Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app): ensure proper API cal data is associated to protocol labware #6280

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Aug 4, 2020

overview

This PR follows up on an #6100 and #6246. A given labware used in a protocol should be associated to a given entry in the labware calibrations results if the labware's:

  1. loadName matches the calibration data entry, and
  2. namespace matches the calibration data entry, and
  3. version matches the calibration data entry, and
  4. parent module (or lack of parent) matches the data entry

The implementation I wrote in #6100 was buggy, and #6246 noted that it resulted in incorrect quantities being displayed in the table. However, it it turns out 6100 was buggy for both quantity linking and calibration data linking. If a labware does not have a module parent, it could get linked (in UI) to module-based calibration data if the module-based calibration entry was earlier in the response array.

This PR adds a test and fix for this case.

changelog

  • refactor(app): ensure proper API cal data is associated to protocol labware

review requests

risk assessment

Low. Unit test coverage was already high for this logic, and bug+fix was TDD'd.

@mcous mcous added app Affects the `app` project ready for review hmg hardware, motion, and geometry labels Aug 4, 2020
@mcous mcous requested a review from a team as a code owner August 4, 2020 20:16
@mcous mcous requested review from shlokamin, sfoster1, Laura-Danielle and b-cooper and removed request for a team and shlokamin August 4, 2020 20:16
Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎲

Copy link
Contributor

@amitlissack amitlissack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me.

@mcous mcous merged commit 8b1afb0 into edge Aug 4, 2020
@mcous mcous deleted the app_fix-map-proto-labware-to-cal-data branch August 4, 2020 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project hmg hardware, motion, and geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants