Y24-396: Refactor Automatic State Changers #2044
Conversation
|
Will likely conflict with #2034, probably best to let that be merged in first, then refactor as required. |
app/models/state_changers.rb
Outdated
|
|
||
| api.work_completion.create!(submissions: in_prog_submissions, target: v2_labware.uuid, user: user_uuid) | ||
| def labware | ||
| raise 'Tubes are not supported by API V1' |
There was a problem hiding this comment.
It's not a path that appears to be used in the current iteration - I'm hoping it will be removed with @sdjmchattie's v1 API removal changes
There was a problem hiding this comment.
Addressed in 7ca499d
…nges-using-mixins
TODO: add tests for tube automated state changer
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## y24-088-tuberacks-epic #2044 +/- ##
==========================================================
+ Coverage 80.78% 80.97% +0.19%
==========================================================
Files 482 483 +1
Lines 18295 18499 +204
Branches 269 269
==========================================================
+ Hits 14780 14980 +200
- Misses 3513 3517 +4
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Further refactoring required after #2099 is merged in |
…nges-using-mixins
|
This is ready for review again. |
|
|
||
| # if all the tubes are already in the target state expect contents to be empty | ||
| # TODO: I'm not sure this is correct behaviour, it should probably raise an error | ||
| # or a validation should catch that the state change is not needed |
There was a problem hiding this comment.
I've seen this before in code on the sequencescape side. It gets an api call for a state change but then has code that works out nothing needs doing. Waste of an api call.
Not sure how that even gets triggered though. If there's a button that triggers a state change that isn't needed, then the button should not be visible (i.e. validation like you said).
Maybe someone decided it was too expensive to pull all the information into Limber and instead let SS decide?
There was a problem hiding this comment.
I think this was an existing comment in the code, it was just indented as part of the refactoring
| @@ -271,14 +271,20 @@ def stub_v2_tag_layout_templates(templates) | |||
| end | |||
|
|
|||
| # Builds the basic v2 tube finding query. | |||
| def stub_v2_tube(tube, stub_search: true, custom_query: nil, custom_includes: nil) | |||
| def stub_v2_tube(tube, stub_search: true, custom_query: nil, custom_includes: false) # rubocop:todo Metrics/AbcSize | |||
There was a problem hiding this comment.
I tend to use rubocop:disable / enable over todo if I am happy with the code and have no intention of coming back to it(!)
Closes #2002
Changes proposed in this pull request
contents_formethod ofDefaultStateChangerwhich was originally designed for plates and not tubesInstructions 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