diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js index 00dee2193..341135127 100644 --- a/lib/model/query/forms.js +++ b/lib/model/query/forms.js @@ -319,7 +319,7 @@ createVersion.audit.withResult = true; // TODO: we need to make more explicit what .def actually represents throughout. // for now, enforce an extra check here just in case. -const publish = (form) => async ({ Forms, Datasets }) => { +const publish = (form) => async ({ Forms, Datasets, FormAttachments }) => { if (form.draftDefId !== form.def.id) throw Problem.internal.unknown(); // Try to push the form to Enketo if it hasn't been pushed already. If doing @@ -329,15 +329,27 @@ const publish = (form) => async ({ Forms, Datasets }) => { : {}; const publishedAt = (new Date()).toISOString(); - return Promise.all([ + await Promise.all([ Forms._update(form, { currentDefId: form.draftDefId, draftDefId: null, ...enketoIds }), Forms._updateDef(form.def, { draftToken: null, enketoId: null, publishedAt }), - Datasets.publishIfExists(form.def.id, publishedAt) ]) .catch(Problem.translate( Problem.user.uniquenessViolation, () => Problem.user.versionUniquenessViolation({ xmlFormId: form.xmlFormId, version: form.def.version }) )); + + const ds = await Datasets.publishIfExists(form.def.id, publishedAt); + // this check is for issue c#554. we will hopefully come back to this flow and improve it later. + // ds contains a list of objects about what happened with the dataset + // like if the dataset was published and which properties were published. + // if it's empty, there is no dataset to work with. + if (ds.length > 0) { + const dataset = await Datasets.getById(ds[0].id).then(o => o.get()); + const attachment = await FormAttachments.getByFormDefIdAndName(form.def.id, `${dataset.name}.csv`); + if (attachment.isDefined() && attachment.get().blobId == null && attachment.get().datasetId == null) { + await FormAttachments.update(form, attachment.get(), null, dataset.id); + } + } }; publish.audit = (form) => (log) => log('form.update.publish', form, { oldDefId: form.currentDefId, newDefId: form.draftDefId }); diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js index ee27d5a2b..f536f102c 100644 --- a/test/integration/api/datasets.js +++ b/test/integration/api/datasets.js @@ -1496,6 +1496,372 @@ describe('datasets and entities', () => { .then(attachment => { should(attachment.value.datasetId).be.null(); }))))))); + + describe('autolink when publishing form that creates and consumes new dataset', () => { + // update form that consumes dataset + const updateForm = ` + + + + + + + + + + + + + + + + + + + + + `; + + it('should NOT autolink on upload new form and simultaneously publish', testService(async (service) => { + // this path of directly publishing a form on upload isn't possible in central + // so it's not going to be supported. if it were, logic would go around line #170 in + // lib/model/query/forms.js in Forms.createNew after the dataset is published. + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(updateForm) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].datasetExists.should.be.false(); + }); + })); + + it('should autolink when first publishing a draft', testService(async (service) => { + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms') + .send(updateForm) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.true(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.true(); + }); + })); + + it('should not autolink if attachment already filled in', testService(async (service) => { + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms') + .send(updateForm) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/attachments/people.csv') + .send('test,csv\n1,2') + .set('Content-Type', 'text/csv') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.true(); + body[0].blobExists.should.be.true(); + body[0].datasetExists.should.be.false(); + body[0].updatedAt.should.not.be.null(); + }); + })); + + it('should not autolink if attachment isnt the dataset in question', testService(async (service) => { + const asAlice = await service.login('alice'); + + const differentDataset = ` + + + + + + + + + + + + + + + + + + + + + `; + + await asAlice.post('/v1/projects/1/forms') + .send(differentDataset) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.false(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.false(); + }); + })); + + it('should not autolink if dataset already published because it will be attached already if dataset made before', testService(async (service) => { + const asAlice = await service.login('alice'); + + // upload form that makes people dataset already + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simpleEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms') + .send(updateForm) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/draft/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.true(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.true(); + body[0].should.not.have.property('updatedAt'); // linked to dataset when created, never updated + }); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.true(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.true(); + body[0].should.not.have.property('updatedAt'); // linked to dataset when created, never updated + }); + })); + + it('should autolink if dataset already published but it was created after the form draft', testService(async (service) => { + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms') + .send(updateForm) + .set('Content-Type', 'application/xml') + .expect(200); + + // upload form that makes people dataset already + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simpleEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + // because the form was uploaded before the dataset was created, this will be null + await asAlice.get('/v1/projects/1/forms/updateEntity/draft/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.false(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.false(); + }); + + // we DO auto-link here but since a Central user can still see the draft, and could + // potentially see the new dataset now, we might want to change this behavior to NOT + // auto-link and have the user explicitly link it themselves. + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.true(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.true(); + }); + })); + + it('should not autolink if the update form doesnt consume dataset', testService(async (service) => { + const asAlice = await service.login('alice'); + + // this form creates a dataset to update but it's not propertly attached to the form + await asAlice.post('/v1/projects/1/forms') + .send(testData.forms.updateEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body.length.should.equal(0); + }); + + await asAlice.get('/v1/projects/1/datasets/people') + .set('X-Extended-Metadata', 'true') + .expect(200) + .then(({ body }) => { + body.linkedForms.length.should.equal(0); + }); + })); + + it('should not autolink if form doesnt create dataset', testService(async (service) => { + const asAlice = await service.login('alice'); + + const noDataset = ` + + + + + + + + + + + + + + + + + + + + `; + + await asAlice.post('/v1/projects/1/forms') + .send(noDataset) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.csv'); + body[0].exists.should.be.false(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.false(); + }); + })); + + it('should not autolink on create/consume dataset if .csv extention doesnt match case', testService(async (service) => { + // we probably want this to be case insensitive but that change will come later. + const asAlice = await service.login('alice'); + + const caseChange = ` + + + + + + + + + + + + + + + + + + + + + `; + + await asAlice.post('/v1/projects/1/forms') + .send(caseChange) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/updateEntity/draft/publish') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.CSV'); + body[0].exists.should.be.false(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.false(); + }); + })); + + it('should not autolink on draft consume-only form if .csv extention doesnt match case', testService(async (service) => { + const asAlice = await service.login('alice'); + + // upload form that makes people dataset already + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simpleEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + const caseChange = ` + + + + + + + + + + + + + + + + + + + + `; + + await asAlice.post('/v1/projects/1/forms') + .send(caseChange) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.get('/v1/projects/1/forms/updateEntity/draft/attachments') + .then(({ body }) => { + body[0].name.should.equal('people.CSV'); + body[0].exists.should.be.false(); + body[0].blobExists.should.be.false(); + body[0].datasetExists.should.be.false(); + }); + })); + }); }); // these scenario will never happen by just using APIs, adding following tests for safety