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

Feature/entities/resolve conflict #1023

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion docs/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

**Added**:

- Entities accessed via API now have conflict and version-specific conflictingProperties fields

* Conflicts are either `hard` (baseVersion conflicts and multiple versions update the same property), `soft` (baseVersion conflicts but data updates are independent), or `null`

**Changed**:

- Conflicts can be resolved using the [PATCH entity](/central-api-entity-management/#updating-an-entity) endpoint with or without new data

### ODK Central v2023.4

**Added**:

Expand Down Expand Up @@ -9957,6 +9969,7 @@ paths:
updatedAt: '2018-03-21T12:45:02.312Z'
deletedAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
conflict: null
creator:
createdAt: '2018-04-18T23:19:14.802Z'
displayName: My Display Name
Expand All @@ -9970,6 +9983,7 @@ paths:
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
creator:
createdAt: '2018-04-18T23:19:14.802Z'
displayName: My Display Name
Expand Down Expand Up @@ -10042,12 +10056,14 @@ paths:
updatedAt: '2018-03-21T12:45:02.312Z'
deletedAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
conflict: null
currentVersion:
label: John (88)
current: true
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand Down Expand Up @@ -10101,12 +10117,14 @@ paths:
updatedAt: '2018-03-21T12:45:02.312Z'
deletedAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
conflict: null
currentVersion:
label: John (88)
current: true
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand All @@ -10121,6 +10139,7 @@ paths:
updatedAt: '2018-03-21T12:45:02.312Z'
deletedAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
conflict: null
creator:
createdAt: '2018-04-18T23:19:14.802Z'
displayName: My Display Name
Expand All @@ -10134,6 +10153,7 @@ paths:
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand Down Expand Up @@ -10235,6 +10255,10 @@ paths:
Use this API to update one or all properties of an Entity. It will throw `400 - Bad Request` if any of the updating properties doesn't exist in the dataset.

To unset value of any property, you can set it to empty string (""). Setting it to `null` will throw an error.

sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
**Resolve the conflict**

You can also use this endpoint to resolve the conflict by passing `resolve=true` query parameter. If you set `resolve` parameter then providing data in the body is optional, in that case only `conflict` status from the Entity will be cleared.
operationId: Updating an Entity
parameters:
- name: projectId
Expand All @@ -10258,6 +10282,20 @@ paths:
schema:
type: string
example: 54a405a0-53ce-4748-9788-d23a30cc3afa
- name: force
in: query
description: Flag to forcefully update the Entity
required: false
schema:
type: boolean
example: true
- name: resolve
in: query
description: Flag to resolve the conflict
required: false
schema:
type: boolean
example: true
requestBody:
content:
'*/*':
Expand Down Expand Up @@ -10287,12 +10325,14 @@ paths:
updatedAt: '2018-03-21T12:45:02.312Z'
deletedAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
conflict: null
currentVersion:
label: John (88)
current: true
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand Down Expand Up @@ -10372,6 +10412,7 @@ paths:
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand All @@ -10387,6 +10428,7 @@ paths:
createdAt: '2018-03-21T12:45:02.312Z'
creatorId: 1
userAgent: Enketo/3.0.4
conflictingProperties: null
data:
firstName: John
age: '88'
Expand Down Expand Up @@ -13175,6 +13217,17 @@ components:
type: number
description: The ID of the Actor (App User, User, or Public Link) that originally created the Entity.
example: 1
conflict:
type: string
description: |-
Type of the conflict.

`hard`: baseVersion conflicts and multiple versions update the same property

`soft`: baseVersion conflicts but data updates are independent
enum:
- soft
- hard
EntityVersionFields:
type: object
properties:
Expand All @@ -13194,6 +13247,11 @@ components:
type: string
description: The self-identified `userAgent` of the device that created the `Entity` version.
example: Enketo/3.0.4
conflictingProperties:
type: array
description: list of properties updated offline simultaneously.
items:
type: string
EntitySummary:
allOf:
- $ref: '#/components/schemas/EntitySummaryFields'
Expand Down
4 changes: 2 additions & 2 deletions lib/model/query/audits.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const actionCondition = (action) => {
// The backup action was logged by a backup script that has been removed.
// Even though the script has been removed, the audit log entries it logged
// have not, so we should continue to exclude those.
return sql`action not in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`;
return sql`action not in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`;
else if (action === 'user')
return sql`action in ('user.create', 'user.update', 'user.delete', 'user.assignment.create', 'user.assignment.delete', 'user.session.create')`;
else if (action === 'field_key')
Expand All @@ -52,7 +52,7 @@ const actionCondition = (action) => {
else if (action === 'dataset')
return sql`action in ('dataset.create', 'dataset.update')`;
else if (action === 'entity')
return sql`action in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete')`;
return sql`action in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete')`;

return sql`action=${action}`;
};
Expand Down
12 changes: 11 additions & 1 deletion lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,16 @@ createVersion.audit = (updatedEntity, dataset, partial, subDef) => (log) => {
};
createVersion.audit.withResult = true;

////////////////////////////////////////////////////////////////////////////////
// RESOLVE CONFLICT

const resolveConflict = (entity, dataset) => ({ run }) => // eslint-disable-line no-unused-vars
run(sql`update entities set conflict=null where "id"=${entity.id}`);
resolveConflict.audit = (entity, dataset) => (log) => log('entity.update.resolve', dataset, {
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
entityId: entity.id,
entityDefId: entity.aux.currentVersion.id,
entity: { uuid: entity.uuid, dataset: dataset.name }
});

/////////////////////////////////////////////////////////////////////////
// Processing submission events to create and update entities
Expand Down Expand Up @@ -449,5 +458,6 @@ module.exports = {
createVersion,
countByDatasetId, getById,
getAll, getAllDefs, del,
createEntitiesFromPendingSubmissions
createEntitiesFromPendingSubmissions,
resolveConflict
};
19 changes: 18 additions & 1 deletion lib/resources/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,15 @@ module.exports = (service, endpoint) => {

const entity = await Entities.getById(dataset.id, params.uuid).then(getOrNotFound);

if (isTrue(query.resolve) && !entity.conflict) return reject(Problem.user.noConflictEntity());

// User just wants to resolve the conflict, so body is empty
// Resolve the conflict and shortcircuit
if (isTrue(query.resolve) && (!body || Object.keys(body).length === 0)) {
await Entities.resolveConflict(entity, dataset);
return entity;
}

const clientEntityVersion = headers['if-match'] && headers['if-match'].replaceAll('"', '');
const serverEntityVersion = entity.aux.currentVersion.version; // aka baseVersion

Expand All @@ -116,7 +125,15 @@ module.exports = (service, endpoint) => {

const sourceId = await Entities.createSource();

return Entities.createVersion(dataset, partial, null, serverEntityVersion + 1, sourceId, serverEntityVersion, userAgent);
const updatedEntity = await Entities.createVersion(dataset, partial, null, serverEntityVersion + 1, sourceId, serverEntityVersion, userAgent);

// User wants to resolve conflict in addition to update the Entity
if (isTrue(query.resolve)) {
await Entities.resolveConflict(updatedEntity, dataset);
}

return updatedEntity;

}));

service.delete('/projects/:projectId/datasets/:name/entities/:uuid', endpoint(async ({ Datasets, Entities }, { auth, params, queryOptions }) => {
Expand Down
2 changes: 2 additions & 0 deletions lib/util/problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ const problems = {
// { expected: "list of expected parameters", actual: "list of provided parameters" }
unexpectedAttributes: problem(400.31, ({ expected, actual }) => `Expected parameters: (${expected.join(', ')}). Got (${actual.join(', ')}).`),

noConflictEntity: problem(400.32, () => 'The Entity doesn\'t have any conflict'),

// no detail information for security reasons.
authenticationFailed: problem(401.2, () => 'Could not authenticate with the provided credentials.'),

Expand Down
21 changes: 20 additions & 1 deletion test/integration/api/audits.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,23 @@ describe('/audits', () => {
data: { age: '77', first_name: 'Alan' }
})
.expect(200);

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.updateEntity)
.set('Content-Type', 'application/xml')
.expect(200);

// all properties changed
await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.one)
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true')
Copy link
Member

Choose a reason for hiding this comment

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

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.

.expect(200);

await asAlice.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200)
.then(({ body }) => {
Expand All @@ -391,9 +408,11 @@ describe('/audits', () => {
await asAlice.get('/v1/audits?action=entity')
.expect(200)
.then(({ body }) => {
body.length.should.equal(4);
body.length.should.equal(6);
body.map(a => a.action).should.eql([
'entity.delete',
'entity.update.resolve',
'entity.update.version',
'entity.update.version',
'entity.create.error',
'entity.create'
Expand Down
Loading