Merge branch 'fj-add-allowed-actions-to-snippet-input-action' into 'master'

Add allowed actions to snippet input action

See merge request gitlab-org/gitlab!34499

app/models/snippet_input_action.rb

@@ -18,12 +18,14 @@ class SnippetInputAction
   validates :file_path, presence: true
   validates :content, presence: true, if: -> (action) { action.create_action? || action.update_action? }
   validate :ensure_same_file_path_and_previous_path, if: :update_action?
+  validate :ensure_allowed_action
 
-  def initialize(action: nil, previous_path: nil, file_path: nil, content: nil)
+  def initialize(action: nil, previous_path: nil, file_path: nil, content: nil, allowed_actions: nil)
     @action = action&.to_sym
     @previous_path = previous_path
     @file_path = file_path
     @content = content
+    @allowed_actions = Array(allowed_actions).map(&:to_sym)
   end
 
   def to_commit_action
@@ -49,4 +51,12 @@ class SnippetInputAction
 
     errors.add(:file_path, "can't be different from the previous_path attribute")
   end
+
+  def ensure_allowed_action
+    return if @allowed_actions.empty?
+
+    unless @allowed_actions.include?(action)
+      errors.add(:action, 'is not allowed')
+    end
+  end
 end

app/models/snippet_input_action_collection.rb

@@ -7,8 +7,8 @@ class SnippetInputActionCollection
 
   delegate :empty?, :any?, :[], to: :actions
 
-  def initialize(actions = [])
-    @actions = actions.map { |action| SnippetInputAction.new(action) }
+  def initialize(actions = [], allowed_actions: nil)
+    @actions = actions.map { |action| SnippetInputAction.new(action.merge(allowed_actions: allowed_actions)) }
   end
 
   def to_commit_actions

changelogs/unreleased/fj-add-allowed-actions-to-snippet-input-action.yml

+---
+title: Add allowed actions to snippet input action
+merge_request: 34499
+author:
+type: changed

spec/models/snippet_input_action_collection_spec.rb

@@ -36,4 +36,12 @@ describe SnippetInputActionCollection do
       end
     end
   end
+
+  context 'when allowed_actions param is passed' do
+    it 'builds SnippetInputAction with that param' do
+      expect(SnippetInputAction).to receive(:new).with(hash_including(allowed_actions: :create))
+
+      described_class.new([action], allowed_actions: :create)
+    end
+  end
 end

spec/models/snippet_input_action_spec.rb

@@ -6,34 +6,42 @@ describe SnippetInputAction do
   describe 'validations' do
     using RSpec::Parameterized::TableSyntax
 
-    where(:action, :file_path, :content, :previous_path, :is_valid, :invalid_field) do
-      :create  | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      :move    | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      :delete  | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      :update  | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      :foo     | 'foobar'  | 'foobar' | 'foobar' | false | :action
-      'create' | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      'move'   | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      'delete' | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      'update' | 'foobar'  | 'foobar' | 'foobar' | true  | nil
-      'foo'    | 'foobar'  | 'foobar' | 'foobar' | false | :action
-      nil      | 'foobar'  | 'foobar' | 'foobar' | false | :action
-      ''       | 'foobar'  | 'foobar' | 'foobar' | false | :action
-      :move    | 'foobar'  | 'foobar' | nil      | false | :previous_path
-      :move    | 'foobar'  | 'foobar' | ''       | false | :previous_path
-      :create  | 'foobar'  | nil      | 'foobar' | false | :content
-      :create  | 'foobar'  | ''       | 'foobar' | false | :content
-      :create  | nil       | 'foobar' | 'foobar' | false | :file_path
-      :create  | ''        | 'foobar' | 'foobar' | false | :file_path
-      :update  | 'foobar'  | nil      | 'foobar' | false | :content
-      :update  | 'foobar'  | ''       | 'foobar' | false | :content
-      :update  | 'other'   | 'foobar' | 'foobar' | false | :file_path
-      :update  | 'foobar'  | 'foobar' | nil      | true  | nil
-      :update  | 'foobar'  | 'foobar' | ''       | true  | nil
+    where(:action, :file_path, :content, :previous_path, :allowed_actions, :is_valid, :invalid_field) do
+      :create  | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      :move    | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      :delete  | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      :update  | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      :foo     | 'foobar'  | 'foobar' | 'foobar' | nil                | false | :action
+      'create' | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      'move'   | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      'delete' | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      'update' | 'foobar'  | 'foobar' | 'foobar' | nil                | true  | nil
+      'foo'    | 'foobar'  | 'foobar' | 'foobar' | nil                | false | :action
+      nil      | 'foobar'  | 'foobar' | 'foobar' | nil                | false | :action
+      ''       | 'foobar'  | 'foobar' | 'foobar' | nil                | false | :action
+      :move    | 'foobar'  | 'foobar' | nil      | nil                | false | :previous_path
+      :move    | 'foobar'  | 'foobar' | ''       | nil                | false | :previous_path
+      :create  | 'foobar'  | nil      | 'foobar' | nil                | false | :content
+      :create  | 'foobar'  | ''       | 'foobar' | nil                | false | :content
+      :create  | nil       | 'foobar' | 'foobar' | nil                | false | :file_path
+      :create  | ''        | 'foobar' | 'foobar' | nil                | false | :file_path
+      :update  | 'foobar'  | nil      | 'foobar' | nil                | false | :content
+      :update  | 'foobar'  | ''       | 'foobar' | nil                | false | :content
+      :update  | 'other'   | 'foobar' | 'foobar' | nil                | false | :file_path
+      :update  | 'foobar'  | 'foobar' | nil      | nil                | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | nil                | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | :update            | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | 'update'           | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | [:update]          | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | [:update, :create] | true  | nil
+      :update  | 'foobar'  | 'foobar' | ''       | :create            | false | :action
+      :update  | 'foobar'  | 'foobar' | ''       | 'create'           | false | :action
+      :update  | 'foobar'  | 'foobar' | ''       | [:create]          | false | :action
+      :foo     | 'foobar'  | 'foobar' | ''       | :foo               | false | :action
     end
 
     with_them do
-      subject { described_class.new(action: action, file_path: file_path, content: content, previous_path: previous_path) }
+      subject { described_class.new(action: action, file_path: file_path, content: content, previous_path: previous_path, allowed_actions: allowed_actions) }
 
       specify do
         expect(subject.valid?).to be is_valid