From 9acaa09ba87b3cbcdc371c32efad78fceaf1b370 Mon Sep 17 00:00:00 2001
From: Thomas Hutterer <thutterer@gitlab.com>
Date: Tue, 6 Aug 2024 19:15:06 +0000
Subject: [PATCH] Add Danger warning about changed SettingBlock instances
Settings sections can be searched through the command palette.
But this only works if they are kept in sync with the files that drive
that search.
---
danger/plugins/settings_sections.rb | 9 +++
danger/settings_sections/Dangerfile | 3 +
lib/search/group_settings.rb | 2 +
lib/search/project_settings.rb | 2 +
spec/tooling/danger/settings_sections_spec.rb | 57 +++++++++++++++++++
tooling/danger/settings_sections.rb | 55 ++++++++++++++++++
6 files changed, 128 insertions(+)
create mode 100644 danger/plugins/settings_sections.rb
create mode 100644 danger/settings_sections/Dangerfile
create mode 100644 spec/tooling/danger/settings_sections_spec.rb
create mode 100644 tooling/danger/settings_sections.rb
diff --git a/danger/plugins/settings_sections.rb b/danger/plugins/settings_sections.rb
new file mode 100644
index 000000000000..c58e2832e4af
--- /dev/null
+++ b/danger/plugins/settings_sections.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/settings_sections'
+
+module Danger
+ class SettingsSections < ::Danger::Plugin
+ include Tooling::Danger::SettingsSections
+ end
+end
diff --git a/danger/settings_sections/Dangerfile b/danger/settings_sections/Dangerfile
new file mode 100644
index 000000000000..05b5f8e1d5da
--- /dev/null
+++ b/danger/settings_sections/Dangerfile
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+settings_sections.check!
diff --git a/lib/search/group_settings.rb b/lib/search/group_settings.rb
index 32903860e7db..96c3fb8878b3 100644
--- a/lib/search/group_settings.rb
+++ b/lib/search/group_settings.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
module Search
+ # Generates a list of all available setting sections of a group.
+ # This list is used by the command palette's search functionality.
class GroupSettings
include Rails.application.routes.url_helpers
diff --git a/lib/search/project_settings.rb b/lib/search/project_settings.rb
index 413b69a0ac8e..645783c83b94 100644
--- a/lib/search/project_settings.rb
+++ b/lib/search/project_settings.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
module Search
+ # Generates a list of all available setting sections of a project.
+ # This list is used by the command palette's search functionality.
class ProjectSettings
include Rails.application.routes.url_helpers
diff --git a/spec/tooling/danger/settings_sections_spec.rb b/spec/tooling/danger/settings_sections_spec.rb
new file mode 100644
index 000000000000..1fbea0e1f3a1
--- /dev/null
+++ b/spec/tooling/danger/settings_sections_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'gitlab/dangerfiles/spec_helper'
+require 'fast_spec_helper'
+
+require_relative '../../../tooling/danger/settings_sections'
+
+RSpec.describe Tooling::Danger::SettingsSections, feature_category: :tooling do
+ include_context 'with dangerfile'
+
+ subject(:settings_section_check) { fake_danger.new(helper: fake_helper) }
+
+ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+ let(:matching_changed_files) { ['app/views/foo/bar.html.haml', 'app/assets/js/foo/bar.vue'] }
+ let(:changed_lines) { ['-render SettingsBlockComponent.new(id: "foo") do', '<settings-block id="foo">'] }
+ let(:stable_branch?) { false }
+
+ before do
+ allow(fake_helper).to receive(:changed_files).and_return(matching_changed_files)
+ allow(fake_helper).to receive(:changed_lines).and_return(changed_lines)
+ allow(fake_helper).to receive(:stable_branch?).and_return(stable_branch?)
+ end
+
+ context 'when on stable branch' do
+ let(:stable_branch?) { true }
+
+ it 'does not write any markdown' do
+ expect(settings_section_check).not_to receive(:markdown)
+ settings_section_check.check!
+ end
+ end
+
+ context 'when none of the changed files are Haml or Vue files' do
+ let(:matching_changed_files) { [] }
+
+ it 'does not write any markdown' do
+ expect(settings_section_check).not_to receive(:markdown)
+ settings_section_check.check!
+ end
+ end
+
+ context 'when none of the changed lines match the pattern' do
+ let(:changed_lines) { ['-foo', '+bar'] }
+
+ it 'does not write any markdown' do
+ expect(settings_section_check).not_to receive(:markdown)
+ settings_section_check.check!
+ end
+ end
+
+ it 'adds a new markdown section listing every matching line' do
+ expect(settings_section_check).to receive(:markdown).with(/Searchable setting sections/)
+ expect(settings_section_check).to receive(:markdown).with(/SettingsBlock/)
+ expect(settings_section_check).to receive(:markdown).with(/settings-block/)
+ settings_section_check.check!
+ end
+end
diff --git a/tooling/danger/settings_sections.rb b/tooling/danger/settings_sections.rb
new file mode 100644
index 000000000000..b05fb721e94e
--- /dev/null
+++ b/tooling/danger/settings_sections.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module Tooling
+ module Danger
+ module SettingsSections
+ def check!
+ return if helper.stable_branch?
+
+ changed_code_files = helper.changed_files(/\.(haml|vue)$/)
+ return if changed_code_files.empty?
+
+ vc_regexp = /(SettingsBlockComponent|settings-block)/
+ lines_with_matches = filter_changed_lines(changed_code_files, vc_regexp)
+ return if lines_with_matches.empty?
+
+ markdown(<<~MARKDOWN)
+ ## Searchable setting sections
+
+ Looks like you have edited the template of some settings section. Please check that all changed sections are still searchable:
+
+ - If you created a new section, make sure to add it to either `lib/search/project_settings.rb` or `lib/search/group_settings.rb`, or in their counterparts in `ee/` if this section is only available behind a licensed feature.
+ - If you removed a section, make sure to also remove it from the files above.
+ - If you changed a section's id, please update it also in the files above.
+ - If you just moved code around within the same page, there is nothing to do.
+
+ MARKDOWN
+
+ lines_with_matches.each do |file, lines|
+ markdown(<<~MARKDOWN)
+ #### `#{file}`
+
+ ```shell
+ #{lines.join("\n")}
+ ```
+
+ MARKDOWN
+ end
+ end
+
+ def filter_changed_lines(files, pattern)
+ files_with_lines = {}
+ files.each do |file|
+ next if file.start_with?('spec/', 'ee/spec/', 'qa/')
+
+ matching_changed_lines = helper.changed_lines(file).select { |line| line =~ pattern }
+ next unless matching_changed_lines.any?
+
+ files_with_lines[file] = matching_changed_lines
+ end
+
+ files_with_lines
+ end
+ end
+ end
+end
--
GitLab