diff options
author | Stan Hu <stanhu@gmail.com> | 2015-11-22 01:27:48 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2015-11-22 01:27:48 +0300 |
commit | 9155c82fdc075b7a3ef7f45224cbe5382c2ec7f9 (patch) | |
tree | e73f9e005e8ff3067faeeaaceef4332e840cd183 | |
parent | 319808fc7be2886224541209eeae153d85386bd0 (diff) | |
parent | 04a3d27eaba0312d99e8d88a3a9ee4b5c83ecce1 (diff) |
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
-rw-r--r-- | app/services/create_release_service.rb | 31 | ||||
-rw-r--r-- | app/services/create_tag_service.rb | 10 | ||||
-rw-r--r-- | app/services/update_release_service.rb | 29 | ||||
-rw-r--r-- | doc/api/tags.md | 39 | ||||
-rw-r--r-- | lib/api/entities.rb | 3 | ||||
-rw-r--r-- | lib/api/tags.rb | 37 | ||||
-rw-r--r-- | spec/requests/api/tags_spec.rb | 71 |
7 files changed, 196 insertions, 24 deletions
diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb new file mode 100644 index 00000000000..e06a6f2f47a --- /dev/null +++ b/app/services/create_release_service.rb @@ -0,0 +1,31 @@ +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 diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index 9917119fce2..2452999382a 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/app/services/update_release_service.rb b/app/services/update_release_service.rb new file mode 100644 index 00000000000..25eb13ef09a --- /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 b5b90cf6b82..085d387e824 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", @@ -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": "1.0.0", + "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_name": "1.0.0", + "description": "Amazing release. Wow" +} +```
\ No newline at end of file diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3da6bc415d6..5dea74db295 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 673342dd447..47621f443e6 100644 --- a/lib/api/tags.rb +++ b/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 diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index cc9a5f47582..17f2643fd45 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) @@ -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 |