Return dataset metadata properties and entities in order (#780)

* Return dataset metadata properties in order * Adding another test, but one that shows off another bug * Publish individual dataset properties as needed * Export entities in csv with newest at top * Ordering by published time, adding a test, and fixing publish query * Still exploring ds prop order with deleted forms
getodk · Feb 28, 2023 · db38d71 · db38d71
1 parent d84662b
commit db38d71
Show file tree

Hide file tree

Showing 5 changed files with 335 additions and 6 deletions.
diff --git a/lib/model/query/datasets.js b/lib/model/query/datasets.js
@@ -107,6 +107,14 @@ insert_property_fields AS (
   INSERT INTO ds_property_fields ("dsPropertyId", "formDefId", "schemaId", "path")
   SELECT "dsPropertyId", "formDefId"::integer, "schemaId"::integer, path FROM all_properties
 )
+${publish ? sql`
+,
+update_ds_properties AS (
+  UPDATE ds_properties SET "publishedAt" = clock_timestamp()
+  FROM all_properties
+  WHERE ds_properties.id = all_properties."dsPropertyId" AND ds_properties."publishedAt" IS NULL
+)
+` : sql``}
 `;
 
 const _createOrMerge = (dataset, fields, acteeId, publish) => sql`
@@ -137,9 +145,12 @@ const _getByNameSql = ((fields, datasetName, projectId, includeForms) => sql`
     ) stats on stats."datasetId" = datasets.id
     LEFT OUTER JOIN ds_properties ON
         datasets.id = ds_properties."datasetId" AND ds_properties."publishedAt" IS NOT NULL
-    ${includeForms ? sql`
     LEFT OUTER JOIN ds_property_fields ON
         ds_properties.id = ds_property_fields."dsPropertyId"
+    LEFT OUTER JOIN form_fields ON
+        ds_property_fields.path = form_fields.path
+        AND ds_property_fields."schemaId" = form_fields."schemaId"
+    ${includeForms ? sql`
     LEFT OUTER JOIN forms ON
         ds_property_fields."formDefId" = forms."currentDefId"
     LEFT JOIN form_defs ON 
@@ -148,6 +159,7 @@ const _getByNameSql = ((fields, datasetName, projectId, includeForms) => sql`
     WHERE datasets.name = ${datasetName}
       AND datasets."projectId" = ${projectId}
       AND datasets."publishedAt" IS NOT NULL
+    ORDER BY ds_properties."publishedAt", form_fields.order, ds_properties.id
  `);
 
 const _getLinkedForms = (datasetName, projectId) => sql`
@@ -333,12 +345,14 @@ WITH properties_update as (
   JOIN form_defs fd ON fs."id" = fd."schemaId"
   WHERE fd."id" = ${formDefId}
   AND dpf."dsPropertyId" = dp.id
+  AND dp."publishedAt" IS NULL
   RETURNING dp.*
 ), datasets_update as (
   UPDATE datasets ds SET "publishedAt" = ${publishedAt}
   FROM dataset_form_defs dfd
   WHERE dfd."formDefId" = ${formDefId}
   AND dfd."datasetId" = ds.id
+  AND ds."publishedAt" IS NULL
   RETURNING *
 )
 -- selecting following for publish.audit

diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js
@@ -147,7 +147,7 @@ WHERE
   entities."datasetId" = ${datasetId}
   AND entity_defs.current=true
   AND  ${odataFilter(options.filter, odataToColumnMap)}
-ORDER BY entities.id
+ORDER BY entities."createdAt" DESC, entities.id DESC
 ${page(options)}`)
     .then(stream.map(_exportUnjoiner));
 

diff --git a/test/data/xml.js b/test/data/xml.js
@@ -368,6 +368,31 @@ module.exports = {
   </h:head>
 </h:html>`,
 
+    multiPropertyEntity: `<?xml version="1.0"?>
+<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms">
+  <h:head>
+    <model entities:entities-version="2022.1.0">
+      <instance>
+        <data id="multiPropertyEntity" orx:version="1.0">
+          <q1/>
+          <q2/>
+          <q3/>
+          <q4/>
+          <meta>
+            <entity dataset="foo" id="" create="">
+              <label/>
+            </entity>
+          </meta>
+        </data>
+      </instance>
+      <bind entities:saveto="a_q3" nodeset="/data/q3" type="string"/>
+      <bind entities:saveto="b_q1" nodeset="/data/q1" type="string"/>
+      <bind entities:saveto="c_q4" nodeset="/data/q4" type="string"/>
+      <bind entities:saveto="d_q2" nodeset="/data/q2" type="string"/>
+    </model>
+  </h:head>
+</h:html>`,
+
     groupRepeat: `<?xml version="1.0"?>
     <h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
         <h:head>
@@ -579,6 +604,32 @@ module.exports = {
           <age>40</age>
         </data>`
     },
+    multiPropertyEntity: {
+      one: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="multiPropertyEntity" version="1.0">
+        <meta>
+          <instanceID>one</instanceID>
+          <entities:entity dataset="foo" id="uuid:12345678-1234-4123-8234-123456789aaa" create="1">
+            <entities:label>one</entities:label>
+          </entities:entity>
+        </meta>
+        <q1>w</q1>
+        <q2>x</q2>
+        <q3>y</q3>
+        <q4>z</q4>
+      </data>`,
+      two: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="multiPropertyEntity" version="1.0">
+        <meta>
+          <instanceID>two</instanceID>
+          <entities:entity dataset="foo" id="uuid:12345678-1234-4123-8234-123456789bbb" create="1">
+            <entities:label>two</entities:label>
+          </entities:entity>
+        </meta>
+        <q1>a</q1>
+        <q2>b</q2>
+        <q3>c</q3>
+        <q4>d</q4>
+      </data>`,
+    },
     groupRepeat: {
       one: `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" id="groupRepeat">
       <text>xyz</text>

diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js
@@ -438,11 +438,11 @@ describe('datasets and entities', () => {
               publishedAt.should.not.be.null();
               return p;
             }).should.be.eql([
-              { name: 'age', forms: [ { name: 'simpleEntity', xmlFormId: 'simpleEntity' }, ] },
               { name: 'first_name', forms: [
                 { name: 'simpleEntity', xmlFormId: 'simpleEntity' },
                 { name: 'simpleEntity2', xmlFormId: 'simpleEntity2' }
               ] },
+              { name: 'age', forms: [ { name: 'simpleEntity', xmlFormId: 'simpleEntity' }, ] },
               { name: 'address', forms: [ { name: 'simpleEntity2', xmlFormId: 'simpleEntity2' }, ] }
             ]);
 
@@ -480,6 +480,215 @@ describe('datasets and entities', () => {
           });
 
       }));
+
+      it('should return properties of a dataset in order', testService(async (service) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'b_q1',
+                'd_q2',
+                'a_q3',
+                'c_q4'
+              ]);
+          });
+      }));
+
+      it('should return dataset properties from multiple forms in order', testService(async (service) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity
+            .replace('multiPropertyEntity', 'multiPropertyEntity2')
+            .replace('b_q1', 'f_q1')
+            .replace('d_q2', 'e_q2'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'b_q1',
+                'd_q2',
+                'a_q3',
+                'c_q4',
+                'f_q1',
+                'e_q2'
+              ]);
+          });
+      }));
+
+      it('should return dataset properties from multiple forms including updated form with updated schema', testService(async (service) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity
+            .replace('multiPropertyEntity', 'multiPropertyEntity2')
+            .replace('b_q1', 'f_q1')
+            .replace('d_q2', 'e_q2'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms/multiPropertyEntity/draft')
+          .send(testData.forms.multiPropertyEntity
+            .replace('orx:version="1.0"', 'orx:version="2.0"')
+            .replace('b_q1', 'g_q1'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms/multiPropertyEntity/draft/publish').expect(200);
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'b_q1',
+                'd_q2',
+                'a_q3',
+                'c_q4',
+                'f_q1',
+                'e_q2',
+                'g_q1'
+              ]);
+          });
+      }));
+
+      it('should return dataset properties when purged draft form shares some properties', testService(async (service, { Forms }) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity
+            .replace('multiPropertyEntity', 'multiPropertyEntity2')
+            .replace('b_q1', 'f_q1')
+            .replace('d_q2', 'e_q2'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.delete('/v1/projects/1/forms/multiPropertyEntity')
+          .expect(200);
+
+        await Forms.purge(true);
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'f_q1',
+                'e_q2',
+                'a_q3',
+                'c_q4'
+              ]);
+          });
+      }));
+
+      it('should return dataset properties when draft form (purged before second form publish) shares some properties', testService(async (service, { Forms }) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.post('/v1/projects/1/forms')
+          .send(testData.forms.multiPropertyEntity
+            .replace('multiPropertyEntity', 'multiPropertyEntity2')
+            .replace('b_q1', 'f_q1')
+            .replace('d_q2', 'e_q2'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.delete('/v1/projects/1/forms/multiPropertyEntity')
+          .expect(200);
+
+        await Forms.purge(true);
+
+        await asAlice.post('/v1/projects/1/forms/multiPropertyEntity2/draft/publish');
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'f_q1',
+                'e_q2',
+                'a_q3',
+                'c_q4'
+              ]);
+          });
+      }));
+
+      it.skip('should return ordered dataset properties including from deleted published form', testService(async (service, { Forms }) => {
+        const asAlice = await service.login('alice', identity);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.delete('/v1/projects/1/forms/multiPropertyEntity')
+          .expect(200);
+
+        await Forms.purge(true);
+
+        await asAlice.post('/v1/projects/1/forms?publish=true')
+          .send(testData.forms.multiPropertyEntity
+            .replace('multiPropertyEntity', 'multiPropertyEntity2')
+            .replace('b_q1', 'f_q1')
+            .replace('d_q2', 'e_q2'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+
+        await asAlice.get('/v1/projects/1/datasets/foo')
+          .expect(200)
+          .then(({ body }) => {
+            const { properties } = body;
+            // Properties are coming out in this other order:
+            // [ 'a_q3', 'c_q4', 'b_q1', 'd_q2', 'f_q1', 'e_q2' ]
+            // It's not terrible but would rather all the props of the first form
+            // show up first.
+            properties.map((p) => p.name)
+              .should.be.eql([
+                'b_q1',
+                'd_q2',
+                'a_q3',
+                'c_q4',
+                'f_q1',
+                'e_q2'
+              ]);
+          });
+      }));
     });
   });