fix(protocol-designer): append definitionId to the labwareId in migration #15396
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jerader
commented
Jun 12, 2024
| @@ -79,7 +79,7 @@ export const migrateFile = ( | |||
| .filter(labwareId => getIsAdapter(labwareId)) | |||
| .reduce((acc: LabwareIdMapping, labwareId: string): LabwareIdMapping => { | |||
| const { labwareUri, adapterUri } = getAdapterAndLabwareSplitInfo( | |||
There was a problem hiding this comment.
if you search getAdapterAndLabwareSplitInfo util, it relies on having the labwareDefURI info, which definitionId === defURI (i know its confusing but we removed definitionIds starting in 7.0.0 and started only using defURI or labwareDefURI.
koji
reviewed
Jun 12, 2024
| acc[labwareId] = labwareLocationUpdate[labwareId] | ||
| const definitionId = labware[labwareId].definitionId | ||
| const labId = | ||
| labwareId === 'fixedTrash' || labwareId.includes(definitionId) |
There was a problem hiding this comment.
nit
for consistency
Suggested change
| labwareId === 'fixedTrash' || labwareId.includes(definitionId) | |
| labwareId.includes(definitionId) || labwareId === 'fixedTrash' |
koji
approved these changes
Jun 12, 2024
shlokamin
reviewed
Jun 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes RESC-271
Overview
This PR addresses 2 bugs in 1.
The first bug has to do with the 7.0.0 migration file. Lots of the logic relies on the labware id having the
definitionIdappended to the end of the uuid string. This is NOT GREAT but that's how i architectured it so the migration should handle appending it. It does in most cases but somehow missed some cases which are fixed in this PR. So basically the bug is the migration couldn't find certain labwareIds and labware definitions because it was searching via a string that didn't include thedefinitionId.The 2nd bug is in the 8.1.0 migration file where to populate the tiprack command, it is incorrectly searching for a tipRack key in the step form which doesn't exist yet? idk how we didn't notice this bug sooner. I guess it doesn't whitescreen but it does require the user to add their tiprack location manually instead of auto-populating upon migrating.
Test Plan
Upload the attached protocol and see that it migrated correctly. Go to the deck map and see that there are no errors in the steps (NOTE they will have the tuberack warnings but that is fine). Export the protocol. Then, reimport the migrated protocol and see that there are no errors.
16S Bakterien qPCR 3 Primer 5mL JoLo tubes_V6.json
Changelog
Review requests
see test plan
Risk assessment
low