Skip to content

Commit

Permalink
Implement better error message in Staging::StageRequest
Browse files Browse the repository at this point in the history
to let the user know what is going wrong:
* Request not found
* Backend exception
* Already branched
  • Loading branch information
ChrisBr committed Nov 22, 2018
1 parent 4ecdd4d commit 6a572f4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 10 deletions.
13 changes: 7 additions & 6 deletions src/api/app/controllers/staging/staged_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@ def index

def create
authorize @staging_project, :update?
requests = @staging_workflow.unassigned_requests.where(number: request_numbers)

result = ::Staging::StageRequests.new(requests: requests, staging_project_name: @staging_project.name).perform
@staging_project.staged_requests << result
unassigned_requests = request_numbers - result.pluck(:number).map(&:to_s)
result = ::Staging::StageRequests.new(
request_numbers: request_numbers,
staging_workflow: @staging_workflow,
staging_project_name: @staging_project.name
).perform

if unassigned_requests.empty?
if result.valid?
render_ok
else
render_error(
status: 400,
errorcode: 'invalid_request',
message: "Could not assign requests '#{unassigned_requests.to_sentence}' to #{@staging_project}."
message: "Assigning requests to #{@staging_project} failed: #{result.errors.to_sentence}."
)
end
end
Expand Down
38 changes: 34 additions & 4 deletions src/api/app/models/staging/stage_requests.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,51 @@
class Staging::StageRequests
include ActiveModel::Model
attr_accessor :requests, :staging_project_name
attr_accessor :request_numbers, :staging_project_name, :staging_workflow

def perform
self.result = []
add_request_not_found_errors
requests.each do |request|
bs_request_action = request.bs_request_actions.first
if bs_request_action.is_submit?
branch_package(bs_request_action)
elsif bs_request_action.is_delete?
# TODO: implement delete requests
end
result
end
self
end

def errors
@errors ||= []
end

def valid?
errors.empty?
end

private

attr_accessor :result
def result
@result ||= []
end

def add_request_not_found_errors
not_found_requests.each do |request_number|
errors << "Request '#{request_number}' does not exist or target_project is not '#{request_target_project}'"
end
end

def request_target_project
staging_workflow.project
end

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

def requests
staging_workflow.unassigned_requests.where(number: request_numbers)
end

def branch_package(bs_request_action)
request = bs_request_action.bs_request
Expand All @@ -32,7 +60,9 @@ def branch_package(bs_request_action)
rescue BranchPackage::DoubleBranchPackageError
# we leave the package there and do not report as success
# because packages might differ
errors << "Request '#{request.number}' already branched into '#{staging_project_name}'"
rescue APIError, Backend::Error => e
errors << "Request '#{request.number}' branching failed: '#{e.message}'"
Airbrake.notify(e, bs_request: request.number)
end
end

0 comments on commit 6a572f4

Please sign in to comment.