From 2cba93a0d2d12ee36bf98569e5c6c14ac7ea40e0 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Sat, 21 Nov 2015 17:29:26 +0100
Subject: [PATCH 1/5] Make tag API consistent for release feature

---
 doc/api/tags.md                | 10 +++++-----
 lib/api/entities.rb            |  3 ++-
 lib/api/tags.rb                |  8 ++++----
 spec/requests/api/tags_spec.rb |  6 +++---
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/doc/api/tags.md b/doc/api/tags.md
index b5b90cf6b8..cf95f87dc3 100644
--- a/doc/api/tags.md
+++ b/doc/api/tags.md
@@ -29,7 +29,7 @@ Parameters:
       ]
     },
     "release": {
-      "tag": "1.0.0",
+      "tag_name": "1.0.0",
       "description": "Amazing release. Wow"
     },
     "name": "v1.0.0",
@@ -70,7 +70,7 @@ Parameters:
     ]
   },
   "release": {
-    "tag": "1.0.0",
+    "tag_name": "1.0.0",
     "description": "Amazing release. Wow"
   },
   "name": "v1.0.0",
@@ -89,18 +89,18 @@ It returns 200 if the operation succeed. In case of an error,
 Add release notes to the existing git tag
 
 ```
-PUT /projects/:id/repository/:tag/release
+PUT /projects/:id/repository/tags/:tag_name/release
 ```
 
 Parameters:
 
 - `id` (required) - The ID of a project
-- `tag` (required) - The name of a tag
+- `tag_name` (required) - The name of a tag
 - `description` (required) - Release notes with markdown support
 
 ```json
 {
-  "tag": "1.0.0",
+  "tag_name": "1.0.0",
   "description": "Amazing release. Wow"
 }
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3da6bc415d..5dea74db29 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -322,7 +322,8 @@ module API
     end
 
     class Release < Grape::Entity
-      expose :tag, :description
+      expose :tag, as: :tag_name
+      expose :description
     end
 
     class RepoTag < Grape::Entity
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index 673342dd44..2c6c73da08 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -44,14 +44,14 @@ module API
       #
       # Parameters:
       #   id (required) - The ID of a project
-      #   tag (required) - The name of the tag
+      #   tag_name (required) - The name of the tag
       #   description (required) - Release notes with markdown support
       # Example Request:
-      #   PUT /projects/:id/repository/tags
-      put ':id/repository/:tag/release', requirements: { tag: /.*/ } do
+      #   PUT /projects/:id/repository/tags/:tag_name/release
+      put ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
         authorize_push_project
         required_attributes! [:description]
-        release = user_project.releases.find_or_initialize_by(tag: params[:tag])
+        release = user_project.releases.find_or_initialize_by(tag: params[:tag_name])
         release.update_attributes(description: params[:description])
 
         present release, with: Entities::Release
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index cc9a5f4758..e3fd2d547c 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -119,16 +119,16 @@ describe API::API, api: true  do
     end
   end
 
-  describe 'PUT /projects/:id/repository/:tag/release' do
+  describe 'PUT /projects/:id/repository/tags/:tag_name/release' do
     let(:tag_name) { project.repository.tag_names.first }
     let(:description) { 'Awesome release!' }
 
     it 'should create description for existing git tag' do
-      put api("/projects/#{project.id}/repository/#{tag_name}/release", user),
+      put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
         description: description
 
       expect(response.status).to eq(200)
-      expect(json_response['tag']).to eq(tag_name)
+      expect(json_response['tag_name']).to eq(tag_name)
       expect(json_response['description']).to eq(description)
     end
   end

From faef95af1a07bdcfd02eead36d144f332b428f1f Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Sat, 21 Nov 2015 18:08:45 +0100
Subject: [PATCH 2/5] API: Return 404 if the tag for a release does not exist

---
 app/services/create_release_service.rb | 25 +++++++++++++++++++++++++
 app/services/create_tag_service.rb     | 10 +++++-----
 doc/api/tags.md                        |  3 ++-
 lib/api/tags.rb                        | 10 +++++++---
 spec/requests/api/tags_spec.rb         |  8 ++++++++
 5 files changed, 47 insertions(+), 9 deletions(-)
 create mode 100644 app/services/create_release_service.rb

diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb
new file mode 100644
index 0000000000..355374ce25
--- /dev/null
+++ b/app/services/create_release_service.rb
@@ -0,0 +1,25 @@
+require_relative 'base_service'
+
+class CreateReleaseService < BaseService
+  def execute(tag_name, release_description)
+
+    repository = project.repository
+    existing_tag = repository.find_tag(tag_name)
+
+    # Only create a release if the tag exists
+    if existing_tag
+      release = project.releases.find_or_initialize_by(tag: tag_name)
+      release.update_attributes(description: release_description)
+
+      success(release)
+    else
+      error('Tag does not exist')
+    end
+  end
+
+  def success(release)
+    out = super()
+    out[:release] = release
+    out
+  end
+end
diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb
index 9917119fce..2452999382 100644
--- a/app/services/create_tag_service.rb
+++ b/app/services/create_tag_service.rb
@@ -19,16 +19,16 @@ class CreateTagService < BaseService
     new_tag = repository.find_tag(tag_name)
 
     if new_tag
-      if release_description
-        release = project.releases.find_or_initialize_by(tag: tag_name)
-        release.update_attributes(description: release_description)
-      end
-
       push_data = create_push_data(project, current_user, new_tag)
       EventCreateService.new.push(project, current_user, push_data)
       project.execute_hooks(push_data.dup, :tag_push_hooks)
       project.execute_services(push_data.dup, :tag_push_hooks)
 
+      if release_description
+        CreateReleaseService.new(@project, @current_user).
+          execute(tag_name, release_description)
+      end
+
       success(new_tag)
     else
       error('Invalid reference name')
diff --git a/doc/api/tags.md b/doc/api/tags.md
index cf95f87dc3..ca9e2cef65 100644
--- a/doc/api/tags.md
+++ b/doc/api/tags.md
@@ -86,7 +86,8 @@ It returns 200 if the operation succeed. In case of an error,
 
 ## New release
 
-Add release notes to the existing git tag
+Add release notes to the existing git tag. It returns 200 if the release is
+created successfully. If the tag does not exist, 404 is returned.
 
 ```
 PUT /projects/:id/repository/tags/:tag_name/release
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index 2c6c73da08..0721b9cc84 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -51,10 +51,14 @@ module API
       put ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
         authorize_push_project
         required_attributes! [:description]
-        release = user_project.releases.find_or_initialize_by(tag: params[:tag_name])
-        release.update_attributes(description: params[:description])
+        result = CreateReleaseService.new(user_project, current_user).
+          execute(params[:tag_name], params[:description])
 
-        present release, with: Entities::Release
+        if result[:status] == :success
+          present result[:release], with: Entities::Release
+        else
+          render_api_error!(result[:message], 404)
+        end
       end
     end
   end
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index e3fd2d547c..4dac3eec84 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -131,5 +131,13 @@ describe API::API, api: true  do
       expect(json_response['tag_name']).to eq(tag_name)
       expect(json_response['description']).to eq(description)
     end
+
+    it 'should return 404 if the tag does not exist' do
+      put api("/projects/#{project.id}/repository/tags/foobar/release", user),
+        description: description
+
+      expect(response.status).to eq(404)
+      expect(json_response['message']).to eq('Tag does not exist')
+    end
   end
 end

From 6f7e90f6dba300591281aba08ffbe30ce3cc5c90 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Sat, 21 Nov 2015 18:51:41 +0100
Subject: [PATCH 3/5] Use POST to create a new release instead of PUT

---
 doc/api/tags.md                | 6 +++---
 lib/api/tags.rb                | 2 +-
 spec/requests/api/tags_spec.rb | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/api/tags.md b/doc/api/tags.md
index ca9e2cef65..ab13511725 100644
--- a/doc/api/tags.md
+++ b/doc/api/tags.md
@@ -84,13 +84,13 @@ It returns 200 if the operation succeed. In case of an error,
 405 with an explaining error message is returned.
 
 
-## New release
+## Create a new release
 
-Add release notes to the existing git tag. It returns 200 if the release is
+Add release notes to the existing git tag. It returns 201 if the release is
 created successfully. If the tag does not exist, 404 is returned.
 
 ```
-PUT /projects/:id/repository/tags/:tag_name/release
+POST /projects/:id/repository/tags/:tag_name/release
 ```
 
 Parameters:
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index 0721b9cc84..48f630d58e 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -48,7 +48,7 @@ module API
       #   description (required) - Release notes with markdown support
       # Example Request:
       #   PUT /projects/:id/repository/tags/:tag_name/release
-      put ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
+      post ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
         authorize_push_project
         required_attributes! [:description]
         result = CreateReleaseService.new(user_project, current_user).
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index 4dac3eec84..0ee819ae44 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -119,21 +119,21 @@ describe API::API, api: true  do
     end
   end
 
-  describe 'PUT /projects/:id/repository/tags/:tag_name/release' do
+  describe 'POST /projects/:id/repository/tags/:tag_name/release' do
     let(:tag_name) { project.repository.tag_names.first }
     let(:description) { 'Awesome release!' }
 
     it 'should create description for existing git tag' do
-      put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
+      post api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
         description: description
 
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(201)
       expect(json_response['tag_name']).to eq(tag_name)
       expect(json_response['description']).to eq(description)
     end
 
     it 'should return 404 if the tag does not exist' do
-      put api("/projects/#{project.id}/repository/tags/foobar/release", user),
+      post api("/projects/#{project.id}/repository/tags/foobar/release", user),
         description: description
 
       expect(response.status).to eq(404)

From 3ea05c5b5b253de33d8bf8d615c66e2935b940ef Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Sat, 21 Nov 2015 22:24:34 +0100
Subject: [PATCH 4/5] Only allow to create a release if it does not exist yet

---
 app/services/create_release_service.rb | 14 ++++++++++----
 doc/api/tags.md                        |  3 ++-
 lib/api/tags.rb                        |  4 ++--
 spec/requests/api/tags_spec.rb         | 17 ++++++++++++++++-
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb
index 355374ce25..54e87478e6 100644
--- a/app/services/create_release_service.rb
+++ b/app/services/create_release_service.rb
@@ -8,12 +8,18 @@ class CreateReleaseService < BaseService
 
     # Only create a release if the tag exists
     if existing_tag
-      release = project.releases.find_or_initialize_by(tag: tag_name)
-      release.update_attributes(description: release_description)
+      release = project.releases.find_by(tag: tag_name)
 
-      success(release)
+      if(release)
+        error('Release already exists', 409)
+      else
+        release = project.releases.new({ tag: tag_name, description: release_description })
+        release.save
+
+        success(release)
+      end
     else
-      error('Tag does not exist')
+      error('Tag does not exist', 404)
     end
   end
 
diff --git a/doc/api/tags.md b/doc/api/tags.md
index ab13511725..bc61aa1118 100644
--- a/doc/api/tags.md
+++ b/doc/api/tags.md
@@ -87,7 +87,8 @@ It returns 200 if the operation succeed. In case of an error,
 ## Create a new release
 
 Add release notes to the existing git tag. It returns 201 if the release is
-created successfully. If the tag does not exist, 404 is returned.
+created successfully. If the tag does not exist, 404 is returned. If there
+already exists a release for the given tag, 409 is returned.
 
 ```
 POST /projects/:id/repository/tags/:tag_name/release
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index 48f630d58e..e46d9bb96f 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -47,7 +47,7 @@ module API
       #   tag_name (required) - The name of the tag
       #   description (required) - Release notes with markdown support
       # Example Request:
-      #   PUT /projects/:id/repository/tags/:tag_name/release
+      #   POST /projects/:id/repository/tags/:tag_name/release
       post ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
         authorize_push_project
         required_attributes! [:description]
@@ -57,7 +57,7 @@ module API
         if result[:status] == :success
           present result[:release], with: Entities::Release
         else
-          render_api_error!(result[:message], 404)
+          render_api_error!(result[:message], result[:http_status])
         end
       end
     end
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index 0ee819ae44..0a456fc3b5 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -28,10 +28,10 @@ describe API::API, api: true  do
       before do
         release = project.releases.find_or_initialize_by(tag: tag_name)
         release.update_attributes(description: description)
-        get api("/projects/#{project.id}/repository/tags", user)
       end
 
       it "should return an array of project tags with release info" do
+        get api("/projects/#{project.id}/repository/tags", user)
         expect(response.status).to eq(200)
         expect(json_response).to be_an Array
         expect(json_response.first['name']).to eq(tag_name)
@@ -139,5 +139,20 @@ describe API::API, api: true  do
       expect(response.status).to eq(404)
       expect(json_response['message']).to eq('Tag does not exist')
     end
+
+    context 'on tag with existing release' do
+      before do
+        release = project.releases.find_or_initialize_by(tag: tag_name)
+        release.update_attributes(description: description)
+      end
+
+      it 'should return 409 if there is already a release' do
+        post api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
+          description: description
+
+        expect(response.status).to eq(409)
+        expect(json_response['message']).to eq('Release already exists')
+      end
+    end
   end
 end

From 04a3d27eaba0312d99e8d88a3a9ee4b5c83ecce1 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Sat, 21 Nov 2015 22:34:53 +0100
Subject: [PATCH 5/5] Allow editing a release in API via PUT method

---
 app/services/create_release_service.rb |  2 +-
 app/services/update_release_service.rb | 29 ++++++++++++++++++++
 doc/api/tags.md                        | 23 ++++++++++++++++
 lib/api/tags.rb                        | 21 ++++++++++++++
 spec/requests/api/tags_spec.rb         | 38 ++++++++++++++++++++++++++
 5 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 app/services/update_release_service.rb

diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb
index 54e87478e6..e06a6f2f47 100644
--- a/app/services/create_release_service.rb
+++ b/app/services/create_release_service.rb
@@ -10,7 +10,7 @@ class CreateReleaseService < BaseService
     if existing_tag
       release = project.releases.find_by(tag: tag_name)
 
-      if(release)
+      if release
         error('Release already exists', 409)
       else
         release = project.releases.new({ tag: tag_name, description: release_description })
diff --git a/app/services/update_release_service.rb b/app/services/update_release_service.rb
new file mode 100644
index 0000000000..25eb13ef09
--- /dev/null
+++ b/app/services/update_release_service.rb
@@ -0,0 +1,29 @@
+require_relative 'base_service'
+
+class UpdateReleaseService < BaseService
+  def execute(tag_name, release_description)
+
+    repository = project.repository
+    existing_tag = repository.find_tag(tag_name)
+
+    if existing_tag
+      release = project.releases.find_by(tag: tag_name)
+
+      if release
+        release.update_attributes(description: release_description)
+
+        success(release)
+      else
+        error('Release does not exist', 404)
+      end
+    else
+      error('Tag does not exist', 404)
+    end
+  end
+
+  def success(release)
+    out = super()
+    out[:release] = release
+    out
+  end
+end
diff --git a/doc/api/tags.md b/doc/api/tags.md
index bc61aa1118..085d387e82 100644
--- a/doc/api/tags.md
+++ b/doc/api/tags.md
@@ -106,3 +106,26 @@ Parameters:
   "description": "Amazing release. Wow"
 }
 ```
+
+## Update a release
+
+Updates the release notes of a given release. It returns 200 if the release is
+successfully updated. If the tag or the release does not exist, it returns 404
+with a proper error message.
+
+```
+PUT /projects/:id/repository/tags/:tag_name/release
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `tag_name` (required) - The name of a tag
+- `description` (required) - Release notes with markdown support
+
+```json
+{
+  "tag_name": "1.0.0",
+  "description": "Amazing release. Wow"
+}
+```
\ No newline at end of file
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index e46d9bb96f..47621f443e 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -60,6 +60,27 @@ module API
           render_api_error!(result[:message], result[:http_status])
         end
       end
+
+      # Updates a release notes of a tag
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   tag_name (required) - The name of the tag
+      #   description (required) - Release notes with markdown support
+      # Example Request:
+      #   PUT /projects/:id/repository/tags/:tag_name/release
+      put ':id/repository/tags/:tag_name/release', requirements: { tag_name: /.*/ } do
+        authorize_push_project
+        required_attributes! [:description]
+        result = UpdateReleaseService.new(user_project, current_user).
+          execute(params[:tag_name], params[:description])
+
+        if result[:status] == :success
+          present result[:release], with: Entities::Release
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
     end
   end
 end
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index 0a456fc3b5..17f2643fd4 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -155,4 +155,42 @@ describe API::API, api: true  do
       end
     end
   end
+
+  describe 'PUT id/repository/tags/:tag_name/release' do
+    let(:tag_name) { project.repository.tag_names.first }
+    let(:description) { 'Awesome release!' }
+    let(:new_description) { 'The best release!' }
+
+    context 'on tag with existing release' do
+      before do
+        release = project.releases.find_or_initialize_by(tag: tag_name)
+        release.update_attributes(description: description)
+      end
+
+      it 'should update the release description' do
+        put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
+          description: new_description
+
+        expect(response.status).to eq(200)
+        expect(json_response['tag_name']).to eq(tag_name)
+        expect(json_response['description']).to eq(new_description)
+      end
+    end
+
+    it 'should return 404 if the tag does not exist' do
+      put api("/projects/#{project.id}/repository/tags/foobar/release", user),
+        description: new_description
+
+      expect(response.status).to eq(404)
+      expect(json_response['message']).to eq('Tag does not exist')
+    end
+
+    it 'should return 404 if the release does not exist' do
+      put api("/projects/#{project.id}/repository/tags/#{tag_name}/release", user),
+        description: new_description
+
+      expect(response.status).to eq(404)
+      expect(json_response['message']).to eq('Release does not exist')
+    end
+  end
 end