-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gpl711 create events for external subjects #2959
Conversation
# properties attribute, instead of the default BroadcastEvent procedure | ||
module ExternalSubjects | ||
def subjects | ||
return [] unless properties&.key?(:subjects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BroadcastEvent::Helpers::ExternalSubjects#subjects performs a nil-check
end | ||
|
||
def params_for_event | ||
return unless @params[:events] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Concerns::Eventful#params_for_event calls '@params[:events]' 2 times
|
||
def add_all_errors_from_events(events) | ||
events.each do |event| | ||
event.errors.each do |k, v| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Concerns::Eventful#add_all_errors_from_events contains iterators nested 2 deep
|
||
def add_all_errors_from_events(events) | ||
events.each do |event| | ||
event.errors.each do |k, v| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Concerns::Eventful#add_all_errors_from_events has the variable name 'k'
|
||
def add_all_errors_from_events(events) | ||
events.each do |event| | ||
event.errors.each do |k, v| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Concerns::Eventful#add_all_errors_from_events has the variable name 'v'
app/heron/factories/event.rb
Outdated
return if errors.count.positive? | ||
return unless broadcast_event | ||
|
||
broadcast_event.errors.each { |k, v| errors.add(k, v) } unless broadcast_event.valid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Event#check_broadcast_event has the variable name 'k'
app/heron/factories/event.rb
Outdated
return if errors.count.positive? | ||
return unless broadcast_event | ||
|
||
broadcast_event.errors.each { |k, v| errors.add(k, v) } unless broadcast_event.valid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Event#check_broadcast_event has the variable name 'v'
app/heron/factories/sample.rb
Outdated
sample.sample_metadata.update!(params_for_sample_metadata_table) | ||
sample.studies << study | ||
end | ||
end | ||
|
||
def replace_uuid(sample) | ||
handle_uuid_duplication(@params[:uuid]) if Uuid.with_external_id(@params[:uuid]).count.positive? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Sample#replace_uuid calls '@params[:uuid]' 4 times
app/heron/factories/event.rb
Outdated
@@ -0,0 +1,46 @@ | |||
module Heron | |||
module Factories | |||
# Factory class to create Heron tube racks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment wrong
it 'includes all required subjects in the message' do | ||
event_info = JSON.parse(instance.to_json) | ||
expect(event_info['event']).to have_key('subjects') | ||
expect(event_info['event']['subjects'].size).to eq(6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check elements have right role types
Corrected uuid generation to only generate one uuid on sample creation
…o gpl711_create_events_for_external_subjects
def replace_uuid(sample) | ||
uuid = @params[:uuid] | ||
handle_uuid_duplication(uuid) if Uuid.with_external_id(uuid).count.positive? | ||
sample.lazy_uuid_generation = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heron::Factories::Sample#replace_uuid refers to 'sample' more than self (maybe move it to another class?)
Code Climate has analyzed commit b0325b5 and detected 6 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 87.0% (0.0% change). View more on Code Climate. |
Closes GPL711
Changes proposed in this pull request: