Feature/entities/resolve conflict #1023
Conversation
sadiqkhoja
force-pushed
the
feature/entities/resolve-conflict
branch
from
81cb1b4 to
b704533
Compare
| .then(({ body: person }) => { | ||
| should(person.conflict).be.null(); | ||
| }); | ||
| })); |
There was a problem hiding this comment.
Can you add a test checking that the right person/actor is associated with the resolve conflict audit event?
sadiqkhoja
force-pushed
the
feature/entities/resolve-conflict
branch
from
12b2479 to
2004926
Compare
There was a problem hiding this comment.
Thanks for adding the doc changes and the audit action tests.
One of the additions to a test made me curious about calling 'resolve' on an entity without any conflict. It seems like it shouldn't resolve a conflict that doesn't exist. Mainly it shouldn't log it in the audit log, because that might be confusing later.
docs/api.yaml
Outdated
| @@ -36,7 +36,19 @@ info: | |||
|
|
|||
| Here major and breaking changes to the API are listed by version. | |||
|
|
|||
| ## ODK Central v2023.4 | |||
| ### ODK Central v2023.5 | |||
There was a problem hiding this comment.
Should this just be ##? Can you put backticks around the appropriate words below?
|
|
||
| await exhaust(container); | ||
|
|
||
| await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') |
There was a problem hiding this comment.
Is this suggesting that you can resolve a conflict even if there is no conflict? It seems like it shouldn't log anything if you try to resolve a conflict that doesn't exist.
Part of getodk/central#506
What has been done to verify that this works as intended?
Test
Why is this the best possible solution? Were any other approaches considered?
Adding a separate function to resolve the conflict felt clean thing to do; instead of modifying existing update function
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?
None.
Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.
Done
Before submitting this PR, please make sure you have:
make test-fulland confirmed all checks still pass OR confirm CircleCI build passes