Skip to content

Commit

Permalink
Return conflicts count in Project and Dataset GET APIs (#1026)
Browse files Browse the repository at this point in the history
* Return conflicts count in Project and Dataset GET APIs

* moved createConflict() to a separate file
  • Loading branch information
sadiqkhoja authored Oct 17, 2023
1 parent 43474fd commit 9dc21ed
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 11 deletions.
5 changes: 5 additions & 0 deletions docs/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9675,6 +9675,7 @@ paths:
approvalRequired: true
lastEntity: '2018-04-18T03:04:51.695Z'
entities: 10
conflicts: 2
403:
description: Forbidden
content:
Expand Down Expand Up @@ -13580,6 +13581,10 @@ components:
entities:
type: number
description: The number of Entities in the Dataset.
example: 10
conflicts:
type: number
description: The number of Entities that have conflicts.
example: 10.0
PatchAttachment:
required:
Expand Down
6 changes: 4 additions & 2 deletions lib/model/frames/dataset.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ const Dataset = Frame.define(
);

Dataset.Extended = class extends Frame.define(
'entities', readable, 'lastEntity', readable
'entities', readable, 'lastEntity', readable,
'conflicts', readable
) {
// default these properties to 0, since sql gives null if they're 0.
forApi() {
return {
entities: this.entities || 0,
lastEntity: this.lastEntity
lastEntity: this.lastEntity,
conflicts: this.conflicts || 0
};
}
};
Expand Down
2 changes: 1 addition & 1 deletion lib/model/query/datasets.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ const _get = extender(Dataset)(Dataset.Extended)((fields, extend, options, publi
SELECT ${fields} FROM datasets
${extend|| sql`
LEFT JOIN (
SELECT "datasetId", COUNT(1) "entities", MAX(COALESCE("updatedAt", "createdAt")) "lastEntity"
SELECT "datasetId", COUNT(1) "entities", MAX(COALESCE("updatedAt", "createdAt")) "lastEntity", COUNT(1) FILTER (WHERE conflict IS NOT NULL) "conflicts"
FROM entities e
GROUP BY "datasetId"
) stats on stats."datasetId" = datasets.id`}
Expand Down
2 changes: 1 addition & 1 deletion test/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ should.Assertion.add('ExtendedDataset', function assertExtendedDataset() {
this.params = { operator: 'to be an extended Dataset' };

this.obj.should.be.a.Dataset();
Object.keys(this.obj).should.containDeep([ 'entities', 'lastEntity' ]);
Object.keys(this.obj).should.containDeep([ 'entities', 'lastEntity', 'conflicts' ]);
this.obj.entities.should.be.a.Number();
if (this.obj.lastEntity != null) this.obj.lastEntity.should.be.an.isoDate();
});
Expand Down
28 changes: 24 additions & 4 deletions test/integration/api/datasets.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('datasets and entities', () => {
.then(({ body }) => {
body[0].should.be.an.ExtendedDataset();
body.map(({ createdAt, lastEntity, ...d }) => d).should.eql([
{ name: 'people', projectId: 1, entities: 1, approvalRequired: false }
{ name: 'people', projectId: 1, entities: 1, approvalRequired: false, conflicts: 0 }
]);
});
}));
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('datasets and entities', () => {
should(lastEntity).be.null();
return d;
}).should.eql([
{ name: 'people', projectId: 1, entities: 0, approvalRequired: false }
{ name: 'people', projectId: 1, entities: 0, approvalRequired: false, conflicts: 0 }
]);
});
}));
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('datasets and entities', () => {
lastEntity.should.not.be.null();
return d;
}).should.eql([
{ name: 'people', projectId: 1, entities: 1, approvalRequired: false }
{ name: 'people', projectId: 1, entities: 1, approvalRequired: false, conflicts: 0 }
]);
});
}));
Expand All @@ -171,6 +171,23 @@ describe('datasets and entities', () => {

await exhaust(container);

await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?force=true')
.send({ data: { age: '99' } })
.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 container.run(sql`UPDATE entities SET "createdAt" = '1999-1-1' WHERE TRUE`);

await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions')
Expand All @@ -189,12 +206,14 @@ describe('datasets and entities', () => {
.expect(200)
.then(({ body }) => {
body[0].should.be.an.ExtendedDataset();
body.map(({ createdAt, lastEntity, ...d }) => {
body.map(({ createdAt, lastEntity, conflicts, ...d }) => {
lastEntity.should.not.startWith('1999');
return d;
}).should.eql([
{ name: 'people', projectId: 1, entities: 2, approvalRequired: false }
]);

body[0].conflicts.should.equal(1);
});
}));

Expand Down Expand Up @@ -695,6 +714,7 @@ describe('datasets and entities', () => {
projectId: 1,
approvalRequired: false,
entities: 1,
conflicts: 0,
linkedForms: [],
sourceForms: [{ name: 'simpleEntity', xmlFormId: 'simpleEntity' }]
});
Expand Down
9 changes: 7 additions & 2 deletions test/integration/api/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { testService } = require('../setup');
const testData = require('../../data/xml');
const { QueryOptions } = require('../../../lib/util/db');
const { Actor } = require('../../../lib/model/frames');
const { createConflict } = require('../fixtures/scenarios');
// eslint-disable-next-line import/no-dynamic-require
const { exhaust } = require(appRoot + '/lib/worker/worker');

Expand Down Expand Up @@ -1571,7 +1572,7 @@ describe('api: /projects?forms=true', () => {
form.reviewStates.received.should.equal(2);
})))));

it('should set project data from datasetList even on non-extended projects', testService(async (service) => {
it('should set project data from datasetList even on non-extended projects', testService(async (service, container) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/forms?publish=true')
Expand Down Expand Up @@ -1608,17 +1609,21 @@ describe('api: /projects?forms=true', () => {
})
.expect(200);

await createConflict(asAlice, container);

await asAlice.get('/v1/projects?datasets=true')
.expect(200)
.then(({ body }) => {
body.length.should.equal(1);
const project = body[0];
project.should.be.a.Project();
project.datasets.should.equal(2);
project.lastEntity.should.be.eql(body[0].datasetList[0].lastEntity);
project.lastEntity.should.be.eql(body[0].datasetList[1].lastEntity);
const dataset = body[0].datasetList[0];
dataset.should.be.a.ExtendedDataset();
dataset.name.should.equal('foo');

body[0].datasetList[1].conflicts.should.equal(1);
});
}));

Expand Down
33 changes: 33 additions & 0 deletions test/integration/fixtures/scenarios.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const appRoot = require('app-root-path');
const { exhaust } = require(appRoot + '/lib/worker/worker');
const testData = require('../../data/xml');

const createConflict = async (user, container) => {
await user.post('/v1/projects/1/forms/simpleEntity/submissions')
.send(testData.instances.simpleEntity.one)
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

await user.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?force=true')
.send({ data: { age: '99' } })
.expect(200);

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

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

await exhaust(container);
};

module.exports = {
createConflict
};
1 change: 0 additions & 1 deletion test/integration/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,3 @@ const withClosedForm = (f) => async (service) => {
};

module.exports = { testService, testServiceFullTrx, testContainer, testContainerFullTrx, testTask, withClosedForm };

0 comments on commit 9dc21ed

Please sign in to comment.