diff options
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 11 | ||||
-rw-r--r-- | app/models/concerns/internal_id.rb | 7 | ||||
-rw-r--r-- | app/models/issue.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | app/views/projects/issues/show.html.haml | 5 | ||||
-rw-r--r-- | app/views/projects/merge_requests/show/_mr_title.html.haml | 6 | ||||
-rw-r--r-- | config/routes.rb | 4 | ||||
-rw-r--r-- | db/migrate/20160225090018_add_delete_at_to_issues.rb | 6 | ||||
-rw-r--r-- | db/migrate/20160225101956_add_delete_at_to_merge_requests.rb | 6 | ||||
-rw-r--r-- | db/schema.rb | 4 | ||||
-rw-r--r-- | doc/api/issues.md | 44 | ||||
-rw-r--r-- | doc/api/merge_requests.md | 62 | ||||
-rw-r--r-- | lib/api/issues.rb | 9 | ||||
-rw-r--r-- | lib/api/merge_requests.rb | 14 | ||||
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 28 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 21 | ||||
-rw-r--r-- | spec/models/issue_spec.rb | 5 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 5 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 13 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 18 |
20 files changed, 254 insertions, 18 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 6603f28a082..2367ad20574 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -108,6 +108,17 @@ class Projects::IssuesController < Projects::ApplicationController end end + def destroy + return access_denied! unless current_user.admin? + + issue.destroy + + respond_to do |format| + format.html { redirect_to namespace_project_issues_path(@project.namespace, @project), notice: "The issues was deleted." } + format.json { head :ok } + end + end + def bulk_update result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute redirect_back_or_default(default: { action: 'index' }, options: { notice: "#{result[:count]} issues updated" }) diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb index 821ed54fb98..6fc12202dde 100644 --- a/app/models/concerns/internal_id.rb +++ b/app/models/concerns/internal_id.rb @@ -7,7 +7,12 @@ module InternalId end def set_iid - max_iid = project.send(self.class.name.tableize).maximum(:iid) + max_iid = case self.class + when Issue, MergeRequest + project.send(self.class.name.tableize).with_deleted.maximum(:iid) + else + project.send(self.class.name.tableize).maximum(:iid) + end self.iid = max_iid.to_i + 1 end diff --git a/app/models/issue.rb b/app/models/issue.rb index 5347d4fa1be..4bd91d57ffc 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -54,6 +54,8 @@ class Issue < ActiveRecord::Base state :closed end + acts_as_paranoid + def hook_attrs attributes end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a015a9ef394..5488aa685f1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -142,6 +142,8 @@ class MergeRequest < ActiveRecord::Base scope :join_project, -> { joins(:target_project) } scope :references_project, -> { references(:target_project) } + acts_as_paranoid + def self.reference_prefix '!' end diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 52df3de8a27..2f0a91267eb 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -45,7 +45,10 @@ - if can?(current_user, :update_issue, @issue) = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' - + - if current_user.admin? + = link_to namespace_project_issue_path(@project.namespace, @project, @issue), method: :delete, class: 'btn btn-grouped' do + = icon('trash-o') + Delete = link_to edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'btn btn-nr btn-grouped issuable-edit' do = icon('pencil-square-o') Edit diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index eeb605e2dc5..3a7208cb3e8 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -29,7 +29,11 @@ - if @merge_request.open? = link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, class: 'btn btn-nr btn-grouped btn-close', title: 'Close merge request' = link_to edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-nr btn-grouped issuable-edit', id: 'edit_merge_request' do - %i.fa.fa-pencil-square-o + =icon('pencil-square-o') #%i.fa.fa-pencil-square-o Edit - if @merge_request.closed? = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: 'Reopen merge request' + - if current_user.admin? + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), method: :delete, class: 'btn btn-grouped' do + = icon('trash-o') + Delete diff --git a/config/routes.rb b/config/routes.rb index 561987322b2..90d858d7fc1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -613,7 +613,7 @@ Rails.application.routes.draw do end end - resources :merge_requests, constraints: { id: /\d+/ }, except: [:destroy] do + resources :merge_requests, constraints: { id: /\d+/ } do member do get :commits get :diffs @@ -684,7 +684,7 @@ Rails.application.routes.draw do end end - resources :issues, constraints: { id: /\d+/ }, except: [:destroy] do + resources :issues, constraints: { id: /\d+/ } do member do post :toggle_subscription end diff --git a/db/migrate/20160225090018_add_delete_at_to_issues.rb b/db/migrate/20160225090018_add_delete_at_to_issues.rb new file mode 100644 index 00000000000..3ddbef92978 --- /dev/null +++ b/db/migrate/20160225090018_add_delete_at_to_issues.rb @@ -0,0 +1,6 @@ +class AddDeleteAtToIssues < ActiveRecord::Migration + def change + add_column :issues, :deleted_at, :datetime + add_index :issues, :deleted_at + end +end diff --git a/db/migrate/20160225101956_add_delete_at_to_merge_requests.rb b/db/migrate/20160225101956_add_delete_at_to_merge_requests.rb new file mode 100644 index 00000000000..9d09105f17d --- /dev/null +++ b/db/migrate/20160225101956_add_delete_at_to_merge_requests.rb @@ -0,0 +1,6 @@ +class AddDeleteAtToMergeRequests < ActiveRecord::Migration + def change + add_column :merge_requests, :deleted_at, :datetime + add_index :merge_requests, :deleted_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 5b2f5aa3ddd..a4a17770cd6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -417,6 +417,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.integer "iid" t.integer "updated_by_id" t.boolean "confidential", default: false + t.datetime "deleted_at" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -424,6 +425,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree add_index "issues", ["created_at", "id"], name: "index_issues_on_created_at_and_id", using: :btree add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree + add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree @@ -546,12 +548,14 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.boolean "merge_when_build_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" + t.datetime "deleted_at" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree add_index "merge_requests", ["created_at", "id"], name: "index_merge_requests_on_created_at_and_id", using: :btree add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree + add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree diff --git a/doc/api/issues.md b/doc/api/issues.md index 9e704648b25..5df7ce9f006 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -326,17 +326,51 @@ Example response: } ``` -## Delete existing issue (**Deprecated**) +## Delete existing issue -This call is deprecated and returns a `405 Method Not Allowed` error if called. -An issue gets now closed and is done by calling -`PUT /projects/:id/issues/:issue_id` with the parameter `state_event` set to -`close`. See [edit issue](#edit-issue) for more details. +Only for admins. Soft deletes the issue in question. Returns the issue which was deleted. ``` DELETE /projects/:id/issues/:issue_id ``` +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | + +```bash +curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/4/issues/85 +``` + +Example response: + +```json +{ + "created_at" : "2016-01-07T12:46:01.410Z", + "author" : { + "name" : "Alexandra Bashirian", + "avatar_url" : null, + "username" : "eileen.lowe", + "id" : 18, + "state" : "active", + "web_url" : "https://gitlab.example.com/u/eileen.lowe" + }, + "state" : "closed", + "title" : "Issues with auth", + "project_id" : 4, + "description" : null, + "updated_at" : "2016-01-07T12:55:16.213Z", + "iid" : 15, + "labels" : [ + "bug" + ], + "id" : 85, + "assignee" : null, + "milestone" : null +} +``` + ## Comments on issues Comments are done via the [notes](notes.md) resource. diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 5c527d55481..977e16b8601 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -380,6 +380,68 @@ Parameters: If the operation is successful, 200 and the updated merge request is returned. If an error occurs, an error number and a message explaining the reason is returned. +## Delete a MR + +Soft deletes a merge request. For admins only. + +``` +DELETE /projects/:id/merge_requests/:merge_request_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge request | + +Example response: + +```json +{ + "id": 1, + "target_branch": "master", + "source_branch": "test1", + "project_id": 3, + "title": "test1", + "state": "merged", + "upvotes": 0, + "downvotes": 0, + "author": { + "id": 1, + "username": "admin", + "email": "admin@example.com", + "name": "Administrator", + "state": "active", + "created_at": "2012-04-29T08:46:00Z" + }, + "assignee": { + "id": 1, + "username": "admin", + "email": "admin@example.com", + "name": "Administrator", + "state": "active", + "created_at": "2012-04-29T08:46:00Z" + }, + "source_project_id": 4, + "target_project_id": 4, + "labels": [ ], + "description":"fixed login page css paddings", + "work_in_progress": false, + "milestone": { + "id": 5, + "iid": 1, + "project_id": 4, + "title": "v2.0", + "description": "Assumenda aut placeat expedita exercitationem labore sunt enim earum.", + "state": "closed", + "created_at": "2015-02-02T19:49:26.013Z", + "updated_at": "2015-02-02T19:49:26.013Z", + "due_date": null + }, + "merge_when_build_succeeds": true, + "merge_status": "can_be_merged" +} +``` + ## Accept MR Merge changes submitted with MR using this API. diff --git a/lib/api/issues.rb b/lib/api/issues.rb index fda6f841438..5b47bbc0b1b 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -191,7 +191,7 @@ module API end end - # Delete a project issue (deprecated) + # Delete a project issue # # Parameters: # id (required) - The ID of a project @@ -199,7 +199,12 @@ module API # Example Request: # DELETE /projects/:id/issues/:issue_id delete ":id/issues/:issue_id" do - not_allowed! + authenticated_as_admin! + + issue = user_project.issues.find(params[:issue_id]) + issue.destroy + + present issue, with: Entities::Issue end end end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index c5e5d57ed4d..09ce02b0912 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -100,6 +100,20 @@ module API end end + # Delete a MR + # + # Parameters: + # id (required) - The ID of the project + # merge_request_id (required) - The MR id + delete ":id/merge_requests/:merge_request_id" do + authenticated_as_admin! + + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request.destroy + + present merge_request, with: Entities::MergeRequest + end + # Routing "merge_request/:merge_request_id/..." is DEPRECATED and WILL BE REMOVED in version 9.0 # Use "merge_requests/:merge_request_id/..." instead. # diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 2cd81231144..62643a755b7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,11 +1,11 @@ require('spec_helper') describe Projects::IssuesController do - describe "GET #index" do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:issue) { create(:issue, project: project) } + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + describe "GET #index" do before do sign_in(user) project.team << [user, :developer] @@ -186,4 +186,24 @@ describe Projects::IssuesController do end end end + + describe "DELETE #destroy" do + it "rejects a developer to destory an issue" do + delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid + expect(response.status).to eq 404 + end + + context "user is an admin" do + before do + user.admin = true + user.save + end + + it "lets an admin delete an issue" do + delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid + + expect(response.status).to eq 302 + end + end + end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index e82fe26c7a6..6c1aa8368ea 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -157,6 +157,27 @@ describe Projects::MergeRequestsController do end end + describe "DELETE #destroy" do + it "lets mere mortals not acces this endpoint" do + delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid + + expect(response.status).to eq 404 + end + + context "user is an admin" do + before do + user.admin = true + user.save + end + + it "lets an admin delete an issue" do + delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid + + expect(response.status).to be 302 + end + end + end + describe 'GET diffs' do def go(format: 'html') get :diffs, diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 540a62eb1f8..5107e9a5030 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -37,6 +37,11 @@ describe Issue, models: true do subject { create(:issue) } + describe "act_as_paranoid" do + it { is_expected.to have_db_column(:deleted_at) } + it { is_expected.to have_db_index(:deleted_at) } + end + describe '#to_reference' do it 'returns a String reference to the object' do expect(subject.to_reference).to eq "##{subject.iid}" diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index f2f07e4ee17..bd0a4ebe337 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -49,6 +49,11 @@ describe MergeRequest, models: true do it { is_expected.to include_module(Taskable) } end + describe "act_as_paranoid" do + it { is_expected.to have_db_column(:deleted_at) } + it { is_expected.to have_db_index(:deleted_at) } + end + describe 'validation' do it { is_expected.to validate_presence_of(:target_branch) } it { is_expected.to validate_presence_of(:source_branch) } diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index bb2ab058003..7d6ebc9b76e 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -469,9 +469,18 @@ describe API::API, api: true do end describe "DELETE /projects/:id/issues/:issue_id" do - it "should delete a project issue" do + it "should reject non admins form deleting an issue" do delete api("/projects/#{project.id}/issues/#{issue.id}", user) - expect(response.status).to eq(405) + expect(response.status).to eq(403) + end + + it "deletes the issue if an admin requests it" do + user.admin = true + user.save + + delete api("/projects/#{project.id}/issues/#{issue.id}", user) + expect(response.status).to eq(200) + expect(json_response['state']).to eq 'opened' end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4fd1df25568..5a0bc4ff076 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -315,6 +315,24 @@ describe API::API, api: true do end end + describe "DELETE /projects/:id/merge_request/:merge_request_id" do + it "rejects non admin users from deletions" do + delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + + expect(response.status).to eq(403) + end + + it "let's Admins delete a merge request" do + user.admin = true + user.save + + delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + + expect(response.status).to eq(200) + expect(json_response['id']).to eq merge_request.id + end + end + describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do it "should return merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close" |