Merge branch '212873-fix-can-edit-logic' into 'master'

Remove "Edit dashboard" button from self monitoring dashboard

See merge request gitlab-org/gitlab!35521

app/controllers/concerns/metrics_dashboard.rb

@@ -41,7 +41,7 @@ module MetricsDashboard
   end
 
   def amend_dashboard(dashboard)
-    project_dashboard = project_for_dashboard && !dashboard[:system_dashboard]
+    project_dashboard = project_for_dashboard && !dashboard[:out_of_the_box_dashboard]
 
     dashboard[:can_edit] = project_dashboard ? can_edit?(dashboard) : false
     dashboard[:project_blob_path] = project_dashboard ? dashboard_project_blob_path(dashboard) : nil

app/services/metrics/dashboard/base_service.rb

@@ -37,6 +37,14 @@ module Metrics
         Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard }
       end
 
+      # Should return true if this dashboard service is for an out-of-the-box
+      # dashboard.
+      # This method is overridden in app/services/metrics/dashboard/predefined_dashboard_service.rb.
+      # @return Boolean
+      def self.out_of_the_box_dashboard?
+        false
+      end
+
       private
 
       # Determines whether users should be able to view

app/services/metrics/dashboard/custom_dashboard_service.rb

@@ -21,7 +21,8 @@ module Metrics
                 path: filepath,
                 display_name: name_for_path(filepath),
                 default: false,
-                system_dashboard: false
+                system_dashboard: false,
+                out_of_the_box_dashboard: out_of_the_box_dashboard?
               }
             end
         end

app/services/metrics/dashboard/predefined_dashboard_service.rb

@@ -24,6 +24,10 @@ module Metrics
         def matching_dashboard?(filepath)
           filepath == self::DASHBOARD_PATH
         end
+
+        def out_of_the_box_dashboard?
+          true
+        end
       end
 
       private

app/services/metrics/dashboard/self_monitoring_dashboard_service.rb

@@ -26,7 +26,8 @@ module Metrics
             path: DASHBOARD_PATH,
             display_name: _(DASHBOARD_NAME),
             default: true,
-            system_dashboard: false
+            system_dashboard: false,
+            out_of_the_box_dashboard: out_of_the_box_dashboard?
           }]
         end
 

app/services/metrics/dashboard/system_dashboard_service.rb

@@ -25,7 +25,8 @@ module Metrics
             path: DASHBOARD_PATH,
             display_name: _(DASHBOARD_NAME),
             default: true,
-            system_dashboard: true
+            system_dashboard: true,
+            out_of_the_box_dashboard: out_of_the_box_dashboard?
           }]
         end
       end

changelogs/unreleased/212873-fix-can-edit-logic.yml

+---
+title: Remove Edit dashboard button from self monitoring dashboard
+merge_request: 35521
+author:
+type: fixed

spec/controllers/concerns/metrics_dashboard_spec.rb

@@ -88,13 +88,28 @@ RSpec.describe MetricsDashboard do
 
         context 'in all_dashboard list' do
           let(:system_dashboard) { json_response['all_dashboards'].find { |dashboard| dashboard["system_dashboard"] == true } }
-          let(:project_dashboard) { json_response['all_dashboards'].find { |dashboard| dashboard["system_dashboard"] == false } }
+
+          let(:project_dashboard) do
+            json_response['all_dashboards'].find do |dashboard|
+              dashboard['path'] == '.gitlab/dashboards/test.yml'
+            end
+          end
 
           it 'includes project_blob_path only for project dashboards' do
             expect(system_dashboard['project_blob_path']).to be_nil
             expect(project_dashboard['project_blob_path']).to eq("/#{project.namespace.path}/#{project.name}/-/blob/master/.gitlab/dashboards/test.yml")
           end
 
+          it 'allows editing only for project dashboards' do
+            expect(system_dashboard['can_edit']).to be(false)
+            expect(project_dashboard['can_edit']).to be(true)
+          end
+
+          it 'includes out_of_the_box_dashboard key' do
+            expect(system_dashboard['out_of_the_box_dashboard']).to be(true)
+            expect(project_dashboard['out_of_the_box_dashboard']).to be(false)
+          end
+
           describe 'project permissions' do
             using RSpec::Parameterized::TableSyntax
 

spec/lib/gitlab/metrics/dashboard/finder_spec.rb

@@ -142,7 +142,7 @@ RSpec.describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store
 
   describe '.find_all_paths' do
     let(:all_dashboard_paths) { described_class.find_all_paths(project) }
-    let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default dashboard', default: true, system_dashboard: true } }
+    let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default dashboard', default: true, system_dashboard: true, out_of_the_box_dashboard: true } }
 
     it 'includes only the system dashboard by default' do
       expect(all_dashboard_paths).to eq([system_dashboard])
@@ -153,7 +153,7 @@ RSpec.describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store
       let(:project) { project_with_dashboard(dashboard_path) }
 
       it 'includes system and project dashboards' do
-        project_dashboard = { path: dashboard_path, display_name: 'test.yml', default: false, system_dashboard: false }
+        project_dashboard = { path: dashboard_path, display_name: 'test.yml', default: false, system_dashboard: false, out_of_the_box_dashboard: false }
 
         expect(all_dashboard_paths).to contain_exactly(system_dashboard, project_dashboard)
       end
@@ -165,7 +165,8 @@ RSpec.describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store
           path: self_monitoring_dashboard_path,
           display_name: 'Default dashboard',
           default: true,
-          system_dashboard: false
+          system_dashboard: false,
+          out_of_the_box_dashboard: true
         }
       end
       let(:dashboard_path) { '.gitlab/dashboards/test.yml' }
@@ -180,7 +181,8 @@ RSpec.describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store
           path: dashboard_path,
           display_name: 'test.yml',
           default: false,
-          system_dashboard: false
+          system_dashboard: false,
+          out_of_the_box_dashboard: false
         }
 
         expect(all_dashboard_paths).to contain_exactly(self_monitoring_dashboard, project_dashboard)

spec/services/metrics/dashboard/custom_dashboard_service_spec.rb

@@ -99,7 +99,8 @@ RSpec.describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memo
             path: dashboard_path,
             display_name: 'test.yml',
             default: false,
-            system_dashboard: false
+            system_dashboard: false,
+            out_of_the_box_dashboard: false
           }]
         )
       end

spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb

@@ -44,7 +44,8 @@ RSpec.describe Metrics::Dashboard::SelfMonitoringDashboardService, :use_clean_ra
           path: described_class::DASHBOARD_PATH,
           display_name: described_class::DASHBOARD_NAME,
           default: true,
-          system_dashboard: false
+          system_dashboard: false,
+          out_of_the_box_dashboard: true
         }]
       )
     end

spec/services/metrics/dashboard/system_dashboard_service_spec.rb

@@ -47,7 +47,8 @@ RSpec.describe Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memo
           path: described_class::DASHBOARD_PATH,
           display_name: described_class::DASHBOARD_NAME,
           default: true,
-          system_dashboard: true
+          system_dashboard: true,
+          out_of_the_box_dashboard: true
         }]
       )
     end