Merge branch...

Merge branch '8376-ops-dashboard-should-be-available-for-public-projects-on-gitlab-com' into 'master'

Resolve "Ops Dashboard should be available for public projects on GitLab.com"

Closes #8376

See merge request gitlab-org/gitlab-ee!8399

ee/app/services/dashboard/operations/projects_service.rb

@@ -8,7 +8,11 @@ module Dashboard
       end
 
       def execute(project_ids)
+        return [] unless License.feature_available?(:operations_dashboard)
+
         find_projects(user, project_ids)
+          .to_a
+          .select { |project| project.feature_available?(:operations_dashboard) }
       end
 
       private
@@ -20,17 +24,10 @@ module Dashboard
           current_user: user,
           project_ids_relation: project_ids,
           params: {
-            plans: plan_names_for_operations_dashboard,
             min_access_level: ProjectMember::DEVELOPER
           }
         ).execute
       end
-
-      def plan_names_for_operations_dashboard
-        return unless Gitlab::CurrentSettings.should_check_namespace_plan?
-
-        Namespace.plans_with_feature(:operations_dashboard)
-      end
     end
   end
 end

ee/changelogs/unreleased/8376-ops-dashboard-should-be-available-for-public-projects-on-gitlab-com.yml

+---
+title: Ops Dashboard should be available for public projects on GitLab.com
+merge_request: 8399
+author:
+type: fixed

ee/spec/services/dashboard/operations/list_service_spec.rb

@@ -10,6 +10,13 @@ describe Dashboard::Operations::ListService do
   let!(:user) { create(:user) }
 
   describe '#execute' do
+    let(:projects_service) { double(Dashboard::Operations::ProjectsService) }
+
+    before do
+      allow(Dashboard::Operations::ProjectsService)
+        .to receive(:new).with(user).and_return(projects_service)
+    end
+
     shared_examples 'no projects' do
       it 'returns an empty list' do
         expect(subject).to be_empty
@@ -51,7 +58,9 @@ describe Dashboard::Operations::ListService do
 
       before do
         user.ops_dashboard_projects << project
-        project.add_developer(user)
+
+        allow(projects_service)
+          .to receive(:execute).with([project]).and_return([project])
       end
 
       it 'returns a list of projects' do
@@ -107,42 +116,15 @@ describe Dashboard::Operations::ListService do
               alert2_prd = create(:prometheus_alert, project: project2, environment: production2)
               create(:prometheus_alert_event, prometheus_alert: alert2_prd)
 
-              project2.add_developer(user)
               user.ops_dashboard_projects << project2
-            end
-
-            it_behaves_like 'avoiding N+1 queries'
-          end
-        end
-
-        describe 'checking plans' do
-          using RSpec::Parameterized::TableSyntax
-
-          where(:check_namespace_plan, :plan, :available) do
-            true  | :gold_plan   | true
-            true  | :silver_plan | false
-            true  | nil          | false
-            false | :gold_plan   | true
-            false | :silver_plan | true
-            false | nil          | true
-          end
 
-          with_them do
-            before do
-              stub_application_setting(check_namespace_plan: check_namespace_plan)
-              project.namespace.update!(plan: create(plan)) if plan
+              allow(projects_service)
+                .to receive(:execute)
+                .with([project, project2])
+                .and_return([project, project2])
             end
 
-            if params[:available]
-              it 'returns this project' do
-                expect(subject.size).to eq(1)
-                expect(dashboard_project.project).to eq(project)
-              end
-            else
-              it 'does not return this project' do
-                expect(subject).to be_empty
-              end
-            end
+            it_behaves_like 'avoiding N+1 queries'
           end
         end
       end
@@ -158,17 +140,14 @@ describe Dashboard::Operations::ListService do
       context 'without deployments' do
         it_behaves_like 'no deployment information'
       end
-
-      context 'without sufficient access level' do
-        before do
-          project.add_reporter(user)
-        end
-
-        it_behaves_like 'no projects'
-      end
     end
 
     context 'without added projects' do
+      before do
+        allow(projects_service)
+          .to receive(:execute).with([]).and_return([])
+      end
+
       it_behaves_like 'no projects'
     end
   end

ee/spec/services/dashboard/operations/projects_service_spec.rb

@@ -3,77 +3,150 @@
 require 'spec_helper'
 
 describe Dashboard::Operations::ProjectsService do
+  PUBLIC = Gitlab::VisibilityLevel::PUBLIC
+  PRIVATE = Gitlab::VisibilityLevel::PRIVATE
+
+  let!(:license) { create(:license, plan: License::ULTIMATE_PLAN) }
+
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, namespace: namespace, visibility_level: PRIVATE) }
+  let(:namespace) { create(:namespace, visibility_level: PRIVATE) }
   let(:service) { described_class.new(user) }
 
   describe '#execute' do
+    let(:result) { service.execute(projects) }
+
+    shared_examples 'project not found' do
+      it 'returns an empty list' do
+        expect(result).to be_empty
+      end
+    end
+
+    shared_examples 'project found' do
+      it 'returns the project' do
+        expect(result).to contain_exactly(project)
+      end
+    end
+
     before do
       project.add_developer(user)
     end
 
-    it 'returns the project when passing a project id' do
-      projects = service.execute([project.id])
+    context 'when passing a project id' do
+      let(:projects) { [project.id] }
 
-      expect(projects).to contain_exactly(project)
+      it_behaves_like 'project found'
     end
 
-    it 'returns the project when passing a project record' do
-      projects = service.execute([project])
+    context 'when passing a project record' do
+      let(:projects) { [project] }
 
-      expect(projects).to contain_exactly(project)
+      it_behaves_like 'project found'
     end
 
-    describe 'with plans' do
-      let!(:gold_project) { create(:project, namespace: create(:namespace, plan: :gold_plan)) }
-      let!(:silver_project) { create(:project, namespace: create(:namespace, plan: :silver_plan)) }
-      let!(:no_plan_project) { create(:project, namespace: create(:namespace)) }
+    context 'when passing invalid project id' do
+      let(:projects) { [-1] }
 
-      let(:projects) { service.execute([gold_project, silver_project, no_plan_project]) }
+      it_behaves_like 'project not found'
+    end
+
+    context 'with insufficient access' do
+      let(:projects) { [project] }
 
       before do
-        gold_project.add_developer(user)
-        silver_project.add_developer(user)
-        no_plan_project.add_developer(user)
+        project.add_reporter(user)
       end
 
-      context 'when namespace plan check is enabled' do
-        before do
-          stub_application_setting(check_namespace_plan: true)
-        end
+      it_behaves_like 'project not found'
+    end
+
+    describe 'checking license' do
+      let(:projects) { [project] }
+
+      using RSpec::Parameterized::TableSyntax
+
+      before do
+        stub_application_setting(check_namespace_plan: true)
+        namespace.update!(plan: create(:gold_plan))
+      end
+
+      where(:plan, :trial, :expired, :available) do
+        License::ULTIMATE_PLAN  | false | false | true
+        License::ULTIMATE_PLAN  | false | true  | true
+        License::ULTIMATE_PLAN  | true  | false | false
+        License::ULTIMATE_PLAN  | true  | true  | false
+        License::PREMIUM_PLAN   | false | false | false
+        nil                     | false | false | false
+      end
+
+      with_them do
+        let!(:license) { create(:license, plan: plan, trial: trial, expired: expired) }
 
-        it 'returns the gold project' do
-          expect(projects).to contain_exactly(gold_project)
+        if params[:available]
+          it_behaves_like 'project found'
+        else
+          it_behaves_like 'project not found'
         end
       end
+    end
+
+    describe 'checking plans' do
+      let(:projects) { [project] }
+
+      using RSpec::Parameterized::TableSyntax
+
+      where(:check_namespace_plan, :plan, :available) do
+        true  | :gold_plan   | true
+        true  | :silver_plan | false
+        true  | nil          | false
+        false | :gold_plan   | true
+        false | :silver_plan | true
+        false | nil          | true
+      end
 
-      context 'when namespace plan check is disabled' do
+      with_them do
         before do
-          stub_application_setting(check_namespace_plan: false)
+          stub_application_setting(check_namespace_plan: check_namespace_plan)
+          namespace.update!(plan: create(plan)) if plan
         end
 
-        it 'returns all projects' do
-          expect(projects).to contain_exactly(gold_project, silver_project, no_plan_project)
+        if params[:available]
+          it_behaves_like 'project found'
+        else
+          it_behaves_like 'project not found'
         end
       end
     end
 
-    context 'with insufficient access' do
-      before do
-        project.add_reporter(user)
-      end
+    describe 'checking availability of public projects on GitLab.com' do
+      let(:projects) { [project] }
 
-      it 'returns an empty list' do
-        projects = service.execute([project.id])
+      using RSpec::Parameterized::TableSyntax
 
-        expect(projects).to be_empty
+      where(:check_namespace_plan, :project_visibility, :namespace_visibility, :available) do
+        true  | PUBLIC  | PUBLIC  | true
+        true  | PRIVATE | PUBLIC  | false
+        true  | PUBLIC  | PRIVATE | false
+        true  | PRIVATE | PRIVATE | false
+        false | PUBLIC  | PUBLIC  | true
+        false | PRIVATE | PUBLIC  | true
+        false | PUBLIC  | PRIVATE | true
+        false | PRIVATE | PRIVATE | true
       end
-    end
 
-    it 'does not find by invalid project id' do
-      projects = service.execute([-1])
+      with_them do
+        before do
+          stub_application_setting(check_namespace_plan: check_namespace_plan)
+          project.update!(visibility_level: project_visibility)
+          namespace.update!(visibility_level: namespace_visibility)
+        end
 
-      expect(projects).to be_empty
+        if params[:available]
+          it_behaves_like 'project found'
+        else
+          it_behaves_like 'project not found'
+        end
+      end
     end
   end
 end

ee/spec/services/users_ops_dashboard_projects/create_service_spec.rb

@@ -5,96 +5,65 @@ require 'spec_helper'
 describe UsersOpsDashboardProjects::CreateService do
   let(:user) { create(:user) }
   let(:service) { described_class.new(user) }
-  let(:project) { create(:project, :private) }
+  let(:project) { create(:project) }
 
   describe '#execute' do
-    context 'with at least developer access level' do
-      before do
-        project.add_developer(user)
-      end
+    let(:projects_service) { double(Dashboard::Operations::ProjectsService) }
+    let(:result) { service.execute(input) }
+
+    before do
+      allow(Dashboard::Operations::ProjectsService)
+        .to receive(:new).with(user).and_return(projects_service)
+      allow(projects_service)
+        .to receive(:execute).with(input).and_return(output)
+    end
+
+    context 'with projects' do
+      let(:output) { [project] }
 
-      it 'adds a project' do
-        result = service.execute([project.id])
+      context 'with integer id' do
+        let(:input) { [project.id] }
 
-        expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        it 'adds a project' do
+          expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        end
       end
 
-      it 'adds a project with a string id' do
-        result = service.execute([project.id.to_s])
+      context 'with string id' do
+        let(:input) { [project.id.to_s] }
 
-        expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        it 'adds a project' do
+          expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        end
       end
 
-      it 'adds a project only once' do
-        result = service.execute([project.id, project.id])
+      context 'with repeating project id' do
+        let(:input) { [project.id, project.id] }
 
-        expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        it 'adds a project only once' do
+          expect(result).to eq(expected_result(added_project_ids: [project.id]))
+        end
       end
 
       context 'with already added project' do
+        let(:input) { [project.id] }
+
         before do
           user.ops_dashboard_projects << project
         end
 
         it 'does not add duplicates' do
-          result = service.execute([project.id])
-
           expect(result).to eq(expected_result(duplicate_project_ids: [project.id]))
         end
       end
-
-      context 'checking plans' do
-        using RSpec::Parameterized::TableSyntax
-
-        where(:check_namespace_plan, :plan, :can_add) do
-          true  | :gold_plan   | true
-          true  | :silver_plan | false
-          true  | nil          | false
-          false | :gold_plan   | true
-          false | :silver_plan | true
-          false | nil          | true
-        end
-
-        with_them do
-          before do
-            stub_application_setting(check_namespace_plan: check_namespace_plan)
-            project.namespace.update!(plan: create(plan)) if plan
-          end
-
-          subject { service.execute([project.id]) }
-
-          if params[:can_add]
-            it 'adds a project' do
-              expect(subject).to eq(expected_result(added_project_ids: [project.id]))
-            end
-          else
-            it 'is not allowed to add a project' do
-              expect(subject).to eq(expected_result(invalid_project_ids: [project.id]))
-            end
-          end
-        end
-      end
-    end
-
-    context 'with access level lower than developer' do
-      before do
-        project.add_reporter(user)
-      end
-
-      it 'does not add a project' do
-        result = service.execute([project.id])
-
-        expect(result).to eq(expected_result(invalid_project_ids: [project.id]))
-      end
     end
 
     context 'with invalid project ids' do
-      let(:invalid_ids) { [nil, -1, '-1', :symbol] }
+      let(:input) { [nil, -1, '-1', :symbol] }
+      let(:output) { [] }
 
       it 'does not add invalid project ids' do
-        result = service.execute(invalid_ids)
-
-        expect(result).to eq(expected_result(invalid_project_ids: invalid_ids))
+        expect(result).to eq(expected_result(invalid_project_ids: input))
       end
     end
   end