Merge branch '332810-dropping-columns-ignoring-the-column-release-14-1' into 'master'

Remove seat_link_enabled from application settings

See merge request gitlab-org/gitlab!64160

app/views/admin/application_settings/metrics_and_profiling.html.haml

@@ -51,6 +51,4 @@
   .settings-content
     = render 'usage'
 
-= render_if_exists 'admin/application_settings/seat_link_setting', expanded: expanded_by_default?
-
 = render_if_exists 'admin/application_settings/pseudonymizer_settings', expanded: expanded_by_default?

config/initializers/1_settings.rb

@@ -222,7 +222,6 @@ Gitlab.ee do
   Settings.gitlab['mirror_max_delay'] ||= 300
   Settings.gitlab['mirror_max_capacity'] ||= 30
   Settings.gitlab['mirror_capacity_threshold'] ||= 15
-  Settings.gitlab['seat_link_enabled'] = true if Settings.gitlab['seat_link_enabled'].nil?
 end
 
 #

ee/app/helpers/ee/application_settings_helper.rb

@@ -60,7 +60,6 @@ module EE
         :max_personal_access_token_lifetime,
         :pseudonymizer_enabled,
         :repository_size_limit,
-        :seat_link_enabled,
         :secret_detection_token_revocation_enabled,
         :secret_detection_token_revocation_url,
         :secret_detection_token_revocation_token,

ee/app/models/ee/application_setting.rb

@@ -13,6 +13,7 @@ module EE
       include IgnorableColumns
 
       ignore_columns %i[elasticsearch_shards elasticsearch_replicas], remove_with: '14.1', remove_after: '2021-06-22'
+      ignore_column :seat_link_enabled, remove_with: '14.2', remove_after: '2021-07-22'
 
       EMAIL_ADDITIONAL_TEXT_CHARACTER_LIMIT = 10_000
       DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL = 7
@@ -162,7 +163,6 @@ module EE
           mirror_max_delay: Settings.gitlab['mirror_max_delay'],
           pseudonymizer_enabled: false,
           repository_size_limit: 0,
-          seat_link_enabled: Settings.gitlab['seat_link_enabled'],
           secret_detection_token_revocation_enabled: false,
           secret_detection_token_revocation_url: nil,
           secret_detection_token_revocation_token: nil,
@@ -245,19 +245,6 @@ module EE
       pseudonymizer_available? && super
     end
 
-    def seat_link_available?
-      License.feature_available?(:seat_link)
-    end
-
-    def seat_link_can_be_configured?
-      Settings.gitlab.seat_link_enabled
-    end
-
-    def seat_link_enabled
-      seat_link_available? && seat_link_can_be_configured? && super
-    end
-    alias_method :seat_link_enabled?, :seat_link_enabled
-
     def should_check_namespace_plan?
       check_namespace_plan? && (Rails.env.test? || ::Gitlab.dev_env_org_or_com?)
     end

ee/app/models/gitlab/seat_link_data.rb

@@ -27,20 +27,9 @@ module Gitlab
     end
 
     def should_sync_seats?
-      return false unless license
-
-      if license.cloud_license?
-        !license.trial? &&
-          license.expires_at.present? # Skip sync if license has no expiration
-      else
-        # Only sync paid licenses from start date until 14 days after expiration
-        # when seat link feature is enabled.
-        Gitlab::CurrentSettings.seat_link_enabled? &&
-          !license.trial? &&
-          license.expires_at && # Skip sync if license has no expiration
-          historical_data.present? && # Skip sync if there is no historical data
-          timestamp.between?(license.starts_at.beginning_of_day, license.expires_at.end_of_day + 14.days)
-      end
+      return false unless license&.cloud_license?
+
+      !license.trial? && license.expires_at.present? # Skip sync if license has no expiration
     end
 
     private

ee/app/views/admin/application_settings/_seat_link_form.html.haml

-- payload_class = 'js-seat-link-payload'
-
-= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-seat-link-settings'), html: { class: 'fieldset-form' } do |f|
-  = form_errors(@application_setting)
-
-  %fieldset
-    .form-group
-      - can_be_configured = @application_setting.seat_link_can_be_configured?
-      .form-check
-        = f.check_box :seat_link_enabled, disabled: !can_be_configured, class: 'form-check-input'
-        = f.label :seat_link_enabled, class: 'form-check-label' do
-          = _('Enable Seat Link')
-        .form-text.text-muted
-          - if can_be_configured
-            %p.mb-2= _('To simplify the billing process, GitLab will collect user counts in order to prorate charges for user growth throughout the year using a quarterly reconciliation process.')
-
-            - link_path = help_page_path('subscriptions/self_managed/index', anchor: 'seat-link')
-            - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: link_path }
-            %p.mb-2= s_('%{link_start}Learn more%{link_end} about what information is shared with GitLab Inc.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
-
-            %button.btn.gl-button.btn-default.js-payload-preview-trigger{ type: 'button', data: { payload_selector: ".#{payload_class}" } }
-              .gl-spinner.js-spinner.gl-display-none.gl-mr-2
-              .js-text.d-inline= _('Preview payload')
-            %pre.usage-data.js-syntax-highlight.code.highlight.mt-2.d-none{ class: payload_class, data: { endpoint: seat_link_payload_admin_application_settings_path(format: :html) } }
-          - else
-            = _('Seat Link is disabled, and cannot be configured through this form.')
-            - link_path = help_page_path('subscriptions/self_managed/index', anchor: 'disable-seat-link')
-            - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: link_path }
-            = s_('For more information, see the documentation on %{link_start}disabling Seat Link%{link_end}.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
-
-  = f.submit 'Save changes', class: "gl-button btn btn-confirm"

ee/app/views/admin/application_settings/_seat_link_setting.html.haml

-- return unless Gitlab::CurrentSettings.seat_link_available?
-
-%section.settings.as-seat-link.no-animate#js-seat-link-settings{ class: ('expanded' if expanded) }
-  .settings-header
-    %h4
-      = _('Seat Link')
-    %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
-      = expanded_by_default? ? _('Collapse') : _('Expand')
-    %p
-      = _('Enable or disable Seat Link.')
-  .settings-content
-    = render 'seat_link_form'

ee/spec/controllers/admin/licenses_controller_spec.rb

@@ -122,12 +122,11 @@ RSpec.describe Admin::LicensesController do
     let_it_be(:historical_data) { create(:historical_data, recorded_at: Time.current) }
 
     before do
-      allow(License).to receive(:current).and_return(create(:license))
-      allow(Settings.gitlab).to receive(:seat_link_enabled).and_return(seat_link_enabled)
+      allow(License).to receive(:current).and_return(create(:license, cloud: cloud_license_enabled ))
     end
 
-    context 'with seat link enabled' do
-      let(:seat_link_enabled) { true }
+    context 'with a cloud license' do
+      let(:cloud_license_enabled) { true }
 
       it 'returns a success response' do
         post :sync_seat_link, format: :json
@@ -137,8 +136,8 @@ RSpec.describe Admin::LicensesController do
       end
     end
 
-    context 'with seat link disabled' do
-      let(:seat_link_enabled) { false }
+    context 'without a cloud license' do
+      let(:cloud_license_enabled) { false }
 
       it 'returns a failure response' do
         post :sync_seat_link, format: :json

ee/spec/features/admin/admin_settings_spec.rb

@@ -276,28 +276,6 @@ RSpec.describe 'Admin updates EE-only settings' do
     end
   end
 
-  context 'Metrics and profiling page' do
-    before do
-      visit metrics_and_profiling_admin_application_settings_path
-    end
-
-    it 'loads seat link payload on click', :js do
-      page.within('#js-seat-link-settings') do
-        expected_payload_content = /(?=.*"date")(?=.*"timestamp")(?=.*"license_key")(?=.*"max_historical_user_count")(?=.*"billable_users_count")/m
-
-        expect(page).not_to have_content expected_payload_content
-
-        click_button('Preview payload')
-
-        wait_for_requests
-
-        expect(page).to have_selector '.js-seat-link-payload'
-        expect(page).to have_button 'Hide payload'
-        expect(page).to have_content expected_payload_content
-      end
-    end
-  end
-
   context 'sign up settings', :js do
     context 'when license has active user count' do
       let(:license) { create(:license, restrictions: { active_user_count: 1 }) }

ee/spec/models/application_setting_spec.rb

@@ -723,71 +723,6 @@ RSpec.describe ApplicationSetting do
     end
   end
 
-  describe '#seat_link_available?' do
-    subject { setting.seat_link_available? }
-
-    before do
-      allow(License).to receive(:feature_available?).and_call_original
-      allow(License).to receive(:feature_available?).with(:seat_link).and_return(seat_link)
-    end
-
-    context 'when the seat_link feature is available' do
-      let(:seat_link) { true }
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when the seat_link feature is not available' do
-      let(:seat_link) { false }
-
-      it { is_expected.to eq(false) }
-    end
-  end
-
-  describe '#seat_link_can_be_configured?' do
-    subject { setting.seat_link_can_be_configured? }
-
-    before do
-      allow(Settings.gitlab).to receive(:seat_link_enabled).and_return(seat_link_enabled)
-    end
-
-    context 'when the seat_link_enabled configuration is enabled' do
-      let(:seat_link_enabled) { true }
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when the seat_link_enabled configuration is disabled' do
-      let(:seat_link_enabled) { false }
-
-      it { is_expected.to eq(false) }
-    end
-  end
-
-  describe '#seat_link_enabled?' do
-    subject { setting.seat_link_enabled? }
-
-    using RSpec::Parameterized::TableSyntax
-
-    where(:seat_link_available, :seat_link_can_be_configured, :seat_link_enabled, :result) do
-      true  | true  | true  | true
-      false | true  | true  | false
-      true  | false | true  | false
-      true  | true  | false | false
-      false | false | false | false
-    end
-
-    with_them do
-      before do
-        allow(setting).to receive(:seat_link_available?).and_return(seat_link_available)
-        allow(setting).to receive(:seat_link_can_be_configured?).and_return(seat_link_can_be_configured)
-        setting.seat_link_enabled = seat_link_enabled
-      end
-
-      it { is_expected.to eq(result) }
-    end
-  end
-
   describe '#compliance_frameworks' do
     it 'sorts the list' do
       setting.compliance_frameworks = [5, 4, 1, 3, 2]

ee/spec/models/gitlab/seat_link_data_spec.rb

@@ -174,58 +174,24 @@ RSpec.describe Gitlab::SeatLinkData do
       it { is_expected.to be_falsey }
     end
 
-    context 'cloud license' do
-      context 'when license is trial' do
-        let(:license) { build(:license, trial: true) }
+    context 'when license is trial' do
+      let(:license) { build(:license, trial: true) }
 
-        it { is_expected.to be_falsey }
-      end
-
-      context 'when timestamp is out of the range' do
-        let(:timestamp) { license.starts_at - 1.day }
-
-        it { is_expected.to eq(true) }
-      end
-
-      context 'when historical data not found' do
-        before do
-          historical_data.destroy!
-        end
-
-        it { is_expected.to eq(true) }
-      end
+      it { is_expected.to be_falsey }
     end
 
-    context 'legacy license' do
-      let(:license) { build(:license) }
-
-      context 'when seat link is disabled' do
-        before do
-          allow(Settings.gitlab).to receive(:seat_link_enabled).and_return(false)
-        end
-
-        it { is_expected.to be_falsey }
-      end
-
-      context 'when license is trial' do
-        let(:license) { build(:license, trial: true) }
+    context 'when timestamp is out of the range' do
+      let(:timestamp) { license.starts_at - 1.day }
 
-        it { is_expected.to be_falsey }
-      end
-
-      context 'when timestamp is out of the range' do
-        let(:timestamp) { license.starts_at - 1.day }
+      it { is_expected.to eq(true) }
+    end
 
-        it { is_expected.to be_falsey }
+    context 'when historical data not found' do
+      before do
+        historical_data.destroy!
       end
 
-      context 'when historical data not found' do
-        before do
-          historical_data.destroy!
-        end
-
-        it { is_expected.to eq(false) }
-      end
+      it { is_expected.to eq(true) }
     end
   end
 end

ee/spec/workers/sync_seat_link_worker_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe SyncSeatLinkWorker, type: :worker do
 
       before do
         # Setting the date as 12th March 2020 12:00 UTC for tests and creating new license
-        create_current_license(starts_at: '2020-02-12'.to_date)
+        create_current_license(cloud_licensing_enabled: true, starts_at: '2020-02-12'.to_date)
 
         create(:historical_data, recorded_at: '2020-02-11T00:00:00Z', active_user_count: 100)
         create(:historical_data, recorded_at: '2020-02-12T00:00:00Z', active_user_count: 10)
@@ -90,6 +90,14 @@ RSpec.describe SyncSeatLinkWorker, type: :worker do
       end
     end
 
+    shared_examples 'seat link sync' do
+      it 'executes the SyncSeatLinkRequestWorker' do
+        expect(SyncSeatLinkRequestWorker).to receive(:perform_async).and_return(true)
+
+        subject.perform
+      end
+    end
+
     context 'license checks' do
       let_it_be(:historical_data) { create(:historical_data) }
 
@@ -111,7 +119,7 @@ RSpec.describe SyncSeatLinkWorker, type: :worker do
 
       context 'when the license has no expiration date' do
         before do
-          create_current_license(expires_at: nil, block_changes_at: nil)
+          create_current_license(cloud_licensing_enabled: true, expires_at: nil, block_changes_at: nil)
         end
 
         include_examples 'no seat link sync'
@@ -119,39 +127,19 @@ RSpec.describe SyncSeatLinkWorker, type: :worker do
 
       context 'when using an expired license' do
         before do
-          create_current_license(expires_at: expiration_date)
+          create_current_license(cloud_licensing_enabled: true, expires_at: Time.zone.now.utc.to_date - 15.days)
         end
 
-        context 'the license expired over 14 days ago' do
-          let(:expiration_date) { Time.zone.now.utc.to_date - 15.days }
-
-          include_examples 'no seat link sync'
-        end
-
-        context 'the license expired less than or equal to 14 days ago' do
-          let(:expiration_date) { Time.zone.now.utc.to_date - 14.days }
-
-          it 'executes the SyncSeatLinkRequestWorker' do
-            expect(SyncSeatLinkRequestWorker).to receive(:perform_async).and_return(true)
-
-            subject.perform
-          end
-        end
+        include_examples 'seat link sync'
       end
     end
 
-    context 'when seat link has been disabled' do
+    context 'with a non cloud license' do
       before do
-        create(:historical_data)
-
-        allow(Gitlab::CurrentSettings).to receive(:seat_link_enabled?).and_return(false)
+        create_current_license(starts_at: '2020-02-12'.to_date)
       end
 
       include_examples 'no seat link sync'
     end
-
-    context 'when no historical data exists' do
-      include_examples 'no seat link sync'
-    end
   end
 end

locale/gitlab.pot

@@ -11953,9 +11953,6 @@ msgstr ""
 msgid "Enable SSL verification"
 msgstr ""
 
-msgid "Enable Seat Link"
-msgstr ""
-
 msgid "Enable Spam Check via external API endpoint"
 msgstr ""
 
@@ -12034,9 +12031,6 @@ msgstr ""
 msgid "Enable mirror configuration"
 msgstr ""
 
-msgid "Enable or disable Seat Link."
-msgstr ""
-
 msgid "Enable or disable the Pseudonymizer data collection."
 msgstr ""
 
@@ -14098,9 +14092,6 @@ msgstr ""
 msgid "For more information, see the documentation on %{deactivating_usage_ping_link_start}deactivating the usage ping%{deactivating_usage_ping_link_end}."
 msgstr ""
 
-msgid "For more information, see the documentation on %{link_start}disabling Seat Link%{link_end}."
-msgstr ""
-
 msgid "Forgot your password?"
 msgstr ""
 
@@ -28610,12 +28601,6 @@ msgstr[1] ""
 msgid "Searching by both author and message is currently not supported."
 msgstr ""
 
-msgid "Seat Link"
-msgstr ""
-
-msgid "Seat Link is disabled, and cannot be configured through this form."
-msgstr ""
-
 msgid "SeatUsage|Seat usage"
 msgstr ""
 
@@ -34006,9 +33991,6 @@ msgstr ""
 msgid "To set up this service:"
 msgstr ""
 
-msgid "To simplify the billing process, GitLab will collect user counts in order to prorate charges for user growth throughout the year using a quarterly reconciliation process."
-msgstr ""
-
 msgid "To specify the notification level per project of a group you belong to, you need to visit project page and change notification level there."
 msgstr ""