Y25-483 - Show Lot information using the v2 API#652
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #652 +/- ##
===========================================
- Coverage 80.31% 78.68% -1.64%
===========================================
Files 74 75 +1
Lines 1128 1201 +73
===========================================
+ Hits 906 945 +39
- Misses 222 256 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # Provides a V1-compatible barcode interface wrapping the V2 labware_barcode hash. | ||
| # The V2 API returns labware_barcode as { 'human_barcode' => 'DN1S', ... } whereas | ||
| # the V1 API returned barcode as an object with .prefix and .number accessors. | ||
| # This method bridges the gap so that Presenter::Qcable (and Presenter::Lot#prefix) |
There was a problem hiding this comment.
Its slightly more work but I think we can get rid of the prefix method and use human_readable for label printing. I came across this in another gatekeeper story and think that should fix #582 too. It means we wont need this barcode/prefix/number workaround.
StephenHulme
left a comment
StephenHulme
left a comment
There was a problem hiding this comment.
Thanks for the change :)
StephenHulme
left a comment
StephenHulme
left a comment
There was a problem hiding this comment.
Wrote the review, but forgot to submit it 🙈 - looks good but a couple questions.
| labware_prefix(qcable) || | ||
| barcode_prefix(qcable) || | ||
| '' |
There was a problem hiding this comment.
Nitpick: could this be on one line?
| lb = qcable.labware_barcode | ||
| return unless lb | ||
|
|
||
| (lb['human_barcode'] || lb[:human_barcode])&.to_s |
There was a problem hiding this comment.
Optional: I see this is old code, but I think the intent could be better indicated using symbolize_keys?
Suggest something like:
lb = qcable.labware_barcode&.symbolize_keys
return unless lb
lb[:human_barcode].to_s|
|
||
| prefix = barcode['prefix'] || barcode[:prefix] | ||
| number = barcode['number'] || barcode[:number] | ||
| return unless prefix || number |
There was a problem hiding this comment.
Optional: possibly use symbolize_keys here too
| lb = @qcable.labware_barcode | ||
| return unless lb.is_a?(Hash) | ||
|
|
||
| (lb['human_barcode'] || lb[:human_barcode])&.to_s |
There was a problem hiding this comment.
Optional: possibly use symbolize_keys here too
| bc = @qcable.barcode | ||
|
|
||
| if bc.is_a?(Hash) | ||
| machine = bc['machine'] || bc[:machine] |
There was a problem hiding this comment.
Optional: possibly use symbolize_keys here too
| def barcode_prefix_number | ||
| return unless @qcable.respond_to?(:barcode) | ||
|
|
||
| bc = @qcable.barcode |
There was a problem hiding this comment.
Optional: possibly use symbolize_keys here too
| assert_select '#pending_plate_7 > .plate_barcode', 'DN8' | ||
| assert_select '#pending_plate_7 > .plate_barcode', 'DN8C' |
There was a problem hiding this comment.
This is an interesting change, what happened to the C?
3 participants
Closes #563
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version