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

fix: MCR-3988 document dates working again #2369

Merged
merged 9 commits into from
Apr 22, 2024
Merged

fix: MCR-3988 document dates working again #2369

merged 9 commits into from
Apr 22, 2024

Conversation

haworku
Copy link
Contributor

@haworku haworku commented Apr 18, 2024

Summary

This PR is related to #1827. That code only partially fixed the problem.

Product review found there were still caseswhere dateAdded was missing in the UploadedDocumentsTable. It seemed to depend on if package was submitted once or resubmitted.

I found that

  1. Contract + Rates shared history #2349 fixed one of the bugs. I was seeing an issue with linked rates flag on where the packageSubmission array was empty on an unlocked submission and only the draft revision was populated. However, after merging main today that bug seems gone
  2. 8b8044b fixed the bug for linked rate flag off on a package that's only been submitted once and not unlocked I think this might have been an existing bug not related to recent work. That file hadn't been touched in 9 months.

Related issues

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

Test cases covered

I added one unit test.

I also added a check to cypress that N/A is not appearing on submission summary pages (that's the screen reader only experience when fields are missing).

QA guidance

Steps for full regression test of date added document table

  1. Set up submissions with flag off in various states (draft, unlocked, submitted)
  2. Test these cases BOTH with flag on and flag off

Submission summary

  • date added on document tables for submitted packages
  • date added on document tables for unlocked packages

Rate summary

  • date added on document tables for submitted rates
  • date added for document tables for unlocked rates

ReviewSubmit (aka state user view)

  • date added on document tables on initial draft packages
  • date added on document tables for submitted packages
  • date added on document tables for submitted packages

QuestionResponse

  • date added on question and response tables for cms user(no need to test both cms and state user for this UI)

@haworku haworku changed the title fix: MCR- 3988 document dates working again. Also add filtering to state analysts table fix: MCR-3988 document dates working again Apr 18, 2024
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.

Looks good! Your change to the mock data may conflict with stuff on Pearl's branch

@@ -309,7 +309,7 @@ export const ContractDetailsSummarySection = ({
? new Date(
documentDateLookupTable.previousSubmissionDate
)
: undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why we had to switch from undefined to nulls here. Totally fine but I'm curious. in general I only expect to see nulls when dealing directly with Apollo GQL data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this choice for React reasons, not GQL. Because this was previously an optional prop, the component is used in about 6 places, and the bug was that the date was present in some expected places and not in others it was very for me difficult to tell if data was coming back unexpected as undefined OR if it was just a case where the prop wasn't passed in at all intentionally.

I changed to a required prop. That means places using this component have to be clear when there is no previousSubmissionDate using explicit null

@haworku haworku merged commit ca138d4 into main Apr 22, 2024
28 checks passed
@haworku haworku deleted the mcr-3988-fix branch April 22, 2024 15:55
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.

2 participants