Refactoring entity tests and adding new entity update tests #1021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- moved extra tests of streaming CSVs of entities from worker/entity test to datasets test (multi form and updated entities in particular) - removed redundant checks and tests from worker/entity test file - added entity update tests to relevant sections of entity API test file * returning current version and data of updated entity * audit log events for an entity
2 tasks
This was referenced Oct 9, 2023
sadiqkhoja
approved these changes
Oct 10, 2023
| @@ -156,11 +157,14 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss | |||
| }; | |||
|
|
|||
| const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event) => async ({ Audits, Entities }) => { | |||
| if (!(event.action === 'submission.create')) // only update on submission.create | |||
There was a problem hiding this comment.
should this condition be in _processSubmissionEvent function?
There was a problem hiding this comment.
Because I moved these lines into _createEntity
if ((dataset.approvalRequired && event.details.reviewState !== 'approved') ||
(!dataset.approvalRequired && event.action === 'submission.update'))
it felt reasonable to move this event action check into _updateEntity
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes the following issues:
In particular:
commit 1
commit 2
worker/entitytest toapi/datasetstest (multi form and updated entities in particular)worker/entitytest fileapi/entitiesAPI test filecommit 3
commit 4
commit 5
What has been done to verify that this works as intended?
it's mostly all tests!
Why is this the best possible solution? Were any other approaches considered?
Some of the tests got refactored. I moved tests that started in worker/entity but weren't really about the worker into other files, e.g. csv streaming of entities, which was one of the first tests about entities to exist.
Many tests about entity updates have an API endpoint that is most relevant, and it seems reasonable to move tests/parts of tests to those places.
I added one special section to the end of
integration/api/entitiesfor capturing update logic. There are some special sections at the bottom ofintegration/api/datasetsspecifically for dataset scenarios.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
More reliable entity update behavior
Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.
No
Before submitting this PR, please make sure you have:
make test-fulland confirmed all checks still pass OR confirm CircleCI build passes