Add latest changes from gitlab-org/gitlab@master

5 files changed, 183 insertions, 53 deletions

diff --git a/spec/frontend/admin/abuse_report/components/user_details_spec.js b/spec/frontend/admin/abuse_report/components/user_details_spec.js
index 24ec0cdb1b2..42c219b1b11 100644
--- a/spec/frontend/admin/abuse_report/components/user_details_spec.js
+++ b/spec/frontend/admin/abuse_report/components/user_details_spec.js
@@ -1,6 +1,5 @@
 import { GlLink, GlSprintf } from '@gitlab/ui';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
-import { sprintf } from '~/locale';
 import UserDetails from '~/admin/abuse_report/components/user_details.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import { USER_DETAILS_I18N } from '~/admin/abuse_report/constants';
@@ -61,7 +60,7 @@ describe('UserDetails', () => {
   describe('verification', () => {
     it('renders the users verification with the correct label', () => {
       expect(findUserDetailLabel('verification')).toBe(USER_DETAILS_I18N.verification);
-      expect(findUserDetailValue('verification')).toBe('Email, Credit card');
+      expect(findUserDetailValue('verification')).toBe('Email, Phone, Credit card');
     });
   });
 
@@ -73,7 +72,7 @@ describe('UserDetails', () => {
     describe('similar credit cards', () => {
       it('renders the number of similar records', () => {
         expect(findUserDetail('credit-card-verification').text()).toContain(
-          sprintf('Card matches %{similarRecordsCount} accounts', { ...user.creditCard }),
+          `Card matches ${user.creditCard.similarRecordsCount} accounts`,
         );
       });
 
@@ -83,7 +82,7 @@ describe('UserDetails', () => {
         );
 
         expect(findLinkFor('credit-card-verification').text()).toBe(
-          sprintf('%{similarRecordsCount} accounts', { ...user.creditCard }),
+          `${user.creditCard.similarRecordsCount} accounts`,
         );
 
         expect(findLinkFor('credit-card-verification').text()).toContain(
@@ -100,7 +99,7 @@ describe('UserDetails', () => {
 
         it('does not render the number of similar records', () => {
           expect(findUserDetail('credit-card-verification').text()).not.toContain(
-            sprintf('Card matches %{similarRecordsCount} accounts', { ...user.creditCard }),
+            `Card matches ${user.creditCard.similarRecordsCount} accounts`,
           );
         });
 
@@ -123,6 +122,60 @@ describe('UserDetails', () => {
     });
   });
 
+  describe('phoneNumber', () => {
+    it('renders the correct label', () => {
+      expect(findUserDetailLabel('phone-number-verification')).toBe(USER_DETAILS_I18N.phoneNumber);
+    });
+
+    describe('similar phone numbers', () => {
+      it('renders the number of similar records', () => {
+        expect(findUserDetail('phone-number-verification').text()).toContain(
+          `Phone matches ${user.phoneNumber.similarRecordsCount} accounts`,
+        );
+      });
+
+      it('renders a link to the matching phone numbers', () => {
+        expect(findLinkFor('phone-number-verification').attributes('href')).toBe(
+          user.phoneNumber.phoneMatchesLink,
+        );
+
+        expect(findLinkFor('phone-number-verification').text()).toBe(
+          `${user.phoneNumber.similarRecordsCount} accounts`,
+        );
+      });
+
+      describe('when the number of similar phone numbers is less than 2', () => {
+        beforeEach(() => {
+          createComponent({
+            user: { ...user, phoneNumber: { ...user.phoneNumber, similarRecordsCount: 1 } },
+          });
+        });
+
+        it('does not render the number of similar records', () => {
+          expect(findUserDetail('phone-number-verification').text()).not.toContain(
+            `Phone matches ${user.phoneNumber.similarRecordsCount} accounts`,
+          );
+        });
+
+        it('does not render a link to the matching phone numbers', () => {
+          expect(findLinkFor('phone-number-verification').exists()).toBe(false);
+        });
+      });
+    });
+
+    describe('when the users phoneNumber is blank', () => {
+      beforeEach(() => {
+        createComponent({
+          user: { ...user, phoneNumber: undefined },
+        });
+      });
+
+      it('does not render the users phoneNumber', () => {
+        expect(findUserDetail('phone-number-verification').exists()).toBe(false);
+      });
+    });
+  });
+
   describe('otherReports', () => {
     it('renders the correct label', () => {
       expect(findUserDetailLabel('past-closed-reports')).toBe(USER_DETAILS_I18N.pastReports);
@@ -132,9 +185,7 @@ describe('UserDetails', () => {
       const index = user.pastClosedReports.indexOf(pastReport);
 
       it('renders the category', () => {
-        expect(findPastReport(index).text()).toContain(
-          sprintf('Reported for %{category}', { ...pastReport }),
-        );
+        expect(findPastReport(index).text()).toContain(`Reported for ${pastReport.category}`);
       });
 
       it('renders a link to the report', () => {
diff --git a/spec/frontend/admin/abuse_report/mock_data.js b/spec/frontend/admin/abuse_report/mock_data.js
index 9790b44c976..f02986fb5bb 100644
--- a/spec/frontend/admin/abuse_report/mock_data.js
+++ b/spec/frontend/admin/abuse_report/mock_data.js
@@ -9,12 +9,16 @@ export const mockAbuseReport = {
     path: '/spamuser417',
     adminPath: '/admin/users/spamuser417',
     plan: 'Free',
-    verificationState: { email: true, phone: false, creditCard: true },
+    verificationState: { email: true, phone: true, creditCard: true },
     creditCard: {
       name: 'S. User',
       similarRecordsCount: 2,
       cardMatchesLink: '/admin/users/spamuser417/card_match',
     },
+    phoneNumber: {
+      similarRecordsCount: 2,
+      phoneMatchesLink: '/admin/users/spamuser417/phone_match',
+    },
     pastClosedReports: [
       {
         category: 'offensive',
diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb
index eb7727529cc..89fcb4b43a6 100644
--- a/spec/lib/api/helpers_spec.rb
+++ b/spec/lib/api/helpers_spec.rb
@@ -545,7 +545,7 @@ RSpec.describe API::Helpers, feature_category: :shared do
         allow(helper).to receive(:authenticate_non_public?).and_return(false)
       end
 
-      subject { helper.find_group!(group.id, organization) }
+      subject { helper.find_group!(group.id, organization: organization) }
 
       context 'when group exists in the organization' do
         before do
diff --git a/spec/models/users/phone_number_validation_spec.rb b/spec/models/users/phone_number_validation_spec.rb
index 15bbb507dee..788df05763c 100644
--- a/spec/models/users/phone_number_validation_spec.rb
+++ b/spec/models/users/phone_number_validation_spec.rb
@@ -14,12 +14,12 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
   it { is_expected.to validate_presence_of(:international_dial_code) }
 
-  it {
+  it do
     is_expected.to validate_numericality_of(:international_dial_code)
       .only_integer
       .is_greater_than_or_equal_to(1)
       .is_less_than_or_equal_to(999)
-  }
+  end
 
   it { is_expected.to validate_presence_of(:phone_number) }
   it { is_expected.to validate_length_of(:phone_number).is_at_most(12) }
@@ -30,6 +30,27 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
   it { is_expected.to validate_length_of(:telesign_reference_xid).is_at_most(255) }
 
+  describe '#similar_records' do
+    let_it_be(:phone_number_validation) { create(:phone_number_validation, :validated) }
+
+    let_it_be(:phone_number) do
+      phone_number_validation.attributes.with_indifferent_access.slice(
+        :international_dial_code, :phone_number
+      )
+    end
+
+    let_it_be(:match) { create(:phone_number_validation, :validated, phone_number) }
+    let_it_be(:unvalidated_match) { create(:phone_number_validation, phone_number) }
+
+    let_it_be(:non_match_1) { create(:phone_number_validation, phone_number.merge(international_dial_code: 81)) }
+    let_it_be(:non_match_2) { create(:phone_number_validation, phone_number.merge(phone_number: '5555555555')) }
+
+    it 'returns matches with the same international dialing code and phone number' do
+      expect(phone_number_validation.similar_records).to match_array([unvalidated_match, match,
+        phone_number_validation])
+    end
+  end
+
   describe '.related_to_banned_user?' do
     let_it_be(:international_dial_code) { 1 }
     let_it_be(:phone_number) { '555' }
@@ -41,7 +62,12 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
     context 'when banned user has the same international dial code and phone number' do
       context 'and the matching record has not been verified' do
         before do
-          create(:phone_number_validation, user: banned_user)
+          create(
+            :phone_number_validation,
+            user: banned_user,
+            international_dial_code: international_dial_code,
+            phone_number: phone_number
+          )
         end
 
         it { is_expected.to eq(false) }
@@ -49,7 +75,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
       context 'and the matching record has been verified' do
         before do
-          create(:phone_number_validation, :validated, user: banned_user)
+          create(
+            :phone_number_validation,
+            :validated,
+            user: banned_user,
+            international_dial_code: international_dial_code,
+            phone_number: phone_number
+          )
         end
 
         it { is_expected.to eq(true) }
@@ -58,7 +90,14 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
     context 'when banned user has the same international dial code and phone number, but different country code' do
       before do
-        create(:phone_number_validation, :validated, user: banned_user, country: 'CA')
+        create(
+          :phone_number_validation,
+          :validated,
+          user: banned_user,
+          international_dial_code: international_dial_code,
+          phone_number: phone_number,
+          country: 'CA'
+        )
       end
 
       it { is_expected.to eq(true) }
@@ -66,7 +105,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
     context 'when banned user does not have the same international dial code' do
       before do
-        create(:phone_number_validation, :validated, user: banned_user, international_dial_code: 61)
+        create(
+          :phone_number_validation,
+          :validated,
+          user: banned_user,
+          international_dial_code: 81,
+          phone_number: phone_number
+        )
       end
 
       it { is_expected.to eq(false) }
@@ -74,7 +119,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
     context 'when banned user does not have the same phone number' do
       before do
-        create(:phone_number_validation, :validated, user: banned_user, phone_number: '666')
+        create(
+          :phone_number_validation,
+          :validated,
+          user: banned_user,
+          international_dial_code: international_dial_code,
+          phone_number: '666'
+        )
       end
 
       it { is_expected.to eq(false) }
@@ -82,7 +133,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
 
     context 'when not-banned user has the same international dial code and phone number' do
       before do
-        create(:phone_number_validation, :validated, user: user)
+        create(
+          :phone_number_validation,
+          :validated,
+          user: user,
+          international_dial_code: international_dial_code,
+          phone_number: phone_number
+        )
       end
 
       it { is_expected.to eq(false) }
@@ -105,6 +162,57 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie
         end
       end
     end
+
+    describe '.similar_to' do
+      subject(:similar_to) { described_class.similar_to(phone_number_validation) }
+
+      let_it_be(:international_dial_code) { 44 }
+      let_it_be(:phone_number) { '111' }
+
+      let_it_be(:phone_number_validation) do
+        create(:phone_number_validation,
+          :validated,
+          international_dial_code: international_dial_code,
+          phone_number: phone_number
+        )
+      end
+
+      let_it_be(:match) do
+        create(:phone_number_validation,
+          :validated,
+          international_dial_code: phone_number_validation.international_dial_code,
+          phone_number: phone_number_validation.phone_number
+        )
+      end
+
+      let_it_be(:non_match_1) do
+        create(:phone_number_validation,
+          :validated,
+          international_dial_code: phone_number_validation.international_dial_code,
+          phone_number: '222'
+        )
+      end
+
+      let_it_be(:non_match_2) do
+        create(:phone_number_validation,
+          :validated,
+          international_dial_code: 81,
+          phone_number: phone_number_validation.phone_number
+        )
+      end
+
+      let_it_be(:non_match_3) do
+        create(:phone_number_validation,
+          :validated,
+          international_dial_code: 82,
+          phone_number: '333'
+        )
+      end
+
+      it 'returns only records with the same international dialing code and phone number' do
+        expect(similar_to).to match_array([phone_number_validation, match])
+      end
+    end
   end
 
   describe '#validated?' do
diff --git a/spec/serializers/admin/abuse_report_details_entity_spec.rb b/spec/serializers/admin/abuse_report_details_entity_spec.rb
index 67f6bdfee85..9260cfc0799 100644
--- a/spec/serializers/admin/abuse_report_details_entity_spec.rb
+++ b/spec/serializers/admin/abuse_report_details_entity_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Admin::AbuseReportDetailsEntity, feature_category: :insider_threat do
   include Gitlab::Routing
 
-  let_it_be(:report) { build_stubbed(:abuse_report) }
+  let_it_be(:report) { create(:abuse_report) }
   let_it_be(:user) { report.user }
   let_it_be(:reporter) { report.reporter }
   let_it_be(:past_report) { create_default(:abuse_report, :closed, user: user) }
@@ -136,7 +136,7 @@ RSpec.describe Admin::AbuseReportDetailsEntity, feature_category: :insider_threa
       context 'when on .com', :saas, if: Gitlab.ee? do
         before do
           stub_ee_application_setting(should_check_namespace_plan: true)
-          create(:namespace_with_plan, plan: :bronze_plan, owner: user)  # rubocop:disable RSpec/FactoryBot/AvoidCreate
+          create(:gitlab_subscription, :bronze, namespace: user.namespace)
         end
 
         it 'includes the plan' do
@@ -144,38 +144,5 @@ RSpec.describe Admin::AbuseReportDetailsEntity, feature_category: :insider_threa
         end
       end
     end
-
-    describe 'users credit card' do
-      let(:credit_card_hash) { entity_hash[:user][:credit_card] }
-
-      context 'when the user has no verified credit card' do
-        it 'does not expose the credit card' do
-          expect(credit_card_hash).to be_nil
-        end
-      end
-
-      context 'when the user does have a verified credit card' do
-        let!(:credit_card) { build_stubbed(:credit_card_validation, user: user) }
-
-        it 'exposes the credit card' do
-          expect(credit_card_hash.keys).to match_array([
-            :similar_records_count,
-            :card_matches_link
-          ])
-        end
-
-        context 'when not on ee', unless: Gitlab.ee? do
-          it 'does not include the path to the admin card matches page' do
-            expect(credit_card_hash[:card_matches_link]).to be_nil
-          end
-        end
-
-        context 'when on ee', if: Gitlab.ee? do
-          it 'includes the path to the admin card matches page' do
-            expect(credit_card_hash[:card_matches_link]).not_to be_nil
-          end
-        end
-      end
-    end
   end
 end