summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-03-21 19:28:48 +0000
committerDouwe Maan <douwe@gitlab.com>2016-03-21 19:28:48 +0000
commit4a1680261513926f0fa4634bae02e8d599d47050 (patch)
tree8b48f8e7c8fbb570aa563c4a0f3444e8991a0f1a
parent642f56c6465b5f2e0f4e44e59e018391f43d1acf (diff)
parent6e5461c6ee8b408e35324b32c0b5ba99328ec763 (diff)
downloadgitlab-ce-4a1680261513926f0fa4634bae02e8d599d47050.tar.gz
Merge branch '2489-soft-delete-issues' into 'master'
Soft delete issuables Fixes #2489 What still needs to happen: research on the indexes, the gem suggests a [lot of changes](https://github.com/rubysherpas/paranoia#about-indexes) though this is probably a good idea to discuss and I'm unsure on the impact of an omnibus upgrade as I suspect creating about 10 new indexes has a large impact on the downtime. TODO: - [x] Also group owners can ***soft*** delete - [x] Button should be hidden See merge request !2982
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/concerns/issuable_actions.rb23
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/models/ability.rb4
-rw-r--r--app/models/concerns/internal_id.rb5
-rw-r--r--app/models/concerns/issuable.rb2
-rw-r--r--app/views/projects/issues/show.html.haml1
-rw-r--r--app/views/projects/merge_requests/show/_mr_title.html.haml2
-rw-r--r--app/views/shared/issuable/_form.html.haml10
-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.md18
-rw-r--r--doc/api/merge_requests.md19
-rw-r--r--lib/api/helpers.rb4
-rw-r--r--lib/api/issues.rb7
-rw-r--r--lib/api/merge_requests.rb12
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb33
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb23
-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.rb34
-rw-r--r--spec/requests/api/merge_requests_spec.rb37
25 files changed, 231 insertions, 38 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 6a6f0427745..20a21abfb69 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -61,6 +61,7 @@ v 8.6.0 (unreleased)
- User deletion is now done in the background so the request can not time out
- Canceled builds are now ignored in compound build status if marked as `allowed to fail`
- Trigger a todo for mentions on commits page
+ - Let project owners and admins soft delete issues and merge requests
v 8.5.8
- Bump Git version requirement to 2.7.4
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
new file mode 100644
index 00000000000..f40b62446e5
--- /dev/null
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -0,0 +1,23 @@
+module IssuableActions
+ extend ActiveSupport::Concern
+
+ included do
+ before_action :authorize_destroy_issuable!, only: :destroy
+ end
+
+ def destroy
+ issuable.destroy
+
+ name = issuable.class.name.titleize.downcase
+ flash[:notice] = "The #{name} was successfully deleted."
+ redirect_to polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable.class])
+ end
+
+ private
+
+ def authorize_destroy_issuable!
+ unless current_user.can?(:"destroy_#{issuable.to_ability_name}", issuable)
+ return access_denied!
+ end
+ end
+end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index a3c28a82d26..877b39c9b1b 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -1,5 +1,6 @@
class Projects::IssuesController < Projects::ApplicationController
include ToggleSubscriptionAction
+ include IssuableActions
before_action :module_enabled
before_action :issue, only: [:edit, :update, :show]
@@ -133,6 +134,7 @@ class Projects::IssuesController < Projects::ApplicationController
end
end
alias_method :subscribable_resource, :issue
+ alias_method :issuable, :issue
def authorize_read_issue!
return render_404 unless can?(current_user, :read_issue, @issue)
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 99cb5981940..b830d777752 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -1,6 +1,7 @@
class Projects::MergeRequestsController < Projects::ApplicationController
include ToggleSubscriptionAction
include DiffHelper
+ include IssuableActions
before_action :module_enabled
before_action :merge_request, only: [
@@ -255,6 +256,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
end
alias_method :subscribable_resource, :merge_request
+ alias_method :issuable, :merge_request
def closes_issues
@closes_issues ||= @merge_request.closes_issues
diff --git a/app/models/ability.rb b/app/models/ability.rb
index e22da4806e6..40c529e8117 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -235,7 +235,9 @@ class Ability
:rename_project,
:remove_project,
:archive_project,
- :remove_fork_project
+ :remove_fork_project,
+ :destroy_merge_request,
+ :destroy_issue
]
end
diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb
index 821ed54fb98..51288094ef1 100644
--- a/app/models/concerns/internal_id.rb
+++ b/app/models/concerns/internal_id.rb
@@ -7,7 +7,10 @@ module InternalId
end
def set_iid
- max_iid = project.send(self.class.name.tableize).maximum(:iid)
+ records = project.send(self.class.name.tableize)
+ records = records.with_deleted if self.paranoid?
+ max_iid = records.maximum(:iid)
+
self.iid = max_iid.to_i + 1
end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 9ab72652190..476e1ce7af0 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -58,6 +58,8 @@ module Issuable
attr_mentionable :description, cache: true
participant :author, :assignee, :notes_with_associations
strip_attributes :title
+
+ acts_as_paranoid
end
module ClassMethods
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 52df3de8a27..6fa059cbe68 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -45,7 +45,6 @@
- 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'
-
= 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..ab4b1f14be5 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,7 @@
- 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')
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'
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 1740b128ee4..551f0cc0b51 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -127,7 +127,11 @@
for this project.
- if issuable.new_record?
- - cancel_project = issuable.source_project
+ = link_to 'Cancel', namespace_project_issues_path(@project.namespace, @project), class: 'btn btn-cancel'
- else
- - cancel_project = issuable.project
- = link_to 'Cancel', [cancel_project.namespace.becomes(Namespace), cancel_project, issuable], class: 'btn btn-cancel'
+ .pull-right
+ - if current_user.can?(:"destroy_#{issuable.to_ability_name}", @project)
+ = link_to polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), method: :delete, class: 'btn btn-grouped' do
+ = icon('trash-o')
+ Delete
+ = link_to 'Cancel', namespace_project_issue_path(@project.namespace, @project, issuable), class: 'btn btn-grouped btn-cancel'
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 36b37d3944c..692f8e0616a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -418,6 +418,7 @@ ActiveRecord::Schema.define(version: 20160317092222) do
t.integer "updated_by_id"
t.integer "moved_to_id"
t.boolean "confidential", default: false
+ t.datetime "deleted_at"
end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -425,6 +426,7 @@ ActiveRecord::Schema.define(version: 20160317092222) 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
@@ -547,12 +549,14 @@ ActiveRecord::Schema.define(version: 20160317092222) 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..18d64c41986 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -326,17 +326,25 @@ Example response:
}
```
-## Delete existing issue (**Deprecated**)
+## Delete an 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 and project owners. Soft deletes the issue in question.
+If the operation is successful, a status code `200` is returned. In case you cannot
+destroy this issue, or it is not present, code `404` is given.
```
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
+```
+
## 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..b20a6300b7a 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -380,6 +380,25 @@ 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 merge request
+
+Only for admins and project owners. Soft deletes the merge request in question.
+If the operation is successful, a status code `200` is returned. In case you cannot
+destroy this merge request, or it is not present, code `404` is given.
+
+```
+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 |
+
+```bash
+curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/4/merge_request/85
+```
+
## Accept MR
Merge changes submitted with MR using this API.
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index a72044e8058..4921ae99e78 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -118,9 +118,7 @@ module API
end
def authorize!(action, subject)
- unless abilities.allowed?(current_user, action, subject)
- forbidden!
- end
+ forbidden! unless abilities.allowed?(current_user, action, subject)
end
def authorize_push_project
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index fda6f841438..e5ae88eb96f 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,10 @@ module API
# Example Request:
# DELETE /projects/:id/issues/:issue_id
delete ":id/issues/:issue_id" do
- not_allowed!
+ issue = user_project.issues.find_by(id: params[:issue_id])
+
+ authorize!(:destroy_issue, issue)
+ issue.destroy
end
end
end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index c5e5d57ed4d..93052fba06b 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -100,6 +100,18 @@ 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
+ merge_request = user_project.merge_requests.find_by(id: params[:merge_request_id])
+
+ authorize!(:destroy_merge_request, merge_request)
+ merge_request.destroy
+ 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 a49bd8960ee..d6e4cd71ce6 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_empty_repo) }
- let(:user) { create(:user) }
- let(:issue) { create(:issue, project: project) }
+ let(:project) { create(:project_empty_repo) }
+ 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,29 @@ describe Projects::IssuesController do
end
end
end
+
+ describe "DELETE #destroy" do
+ context "when the user is a developer" do
+ before { sign_in(user) }
+ it "rejects a developer to destroy an issue" do
+ delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+ expect(response.status).to eq(404)
+ end
+ end
+
+ context "when the user is owner" do
+ let(:owner) { create(:user) }
+ let(:namespace) { create(:namespace, owner: owner) }
+ let(:project) { create(:project, namespace: namespace) }
+
+ before { sign_in(owner) }
+
+ it "deletes the issue" do
+ delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+
+ expect(response.status).to eq(302)
+ expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./).now
+ 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..c5b034dc064 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -157,6 +157,29 @@ describe Projects::MergeRequestsController do
end
end
+ describe "DELETE #destroy" do
+ it "denies access to users unless they're admin or project owner" do
+ delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
+
+ expect(response.status).to eq(404)
+ end
+
+ context "when the user is owner" do
+ let(:owner) { create(:user) }
+ let(:namespace) { create(:namespace, owner: owner) }
+ let(:project) { create(:project, namespace: namespace) }
+
+ before { sign_in owner }
+
+ it "deletes the merge request" do
+ delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
+
+ expect(response.status).to eq(302)
+ expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./).now
+ 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 8334545b759..3c34b1d397f 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..ce55cb7b0ae 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -2,12 +2,12 @@ require 'spec_helper'
describe API::API, api: true do
include ApiHelpers
- let(:user) { create(:user) }
- let(:non_member) { create(:user) }
- let(:author) { create(:author) }
- let(:assignee) { create(:assignee) }
- let(:admin) { create(:admin) }
- let!(:project) { create(:project, :public, namespace: user.namespace ) }
+ let(:user) { create(:user) }
+ let(:non_member) { create(:user) }
+ let(:author) { create(:author) }
+ let(:assignee) { create(:assignee) }
+ let(:admin) { create(:user, :admin) }
+ let!(:project) { create(:project, :public, namespace: user.namespace ) }
let!(:closed_issue) do
create :closed_issue,
author: user,
@@ -469,9 +469,25 @@ describe API::API, api: true do
end
describe "DELETE /projects/:id/issues/:issue_id" do
- it "should delete a project issue" do
- delete api("/projects/#{project.id}/issues/#{issue.id}", user)
- expect(response.status).to eq(405)
+ it "rejects a non member from deleting an issue" do
+ delete api("/projects/#{project.id}/issues/#{issue.id}", non_member)
+ expect(response.status).to be(403)
+ end
+
+ it "rejects a developer from deleting an issue" do
+ delete api("/projects/#{project.id}/issues/#{issue.id}", author)
+ expect(response.status).to be(403)
+ end
+
+ context "when the user is project owner" do
+ let(:owner) { create(:user) }
+ let(:project) { create(:project, namespace: owner.namespace) }
+
+ it "deletes the issue if an admin requests it" do
+ delete api("/projects/#{project.id}/issues/#{issue.id}", owner)
+ expect(response.status).to eq(200)
+ expect(json_response['state']).to eq 'opened'
+ end
end
end
end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 4fd1df25568..c9175a4d6eb 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -2,15 +2,17 @@ require "spec_helper"
describe API::API, api: true do
include ApiHelpers
- let(:base_time) { Time.now }
- let(:user) { create(:user) }
- let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) }
+ let(:base_time) { Time.now }
+ let(:user) { create(:user) }
+ let(:admin) { create(:user, :admin) }
+ let(:non_member) { create(:user) }
+ let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds) }
- let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
- let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
- let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+ let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
+ let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
+ let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
before do
project.team << [user, :reporters]
@@ -315,6 +317,29 @@ describe API::API, api: true do
end
end
+ describe "DELETE /projects/:id/merge_requests/:merge_request_id" do
+ context "when the user is developer" do
+ let(:developer) { create(:user) }
+
+ before do
+ project.team << [developer, :developer]
+ end
+
+ it "denies the deletion of the merge request" do
+ delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", developer)
+ expect(response.status).to be(403)
+ end
+ end
+
+ context "when the user is project owner" do
+ it "destroys the merge request owners can destroy" do
+ delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+ expect(response.status).to eq(200)
+ end
+ 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"