Merge branch 'mwaw/214582-add-starred-dashboard-services' into 'master'

Add metrics users starred dashboard services

See merge request gitlab-org/gitlab!30051

app/finders/metrics/users_starred_dashboards_finder.rb

+# frozen_string_literal: true
+
+module Metrics
+  class UsersStarredDashboardsFinder
+    def initialize(user:, project:, params: {})
+      @user, @project, @params = user, project, params
+    end
+
+    def execute
+      return ::Metrics::UsersStarredDashboard.none unless Ability.allowed?(user, :read_metrics_user_starred_dashboard, project)
+
+      items = starred_dashboards
+      items = by_project(items)
+      by_dashboard(items)
+    end
+
+    private
+
+    attr_reader :user, :project, :params
+
+    def by_project(items)
+      items.for_project(project)
+    end
+
+    def by_dashboard(items)
+      return items unless params[:dashboard_path]
+
+      items.merge(starred_dashboards.for_project_dashboard(project, params[:dashboard_path]))
+    end
+
+    def starred_dashboards
+      @starred_dashboards ||= user.metrics_users_starred_dashboards
+    end
+  end
+end

app/models/metrics/users_starred_dashboard.rb

@@ -11,5 +11,8 @@ module Metrics
     validates :project_id, presence: true
     validates :dashboard_path, presence: true, length: { maximum: 255 }
     validates :dashboard_path, uniqueness: { scope: %i[user_id project_id] }
+
+    scope :for_project, ->(project) { where(project: project) }
+    scope :for_project_dashboard, ->(project, path) { for_project(project).where(dashboard_path: path) }
   end
 end

app/policies/project_policy.rb

@@ -269,6 +269,8 @@ class ProjectPolicy < BasePolicy
     enable :read_prometheus
     enable :read_environment
     enable :read_deployment
+    enable :create_metrics_user_starred_dashboard
+    enable :read_metrics_user_starred_dashboard
   end
 
   rule { owner | admin | guest | group_member }.prevent :request_access

app/services/metrics/users_starred_dashboards/create_service.rb

+# frozen_string_literal: true
+
+# Create Metrics::UsersStarredDashboard entry for given user based on matched dashboard_path, project
+module Metrics
+  module UsersStarredDashboards
+    class CreateService < ::BaseService
+      include Stepable
+
+      steps :authorize_create_action,
+            :parse_dashboard_path,
+            :create
+
+      def initialize(user, project, dashboard_path)
+        @user, @project, @dashboard_path = user, project, dashboard_path
+      end
+
+      def execute
+        keys = %i[status message starred_dashboard]
+        status, message, dashboards = execute_steps.values_at(*keys)
+
+        if status != :success
+          ServiceResponse.error(message: message)
+        else
+          ServiceResponse.success(payload: dashboards)
+        end
+      end
+
+      private
+
+      attr_reader :user, :project, :dashboard_path
+
+      def authorize_create_action(_options)
+        if Ability.allowed?(user, :create_metrics_user_starred_dashboard, project)
+          success(user: user, project: project)
+        else
+          error(s_('Metrics::UsersStarredDashboards|You are not authorized to add star to this dashboard'))
+        end
+      end
+
+      def parse_dashboard_path(options)
+        if dashboard_path_exists?
+          options[:dashboard_path] = dashboard_path
+          success(options)
+        else
+          error(s_('Metrics::UsersStarredDashboards|Dashboard with requested path can not be found'))
+        end
+      end
+
+      def create(options)
+        starred_dashboard = build_starred_dashboard_from(options)
+
+        if starred_dashboard.save
+          success(starred_dashboard: starred_dashboard)
+        else
+          error(starred_dashboard.errors.messages)
+        end
+      end
+
+      def build_starred_dashboard_from(options)
+        Metrics::UsersStarredDashboard.new(
+          user: options.fetch(:user),
+          project: options.fetch(:project),
+          dashboard_path: options.fetch(:dashboard_path)
+        )
+      end
+
+      def dashboard_path_exists?
+        Gitlab::Metrics::Dashboard::Finder
+          .find_all_paths(project)
+          .any? { |dashboard| dashboard[:path] == dashboard_path }
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -13259,6 +13259,12 @@ msgstr ""
 msgid "Metrics::Dashboard::Annotation|You are not authorized to delete this annotation"
 msgstr ""
 
+msgid "Metrics::UsersStarredDashboards|Dashboard with requested path can not be found"
+msgstr ""
+
+msgid "Metrics::UsersStarredDashboards|You are not authorized to add star to this dashboard"
+msgstr ""
+
 msgid "Metrics|Add metric"
 msgstr ""
 

spec/finders/metrics/users_starred_dashboards_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Metrics::UsersStarredDashboardsFinder do
+  describe '#execute' do
+    subject(:starred_dashboards) { described_class.new(user: user, project: project, params: params).execute }
+
+    let_it_be(:user) { create(:user) }
+    let(:project) { create(:project) }
+    let(:dashboard_path) { 'config/prometheus/common_metrics.yml' }
+    let(:params) { {} }
+
+    context 'there are no starred dashboard records' do
+      it 'returns empty array' do
+        expect(starred_dashboards).to be_empty
+      end
+    end
+
+    context 'with annotation records' do
+      let!(:starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project) }
+      let!(:starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: dashboard_path) }
+      let!(:other_project_dashboard) { create(:metrics_users_starred_dashboard, user: user, dashboard_path: dashboard_path) }
+      let!(:other_user_dashboard) { create(:metrics_users_starred_dashboard, project: project, dashboard_path: dashboard_path) }
+
+      context 'user without read access to project' do
+        it 'returns empty relation' do
+          expect(starred_dashboards).to be_empty
+        end
+      end
+
+      context 'user with read access to project' do
+        before do
+          project.add_reporter(user)
+        end
+
+        it 'loads starred dashboards' do
+          expect(starred_dashboards).to contain_exactly starred_dashboard_1, starred_dashboard_2
+        end
+
+        context 'when the dashboard_path filter is present' do
+          let(:params) do
+            {
+              dashboard_path: dashboard_path
+            }
+          end
+
+          it 'loads filtered starred dashboards' do
+            expect(starred_dashboards).to contain_exactly starred_dashboard_2
+          end
+        end
+      end
+    end
+  end
+end

spec/models/metrics/users_starred_dashboard_spec.rb

@@ -17,4 +17,23 @@ describe Metrics::UsersStarredDashboard do
     it { is_expected.to validate_length_of(:dashboard_path).is_at_most(255) }
     it { is_expected.to validate_uniqueness_of(:dashboard_path).scoped_to(%i[user_id project_id]) }
   end
+
+  context 'scopes' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:starred_dashboard_a) { create(:metrics_users_starred_dashboard, project: project, dashboard_path: 'path_a') }
+    let_it_be(:starred_dashboard_b) { create(:metrics_users_starred_dashboard, project: project, dashboard_path: 'path_b') }
+    let_it_be(:starred_dashboard_c) { create(:metrics_users_starred_dashboard, dashboard_path: 'path_b') }
+
+    describe '#for_project' do
+      it 'selects only starred dashboards belonging to project' do
+        expect(described_class.for_project(project)).to contain_exactly starred_dashboard_a, starred_dashboard_b
+      end
+    end
+
+    describe '#for_project_dashboard' do
+      it 'selects only starred dashboards belonging to project with given dashboard path' do
+        expect(described_class.for_project_dashboard(project, 'path_b')).to contain_exactly starred_dashboard_b
+      end
+    end
+  end
 end

spec/policies/project_policy_spec.rb

@@ -501,6 +501,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do
@@ -527,6 +529,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do
@@ -535,6 +539,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with anonymous' do
@@ -543,6 +549,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
       end
     end
@@ -557,6 +565,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do
@@ -583,6 +593,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do
@@ -591,6 +603,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with anonymous' do
@@ -611,6 +625,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do
@@ -633,6 +649,8 @@ describe ProjectPolicy do
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
           it { is_expected.to be_allowed(:read_deployment) }
+          it { is_expected.to be_allowed(:read_metrics_user_starred_dashboard) }
+          it { is_expected.to be_allowed(:create_metrics_user_starred_dashboard) }
         end
 
         context 'with guest' do

spec/services/metrics/users_starred_dashboards/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Metrics::UsersStarredDashboards::CreateService do
+  let_it_be(:user) { create(:user) }
+  let(:dashboard_path) { 'config/prometheus/common_metrics.yml' }
+  let(:service_instance) { described_class.new(user, project, dashboard_path) }
+  let(:project) { create(:project) }
+  let(:starred_dashboard_params) do
+    {
+      user: user,
+      project: project,
+      dashboard_path: dashboard_path
+    }
+  end
+
+  shared_examples 'prevented starred dashboard creation' do |message|
+    it 'returns error response', :aggregate_failures do
+      expect(Metrics::UsersStarredDashboard).not_to receive(:new)
+
+      response = service_instance.execute
+
+      expect(response.status).to be :error
+      expect(response.message).to eql message
+    end
+  end
+
+  describe '.execute' do
+    context 'with anonymous user' do
+      it_behaves_like 'prevented starred dashboard creation', 'You are not authorized to add star to this dashboard'
+    end
+
+    context 'with reporter user' do
+      before do
+        project.add_reporter(user)
+      end
+
+      context 'incorrect dashboard_path' do
+        let(:dashboard_path) { 'something_incorrect.yml' }
+
+        it_behaves_like 'prevented starred dashboard creation', 'Dashboard with requested path can not be found'
+      end
+
+      context 'with valid dashboard path' do
+        it 'creates starred dashboard and returns success response', :aggregate_failures do
+          expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard|
+            expect(starred_dashboard).to receive(:save).and_return true
+          end
+
+          response = service_instance.execute
+
+          expect(response.status).to be :success
+        end
+
+        context 'Metrics::UsersStarredDashboard has validation errors' do
+          it 'returns error response', :aggregate_failures do
+            expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard|
+              expect(starred_dashboard).to receive(:save).and_return(false)
+              expect(starred_dashboard).to receive(:errors).and_return(double(messages: { base: ['Model validation error'] }))
+            end
+
+            response = service_instance.execute
+
+            expect(response.status).to be :error
+            expect(response.message).to eql(base: ['Model validation error'])
+          end
+        end
+      end
+    end
+  end
+end