[ ] PR title descriptive (can be used in release notes)
Description
Fix issue where calling multiple dynamic-data service queries in parallel could lead to multiple db collection creation requests throwing rxdb error. Specifically:
Implement a queue-type system to prevent duplicate db create requests when creating multiple queries in parallel
Add test case
Dev Notes
Additional work is included to refactor the app-data mocks 56a749a, to fix a regression issue brought in by changes to the app-data mocks in #2107. Data had been copy-pasted from the app-data mock to the dynamic-data mock (which is super fragile), breaking tests when source app-data mock changed. Now mock services can specify their own data when implementing the app-data mock.
This is also another good reminder why we should try to fix up the rest of the ng test suite and integrate into ci (raised in #2080 and various messages on mattermost). Not sure if this is something still on your radar @jfmcquade ?
One other regression discovered (coming from #1989) was the breaking of a test previously designed to throw error when updating row that does not exist. I can see the code changes removed this behaviour (I'm assuming you decided this is no longer a desirable QA check @jfmcquade (?)), so I've removed the test (see updated spec). If we don't get the ng test suite integrated then it would be good to at least make a mental note to check existing tests when making changes to corresponding services for both dev and review.
Review Notes
Tests can be run via yarn ng test --include src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts, also hoping will pass the case you raised in #2176 @jfmcquade (?)
Git Issues
unblocks #2176
Screenshots/Videos
Before - spec test for parallel requests added (failing)
After - all tests passing (with one qa test removed)
PR Checklist
Description
Fix issue where calling multiple dynamic-data service queries in parallel could lead to multiple db collection creation requests throwing rxdb error. Specifically:
Dev Notes
Additional work is included to refactor the app-data mocks 56a749a, to fix a regression issue brought in by changes to the app-data mocks in #2107. Data had been copy-pasted from the app-data mock to the dynamic-data mock (which is super fragile), breaking tests when source app-data mock changed. Now mock services can specify their own data when implementing the app-data mock.
This is also another good reminder why we should try to fix up the rest of the ng test suite and integrate into ci (raised in #2080 and various messages on mattermost). Not sure if this is something still on your radar @jfmcquade ?
One other regression discovered (coming from #1989) was the breaking of a test previously designed to throw error when updating row that does not exist. I can see the code changes removed this behaviour (I'm assuming you decided this is no longer a desirable QA check @jfmcquade (?)), so I've removed the test (see updated spec). If we don't get the ng test suite integrated then it would be good to at least make a mental note to check existing tests when making changes to corresponding services for both dev and review.
Review Notes
Tests can be run via
yarn ng test --include src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts, also hoping will pass the case you raised in #2176 @jfmcquade (?)Git Issues
unblocks #2176
Screenshots/Videos
Before - spec test for parallel requests added (failing)
After - all tests passing (with one qa test removed)