Fix an error where we were unable to create a CommitStatus for running statefix-commit-status

3 files changed, 58 insertions, 36 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 132122d460d..1d13c9872cc 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -121,6 +121,7 @@ v 8.12.0 (unreleased)
   - Add notification_settings API calls !5632 (mahcsig)
 
 v 8.11.6 (unreleased)
+  - Fix an error where we were unable to create a CommitStatus for running state
 
 v 8.11.5
   - Optimize branch lookups and force a repository reload for Repository#find_branch. !6087
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 5e3c9563703..dfbdd597d29 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -37,7 +37,7 @@ module API
       #   id (required) - The ID of a project
       #   sha (required) - The commit hash
       #   ref (optional) - The ref
-      #   state (required) - The state of the status. Can be: pending, running, success, error or failure
+      #   state (required) - The state of the status. Can be: pending, running, success, failed or canceled
       #   target_url (optional) - The target URL to associate with this status
       #   description (optional) - A short description of the status
       #   name or context (optional) - A string label to differentiate this status from the status of other systems. Default: "default"
@@ -46,7 +46,7 @@ module API
       post ':id/statuses/:sha' do
         authorize! :create_commit_status, user_project
         required_attributes! [:state]
-        attrs = attributes_for_keys [:ref, :target_url, :description, :context, :name]
+        attrs = attributes_for_keys [:target_url, :description]
         commit = @project.commit(params[:sha])
         not_found! 'Commit' unless commit
 
@@ -58,36 +58,38 @@ module API
         # the first found branch on that commit
 
         ref = params[:ref]
-        unless ref
-          branches = @project.repository.branch_names_contains(commit.sha)
-          not_found! 'References for commit' if branches.none?
-          ref = branches.first
-        end
+        ref ||= @project.repository.branch_names_contains(commit.sha).first
+        not_found! 'References for commit' unless ref
+
+        name = params[:name] || params[:context] || 'default'
 
         pipeline = @project.ensure_pipeline(ref, commit.sha, current_user)
 
-        name = params[:name] || params[:context]
-        status = GenericCommitStatus.running_or_pending.find_by(pipeline: pipeline, name: name, ref: params[:ref])
-        status ||= GenericCommitStatus.new(project: @project, pipeline: pipeline, user: current_user)
-        status.update(attrs)
+        status = GenericCommitStatus.running_or_pending.find_or_initialize_by(
+          project: @project, pipeline: pipeline,
+          user: current_user, name: name, ref: ref)
+        status.attributes = attrs
 
-        case params[:state].to_s
-        when 'running'
-          status.run
-        when 'success'
-          status.success
-        when 'failed'
-          status.drop
-        when 'canceled'
-          status.cancel
-        else
-          status.status = params[:state].to_s
-        end
+        begin
+          case params[:state].to_s
+          when 'pending'
+            status.enqueue!
+          when 'running'
+            status.enqueue
+            status.run!
+          when 'success'
+            status.success!
+          when 'failed'
+            status.drop!
+          when 'canceled'
+            status.cancel!
+          else
+            render_api_error!('invalid state', 400)
+          end
 
-        if status.save
           present status, with: Entities::CommitStatus
-        else
-          render_validation_error!(status)
+        rescue StateMachines::InvalidTransition => e
+          render_api_error!(e.message, 400)
         end
       end
     end
diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index 2d6093fec7a..7aa7e85a9e2 100644
--- a/spec/requests/api/commit_statuses_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -117,17 +117,36 @@ describe API::CommitStatuses, api: true do
     let(:post_url) { "/projects/#{project.id}/statuses/#{sha}" }
 
     context 'developer user' do
-      context 'only required parameters' do
-        before { post api(post_url, developer), state: 'success' }
+      %w[pending running success failed canceled].each do |status|
+        context "for #{status}" do
+          context 'uses only required parameters' do
+            it 'creates commit status' do
+              post api(post_url, developer), state: status
+
+              expect(response).to have_http_status(201)
+              expect(json_response['sha']).to eq(commit.id)
+              expect(json_response['status']).to eq(status)
+              expect(json_response['name']).to eq('default')
+              expect(json_response['ref']).not_to be_empty
+              expect(json_response['target_url']).to be_nil
+              expect(json_response['description']).to be_nil
+            end
+          end
+        end
+      end
 
-        it 'creates commit status' do
-          expect(response).to have_http_status(201)
-          expect(json_response['sha']).to eq(commit.id)
-          expect(json_response['status']).to eq('success')
-          expect(json_response['name']).to eq('default')
-          expect(json_response['ref']).to be_nil
-          expect(json_response['target_url']).to be_nil
-          expect(json_response['description']).to be_nil
+      context 'transitions status from pending' do
+        before do
+          post api(post_url, developer), state: 'pending'
+        end
+
+        %w[running success failed canceled].each do |status|
+          it "to #{status}" do
+            expect { post api(post_url, developer), state: status }.not_to change { CommitStatus.count }
+
+            expect(response).to have_http_status(201)
+            expect(json_response['status']).to eq(status)
+          end
         end
       end