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

MCR-4052 Re-implement unlock submission #2376

Merged
merged 9 commits into from
Apr 24, 2024
Merged

Conversation

haworku
Copy link
Contributor

@haworku haworku commented Apr 22, 2024

Summary

  • Add back the unlock submission button to SubmissionSummaryV2.
  • Add linked rates "contract actions" list to Rate SummaryPage

Related issues

https://jiraent.cms.gov/browse/MCR-3943
https://jiraent.cms.gov/browse/MCR-3943

Screenshots

Test cases covered

  • Contract actions show up on rate summary page
  • ( in progress) Cypress tests for can unlock and resubmit pass when link rates flag on

QA guidance

@haworku
Copy link
Contributor Author

haworku commented Apr 24, 2024

Proposing that we merge this with the skiped new test. I will keep working on that but this enables unlocking submissions for linked rate on

Copy link
Contributor

@pearl-truss pearl-truss left a comment

Choose a reason for hiding this comment

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

I see that unlock is working but visually in the UI on the submission summary page the unlock banner doesn't appear until page reload

@@ -180,23 +193,48 @@ export const UnlockSubmitModalV2 = ({
case 'SUBMIT_RATE' || 'RESUBMIT_RATE':
console.info('submit/resubmit rate not implemented yet')
break
case 'SUBMIT_CONTRACT':
case 'SUBMIT_CONTRACT' || 'RESUBMIT_CONTRACT':
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes a regression with resubmitting. I don't see why || wouldn't work here but I found that it causes the logic of this case not to be hit with resubmit

@haworku
Copy link
Contributor Author

haworku commented Apr 24, 2024

TY! I'll check the resubmit logic. That seems like a blocker

The unlock banner not showing to reload is the bug cypress caught, forgot to mention I'm looking at that but didn't think it should block turning unlock back on for other testing.

Copy link
Contributor

@macrael macrael left a comment

Choose a reason for hiding this comment

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

LGTM, when will we enable the new cypress tests? Once linked rates is on?

@haworku
Copy link
Contributor Author

haworku commented Apr 24, 2024

@macrael I put this up to move it through to unblock acceptance testing. I still plan to bring in the skipped test ASAP, actively working on this today.

After the flag turns on we will need a whole effort around cypress to get the rest of the tests on and cleaned up. I was going to make a Cypress ticket for this in the Contract API epic, I also added it to the frontend refactor plan

@haworku
Copy link
Contributor Author

haworku commented Apr 24, 2024

@pearl-truss FYI I fixed resubmit. I've been resubmitting in review app with linked rates flag on and off and seeing it work, no errors.

I forgot to ask how you knew resubmit wasn't working though, I'm assuming there was an obvious error meesage? Or was it something more subtle?

@pearl-truss
Copy link
Contributor

@pearl-truss FYI I fixed resubmit. I've been resubmitting in review app with linked rates flag on and off and seeing it work, no errors.

I forgot to ask how you knew resubmit wasn't working though, I'm assuming there was an obvious error meesage? Or was it something more subtle?

There wasn't an error message and it appeared to go through (green success banner) but back on the dashboard the submission still appeared as unlocked (even on page refresh)

@pearl-truss pearl-truss self-requested a review April 24, 2024 17:55
Copy link
Contributor

@pearl-truss pearl-truss left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@haworku haworku merged commit 84b2caa into main Apr 24, 2024
28 checks passed
@haworku haworku deleted the mcr-4052-reimplmenet-unlock branch April 24, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants