-
Notifications
You must be signed in to change notification settings - Fork 76
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
Features/entities/relevant to conflict flag #1028
Features/entities/relevant to conflict flag #1028
Conversation
7787851
to
ceff8d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at tests yet, but I thought I'd start with these comments. It's looking good so far!
+ change the definition of lastGoodKnow version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a few places where we need more test coverage, but besides that, I only have small suggestions to make. ✅ Entity updates via submission are a complex feature, so I really think we'll benefit from having excellent test coverage.
Part of getodk/central#504
What has been done to verify that this works as intended?
Added tests
Why is this the best possible solution? Were any other approaches considered?
The name of querystring parameter could be shorter but we came to consensus on
relevantToConflict
in SlackHow 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.
Yes
Before submitting this PR, please make sure you have:
make test-full
and confirmed all checks still pass OR confirm CircleCI build passes