summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZeger-Jan van de Weg <zegerjan@gitlab.com>2016-02-26 09:55:43 +0100
committerZeger-Jan van de Weg <zegerjan@gitlab.com>2016-03-19 21:23:03 +0100
commit212e83bab3f1f9055f1321c3e934b4f4659250bf (patch)
treef3b67cf54ba43ca7d008c470beadfd12df3cfbb2
parent3f22a92f4a561543c2249786b695d0c65120455b (diff)
downloadgitlab-ce-212e83bab3f1f9055f1321c3e934b4f4659250bf.tar.gz
Soft delete issuables
-rw-r--r--app/controllers/projects/issues_controller.rb11
-rw-r--r--app/models/concerns/internal_id.rb7
-rw-r--r--app/models/issue.rb2
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--app/views/projects/issues/show.html.haml5
-rw-r--r--app/views/projects/merge_requests/show/_mr_title.html.haml6
-rw-r--r--config/routes.rb4
-rw-r--r--db/migrate/20160225090018_add_delete_at_to_issues.rb6
-rw-r--r--db/migrate/20160225101956_add_delete_at_to_merge_requests.rb6
-rw-r--r--db/schema.rb4
-rw-r--r--doc/api/issues.md44
-rw-r--r--doc/api/merge_requests.md62
-rw-r--r--lib/api/issues.rb9
-rw-r--r--lib/api/merge_requests.rb14
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb28
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb21
-rw-r--r--spec/models/issue_spec.rb5
-rw-r--r--spec/models/merge_request_spec.rb5
-rw-r--r--spec/requests/api/issues_spec.rb13
-rw-r--r--spec/requests/api/merge_requests_spec.rb18
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"