Fix to populating project viewer and edited submission counts in anal…

…ytics (#437)

* Fix to populating proj viewer count and edited submission state

* Fixing big issue with metricsTemplate getting modified when it shouldnt

* Fixed null check issue

* Added tests of analytics data in final template w/o errors

* split up analytics preview query tests

* Attempting to increase test timeout

* Attempting to increase test timeout

* Fixed promise rejection when analytics not configured properly

* Version bump of analytics form

config/default.json

@@ -31,7 +31,7 @@
       "analytics": {
         "url": "https://data.getodk.cloud/v1/key/eOZ7S4bzyUW!g1PF6dIXsnSqktRuewzLTpmc6ipBtRq$LDfIMTUKswCexvE0UwJ9/projects/1/submission",
         "formId": "odk-analytics",
-        "version": "2021.09.22.01"
+        "version": "2021.12.15.01"
       }
     }
   },

lib/model/query/analytics.js

@@ -17,7 +17,7 @@ const _cutoffDate = sql`current_date - cast(${DAY_RANGE} as int)`;
 // Gets a pointer to the project (repeat) in the metrics report for a specific
 // project, creating it first from the project metrics template if necessary
 const _getProject = (projects, id) => {
-  if (!Object.hasOwnProperty.call(projects, id)) {
+  if (projects[id] == null) {
     projects[id] = clone(metricsTemplate.projects[0]); // eslint-disable-line no-param-reassign
     projects[id].id = id; // eslint-disable-line no-param-reassign
   }
@@ -319,7 +319,7 @@ const projectMetrics = () => (({ Analytics }) => Promise.all([
       case 'formfill':
         project.users.num_data_collectors = { recent: row.recent, total: row.total };
         break;
-      case 'viewers':
+      case 'viewer':
         project.users.num_viewers = { total: row.total, recent: row.recent };
         break;
       default:
@@ -368,19 +368,15 @@ const projectMetrics = () => (({ Analytics }) => Promise.all([
 
   for (const row of subStates) {
     const project = _getProject(projects, row.projectId);
-    switch (row.reviewState) {
-      case 'approved':
-        project.submissions.num_submissions_approved = { total: row.total, recent: row.recent };
-        break;
-      case 'hasIssues':
-        project.submissions.num_submissions_has_issues = { recent: row.recent, total: row.total };
-        break;
-      case 'rejected':
-        project.submissions.num_submissions_rejected = { total: row.total, recent: row.recent };
-        break;
-      default:
-        break;
-    }
+    const counts = { total: row.total, recent: row.recent };
+    const mapping = {
+      approved: 'num_submissions_approved',
+      hasIssues: 'num_submissions_has_issues',
+      rejected: 'num_submissions_rejected',
+      edited: 'num_submissions_edited'
+    };
+    if (mapping[row.reviewState])
+      project.submissions[mapping[row.reviewState]] = counts;
   }
 
   for (const row of subEdited) {
@@ -414,7 +410,7 @@ const previewMetrics = () => (({ Analytics }) => Promise.all([
   Analytics.auditLogs(),
   Analytics.projectMetrics()
 ]).then(([db, backups, encrypt, bigForm, admins, audits, projMetrics]) => {
-  const metrics = metricsTemplate;
+  const metrics = clone(metricsTemplate);
   // system
   for (const [key, value] of Object.entries(db))
     metrics.system[key] = value;

lib/task/analytics.js

@@ -19,7 +19,7 @@ const Problem = require('../util/problem');
 const runAnalytics = task.withContainer(({ Analytics, Audits, odkAnalytics }) => (force) => {
   if (!config.has('default.external.analytics.url') ||
     !config.has('default.external.analytics.formId'))
-    return Problem.internal.analyticsNotConfigured();
+    return Promise.reject(Problem.internal.analyticsNotConfigured());
 
   return getConfiguration('analytics')
     .then((configuration) => ((configuration.value.enabled === false)

test/integration/other/analytics-queries.js

@@ -573,6 +573,143 @@ describe('analytics task queries', () => {
     }));
   });
 
+  describe('combined analytics', function() {
+    // increasing timeouts on this set of tests
+    this.timeout(4000);
+
+    it('should combine system level queries', testService(async (service, container) => {
+      // backups
+      await container.Configs.set('backups.main', {detail: 'dummy'});
+
+      // encrypting a project
+      await service.login('alice', (asAlice) =>
+        asAlice.post(`/v1/projects/1/key`)
+          .send({ passphrase: 'supersecret', hint: 'it is a secret' }));
+
+      const res = await container.Analytics.previewMetrics();
+
+      // everything in system filled in
+      res.system.num_admins.total.should.equal(1);
+      res.system.num_projects_encryption.total.should.equal(1);
+      res.system.num_questions_biggest_form.should.equal(5);
+      res.system.num_audit_log_entries.total.should.be.above(0);
+      res.system.backups_configured.should.equal(1);
+      res.system.database_size.should.be.above(0);
+    }));
+
+    it('should fill in all project.users queries', testService(async (service, container) => {
+      // create more users
+      await createTestUser(service, container, 'Collector1', 'formfill', 1, false);
+      await createTestUser(service, container, 'Viewer1', 'viewer', 1, false); // no recent activity
+
+      // submission with device id
+      await submitToForm(service, 'alice', 1, 'simple', testData.instances.simple.one, 'device1');
+
+      // create app users
+      const token = await createAppUser(service, 1, 'simple');
+      await service.post(`/v1/key/${token}/projects/1/forms/simple/submissions`)
+        .send(simpleInstance('app_user_token'))
+        .set('Content-Type', 'application/xml');
+
+      // public links
+      const publicLink = await createPublicLink(service, 1, 'simple');
+      await service.post(`/v1/key/${publicLink}/projects/1/forms/simple/submissions`)
+        .send(simpleInstance('pub_link'))
+        .set('Content-Type', 'application/xml');
+
+      const res = await container.Analytics.previewMetrics();
+
+      // check everything is non-zero
+      Object.values(res.projects[0].users).forEach((metric) => metric.total.should.be.above(0));
+    }));
+
+    it('should fill in all project.forms queries', testService(async (service, container) => {
+      // geospatial form
+      await createTestForm(service, container, geoForm, 1);
+
+      // encrypted form
+      await createTestForm(service, container, testData.forms.encrypted, 1);
+
+      // form with audit
+      await createTestForm(service, container, testData.forms.clientAudits, 1);
+      await service.login('alice', (asAlice) =>
+        asAlice.post(`/v1/projects/1/submission`)
+          .set('X-OpenRosa-Version', '1.0')
+          .attach('audit.csv', createReadStream(appRoot + '/test/data/audit.csv'), { filename: 'audit.csv' })
+          .attach('xml_submission_file', Buffer.from(testData.instances.clientAudits.one), { filename: 'data.xml' }));
+
+      const res = await container.Analytics.previewMetrics();
+
+      // check everything is non-zero
+      Object.values(res.projects[0].forms).forEach((metric) => metric.total.should.be.above(0));
+    }));
+
+    it('should fill in all project.submissions queries', testService(async (service, container) => {
+     // submission states
+      for (const state of ['approved', 'rejected', 'hasIssues', 'edited']) {
+        await submitToForm(service, 'alice', 1, 'simple', simpleInstance(state));
+        await service.login('alice', (asAlice) =>
+          asAlice.patch(`/v1/projects/1/forms/simple/submissions/${state}`)
+            .send({ reviewState: state }));
+      }
+
+      // with edits
+      await submitToForm(service, 'alice', 1, 'simple', simpleInstance('v1'));
+      await service.login('alice', (asAlice) =>
+        asAlice.post(`/v1/projects/1/submission`)
+          .set('X-OpenRosa-Version', '1.0')
+          .attach('xml_submission_file', Buffer.from(withSimpleIds('v1', 'v2').replace('Alice', 'Alyssa')), { filename: 'data.xml' }));
+
+      // comments
+      await submitToForm(service, 'alice', 1, 'simple', testData.instances.simple.one, 'device1');
+      await service.login('alice', (asAlice) =>
+        asAlice.post(`/v1/projects/1/forms/simple/submissions/one/comments`)
+          .send({ body: 'new comment here' }));
+
+      // public link
+      const publicLink = await createPublicLink(service, 1, 'simple');
+      await service.post(`/v1/key/${publicLink}/projects/1/forms/simple/submissions`)
+        .send(simpleInstance('111'))
+        .set('Content-Type', 'application/xml');
+
+      // app user token
+      const token = await createAppUser(service, 1, 'simple');
+      await service.post(`/v1/key/${token}/projects/1/forms/simple/submissions`)
+        .send(simpleInstance('aaa'))
+        .set('Content-Type', 'application/xml');
+
+      const res = await container.Analytics.previewMetrics();
+
+      // check everything is non-zero
+      Object.values(res.projects[0].submissions).forEach((metric) => metric.total.should.be.above(0));
+    }));
+
+    it('should be idempotent and not cross-polute project counts', testService(async (service, container) => {
+      const proj1 = await createTestProject(service, container, 'New Proj 1');
+      const formId1 = await createTestForm(service, container, testData.forms.simple, proj1);
+
+      // approved submission in original project
+      await submitToForm(service, 'alice', 1, 'simple', simpleInstance('aaa'));
+      await service.login('alice', (asAlice) =>
+        asAlice.patch('/v1/projects/1/forms/simple/submissions/aaa')
+          .send({ reviewState: 'approved' }));
+
+      // rejected submission in new project
+      await submitToForm(service, 'alice', proj1, 'simple', simpleInstance('aaa'));
+      await service.login('alice', (asAlice) =>
+        asAlice.patch(`/v1/projects/${proj1}/forms/simple/submissions/aaa`)
+          .send({ reviewState: 'rejected' }));
+
+      let res = await container.Analytics.previewMetrics();
+      res.projects[1].submissions.num_submissions_approved.total.should.equal(0);
+
+      // there used to be a bug where counts from one project could
+      // pollute another project.
+      res = await container.Analytics.previewMetrics();
+      res.projects[1].submissions.num_submissions_approved.total.should.equal(0);
+    }));
+  });
+
   describe('latest analytics audit log utility', () => {
     it('should find recently created analytics audit log', testService(async (service, container) => {
       await container.Audits.log(null, 'analytics', null, {test: 'foo', success: true});