add labware updated_at value and return the latest one#5780
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5780 +/- ##
===========================================
+ Coverage 84.06% 87.31% +3.24%
===========================================
Files 1422 1479 +57
Lines 32730 33594 +864
Branches 3502 3557 +55
===========================================
+ Hits 27513 29331 +1818
+ Misses 5195 4242 -953
+ Partials 22 21 -1
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:
|
wendyyang
requested review from
KatyTaylor,
StephenHulme,
andrewsparkes and
yoldas
StephenHulme
left a comment
StephenHulme
left a comment
There was a problem hiding this comment.
Some comments and suggestions : )
| return 'Not found - There is a problem with Labwhere' | ||
| return { error: true } |
There was a problem hiding this comment.
Could we maybe return a message as well? I don't think true provides enough feedback.
There was a problem hiding this comment.
added error messages in return
| end | ||
|
|
||
| nil | ||
| end |
There was a problem hiding this comment.
Would it be possible to reduce the complexity of this function (possibly by pulling out some private methods)?
It's difficult to understand exactly what the new code does.
| labware = described_class.new(params) | ||
|
|
||
| expect(labware.updated_at).to eq('Wednesday May 6 2026 10:46') | ||
| expect(labware.location.updated_at).to eq('Tuesday June 6 2023 16:26') |
There was a problem hiding this comment.
This date format is nice for humans, but non-standard and difficult for computers to parse. May I suggest that the updated_at fields are always represented as ISO 8601 standard strings?
| before do | ||
| allow(LabWhereClient::Labware).to receive(:find_by_barcode).and_return(nil) | ||
| end |
There was a problem hiding this comment.
Is this mocking out the actual code we are testing? Or is it not important to this test?
There was a problem hiding this comment.
re-write to include human barcode
2 participants
Closes #
Changes proposed in this pull request
add labware updated_at in API return and return the latest location
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