From 8963644e3187bf602ca37fd33fdea2c6bb5f4f69 Mon Sep 17 00:00:00 2001 From: Eduardo Navarro Date: Tue, 19 Nov 2019 14:14:16 +0100 Subject: [PATCH] Replace 'number' element with 'request' and 'id' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... for consistency with the rest of the API. Co-authored-by: David Kang Co-authored-by: Saray Cabrera Padrón --- docs/api/api/delete_excluded_requests.xml | 2 +- docs/api/api/remove_staged_requests.xml | 4 +- .../staging/staged_requests_controller.rb | 6 +-- .../app/models/staging/request_excluder.rb | 2 +- src/api/app/models/staging/staged_requests.rb | 8 ++-- .../excluded_requests_controller_spec.rb | 6 +-- .../staged_requests_controller_spec.rb | 46 +++++++++++-------- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/docs/api/api/delete_excluded_requests.xml b/docs/api/api/delete_excluded_requests.xml index 2031d0e4a2a..bd36c752c1f 100644 --- a/docs/api/api/delete_excluded_requests.xml +++ b/docs/api/api/delete_excluded_requests.xml @@ -1,3 +1,3 @@ - 33 + diff --git a/docs/api/api/remove_staged_requests.xml b/docs/api/api/remove_staged_requests.xml index 13718442b8c..1b64bc7e0c8 100644 --- a/docs/api/api/remove_staged_requests.xml +++ b/docs/api/api/remove_staged_requests.xml @@ -1,4 +1,4 @@ - 7 - 13 + + diff --git a/src/api/app/controllers/staging/staged_requests_controller.rb b/src/api/app/controllers/staging/staged_requests_controller.rb index ae810e4812b..a6900e0b564 100644 --- a/src/api/app/controllers/staging/staged_requests_controller.rb +++ b/src/api/app/controllers/staging/staged_requests_controller.rb @@ -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 @@ -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( @@ -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? diff --git a/src/api/app/models/staging/request_excluder.rb b/src/api/app/models/staging/request_excluder.rb index 97390c5f025..701df7654d1 100644 --- a/src/api/app/models/staging/request_excluder.rb +++ b/src/api/app/models/staging/request_excluder.rb @@ -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) diff --git a/src/api/app/models/staging/staged_requests.rb b/src/api/app/models/staging/staged_requests.rb index 98e47b6579a..0b0c358731b 100644 --- a/src/api/app/models/staging/staged_requests.rb +++ b/src/api/app/models/staging/staged_requests.rb @@ -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 @@ -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? diff --git a/src/api/spec/controllers/staging/excluded_requests_controller_spec.rb b/src/api/spec/controllers/staging/excluded_requests_controller_spec.rb index f6cbc16e1af..00f5b28d27b 100644 --- a/src/api/spec/controllers/staging/excluded_requests_controller_spec.rb +++ b/src/api/spec/controllers/staging/excluded_requests_controller_spec.rb @@ -128,7 +128,7 @@ subject do delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml }, - body: "#{bs_request.number}" + body: "" end it { expect { subject }.to(change { staging_workflow.request_exclusions.count }.by(-1)) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(400) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(400) } diff --git a/src/api/spec/controllers/staging/staged_requests_controller_spec.rb b/src/api/spec/controllers/staging/staged_requests_controller_spec.rb index 97173ad598a..3bf8f64e7c8 100644 --- a/src/api/spec/controllers/staging/staged_requests_controller_spec.rb +++ b/src/api/spec/controllers/staging/staged_requests_controller_spec.rb @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:forbidden) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:not_found) } @@ -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: "-1#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:bad_request) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(424) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:success) } @@ -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: "#{delete_request.number}" + body: "" end it { expect(response).to have_http_status(:success) } @@ -129,7 +129,7 @@ format: :xml } end - let(:body) { "#{bs_request.number}" } + let(:body) { "" } before do create(:request_exclusion, @@ -153,7 +153,7 @@ format: :xml } end - let(:body) { "#{bs_request.number}" } + let(:body) { "" } before do create(:request_exclusion, @@ -185,7 +185,7 @@ format: :xml } end - let(:body) { "#{bs_request.number}" } + let(:body) { "" } before { subject } @@ -209,7 +209,17 @@ format: :xml } end - let(:body) { "#{bs_request.number}#{another_bs_request.number}" } + let(:body) { "" } + 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 } @@ -269,7 +279,7 @@ format: :xml } end - let(:body) { "#{bs_request.number}#{request_to_exclude.number}" } + let(:body) { "" } before do create(:request_exclusion, @@ -304,7 +314,7 @@ format: :xml } end - let(:body) { "#{bs_request.number}#{request_to_exclude.number}" } + let(:body) { "" } before do create(:request_exclusion, @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:success) } @@ -372,7 +382,7 @@ before do login other_user delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml }, - body: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:forbidden) } @@ -383,7 +393,7 @@ before do login user delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml }, - body: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:success) } @@ -395,7 +405,7 @@ before do login user delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml }, - body: "-1#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:bad_request) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:success) } @@ -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: "#{bs_request.number}" + body: "" end it { expect(response).to have_http_status(:bad_request) }