-
Notifications
You must be signed in to change notification settings - Fork 183
feat(protocol-designer): add 8.5.0 migration modal #18673
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 8.5.0 migration modal #18673
Conversation
This PR adds copy for 8.5.0 migration modal and wires it up. It also updates the copy for import confirmation from "Confirm" to "Import", and updates tests accordingly. Closes AUTH-1975
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## chore_release-pd-8.5.0 #18673 +/- ##
==========================================================
- Coverage 24.04% 24.03% -0.02%
==========================================================
Files 3256 3256
Lines 281502 281383 -119
Branches 28550 28552 +2
==========================================================
- Hits 67695 67625 -70
+ Misses 213782 213733 -49
Partials 25 25
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
@@ -226,7 +242,9 @@ export const getMigrationMessage = ( | |||
) { | |||
return getNoBehaviorChangeMessage({ t }) | |||
} | |||
if (migrationsRan.includes('8.1.0')) { | |||
if (migrationsRan.includes('8.5.0')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit here: since the docs will include all the copy from previous migration versions, i believe we can simplify the message to a singular generic message. then update getGenericDidMigrateMessage
to return the new copy.
export const getMigrationMessage = (
props: MigrationMessageProps
): ModalContents => {
const { t } = props
return getGenericDidMigrateMessage({ t })
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly you can just have the copy directly in getMigrationMessage
and remove the getGenericDidMigrateMessage()
fn all together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, I vastly simplified the migration content logic since we only need to show the latest generic migration copy.
protocol-designer/src/components/organisms/FileUploadMessagesModal/index.tsx
Outdated
Show resolved
Hide resolved
.../src/components/organisms/FileUploadMessagesModal/__tests__/FileUploadMessagesModal.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some clean up nits for the cypress test. after that is addressed, this lgtm!
Overview
This PR adds copy for 8.5.0 migration modal and wires it up. It also updates the copy for import confirmation from "Confirm" to "Import", and updates tests accordingly.
Note that much of the logic determining which migration modal to show has been removed and simplified to show the latest migration content.
Closes AUTH-1975
Test Plan and Hands on Testing
Review requests
see test plan
Risk assessment
low