Disallow string downcast for groups and repeats (getodk#765)

Author: ktuite

lib/model/query/forms.js

@@ -666,11 +666,21 @@ const rejectIfWarnings = () => ({ context }) => {
   }
 };
 
+// This function replaces a database trigger called check_field_collisions that
+// prevents certain kinds of field type changes and downcasts.
+// - Most types can be downcast to a string, but they cannot change between non-string types.
+//    e.g. A date can become a string, but a string can't become a date, int, etc. because if
+//    the data in that string is not a valid date/int, it will cause problems when exporting.
+// - A group or repeat cannot be downcast to a string.
 const checkFieldDowncast = (allFields, fields) => () => {
   const lookup = {};
   for (const f of allFields) lookup[f.path] = f;
-  for (const f of fields.filter(ff => (ff.type !== 'string'))) {
-    if (lookup[f.path] && f.type !== lookup[f.path].type)
+  for (const f of fields) {
+    // If new field type is not a string, types must stay the same
+    if (f.type !== 'string' && lookup[f.path] && f.type !== lookup[f.path].type)
+      return reject(Problem.user.fieldTypeConflict({ path: f.path, type: lookup[f.path].type }));
+    // Downcasting to string is only allowed if not group/structure or repeat.
+    if (f.type === 'string' && lookup[f.path] && (lookup[f.path].type === 'structure' || lookup[f.path].type === 'repeat'))
       return reject(Problem.user.fieldTypeConflict({ path: f.path, type: lookup[f.path].type }));
   }
   return resolve();

test/integration/api/forms/draft.js

@@ -370,7 +370,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
                 body.details.should.eql({ path: '/age', type: 'string' });
               })))));
 
-      it.skip('should complain on downcast from group to string', testService((service) =>
+      it('should complain on downcast from group to string', testService((service) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms/simple/draft')
             .send(testData.forms.simple.replace('nodeset="/data/meta/instanceID"', 'nodeset="/data/meta"'))
@@ -381,11 +381,11 @@ describe('api: /projects/:id/forms (drafts)', () => {
               body.details.should.eql({ path: '/meta', type: 'structure' });
             }))));
 
-      it.skip('should complain on downcast from repeat to string', testService((service) =>
+      it('should complain on downcast from repeat to string', testService((service) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms/withrepeat/draft')
             .send(testData.forms.withrepeat
-              .replace('</model>', '<bind nodeset="/data/children/child" type="int"/></model>')
+              .replace('</model>', '<bind nodeset="/data/children/child" type="string"/></model>')
               .replace('<repeat', '<rpt')
               .replace('</repeat', '</rpt'))
             .set('Content-Type', 'application/xml')