Skip to content

Commit

Permalink
Avoid accessing internals of set_checkable
Browse files Browse the repository at this point in the history
Instead of creating the event in an if-else cascade,
remember the class to create and save it in constant
  • Loading branch information
coolo committed Nov 14, 2018
1 parent 55b72f9 commit 97c1636
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 14 deletions.
14 changes: 1 addition & 13 deletions src/api/app/controllers/status/checks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def update
end

if @check.save
create_event
@event_class.create(check_notify_params)
render :show
else
render_error(status: 422, errorcode: 'invalid_check', message: "Could not save check: #{@check.errors.full_messages.to_sentence}")
Expand All @@ -27,18 +27,6 @@ def update

private

def create_event
if @project
if @architecture
Event::StatusCheckForBuild.create(check_notify_params)
else
Event::StatusCheckForPublished.create(check_notify_params)
end
else
Event::StatusCheckForRequest.create(check_notify_params)
end
end

def check_notify_params
params = { who: User.current.login, state: @check.state, name: @check.name }
params[:url] = @check.url if @check.url
Expand Down
5 changes: 4 additions & 1 deletion src/api/app/controllers/status/concerns/set_checkable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def set_repository
return unless @project

@checkable = @project.repositories.find_by(name: params[:repository_name])
@event_class = Event::StatusCheckForPublished
return @checkable if @checkable

@error_message = "Repository '#{params[:project_name]}/#{params[:repository_name]}' not found."
Expand All @@ -37,15 +38,17 @@ def set_repository
def set_repository_architecture
return unless set_repository
return @checkable unless params[:arch]
@architecture = params[:arch]

@checkable = @checkable.repository_architectures.joins(:architecture).find_by(architectures: { name: params[:arch] })
@event_class = Event::StatusCheckForBuild
return @checkable if @checkable

@error_message = "Repository '#{params[:project_name]}/#{params[:repository_name]}/#{params[:arch]}' not found."
end

def set_bs_request
@checkable = BsRequest.with_submit_requests.find_by(number: params[:bs_request_number])
@event_class = Event::StatusCheckForRequest
return @checkable if @checkable

@error_message = "Submit request with number '#{params[:bs_request_number]}' not found."
Expand Down
10 changes: 10 additions & 0 deletions src/api/app/models/event/status_check_for_build.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Event
class StatusCheckForBuild < StatusCheck
self.description = 'Status Check for Finished Repository Created'
payload_keys :project, :repo, :arch, :buildid

def self.message_bus_routing_key
'repo.status_report'
end
end
end
6 changes: 6 additions & 0 deletions src/api/spec/controllers/status/checks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@
end

include_context 'does create the check'

it 'creates the proper event' do
post :update, body: xml, params: params, format: :xml
expect(Event::StatusCheckForBuild.first.payload).to include('who' => user.login, 'project' => project.name, 'repo' => repository.name,
'arch' => repository_architecture.architecture.name, 'state' => 'pending')
end
end

context 'for a request' do
Expand Down

0 comments on commit 97c1636

Please sign in to comment.