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

feat(protocol-designer): add liquidClassesSupported field to liquid forms #17599

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

ncdiehl11
Copy link
Collaborator

Overview

In order to correctly branch step generation logic for pre- vs. post-liquid class moveLiquid and mix steps, and to maintain functionality for protocols created in PD versions <8.5.0, we need information regarding when the form was created. This PR introduces this field and adds a migration for 8.5.0.

Test Plan and Hands on Testing

  • import a protocol with a transfer and mix step to this PD on this branch
  • create a new transfer step and save
  • create a new mix step and save
  • export the protocol and inspect
  • verify that the old transfer/mix steps from importing contain "liquidClassesSupported": false,
  • verify that the new transfer/mix steps after importing contain "liquidClassesSupported": true,

Changelog

  • add new liquidClassesSupported field (boolean)
  • add migration
  • update types and fixtures

Review requests

see test plan

Risk assessment

low. currently unused

…orms

In order to correctly branch step generation logic for pre- vs. post-liquid class moveLiquid and mix steps, and to maintain functionality for protocols created in PD versions <8.5.0, we need information regarding when the form was created. This PR introduces this field and adds a migration for 8.5.0.
@ncdiehl11 ncdiehl11 self-assigned this Feb 26, 2025
@ncdiehl11 ncdiehl11 requested review from koji and syao1226 February 26, 2025 21:17
@ncdiehl11 ncdiehl11 assigned ncdiehl11 and unassigned ncdiehl11 Feb 26, 2025
@ncdiehl11 ncdiehl11 marked this pull request as ready for review February 26, 2025 21:18
@ncdiehl11 ncdiehl11 requested a review from a team as a code owner February 26, 2025 21:18
@ncdiehl11 ncdiehl11 changed the title feat(protocol-designer): add liquidClassesSupported field to liquid forms feat(protocol-designer): add liquidClassesSupported field to liquid forms Feb 26, 2025
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 25.76%. Comparing base (39dc008) to head (aa525e2).
Report is 4 commits behind head on edge.

Files with missing lines Patch % Lines
protocol-designer/src/load-file/migration/8_5_0.ts 0.00% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17599      +/-   ##
==========================================
+ Coverage   25.64%   25.76%   +0.11%     
==========================================
  Files        2841     2843       +2     
  Lines      218624   219181     +557     
  Branches    17944    18118     +174     
==========================================
+ Hits        56072    56469     +397     
- Misses     162537   162697     +160     
  Partials       15       15              
Flag Coverage Δ
protocol-designer 19.03% <60.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
protocol-designer/src/form-types.ts 79.31% <ø> (-1.34%) ⬇️
...src/step-forms/test/createPresavedStepForm.test.ts 100.00% <100.00%> (ø)
...r/src/steplist/formLevel/getDefaultsForStepType.ts 97.02% <100.00%> (+4.05%) ⬆️
...rmLevel/stepFormToArgs/test/stepFormToArgs.test.ts 100.00% <100.00%> (ø)
...list/formLevel/test/getDefaultsForStepType.test.ts 100.00% <100.00%> (ø)
...tocol-designer/src/ui/steps/test/selectors.test.ts 100.00% <100.00%> (ø)
protocol-designer/src/load-file/migration/8_5_0.ts 1.12% <0.00%> (-0.11%) ⬇️

... and 13 files with indirect coverage changes

Copy link
Collaborator

@syao1226 syao1226 left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

I'll need to dig into selectros.test.ts but from the code the test value is correct.
other changes look good to me.

@ncdiehl11 ncdiehl11 merged commit 50c487c into edge Feb 27, 2025
26 checks passed
@ncdiehl11 ncdiehl11 deleted the pd_feat-liquidclassessupport-field branch February 27, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants