enhances #722: add ability to select parentID (#915)

* enhances #722: add ability to select parentID

* added more tests

lib/formats/odata.js

@@ -391,8 +391,8 @@ const rowStreamToOData = (fields, table, domain, originalUrl, query, inStream, t
 
         for (const field of data) {
 
-          // if $select is there then don't return parent IDs, currently we don't support selecting parent IDs
-          const fieldRefined = options.metadata ? without([parentIdProperty], field) : field;
+          // if $select is there and parentId is not requested then remove it
+          const fieldRefined = options.metadata && !options.metadata[parentIdProperty] ? without([parentIdProperty], field) : field;
 
           if ((counted >= doOffset) && (counted < (doOffset + doLimit))) {
             this.push((counted === doOffset) ? '{"value":[' : ','); // header or fencepost.
@@ -466,8 +466,8 @@ const singleRowToOData = (fields, row, domain, originalUrl, query) => {
     const nextUrl = nextUrlFor(limit, offset, count, originalUrl);
     const pared = filtered.slice(offset, offset + limit);
 
-    // if $select is there then don't return parent IDs, currently we don't support selecting parent IDs
-    const paredRefined = options.metadata ? pared.map(p => without([filterField], p)) : pared;
+    // if $select is there and parentId is not requested then remove it
+    const paredRefined = options.metadata && !options.metadata[filterField] ? pared.map(p => without([filterField], p)) : pared;
 
     // and finally splice together and return our result:
     const dataContents = paredRefined.map(JSON.stringify).join(',');
@@ -527,42 +527,55 @@ const filterFields = (formFields, select, table) => {
 
   let path = '';
 
+  const idKeys = new Set(['__id']);
+
+  // we return parent ID with the subtable, but the key is dynamic
+  // based on the last repeat field
+  let parentIdKey = '__Submissions';
+
   // For subtables we have to include parents fields
   if (table !== 'Submissions') {
-    for (const tableSegment of table.replace(/Submissions\./, '').split('.')) {
+    const tableSegments = table.replace(/Submissions\./, '').split('.');
+    for (let i = 0; i < tableSegments.length; i+=1) {
+      const tableSegment = tableSegments[i];
+
       path += `/${tableSegment}`;
       if (!fieldTree[path]) throw Problem.user.notFound();
       filteredFields.add(fieldTree[path].value);
+
+      if (fieldTree[path].value.type === 'repeat' && i < tableSegments.length-1) {
+        parentIdKey = `__Submissions${path.replaceAll('/', '-')}`;
+      }
     }
+    parentIdKey += '-id';
+    idKeys.add(parentIdKey);
   }
 
   for (const property of select.split(',').map(p => p.trim())) {
     // validate metadata properties. __system/.. properties are only valid for Submission table
-    if (property.startsWith('__id') || property.startsWith('__system')) {
-      if (!(property === '__id' || (table === 'Submissions' && systemFields.has(property))))
-        throw Problem.user.propertyNotFound({ property });
-    } else {
+    if (idKeys.has(property) || (table === 'Submissions' && systemFields.has(property))) {
+      continue;
+    }
 
-      const field = fieldTree[`${path}/${property}`];
-      if (!field) throw Problem.user.propertyNotFound({ property });
+    const field = fieldTree[`${path}/${property}`];
+    if (!field) throw Problem.user.propertyNotFound({ property });
 
-      filteredFields.add(field.value);
+    filteredFields.add(field.value);
 
-      // we have to include parents fields in the result to handle grouped fields
-      let node = field.parent;
-      while (node && !filteredFields.has(node.value)) { // filteredFields set already has the subtables
-        // Child field of a repeat field is not supported
-        if (node.value.type === 'repeat') throw Problem.user.unsupportedODataSelectField({ property });
+    // we have to include parents fields in the result to handle grouped fields
+    let node = field.parent;
+    while (node && !filteredFields.has(node.value)) { // filteredFields set already has the subtables
+      // Child field of a repeat field is not supported
+      if (node.value.type === 'repeat') throw Problem.user.unsupportedODataSelectField({ property });
 
-        filteredFields.add(node.value);
-        node = node.parent;
-      }
+      filteredFields.add(node.value);
+      node = node.parent;
+    }
 
-      // Include the children of structure/group
-      // Note: This doesn't expand 'repeat' fields
-      if (field.value.type === 'structure') {
-        getChildren(field).forEach(filteredFields.add, filteredFields);
-      }
+    // Include the children of structure/group
+    // Note: This doesn't expand 'repeat' fields
+    if (field.value.type === 'structure') {
+      getChildren(field).forEach(filteredFields.add, filteredFields);
     }
   }
 

test/integration/api/odata.js

@@ -521,6 +521,17 @@ describe('api: /forms/:id.svc', () => {
               }]
             });
           }))));
+
+    it('should return only parent ID', testService(async (service) => {
+      const asAlice = await withSubmission(service, identity);
+
+      await asAlice.get('/v1/projects/1/forms/doubleRepeat.svc/Submissions(\'double\')/children/child(\'b6e93a81a53eed0566e65e472d4a4b9ae383ee6d\')/toys/toy?$select=__Submissions-children-child-id')
+        .expect(200)
+        .then(({ body }) => {
+          body.value.forEach(toy => toy.should.have.property('__Submissions-children-child-id'));
+        });
+
+    }));
   });
 
   describe('/Submissions.xyz.* GET', () => {
@@ -1351,6 +1362,27 @@ describe('api: /forms/:id.svc', () => {
             });
           }))));
 
+    it('should return only parent ID', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      await asAlice.post('/v1/projects/1/forms?publish=true')
+        .send(testData.forms.doubleRepeat)
+        .set('Content-Type', 'text/xml')
+        .expect(200);
+
+      await asAlice.post('/v1/projects/1/forms/doubleRepeat/submissions')
+        .send(testData.instances.doubleRepeat.double)
+        .set('Content-Type', 'text/xml')
+        .expect(200);
+
+      await asAlice.get('/v1/projects/1/forms/doubleRepeat.svc/Submissions.children.child.toys.toy?$select=__Submissions-children-child-id')
+        .expect(200)
+        .then(({ body }) => {
+          body.value.forEach(toy => toy.should.have.property('__Submissions-children-child-id'));
+        });
+
+    }));
+
     it('should return subtable results with group properties', testService(async (service) => {
       const asAlice = await service.login('alice');
 

test/unit/formats/odata.js

@@ -1106,6 +1106,11 @@ describe('odata message composition', () => {
         (() => selectFields({ $select: '__id, __system/status' }, 'Submissions')(fields)).should.not.throw();
       }));
 
+    it('should throw error if system properties are requested for non-submissions table', () => fieldsFor(testData.forms.withrepeat)
+      .then((fields) => {
+        (() => selectFields({ $select: '__id, __system/status' }, 'Submissions.children')(fields)).should.throw('Could not find a property named \'__system/status\'');
+      }));
+
     it('should throw error if unknown system property is requested', () => fieldsFor(testData.forms.simple)
       .then((fields) => {
         (() => selectFields({ $select: '__system/etag' }, 'Submissions')(fields)).should.throw('Could not find a property named \'__system/etag\'');