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

MR-3545, MR-2370: Fix change history, add cypress tests, and turn on remaining API Unit tests #1959

Merged
merged 99 commits into from
Oct 9, 2023

Conversation

JasonLin0991
Copy link
Contributor

@JasonLin0991 JasonLin0991 commented Oct 5, 2023

Summary

MR-3545
MR-2370

The cause was duplicate contract revisions and Apollo Client Caching. See Apollo Client storing normalized docs. The duplicate contract revisions are made in contractWithHistoryToDomainModel when we construct a revision history for each change in rate revisions by creating a new contract revision with the same ID as the contract revision the rate change belongs to. When this data is returned to the frontend, Apollo cache sees duplicate contract revision IDs and merges the data.

See notes for more details on the investigation.

Changes:

  • contractWithHistoryToDomainModel removed creating revision history for rate changes.
  • Updated unlockResubmits.spec to test the order and count of the revision history.
  • Updated API unit tests that were testing rate revision history in the contract revisions.
    • Fixing the change history bug also fixed CI cypress test failing.
    • Previous PR to fix all these tests missed this one, because I forgot to turn the flag back on to run CI 🫠
  • Pulled in all work from this PR to turn on remaining API unit tests.

Related issues

Screenshots

Test cases covered

QA guidance

@JasonLin0991 JasonLin0991 changed the title Fix change history and add cypress tests Fix change history, add cypress tests, and turn on remaining API Unit tests Oct 6, 2023
@JasonLin0991 JasonLin0991 changed the title Fix change history, add cypress tests, and turn on remaining API Unit tests MR-3545, MR-2370: Fix change history, add cypress tests, and turn on remaining API Unit tests Oct 6, 2023
@JasonLin0991 JasonLin0991 changed the base branch from wml-enable-all-tests to main October 6, 2023 16:49
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.

I want to be sure we can switch back to the old accounting easily on those long findContractWithHistory tests but otherwise this seems like a good interim fix

@@ -124,7 +124,9 @@ describe('findContract', () => {
if (threeContract instanceof Error) {
throw threeContract
}
expect(threeContract.revisions).toHaveLength(4)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm very hesitant to lose the specific values in this test. This test was written to check that we get the expected number of revisions everywhere. I'd rather mark this test as skip or something than adapt it to work with the interim solution.

Copy link
Contributor Author

@JasonLin0991 JasonLin0991 Oct 6, 2023

Choose a reason for hiding this comment

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

Agree! Let's mark as skip.

@JasonLin0991 JasonLin0991 merged commit b4ab6ce into main Oct 9, 2023
27 checks passed
@JasonLin0991 JasonLin0991 deleted the jl-fix-change-history-and-add-cypress-tests branch October 9, 2023 18:32
haworku pushed a commit that referenced this pull request Oct 16, 2023
…remaining API Unit tests (#1959)

* populating

* keep incoming

* remove default name. not sure why it's here?

* use the right deploy script

* move the migration into a func

* return errors in prepopulate updateinfo

* use error returns on insertContractId

* return the error. no throws here for try/catch

* clean up error messaging

* move contract ops to migrateContract()

* don't forget the associations error handling

* the migrate results type

* more tests

* make it a helper func

* rate revision test

* check that we can get the migrated revision back

* record the exception no need to return a packageId

* fix test

* more logging

* clearAllMocks

* doesnt' test anything now

* some debug

* keep track of migrations that did not complete

* crashing on the user find

* log out some stats

* need to continue

* just use the client we already get

* remove some debug

* all the contract ammendment info

* get unlockInfoID

* rate revision as well

* testing file

* fix up test some more

* Adding test fixes and fixes from test

* fixes in migrate. add DB reset at init.

* use the prisma types in the test

* finish adding contract docs

* adds the rate docs

* add another fake doc

* contract/rate doc migration moved.

* move join table work to the create phase

* cleanup all the things

* format on save?

* enable remaining jest tests

* fix up compare

* Enable `rates-db-refactor` and `supporting-docs-by-rate` flag for cypress tests.

* Skip Q&A tests, resolver not modified for refactor yet.

* logging data objects out for compare

* refactor comparison

* more cleaning on test

* fix the map to flatmap

* fix up which we compare with

* error check

* Add extra test for change history order and count.

* Remove rate history from contract history.

* Add a check for which query to wait for.

* Apply suggestions from code review

Co-authored-by: Jason Lin <98117700+JasonLin0991@users.noreply.github.com>

* some fixes to the migrator

* fix ordering of contract revisions

* update yarn.lock

* Fix contractWithHistoryToDomainModel.

* debug helper.

* Update tests

* Longer timeout looking for unlock button.

* Update services/app-api/src/resolvers/healthPlanPackage/indexHealthPlanPackages.test.ts

* Update services/app-api/src/resolvers/healthPlanPackage/indexHealthPlanPackages.test.ts

* cypress re-run

* add position to actuary contacts

* Increase timeouts for more docs.

* tests are suspiciously fixed

* Skip tests for rate history.

* test and fix how update contract with rates reads rateInfo.id

* update migrators handling of submittedAt and unlocked rates

* pass tx into findContractWithHistory. variable name changes for clarity.

* Change setting id from revision id to rate id.

* Fix tests for rate formData.id change.

* connect related contract

* fix tests proactively

---------

Co-authored-by: Mazdak Atighi <maz@truss.works>
Co-authored-by: Mojo Talantikite <mojo.talantikite@gmail.com>
Co-authored-by: MacRae Linton <macrael@truss.works>
Co-authored-by: MacRae Linton <55759+macrael@users.noreply.github.com>
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.

4 participants