Merge branch 'prefer-assign_to' into feature/runner-lock-on-project

* prefer-assign_to:
  Give 409 Conflict whenever the runner was already enabled
  We're checking return value rather than rescuing exceptions
  Prefer Runner#assign_to instead of creating directly

4 files changed, 8 insertions, 4 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 39c8450caf3..884ae250018 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -40,6 +40,7 @@ v 8.9.0 (unreleased)
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
   - Todos will display target state if issuable target is 'Closed' or 'Merged'
   - Fix bug when sorting issues by milestone due date and filtering by two or more labels
+  - POST to API /projects/:id/runners/:runner_id would give 409 if the runner was already enabled for this project
   - Add support for using Yubikeys (U2F) for two-factor authentication
   - Link to blank group icon doesn't throw a 404 anymore
   - Remove 'main language' feature
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 101817e1f56..df343fc957a 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -63,7 +63,7 @@ module Ci
     def assign_to(project, current_user = nil)
       self.is_shared = false if shared?
       self.save
-      project.runner_projects.create!(runner_id: self.id)
+      project.runner_projects.create(runner_id: self.id).persisted?
     end
 
     def display_name
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 3ae228d61d8..2c2610fc2e7 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -96,9 +96,12 @@ module API
 
         runner = get_runner(params[:runner_id])
         authenticate_enable_runner!(runner)
-        Ci::RunnerProject.create(runner: runner, project: user_project)
 
-        present runner, with: Entities::Runner
+        if runner.assign_to(user_project)
+          present runner, with: Entities::Runner
+        else
+          conflict!("Runner was already enabled for this project")
+        end
       end
 
       # Disable project's runner
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 26dfa7bed05..b4c826522a5 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -379,7 +379,7 @@ describe API::Runners, api: true  do
         expect do
           post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id
         end.to change{ project.runners.count }.by(0)
-        expect(response.status).to eq(201)
+        expect(response.status).to eq(409)
       end
 
       it 'should not enable locked runner' do