Skip to content

Commit

Permalink
Replace 'number' element with 'request' and 'id'
Browse files Browse the repository at this point in the history
... for consistency with the rest of the API.

Co-authored-by: David Kang <dkang@suse.com>
Co-authored-by: Saray Cabrera Padrón <scabrerapadron@suse.de>
  • Loading branch information
3 people committed Nov 20, 2019
1 parent fec2fc0 commit 8963644
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 32 deletions.
2 changes: 1 addition & 1 deletion docs/api/api/delete_excluded_requests.xml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<excluded_requests>
<number>33</number>
<request id='33'/>
</excluded_requests>
4 changes: 2 additions & 2 deletions docs/api/api/remove_staged_requests.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<requests>
<number>7</number>
<number>13</number>
<request id='7'/>
<request id='13'/>
</requests>
6 changes: 3 additions & 3 deletions src/api/app/controllers/staging/staged_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def create

if params[:remove_exclusion]
::Staging::RequestExcluder
.new(requests_xml_hash: { number: @request_numbers },
.new(requests_xml_hash: @parsed_xml,
staging_workflow: @staging_workflow)
.destroy!
end
Expand Down Expand Up @@ -55,7 +55,7 @@ def destroy
private

def set_request_numbers
@request_numbers = @parsed_xml.elements('number')
@request_numbers = [@parsed_xml[:request]].flatten.map { |request| request[:id].to_i }
return if @request_numbers.present?

render_error(
Expand All @@ -67,7 +67,7 @@ def set_request_numbers

def set_xml_hash
request_body = request.body.read
@parsed_xml = Xmlhash.parse(request_body) if request_body.present?
@parsed_xml = Xmlhash.parse(request_body).with_indifferent_access if request_body.present?
return if @parsed_xml

error_options = if request_body.present?
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/models/staging/request_excluder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def requests_to_be_excluded
end

def request_numbers
[requests_xml_hash[:number]].flatten.map(&:to_i)
[requests_xml_hash[:request]].flatten.map { |request| request[:id].to_i }
end

def valid_request?(bs_request, request_number)
Expand Down
8 changes: 4 additions & 4 deletions src/api/app/models/staging/staged_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ def request_target_project
end

def not_found_request_numbers
request_numbers - requests.pluck(:number).map(&:to_s)
request_numbers - requests.pluck(:number)
end

def excluded_request_numbers
staging_workflow.request_exclusions.where(number: @request_numbers).pluck(:number).map(&:to_s)
staging_workflow.request_exclusions.where(number: @request_numbers).pluck(:number)
end

def requests
Expand Down Expand Up @@ -177,10 +177,10 @@ def missing_packages
end

def missing_requests(requests)
not_unassigned_requests = request_numbers - requests.pluck(:number).map(&:to_s)
not_unassigned_requests = request_numbers - requests.pluck(:number)
return if not_unassigned_requests.empty?

requests_found = BsRequest.where(number: not_unassigned_requests).pluck(:number).map(&:to_s)
requests_found = BsRequest.where(number: not_unassigned_requests).pluck(:number)
requests_not_found = not_unassigned_requests - requests_found

errors << "Requests with number: #{requests_found.to_sentence} don't belong to Staging: #{staging_workflow.project}" if requests_found.present?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@

subject do
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect { subject }.to(change { staging_workflow.request_exclusions.count }.by(-1)) }
Expand All @@ -143,7 +143,7 @@
context 'fails: request not excluded' do
before do
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(400) }
Expand All @@ -154,7 +154,7 @@
request_exclusion
allow_any_instance_of(ActiveRecord::Relation).to receive(:destroy_all).and_return([])
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(400) }
Expand Down
46 changes: 28 additions & 18 deletions src/api/spec/controllers/staging/staged_requests_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

login other_user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:forbidden) }
Expand All @@ -57,7 +57,7 @@
before do
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: 'does-not-exist', format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:not_found) }
Expand All @@ -67,7 +67,7 @@
before do
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>-1</number><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='-1'/><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:bad_request) }
Expand All @@ -79,7 +79,7 @@
Delayed::Job.create(handler: "job_class: StagingProjectAcceptJob, project_id: #{staging_project.id}")
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(424) }
Expand All @@ -93,7 +93,7 @@
before do
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:success) }
Expand All @@ -106,7 +106,7 @@
before do
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{delete_request.number}</number></requests>"
body: "<requests><request id='#{delete_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:success) }
Expand All @@ -129,7 +129,7 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/></requests>" }

before do
create(:request_exclusion,
Expand All @@ -153,7 +153,7 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/></requests>" }

before do
create(:request_exclusion,
Expand Down Expand Up @@ -185,7 +185,7 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/></requests>" }

before { subject }

Expand All @@ -209,7 +209,17 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number><number>#{another_bs_request.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/><request id='#{another_bs_request.number}'/></requests>" }
let(:another_bs_request) do
create(:bs_request_with_submit_action,
state: :review,
creator: other_user,
target_package: target_package,
source_package: source_package,
description: 'BsRequest 2',
number: 2,
review_by_group: group)
end

before { subject }

Expand Down Expand Up @@ -269,7 +279,7 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number><number>#{request_to_exclude.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/><request id='#{request_to_exclude.number}'/></requests>" }

before do
create(:request_exclusion,
Expand Down Expand Up @@ -304,7 +314,7 @@
format: :xml
}
end
let(:body) { "<requests><number>#{bs_request.number}</number><number>#{request_to_exclude.number}</number></requests>" }
let(:body) { "<requests><request id='#{bs_request.number}'/><request id='#{request_to_exclude.number}'/></requests>" }

before do
create(:request_exclusion,
Expand Down Expand Up @@ -348,7 +358,7 @@
before do
login user
post :create, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:success) }
Expand All @@ -372,7 +382,7 @@
before do
login other_user
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:forbidden) }
Expand All @@ -383,7 +393,7 @@
before do
login user
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:success) }
Expand All @@ -395,7 +405,7 @@
before do
login user
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>-1</number><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='-1'/><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:bad_request) }
Expand All @@ -409,7 +419,7 @@
bs_request.state = :revoked
bs_request.save
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, staging_project_name: staging_project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:success) }
Expand All @@ -423,7 +433,7 @@
Delayed::Job.create(handler: "job_class: StagingProjectAcceptJob, project_id: #{staging_project.id}")
login user
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><number>#{bs_request.number}</number></requests>"
body: "<requests><request id='#{bs_request.number}'/></requests>"
end

it { expect(response).to have_http_status(:bad_request) }
Expand Down

0 comments on commit 8963644

Please sign in to comment.