Merge branch '42545-milestion-quick-actions-for-groups' into 'master'

Resolve "Milestone Quick Action not displayed with no project milestones but with group milestones"

Closes #42545

See merge request gitlab-org/gitlab-ce!17239

9 files changed, 75 insertions, 60 deletions

diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index 1ae2c40872a..e61ecb696d0 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -50,16 +50,7 @@ module Projects
 
       return [] unless noteable&.is_a?(Issuable)
 
-      opts = {
-        project: project,
-        issuable: noteable,
-        current_user: current_user
-      }
-      QuickActions::InterpretService.command_definitions.map do |definition|
-        next unless definition.available?(opts)
-
-        definition.to_h(opts)
-      end.compact
+      QuickActions::InterpretService.new(project, current_user).available_commands(noteable)
     end
   end
 end
diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb
index 669c1ba0a22..1e9bd84e749 100644
--- a/app/services/quick_actions/interpret_service.rb
+++ b/app/services/quick_actions/interpret_service.rb
@@ -7,6 +7,18 @@ module QuickActions
     SHRUG = '¯\\＿(ツ)＿/¯'.freeze
     TABLEFLIP = '(╯°□°)╯︵ ┻━┻'.freeze
 
+    # Takes an issuable and returns an array of all the available commands
+    # represented with .to_h
+    def available_commands(issuable)
+      @issuable = issuable
+
+      self.class.command_definitions.map do |definition|
+        next unless definition.available?(self)
+
+        definition.to_h(self)
+      end.compact
+    end
+
     # Takes a text and interprets the commands that are extracted from it.
     # Returns the content without commands, and hash of changes to be applied to a record.
     def execute(content, issuable)
@@ -15,8 +27,8 @@ module QuickActions
       @issuable = issuable
       @updates = {}
 
-      content, commands = extractor.extract_commands(content, context)
-      extract_updates(commands, context)
+      content, commands = extractor.extract_commands(content)
+      extract_updates(commands)
 
       [content, @updates]
     end
@@ -28,8 +40,8 @@ module QuickActions
 
       @issuable = issuable
 
-      content, commands = extractor.extract_commands(content, context)
-      commands = explain_commands(commands, context)
+      content, commands = extractor.extract_commands(content)
+      commands = explain_commands(commands)
       [content, commands]
     end
 
@@ -157,11 +169,11 @@ module QuickActions
     params '%"milestone"'
     condition do
       current_user.can?(:"admin_#{issuable.to_ability_name}", project) &&
-        project.milestones.active.any?
+        find_milestones(project, state: 'active').any?
     end
     parse_params do |milestone_param|
       extract_references(milestone_param, :milestone).first ||
-        project.milestones.find_by(title: milestone_param.strip)
+        find_milestones(project, title: milestone_param.strip).first
     end
     command :milestone do |milestone|
       @updates[:milestone_id] = milestone.id if milestone
@@ -544,6 +556,10 @@ module QuickActions
       users
     end
 
+    def find_milestones(project, params = {})
+      MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute
+    end
+
     def find_labels(labels_param)
       extract_references(labels_param, :label) |
         LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute
@@ -557,21 +573,21 @@ module QuickActions
       find_labels(labels_param).map(&:id)
     end
 
-    def explain_commands(commands, opts)
+    def explain_commands(commands)
       commands.map do |name, arg|
         definition = self.class.definition_by_name(name)
         next unless definition
 
-        definition.explain(self, opts, arg)
+        definition.explain(self, arg)
       end.compact
     end
 
-    def extract_updates(commands, opts)
+    def extract_updates(commands)
       commands.each do |name, arg|
         definition = self.class.definition_by_name(name)
         next unless definition
 
-        definition.execute(self, opts, arg)
+        definition.execute(self, arg)
       end
     end
 
@@ -581,14 +597,5 @@ module QuickActions
 
       ext.references(type)
     end
-
-    def context
-      {
-        issuable: issuable,
-        current_user: current_user,
-        project: project,
-        params: params
-      }
-    end
   end
 end
diff --git a/changelogs/unreleased/42545-milestion-quick-actions-for-groups.yml b/changelogs/unreleased/42545-milestion-quick-actions-for-groups.yml
new file mode 100644
index 00000000000..d29f79aaaf8
--- /dev/null
+++ b/changelogs/unreleased/42545-milestion-quick-actions-for-groups.yml
@@ -0,0 +1,5 @@
+---
+title: Allows the usage of /milestone quick action for group milestones
+merge_request: 17239
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb
index 3937d9c153a..96415271316 100644
--- a/lib/gitlab/quick_actions/command_definition.rb
+++ b/lib/gitlab/quick_actions/command_definition.rb
@@ -24,15 +24,14 @@ module Gitlab
         action_block.nil?
       end
 
-      def available?(opts)
+      def available?(context)
         return true unless condition_block
 
-        context = OpenStruct.new(opts)
         context.instance_exec(&condition_block)
       end
 
-      def explain(context, opts, arg)
-        return unless available?(opts)
+      def explain(context, arg)
+        return unless available?(context)
 
         if explanation.respond_to?(:call)
           execute_block(explanation, context, arg)
@@ -41,15 +40,13 @@ module Gitlab
         end
       end
 
-      def execute(context, opts, arg)
-        return if noop? || !available?(opts)
+      def execute(context, arg)
+        return if noop? || !available?(context)
 
         execute_block(action_block, context, arg)
       end
 
-      def to_h(opts)
-        context = OpenStruct.new(opts)
-
+      def to_h(context)
         desc = description
         if desc.respond_to?(:call)
           desc = context.instance_exec(&desc) rescue ''
diff --git a/lib/gitlab/quick_actions/dsl.rb b/lib/gitlab/quick_actions/dsl.rb
index 536765305e1..d82dccd0db5 100644
--- a/lib/gitlab/quick_actions/dsl.rb
+++ b/lib/gitlab/quick_actions/dsl.rb
@@ -62,9 +62,8 @@ module Gitlab
 
         # Allows to define conditions that must be met in order for the command
         # to be returned by `.command_names` & `.command_definitions`.
-        # It accepts a block that will be evaluated with the context given to
-        # `CommandDefintion#to_h`.
-        #
+        # It accepts a block that will be evaluated with the context
+        # of a QuickActions::InterpretService instance
         # Example:
         #
         #   condition do
diff --git a/lib/gitlab/quick_actions/extractor.rb b/lib/gitlab/quick_actions/extractor.rb
index c0878a34fb1..075ff91700c 100644
--- a/lib/gitlab/quick_actions/extractor.rb
+++ b/lib/gitlab/quick_actions/extractor.rb
@@ -29,7 +29,7 @@ module Gitlab
       # commands = extractor.extract_commands(msg) #=> [['labels', '~foo ~"bar baz"']]
       # msg #=> "hello\nworld"
       # ```
-      def extract_commands(content, opts = {})
+      def extract_commands(content)
         return [content, []] unless content
 
         content = content.dup
@@ -37,7 +37,7 @@ module Gitlab
         commands = []
 
         content.delete!("\r")
-        content.gsub!(commands_regex(opts)) do
+        content.gsub!(commands_regex) do
           if $~[:cmd]
             commands << [$~[:cmd], $~[:arg]].reject(&:blank?)
             ''
@@ -60,8 +60,8 @@ module Gitlab
       # It looks something like:
       #
       #   /^\/(?<cmd>close|reopen|...)(?:( |$))(?<arg>[^\/\n]*)(?:\n|$)/
-      def commands_regex(opts)
-        names = command_names(opts).map(&:to_s)
+      def commands_regex
+        names = command_names.map(&:to_s)
 
         @commands_regex ||= %r{
             (?<code>
@@ -133,7 +133,7 @@ module Gitlab
         [content, commands]
       end
 
-      def command_names(opts)
+      def command_names
         command_definitions.flat_map do |command|
           next if command.noop?
 
diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb
index f44a562dc63..b03c1e23ca3 100644
--- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb
+++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb
@@ -40,7 +40,7 @@ describe Gitlab::QuickActions::CommandDefinition do
   end
 
   describe "#available?" do
-    let(:opts) { { go: false } }
+    let(:opts) { OpenStruct.new(go: false) }
 
     context "when the command has a condition block" do
       before do
@@ -78,7 +78,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       it "doesn't execute the command" do
         expect(context).not_to receive(:instance_exec)
 
-        subject.execute(context, {}, nil)
+        subject.execute(context, nil)
 
         expect(context.run).to be false
       end
@@ -95,7 +95,7 @@ describe Gitlab::QuickActions::CommandDefinition do
         end
 
         it "doesn't execute the command" do
-          subject.execute(context, {}, nil)
+          subject.execute(context, nil)
 
           expect(context.run).to be false
         end
@@ -109,7 +109,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -117,7 +117,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be true
             end
@@ -131,7 +131,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -139,7 +139,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "doesn't execute the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be false
             end
@@ -153,7 +153,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -161,7 +161,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be true
             end
@@ -175,7 +175,7 @@ describe Gitlab::QuickActions::CommandDefinition do
           end
 
           it 'executes the command passing the parsed param' do
-            subject.execute(context, {}, 'something   ')
+            subject.execute(context, 'something   ')
 
             expect(context.received_arg).to eq('something')
           end
@@ -192,7 +192,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'returns nil' do
-        result = subject.explain({}, {}, nil)
+        result = subject.explain({}, nil)
 
         expect(result).to be_nil
       end
@@ -204,7 +204,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'returns this static string' do
-        result = subject.explain({}, {}, nil)
+        result = subject.explain({}, nil)
 
         expect(result).to eq 'Explanation'
       end
@@ -216,7 +216,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'invokes the proc' do
-        result = subject.explain({}, {}, 'explanation')
+        result = subject.explain({}, 'explanation')
 
         expect(result).to eq 'Dynamic explanation'
       end
diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb
index ff59dc48bcb..067a30fd7e2 100644
--- a/spec/lib/gitlab/quick_actions/dsl_spec.rb
+++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb
@@ -76,7 +76,7 @@ describe Gitlab::QuickActions::Dsl do
 
       expect(dynamic_description_def.name).to eq(:dynamic_description)
       expect(dynamic_description_def.aliases).to eq([])
-      expect(dynamic_description_def.to_h(noteable: 'issue')[:description]).to eq('A dynamic description for ISSUE')
+      expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE')
       expect(dynamic_description_def.explanation).to eq('')
       expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument'])
       expect(dynamic_description_def.condition_block).to be_nil
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index ae160d104f1..f793f55e51b 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -522,6 +522,22 @@ describe QuickActions::InterpretService do
       let(:issuable) { merge_request }
     end
 
+    context 'only group milestones available' do
+      let(:group) { create(:group) }
+      let(:project) { create(:project, :public, namespace: group) }
+      let(:milestone) { create(:milestone, group: group, title: '10.0') }
+
+      it_behaves_like 'milestone command' do
+        let(:content) { "/milestone %#{milestone.title}" }
+        let(:issuable) { issue }
+      end
+
+      it_behaves_like 'milestone command' do
+        let(:content) { "/milestone %#{milestone.title}" }
+        let(:issuable) { merge_request }
+      end
+    end
+
     it_behaves_like 'remove_milestone command' do
       let(:content) { '/remove_milestone' }
       let(:issuable) { issue }