Merge branch 'mwaw/214581-delete-star-metrics-dashboard-endpoint' into 'master'

#214581 Delete star from metrics dashboard endpoint

See merge request gitlab-org/gitlab!31892

app/controllers/concerns/metrics_dashboard.rb

@@ -46,11 +46,15 @@ module MetricsDashboard
     dashboard[:can_edit] = project_dashboard ? can_edit?(dashboard) : false
     dashboard[:project_blob_path] = project_dashboard ? dashboard_project_blob_path(dashboard) : nil
     dashboard[:starred] = starred_dashboards.include?(dashboard[:path])
-    dashboard[:user_starred_path] = nil # placeholder attribute until API endpoint will be merged https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31316
+    dashboard[:user_starred_path] = project_for_dashboard ? user_starred_path(project_for_dashboard, dashboard[:path]) : nil
 
     dashboard
   end
 
+  def user_starred_path(project, path)
+    expose_path(api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: path }))
+  end
+
   def dashboard_project_blob_path(dashboard)
     project_blob_path(project_for_dashboard, File.join(project_for_dashboard.default_branch, dashboard.fetch(:path, "")))
   end

changelogs/unreleased/mwaw-214581-delete-star-metrics-dashboard-endpoint.yml

+---
+title: New API endpoint for removing stars from metrics dashboards
+merge_request: 31892
+author:
+type: added

doc/api/metrics_user_starred_dashboards.md

@@ -32,3 +32,30 @@ Example Response:
   "project_id": 20
 }
 ```
+
+## Remove a star from a dashboard
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31892) in GitLab 13.0.
+
+```plaintext
+DELETE /projects/:id/metrics/user_starred_dashboards
+```
+
+Parameters:
+
+| Attribute      | Type           | Required | Description                                                                  |
+|:---------------|:---------------|:---------|:-----------------------------------------------------------------------------|
+| `dashboard_path` | string        | no      | URL-encoded path to file defining the dashboard which should no longer be marked as favorite. When not supplied all dashboards within given projects will be removed from favorites.   |
+
+```shell
+curl --request DELETE --header 'Private-Token: <your_access_token>' https://gitlab.example.com/api/v4/projects/20/metrics/user_starred_dashboards \
+ --data-urlencode "dashboard_path=config/prometheus/dashboards/common_metrics.yml"
+```
+
+Example Response:
+
+```json
+{
+  "deleted_rows": 1
+}
+```

lib/api/metrics/user_starred_dashboards.rb

@@ -3,16 +3,16 @@
 module API
   module Metrics
     class UserStarredDashboards < Grape::API
-      desc 'Marks selected metrics dashboard as starred' do
-        success Entities::Metrics::UserStarredDashboard
-      end
+      resource :projects do
+        desc 'Marks selected metrics dashboard as starred' do
+          success Entities::Metrics::UserStarredDashboard
+        end
 
-      params do
-        requires :dashboard_path, type: String, allow_blank: false, coerce_with: ->(val) { CGI.unescape(val) },
-                 desc: 'Url encoded path to a file defining the dashboard to which the star should be added'
-      end
+        params do
+          requires :dashboard_path, type: String, allow_blank: false, coerce_with: ->(val) { CGI.unescape(val) },
+                   desc: 'Url encoded path to a file defining the dashboard to which the star should be added'
+        end
 
-      resource :projects do
         post ':id/metrics/user_starred_dashboards' do
           result = ::Metrics::UsersStarredDashboards::CreateService.new(current_user, user_project, params[:dashboard_path]).execute
 
@@ -22,6 +22,24 @@ module API
             error!({ errors: result.message }, 400)
           end
         end
+
+        desc 'Remove star from selected metrics dashboard'
+
+        params do
+          optional :dashboard_path, type: String, allow_blank: false, coerce_with: ->(val) { CGI.unescape(val) },
+                   desc: 'Url encoded path to a file defining the dashboard from which the star should be removed'
+        end
+
+        delete ':id/metrics/user_starred_dashboards' do
+          result = ::Metrics::UsersStarredDashboards::DeleteService.new(current_user, user_project, params[:dashboard_path]).execute
+
+          if result.success?
+            status :ok
+            result.payload
+          else
+            status :bad_request
+          end
+        end
       end
     end
   end

spec/controllers/concerns/metrics_dashboard_spec.rb

@@ -134,10 +134,10 @@ describe MetricsDashboard do
             it 'adds starred dashboard information and sorts the list' do
               all_dashboards = json_response['all_dashboards'].map { |dashboard| dashboard.slice('display_name', 'starred', 'user_starred_path') }
               expected_response = [
-                { "display_name" => "Default", "starred" => false, 'user_starred_path' => nil },
-                { "display_name" => "anomaly.yml", "starred" => false, 'user_starred_path' => nil },
-                { "display_name" => "errors.yml", "starred" => true, 'user_starred_path' => nil },
-                { "display_name" => "test.yml", "starred" => true, 'user_starred_path' => nil }
+                { "display_name" => "Default", "starred" => false, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: 'config/prometheus/common_metrics.yml' }) },
+                { "display_name" => "anomaly.yml", "starred" => false, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/anomaly.yml' }) },
+                { "display_name" => "errors.yml", "starred" => true, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/errors.yml' }) },
+                { "display_name" => "test.yml", "starred" => true, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/test.yml' }) }
               ]
 
               expect(all_dashboards).to eql expected_response

spec/requests/api/metrics/user_starred_dashboards_spec.rb

@@ -4,20 +4,17 @@ require 'spec_helper'
 
 describe API::Metrics::UserStarredDashboards do
   let_it_be(:user) { create(:user) }
-  let!(:project) { create(:project, :private, :repository, :custom_repo, namespace: user.namespace, files: { dashboard => dashboard_yml }) }
-  let(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') }
-  let(:dashboard) { '.gitlab/dashboards/find&seek.yml' }
+  let_it_be(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') }
+  let_it_be(:dashboard) { '.gitlab/dashboards/find&seek.yml' }
+  let_it_be(:project) { create(:project, :private, :repository, :custom_repo, namespace: user.namespace, files: { dashboard => dashboard_yml }) }
+  let(:url) { "/projects/#{project.id}/metrics/user_starred_dashboards" }
   let(:params) do
     {
-      user: user,
-      project: project,
       dashboard_path: CGI.escape(dashboard)
     }
   end
 
   describe 'POST /projects/:id/metrics/user_starred_dashboards' do
-    let(:url) { "/projects/#{project.id}/metrics/user_starred_dashboards" }
-
     before do
       project.add_reporter(user)
     end
@@ -25,7 +22,7 @@ describe API::Metrics::UserStarredDashboards do
     context 'with correct permissions' do
       context 'with valid parameters' do
         context 'dashboard_path as url param url escaped' do
-          it 'creates a new annotation', :aggregate_failures do
+          it 'creates a new user starred metrics dashboard', :aggregate_failures do
             post api(url, user), params: params
 
             expect(response).to have_gitlab_http_status(:created)
@@ -38,13 +35,11 @@ describe API::Metrics::UserStarredDashboards do
         context 'dashboard_path in request body unescaped' do
           let(:params) do
             {
-              user: user,
-              project: project,
               dashboard_path: dashboard
             }
           end
 
-          it 'creates a new annotation', :aggregate_failures do
+          it 'creates a new user starred metrics dashboard', :aggregate_failures do
             post api(url, user), params: params
 
             expect(response).to have_gitlab_http_status(:created)
@@ -89,4 +84,81 @@ describe API::Metrics::UserStarredDashboards do
       end
     end
   end
+
+  describe 'DELETE /projects/:id/metrics/user_starred_dashboards' do
+    let_it_be(:user_starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: dashboard) }
+    let_it_be(:user_starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project) }
+    let_it_be(:other_user_starred_dashboard) { create(:metrics_users_starred_dashboard, project: project) }
+    let_it_be(:other_project_starred_dashboard) { create(:metrics_users_starred_dashboard, user: user) }
+
+    before do
+      project.add_reporter(user)
+    end
+
+    context 'with correct permissions' do
+      context 'with valid parameters' do
+        context 'dashboard_path as url param url escaped' do
+          it 'deletes given user starred metrics dashboard', :aggregate_failures do
+            delete api(url, user), params: params
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['deleted_rows']).to eq(1)
+            expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard)
+          end
+        end
+
+        context 'dashboard_path in request body unescaped' do
+          let(:params) do
+            {
+              dashboard_path: dashboard
+            }
+          end
+
+          it 'deletes given user starred metrics dashboard', :aggregate_failures do
+            delete api(url, user), params: params
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['deleted_rows']).to eq(1)
+            expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard)
+          end
+        end
+
+        context 'dashboard_path has not been specified' do
+          it 'deletes all starred dashboards for that user within given project', :aggregate_failures do
+            delete api(url, user), params: {}
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['deleted_rows']).to eq(2)
+            expect(::Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard)
+          end
+        end
+      end
+
+      context 'with invalid parameters' do
+        context 'user is missing' do
+          it 'returns 404 not found' do
+            post api(url, nil), params: params
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+
+        context 'project is missing' do
+          it 'returns 404 not found' do
+            post api("/projects/#{project.id + 1}/user_starred_dashboards", user), params: params
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+      end
+    end
+
+    context 'without correct permissions' do
+      it 'returns 404 not found' do
+        post api(url, create(:user)), params: params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
 end