From 7948e0ef58e23336c4698d973b149886831755a4 Mon Sep 17 00:00:00 2001
From: Andrew Jung <ajung@gitlab.com>
Date: Thu, 28 Nov 2024 15:45:02 +0000
Subject: [PATCH] Revert "Merge branch
'480172/add-danger-for-internal-users-change' into 'master'"
This reverts merge request !171439
---
danger/internal_users/Dangerfile | 3 -
danger/plugins/internal_users.rb | 9 -
spec/tooling/danger/internal_users_spec.rb | 256 ---------------------
tooling/danger/internal_users.rb | 110 ---------
4 files changed, 378 deletions(-)
delete mode 100644 danger/internal_users/Dangerfile
delete mode 100644 danger/plugins/internal_users.rb
delete mode 100644 spec/tooling/danger/internal_users_spec.rb
delete mode 100644 tooling/danger/internal_users.rb
diff --git a/danger/internal_users/Dangerfile b/danger/internal_users/Dangerfile
deleted file mode 100644
index c86353394008b..0000000000000
--- a/danger/internal_users/Dangerfile
+++ /dev/null
@@ -1,3 +0,0 @@
-# frozen_string_literal: true
-
-internal_users.add_comment_for_internal_users_changes
diff --git a/danger/plugins/internal_users.rb b/danger/plugins/internal_users.rb
deleted file mode 100644
index 5506b026c5cbc..0000000000000
--- a/danger/plugins/internal_users.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../tooling/danger/internal_users'
-
-module Danger
- class InternalUsers < Plugin
- include Tooling::Danger::InternalUsers
- end
-end
diff --git a/spec/tooling/danger/internal_users_spec.rb b/spec/tooling/danger/internal_users_spec.rb
deleted file mode 100644
index 8e6b6dab3a4b4..0000000000000
--- a/spec/tooling/danger/internal_users_spec.rb
+++ /dev/null
@@ -1,256 +0,0 @@
-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-require 'gitlab/dangerfiles/spec_helper'
-
-require_relative '../../../tooling/danger/internal_users'
-require_relative '../../../tooling/danger/project_helper'
-
-RSpec.describe Tooling::Danger::InternalUsers, feature_category: :tooling do
- include_context "with dangerfile"
-
- let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
- let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) }
- let(:file_path) { 'app/models/user.rb' }
- let(:changed_lines) { [] }
- let(:file_lines) { [] }
-
- subject(:helper) { fake_danger.new(helper: fake_helper) }
-
- before do
- allow(helper).to receive(:project_helper).and_return(fake_project_helper)
- allow(fake_helper).to receive(:all_changed_files).and_return([file_path])
- allow(fake_helper).to receive(:changed_lines).with(file_path).and_return(changed_lines)
- allow(fake_project_helper).to receive(:file_lines).with(file_path).and_return(file_lines)
- end
-
- describe '#add_comment_for_internal_users_changes' do
- shared_examples 'no violations' do
- it 'adds no comments' do
- expect(helper).not_to receive(:markdown)
- expect(helper).not_to receive(:warn)
-
- helper.add_comment_for_internal_users_changes
- end
- end
-
- shared_examples 'has violations' do
- it 'adds file and MR level comments' do
- expect(helper).to receive(:markdown).once
- expect(helper).to receive(:warn).once
-
- helper.add_comment_for_internal_users_changes
- end
- end
-
- context 'when documentation is changed' do
- before do
- allow(fake_helper).to receive(:all_changed_files).and_return([described_class::DOCS_PATH])
- end
-
- include_examples 'no violations'
- end
-
- context 'when method name is changed' do
- let(:changed_lines) do
- [
- '- def security_bot',
- '+ def security_bot_v2'
- ]
- end
-
- let(:file_lines) { [' def security_bot_v2'] }
-
- include_examples 'has violations'
- end
-
- context 'when method body is changed' do
- let(:changed_lines) do
- [
- ' def security_bot',
- '+ super',
- ' end'
- ]
- end
-
- let(:file_lines) do
- [
- 'def security_bot',
- ' super',
- 'end'
- ]
- end
-
- include_examples 'has violations'
- end
-
- context 'when bot module is referenced' do
- let(:changed_lines) do
- [
- '+ Users::Internal.security_bot'
- ]
- end
-
- let(:file_lines) { [' Users::Internal.security_bot'] }
-
- include_examples 'has violations'
- end
-
- context 'when bot symbol is used' do
- let(:changed_lines) do
- [
- '+ user_type: :security_bot'
- ]
- end
-
- let(:file_lines) { [' user_type: :security_bot'] }
-
- include_examples 'has violations'
- end
-
- context 'when changes are unrelated' do
- let(:changed_lines) do
- [
- '+ user.name = "Regular User"'
- ]
- end
-
- let(:file_lines) { [' user.name = "Regular User"'] }
-
- include_examples 'no violations'
- end
-
- context 'when changing lines inside a bot method definition' do
- let(:changed_lines) do
- [
- ' user.name = "Bot Name"'
- ]
- end
-
- let(:file_lines) do
- [
- 'def security_bot',
- ' user.name = "Bot Name"',
- 'end'
- ]
- end
-
- include_examples 'has violations'
- end
-
- context 'when changing lines outside any bot method definition' do
- let(:changed_lines) do
- [
- ' user.name = "Regular Name"'
- ]
- end
-
- let(:file_lines) do
- [
- 'def regular_method',
- ' user.name = "Regular Name"',
- 'end'
- ]
- end
-
- include_examples 'no violations'
- end
-
- context 'when bot method spans multiple ends' do
- let(:changed_lines) do
- [
- ' user.tap do |u|',
- ' u.name = "Bot Name"',
- ' end'
- ]
- end
-
- let(:file_lines) do
- [
- 'def security_bot',
- ' user.tap do |u|',
- ' u.name = "Bot Name"',
- ' end',
- 'end'
- ]
- end
-
- include_examples 'has violations'
- end
-
- context 'when modifying code between bot methods' do
- let(:changed_lines) do
- [
- ' user.name = "Regular Name"'
- ]
- end
-
- let(:file_lines) do
- [
- 'def security_bot',
- ' # bot code',
- 'end',
- ' user.name = "Regular Name"',
- 'def alert_bot',
- ' # bot code',
- 'end'
- ]
- end
-
- include_examples 'no violations'
- end
- end
-
- describe '#file_has_violations?' do
- context 'when tracking bot method boundaries' do
- let(:changed_lines) { [' some_code'] }
-
- it 'correctly tracks entering and exiting bot methods' do
- allow(fake_project_helper).to receive(:file_lines).and_return([
- 'def security_bot',
- ' some_code',
- 'end'
- ])
-
- expect(helper.send(:file_has_violations?, file_path)).to be true
-
- allow(fake_project_helper).to receive(:file_lines).and_return([
- 'def security_bot',
- ' other_code',
- 'end',
- ' some_code'
- ])
-
- expect(helper.send(:file_has_violations?, file_path)).to be false
- end
-
- it 'handles nested end keywords' do
- allow(fake_project_helper).to receive(:file_lines).and_return([
- 'def security_bot',
- ' some_code',
- ' if true',
- ' some_code',
- ' end',
- 'end'
- ])
-
- expect(helper.send(:file_has_violations?, file_path)).to be true
- end
-
- it 'processes all lines in method body' do
- lines = [
- 'def security_bot',
- ' line1',
- ' line2',
- ' line3',
- 'end'
- ]
-
- allow(fake_project_helper).to receive(:file_lines).and_return(lines)
- allow(fake_helper).to receive(:changed_lines).and_return([' line2'])
-
- expect(helper.send(:file_has_violations?, file_path)).to be true
- end
- end
- end
-end
diff --git a/tooling/danger/internal_users.rb b/tooling/danger/internal_users.rb
deleted file mode 100644
index 082fe4391ec80..0000000000000
--- a/tooling/danger/internal_users.rb
+++ /dev/null
@@ -1,110 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'suggestor'
-
-module Tooling
- module Danger
- module InternalUsers
- include ::Tooling::Danger::Suggestor
-
- DOCS_PATH = 'doc/administration/internal_users.md'
-
- INLINE_COMMENT =
- <<~MESSAGE.freeze
- This line modifies internal user behavior or references an internal bot user.
- If this introduces new capabilities or changes existing behavior, please update the [internal users documentation](#{DOCS_PATH}).
-
- This comment will only appear once per file.
- MESSAGE
-
- MR_LEVEL_COMMENT =
- <<~SUGGEST_COMMENT.freeze
- This MR changes internal user behavior or usage.
- Please ensure [internal users documentation](#{DOCS_PATH}) is up to date.
-
- If this is not applicable, please ignore this message.
-
- Consider documenting:
- - Any changes to internal user behavior or capabilities
- - New use cases or integrations
- - Changes in how bots may interact with GitLab and its features
- SUGGEST_COMMENT
-
- BOT_METHODS = %w[
- support_bot
- alert_bot
- visual_review_bot
- ghost
- migration_bot
- security_bot
- automation_bot
- security_policy_bot
- admin_bot
- suggested_reviewers_bot
- llm_bot
- placeholder
- duo_code_review_bot
- import_user
- ].freeze
-
- BOT_NAMES = BOT_METHODS.join('|')
-
- BOT_METHOD_DEF_REGEX = /^\s*def\s+(?:self\.)?\s*(#{BOT_NAMES})(?!\w)/
- BOT_MODULE_CALL_REGEX = /^\s*Users::Internal\.(?:#{BOT_NAMES})(?!\w)/
- BOT_SYMBOL_TYPE_REGEX = /^\s*(?!#|")[^#"]*:(?:#{BOT_NAMES})(?!\w)/
-
- def add_comment_for_internal_users_changes
- return if helper.all_changed_files.include?(DOCS_PATH)
-
- violations_found = false
-
- helper.all_changed_files.each do |filename|
- next unless file_has_violations?(filename)
-
- violations_found = true
- markdown(INLINE_COMMENT, file: filename)
- end
-
- warn(MR_LEVEL_COMMENT) if violations_found
- end
-
- private
-
- def file_has_violations?(filename)
- changed_lines = helper.changed_lines(filename)
- file_lines = project_helper.file_lines(filename)
-
- in_bot_method = false
- has_violation = false
-
- changed_lines.each do |line|
- clean_line = line.delete_prefix('+').delete_prefix('-').strip
- next unless clean_line.match?(BOT_METHOD_DEF_REGEX) ||
- clean_line.match?(BOT_MODULE_CALL_REGEX) ||
- clean_line.match?(BOT_SYMBOL_TYPE_REGEX)
-
- has_violation = true
- break
- end
-
- return has_violation if has_violation
-
- file_lines.each_with_index do |line, _|
- if line.match?(BOT_METHOD_DEF_REGEX)
- in_bot_method = true
- elsif line.strip == 'end'
- in_bot_method = false
- end
-
- next unless in_bot_method
- next unless changed_lines.any? { |cl| cl.delete_prefix('+').delete_prefix('-').strip == line.strip }
-
- has_violation = true
- break
- end
-
- has_violation
- end
- end
- end
-end
--
GitLab