Y26 045 bug integrate 2d barcode re printing#5599
Y26 045 bug integrate 2d barcode re printing#5599
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5599 +/- ##
===========================================
+ Coverage 87.21% 87.23% +0.01%
===========================================
Files 1461 1461
Lines 33003 33011 +8
Branches 3470 3472 +2
===========================================
+ Hits 28783 28796 +13
+ Misses 4199 4194 -5
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wendyyang
requested review from
BenTopping,
KatyTaylor,
dasunpubudumal and
yoldas
|
barcode type drop down in tube reprint page now looks like: 
|
|
Plate page doesn't have the barcode type drop down: |
|
The printer drop down keeps as it is as this share template is used elsewhere. will break if change it. |
There was a problem hiding this comment.
I think it looks okay. Worth trying out the following:
- Label printing for plates: Should remain unaffected.
- 1D Label printing for tubes: Should remain unaffected.
- 2D label printing for tubes: Labels should print with the given template.
You can check if the 2D labels have got the correct data by adding a binding.pry to here and check build_attribtues reflectively. The additional values you provided should be populated as part of labels_attribute.
I think you would need to sanity check the 2D and 1D barcodes with a real printer as well.
andrewsparkes
left a comment
andrewsparkes
left a comment
There was a problem hiding this comment.
Add unit tests please, none.
Code looks good.
| end | ||
|
|
||
| context 'when printing labels for a plate manifest' do | ||
| let!(:sample_manifest) { create(:sample_manifest, asset_type: 'plate') } |
There was a problem hiding this comment.
create(:sample_manifest, asset_type: 'plate') can be moved into the before block and the let! can be removed - 3 other occurances too
There was a problem hiding this comment.
create(:sample_manifest, asset_type: 'plate') move into the before block seems not working.
andrewsparkes
left a comment
andrewsparkes
left a comment
There was a problem hiding this comment.
Minor stuff:
Split out the printjob setup to avoid those rubocop disables.
And add unit tests for the new lines codecov indicates, in sample_manifest_controller lines 122/3.
Otherwise looks good!
| def print_labels # rubocop:todo Metrics/MethodLength | ||
| def print_labels # rubocop:todo Metrics/MethodLength, Metrics/AbcSize | ||
| print_job = | ||
| LabelPrinter::PrintJob.new( |
There was a problem hiding this comment.
maybe split this part out into a separate method (set_up_printjob or something) to fix rubocop issues rather than add to them.
5 participants
Closes #
Changes proposed in this pull request
add barcode type & template name to print job creation;
add barcode type drop down list in the tube reprint page
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