Soft delete issuables

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" })

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
 

app/models/issue.rb

@@ -54,6 +54,8 @@ class Issue < ActiveRecord::Base
     state :closed
   end
 
+  acts_as_paranoid
+
   def hook_attrs
     attributes
   end

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

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

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

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

db/migrate/20160225090018_add_delete_at_to_issues.rb

+class AddDeleteAtToIssues < ActiveRecord::Migration
+  def change
+    add_column :issues, :deleted_at, :datetime
+    add_index :issues, :deleted_at
+  end
+end

db/migrate/20160225101956_add_delete_at_to_merge_requests.rb

+class AddDeleteAtToMergeRequests < ActiveRecord::Migration
+  def change
+    add_column :merge_requests, :deleted_at, :datetime
+    add_index :merge_requests, :deleted_at
+  end
+end

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

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.

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.

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

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.
       #

spec/controllers/projects/issues_controller_spec.rb

 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) }
 
+  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

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,

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}"

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

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

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"