Merge branch 'consistent-tags-api' into 'master'

Make tag API for release feature consistent

Make tags API consistent with other tags methods. This changes the endpoint from `PUT /projects/:id/repository/:tag/release` to `PUT /projects/:id/repository/tags/:tag_name/release`.

On thing the API is still missing, is an error if the tag does not exist. Right now it returns 200 even the tag does not exist. I'll fix that such it returns 404.

@stanhu Can you review?
@rspeicher This MR should go into 8.2

See merge request !1864

app/services/create_release_service.rb

+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_by(tag: tag_name)
+
+      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', 404)
+    end
+  end
+
+  def success(release)
+    out = super()
+    out[:release] = release
+    out
+  end
+end

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')

app/services/update_release_service.rb

+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

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",
@@ -84,23 +84,48 @@ 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
+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. If there
+already exists a release for the given tag, 409 is returned.
 
 ```
-PUT /projects/:id/repository/:tag/release
+POST /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_name": "1.0.0",
+  "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": "1.0.0",
+  "tag_name": "1.0.0",
   "description": "Amazing release. Wow"
 }
 ```
\ No newline at end of file

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

lib/api/tags.rb

@@ -44,17 +44,42 @@ 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:
+      #   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]
+        result = CreateReleaseService.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
+
+      # 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
-      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.update_attributes(description: params[:description])
+        result = UpdateReleaseService.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], result[:http_status])
+        end
       end
     end
   end

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)
@@ -119,17 +119,78 @@ describe API::API, api: true  do
     end
   end
 
-  describe 'PUT /projects/:id/repository/:tag/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/#{tag_name}/release", user),
+      post 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(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
+      post 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
+
+    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
+
+  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