DPL-1031 - File name improvements #1752
Conversation
…lity to export filenames
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1752 +/- ##
===========================================
+ Coverage 91.35% 91.36% +0.01%
===========================================
Files 372 373 +1
Lines 7660 7669 +9
===========================================
+ Hits 6998 7007 +9
Misses 662 662
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| def handle_filename_barcode(filename, labware, options) | ||
| return filename if options.blank? || labware.blank? | ||
|
|
||
| barcode = labware.barcode.human |
There was a problem hiding this comment.
Interested to see labware.barcode.human here, as @StephenHulme found recently that the human attribute wasn't present - see discussion here #1746 (comment)
Perhaps it's because Stephen was working with tubes and you're working with plates - maybe the API returns different things. Wonder if it would be worth doing something like
barcode = labware.barcode.human || labware.barcode.machine
in case human is blank? Although in that case, not sure if labware.barcode.human would return nil or blow up.
There was a problem hiding this comment.
Good point, the reason I did this was because labware.human_barcode didnt exist on some of my tests but labware.barcode.human did. I haven't run into a nil case for it yet but happy to add some handling for it. Would a machine barcode be more useful than just showing nil? They can manually edit it if it there are any issues.
There was a problem hiding this comment.
Not sure! I guess if the machine barcode is a 13-digit ean13 barcode it's not that useful. But in Stephen's case, he had a FluidX ('foreign') tube barcode, and that didn't have a 'human' attribute but had the human readable barcode in 'machine'. So in that case I it would be handy to have that in the download. Not a massive deal here as it's just naming a file 🤷♀️
There was a problem hiding this comment.
I guess from the users point of view the aim is to match the export file with the physical plate using it's barcode label, so unless what's in the filename matches what's displayed on the plate label (human readable) it is not useful. Important for driver files where they are loading a plate on the robot and using the file to drive transfers for it.
And they (mostly) don't transfer to/from tubes on robots.
Odd there is no human attribute on tube labware barcodes. Maybe we should fix that.
|
Code Climate has analyzed commit f3c55a7 and detected 0 issues on this pull request. 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 91.0% (0.0% change). View more on Code Climate. |
Closes #1521
Changes proposed in this pull request
Instructions for Reviewers
There are no independent unit tests, mainly because the exports_controller tests it by proxy and it is has a different style of testing. But I have tested manually with a few variations and it is working.