Back-port free instance review to CE/FOSS

Allow admin users of CE/FOSS instances with 50+ users to contact GitLab
to get a free instance review.

ee/app/controllers/admin/instance_review_controller.rb → app/controllers/admin/instance_review_controller.rb

@@ -3,7 +3,7 @@ class Admin::InstanceReviewController < Admin::ApplicationController
   feature_category :instance_statistics
 
   def index
-    redirect_to("#{EE::SUBSCRIPTIONS_URL}/instance_review?#{instance_review_params}")
+    redirect_to("#{::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL}/instance_review?#{instance_review_params}")
   end
 
   def instance_review_params

app/helpers/application_helper.rb

@@ -212,6 +212,10 @@ module ApplicationHelper
     Gitlab::CurrentSettings.current_application_settings.help_page_support_url.presence || promo_url + '/getting-help/'
   end
 
+  def instance_review_permitted?
+    ::Gitlab::CurrentSettings.instance_review_permitted? && current_user&.admin?
+  end
+
   def static_objects_external_storage_enabled?
     Gitlab::CurrentSettings.static_objects_external_storage_enabled?
   end

app/models/application_setting.rb

@@ -11,6 +11,7 @@ class ApplicationSetting < ApplicationRecord
   ignore_column :instance_statistics_visibility_private, remove_with: '13.6', remove_after: '2020-10-22'
   ignore_column :snowplow_iglu_registry_url, remove_with: '13.6', remove_after: '2020-11-22'
 
+  INSTANCE_REVIEW_MIN_USERS = 50
   GRAFANA_URL_ERROR_MESSAGE = 'Please check your Grafana URL setting in ' \
     'Admin Area > Settings > Metrics and profiling > Metrics - Grafana'
 
@@ -437,6 +438,14 @@ class ApplicationSetting < ApplicationRecord
     !!(sourcegraph_url =~ /\Ahttps:\/\/(www\.)?sourcegraph\.com/)
   end
 
+  def instance_review_permitted?
+    users_count = Rails.cache.fetch('limited_users_count', expires_in: 1.day) do
+      ::User.limit(INSTANCE_REVIEW_MIN_USERS + 1).count(:all)
+    end
+
+    users_count >= INSTANCE_REVIEW_MIN_USERS
+  end
+
   def self.create_from_defaults
     check_schema!
 

app/views/layouts/header/_current_user_dropdown.html.haml

@@ -46,7 +46,7 @@
   - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile)
     %li.d-md-none
       = render 'shared/user_dropdown_contributing_link'
-  = render_if_exists 'shared/user_dropdown_instance_review'
+  = render 'shared/user_dropdown_instance_review'
   - if Gitlab.com_but_not_canary?
     %li.d-md-none
       = link_to _("Switch to GitLab Next"), "https://next.gitlab.com/"

app/views/layouts/header/_help_dropdown.html.haml

@@ -17,7 +17,7 @@
   - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile)
     %li
       = render 'shared/user_dropdown_contributing_link'
-  = render_if_exists 'shared/user_dropdown_instance_review'
+  = render 'shared/user_dropdown_instance_review'
   - if Gitlab.com_but_not_canary?
     %li
       = link_to _("Switch to GitLab Next"), "https://next.gitlab.com/"

config/routes/admin.rb

@@ -81,6 +81,8 @@ namespace :admin do
     post :preview, on: :collection
   end
 
+  get :instance_review, to: 'instance_review#index'
+
   resource :health_check, controller: 'health_check', only: [:show]
   resource :background_jobs, controller: 'background_jobs', only: [:show]
 

doc/administration/instance_review.md

@@ -2,7 +2,7 @@
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/6995) in [GitLab Core](https://about.gitlab.com/pricing/) 11.3.
 
-If you are running a medium size instance of GitLab Core edition you are qualified for a free Instance Review. You can find the button in the User menu.
+If you are running a medium size instance (50+ users) of GitLab Core edition you are qualified for a free Instance Review. You can find the button in the User menu.
 
 ![Instance Review button](img/instance_review_button.png)
 

ee/app/helpers/ee/application_helper.rb

@@ -104,10 +104,6 @@ module EE
       end
     end
 
-    def instance_review_permitted?
-      ::Gitlab::CurrentSettings.instance_review_permitted? && current_user&.admin?
-    end
-
     override :show_last_push_widget?
     def show_last_push_widget?(event)
       show = super

ee/app/models/ee/application_setting.rb

@@ -7,10 +7,10 @@ module EE
   # and be prepended in the `ApplicationSetting` model
   module ApplicationSetting
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
 
     prepended do
       EMAIL_ADDITIONAL_TEXT_CHARACTER_LIMIT = 10_000
-      INSTANCE_REVIEW_MIN_USERS = 50
       DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL = 7
 
       belongs_to :file_template_project, class_name: "Project"
@@ -299,14 +299,11 @@ module EE
       ::Project.where(namespace_id: group_id)
     end
 
+    override :instance_review_permitted?
     def instance_review_permitted?
-      return if License.current
+      return false if License.current
 
-      users_count = Rails.cache.fetch('limited_users_count', expires_in: 1.day) do
-        ::User.limit(INSTANCE_REVIEW_MIN_USERS + 1).count(:all)
-      end
-
-      users_count >= INSTANCE_REVIEW_MIN_USERS
+      super
     end
 
     def max_personal_access_token_lifetime_from_now

ee/config/routes/admin.rb

@@ -15,8 +15,6 @@ namespace :admin do
     end
   end
 
-  get :instance_review, to: 'instance_review#index'
-
   resource :push_rule, only: [:show, :update]
   resource :email, only: [:show, :create]
   resources :audit_logs, controller: 'audit_logs', only: [:index]

ee/lib/ee.rb

 # frozen_string_literal: true
 
 module EE
-  def self.default_subscriptions_url
-    ::Gitlab.dev_or_test_env? ? 'https://customers.stg.gitlab.com' : 'https://customers.gitlab.com'
-  end
-
-  SUBSCRIPTIONS_URL = ENV.fetch('CUSTOMER_PORTAL_URL', default_subscriptions_url).freeze
+  SUBSCRIPTIONS_URL = ::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL
   SUBSCRIPTIONS_COMPARISON_URL = "https://about.gitlab.com/pricing/gitlab-com/feature-comparison".freeze
   SUBSCRIPTIONS_MORE_MINUTES_URL = "#{SUBSCRIPTIONS_URL}/buy_pipeline_minutes".freeze
   SUBSCRIPTIONS_MORE_STORAGE_URL = "#{SUBSCRIPTIONS_URL}/buy_storage".freeze

ee/spec/helpers/application_helper_spec.rb

@@ -200,30 +200,4 @@ RSpec.describe ApplicationHelper do
       end
     end
   end
-
-  describe '#instance_review_permitted?' do
-    let_it_be(:non_admin_user) { create :user }
-    let_it_be(:admin_user) { create :user, :admin }
-
-    before do
-      allow(::Gitlab::CurrentSettings).to receive(:instance_review_permitted?).and_return(app_setting)
-      allow(helper).to receive(:current_user).and_return(current_user)
-    end
-
-    subject { helper.instance_review_permitted? }
-
-    where(app_setting: [true, false], is_admin: [true, false, nil])
-
-    with_them do
-      let(:current_user) do
-        if is_admin.nil?
-          nil
-        else
-          is_admin ? admin_user : non_admin_user
-        end
-      end
-
-      it { is_expected.to be(app_setting && is_admin) }
-    end
-  end
 end

lib/gitlab/subscription_portal.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module SubscriptionPortal
+    def self.default_subscriptions_url
+      ::Gitlab.dev_or_test_env? ? 'https://customers.stg.gitlab.com' : 'https://customers.gitlab.com'
+    end
+
+    SUBSCRIPTIONS_URL = ENV.fetch('CUSTOMER_PORTAL_URL', default_subscriptions_url).freeze
+  end
+end

ee/spec/controllers/admin/instance_review_controller_spec.rb → spec/controllers/admin/instance_review_controller_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Admin::InstanceReviewController do
   include UsageDataHelpers
 
   let(:admin) { create(:admin) }
+  let(:subscriptions_url) { ::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL }
 
   before do
     sign_in(admin)
@@ -43,7 +44,7 @@ RSpec.describe Admin::InstanceReviewController do
           notes_count: 0
         } }.to_query
 
-        expect(response).to redirect_to("#{EE::SUBSCRIPTIONS_URL}/instance_review?#{params}")
+        expect(response).to redirect_to("#{subscriptions_url}/instance_review?#{params}")
       end
     end
 
@@ -60,7 +61,7 @@ RSpec.describe Admin::InstanceReviewController do
           version: ::Gitlab::VERSION
         } }.to_query
 
-        expect(response).to redirect_to("#{EE::SUBSCRIPTIONS_URL}/instance_review?#{params}")
+        expect(response).to redirect_to("#{subscriptions_url}/instance_review?#{params}")
       end
     end
   end

spec/helpers/application_helper_spec.rb

@@ -222,6 +222,32 @@ RSpec.describe ApplicationHelper do
     end
   end
 
+  describe '#instance_review_permitted?' do
+    let_it_be(:non_admin_user) { create :user }
+    let_it_be(:admin_user) { create :user, :admin }
+
+    before do
+      allow(::Gitlab::CurrentSettings).to receive(:instance_review_permitted?).and_return(app_setting)
+      allow(helper).to receive(:current_user).and_return(current_user)
+    end
+
+    subject { helper.instance_review_permitted? }
+
+    where(app_setting: [true, false], is_admin: [true, false, nil])
+
+    with_them do
+      let(:current_user) do
+        if is_admin.nil?
+          nil
+        else
+          is_admin ? admin_user : non_admin_user
+        end
+      end
+
+      it { is_expected.to be(app_setting && is_admin) }
+    end
+  end
+
   describe '#locale_path' do
     it 'returns the locale path with an `_`' do
       Gitlab::I18n.with_locale('pt-BR') do

ee/spec/lib/ee_spec.rb → spec/lib/gitlab/subscription_portal_spec.rb

@@ -2,9 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe EE do
+RSpec.describe ::Gitlab::SubscriptionPortal do
   describe '.default_subscriptions_url' do
-    subject { EE.default_subscriptions_url }
+    subject { described_class.default_subscriptions_url }
 
     context 'on non test and non dev environments' do
       before do

spec/models/application_setting_spec.rb

@@ -807,6 +807,23 @@ RSpec.describe ApplicationSetting do
     end
   end
 
+  describe '#instance_review_permitted?', :request_store do
+    subject { setting.instance_review_permitted? }
+
+    before do
+      RequestStore.store[:current_license] = nil
+      expect(Rails.cache).to receive(:fetch).and_return(
+        ::ApplicationSetting::INSTANCE_REVIEW_MIN_USERS + users_over_minimum
+      )
+    end
+
+    where(users_over_minimum: [-1, 0, 1])
+
+    with_them do
+      it { is_expected.to be(users_over_minimum >= 0) }
+    end
+  end
+
   describe 'email_restrictions' do
     context 'when email restrictions are enabled' do
       before do