Commit 1d778228 authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

minor improvements and fixed specs

parent 7342a456
...@@ -118,9 +118,7 @@ module API ...@@ -118,9 +118,7 @@ module API
end end
def authorize!(action, subject) def authorize!(action, subject)
unless abilities.allowed?(current_user, action, subject) forbidden! unless abilities.allowed?(current_user, action, subject)
forbidden!
end
end end
def authorize_push_project def authorize_push_project
......
...@@ -200,7 +200,8 @@ module API ...@@ -200,7 +200,8 @@ module API
# DELETE /projects/:id/issues/:issue_id # DELETE /projects/:id/issues/:issue_id
delete ":id/issues/:issue_id" do delete ":id/issues/:issue_id" do
issue = user_project.issues.find(params[:issue_id]) issue = user_project.issues.find(params[:issue_id])
!JLJsdf sdfijsf current_user.can?(:remove_issue, issue)
authorize!(:remove_issue, issue)
issue = user_project.issues.find(params[:issue_id]) issue = user_project.issues.find(params[:issue_id])
issue.destroy issue.destroy
......
...@@ -106,9 +106,9 @@ module API ...@@ -106,9 +106,9 @@ module API
# id (required) - The ID of the project # id (required) - The ID of the project
# merge_request_id (required) - The MR id # merge_request_id (required) - The MR id
delete ":id/merge_requests/:merge_request_id" do delete ":id/merge_requests/:merge_request_id" do
authenticated_as_admin!
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = user_project.merge_requests.find(params[:merge_request_id])
authorize!(:remove_merge_request, merge_request)
merge_request.destroy merge_request.destroy
present merge_request, with: Entities::MergeRequest present merge_request, with: Entities::MergeRequest
......
...@@ -164,7 +164,7 @@ describe Projects::MergeRequestsController do ...@@ -164,7 +164,7 @@ describe Projects::MergeRequestsController do
expect(response.status).to eq 404 expect(response.status).to eq 404
end end
context "user is an admin" do context "user is an admin or owner" do
before do before do
user.admin = true user.admin = true
user.save user.save
......
...@@ -2,12 +2,12 @@ require 'spec_helper' ...@@ -2,12 +2,12 @@ require 'spec_helper'
describe API::API, api: true do describe API::API, api: true do
include ApiHelpers include ApiHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
let(:author) { create(:author) } let(:author) { create(:author) }
let(:assignee) { create(:assignee) } let(:assignee) { create(:assignee) }
let(:admin) { create(:admin) } let(:admin) { create(:user, :admin) }
let!(:project) { create(:project, :public, namespace: user.namespace ) } let!(:project) { create(:project, :public, namespace: user.namespace ) }
let!(:closed_issue) do let!(:closed_issue) do
create :closed_issue, create :closed_issue,
author: user, author: user,
...@@ -469,16 +469,18 @@ describe API::API, api: true do ...@@ -469,16 +469,18 @@ describe API::API, api: true do
end end
describe "DELETE /projects/:id/issues/:issue_id" do describe "DELETE /projects/:id/issues/:issue_id" do
it "should reject non admins form deleting an issue" do it "should reject a non member from deleting an issue" do
delete api("/projects/#{project.id}/issues/#{issue.id}", user) delete api("/projects/#{project.id}/issues/#{issue.id}", non_member)
expect(response.status).to eq(403) expect(response.status).to be(403)
end end
it "deletes the issue if an admin requests it" do it "should reject a developer from deleting an issue" do
user.admin = true delete api("/projects/#{project.id}/issues/#{issue.id}", author)
user.save expect(response.status).to be(403)
end
delete api("/projects/#{project.id}/issues/#{issue.id}", user) it "deletes the issue if an admin requests it" do
delete api("/projects/#{project.id}/issues/#{issue.id}", admin)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response['state']).to eq 'opened' expect(json_response['state']).to eq 'opened'
end end
......
...@@ -3,8 +3,10 @@ require "spec_helper" ...@@ -3,8 +3,10 @@ require "spec_helper"
describe API::API, api: true do describe API::API, api: true do
include ApiHelpers include ApiHelpers
let(:base_time) { Time.now } let(:base_time) { Time.now }
let(:user) { create(:user) } let(:user) { create(:user) }
let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } 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) { 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_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!(: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) }
...@@ -316,21 +318,23 @@ describe API::API, api: true do ...@@ -316,21 +318,23 @@ describe API::API, api: true do
end end
describe "DELETE /projects/:id/merge_request/:merge_request_id" do describe "DELETE /projects/:id/merge_request/:merge_request_id" do
it "rejects non admin users from deletions" do it "owners can destroy" do
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
expect(response.status).to eq(403) expect(response.status).to eq(200)
end end
it "let's Admins delete a merge request" do it "let's Admins and owners delete a merge request" do
user.admin = true delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", admin)
user.save
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response['id']).to eq merge_request.id expect(json_response['id']).to eq merge_request.id
end end
it "rejects removal from other users" do
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", non_member)
expect(response.status).to eq(404)
end
end end
describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment