Hotfix: Support treating no countries as any country when syncing sur…

…veys (questions, survey screens, etc.) (beyondessential#4106)
beyondessential · Aug 23, 2022 · 8b109d5 · 8b109d5
1 parent 06351f1
commit 8b109d5
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 39 deletions.
diff --git a/...ages/central-server/src/apiV2/utilities/meditrakSync/permissionsBasedMeditrakSyncQuery.js b/...ages/central-server/src/apiV2/utilities/meditrakSync/permissionsBasedMeditrakSyncQuery.js
@@ -17,12 +17,11 @@ import {
   supportsPermissionsBasedSync,
 } from './supportsPermissionsBasedSync';
 
+const recordTypesToAlwaysSync = ['country', 'permission_group'];
 /**
  * Since all countries, permission_groups, and country entities regardless of permissions
  */
 export const permissionsFreeChanges = since => {
-  const recordTypesToAlwaysSync = ['country', 'permission_group'];
-
   return {
     query: `change_time > ? AND (record_type IN ${SqlQuery.record(
       recordTypesToAlwaysSync,
@@ -32,14 +31,20 @@ export const permissionsFreeChanges = since => {
 };
 
 export const changesWithPermissions = (countryIds, permissionGroups, since) => {
-  let query = 'change_time > ? AND "type" = ?';
-  const params = [since, 'update'];
+  let query = `change_time > ? AND "type" = ? and record_type NOT IN ${SqlQuery.record(
+    recordTypesToAlwaysSync,
+  )}`;
+  const params = [since, 'update', ...recordTypesToAlwaysSync];
 
-  if (countryIds) {
-    query = query.concat(` AND country_ids && ${SqlQuery.array(countryIds, 'text')}`);
+  // When determining survey permissions, NULL countries means permission to all countries
+  if (permissionGroups) {
+    query = query.concat(
+      ` AND (country_ids IS NULL OR country_ids && ${SqlQuery.array(countryIds, 'text')})`,
+    );
     params.push(...countryIds);
   } else {
-    query = query.concat(` AND country_ids IS NULL`);
+    query = query.concat(` AND country_ids && ${SqlQuery.array(countryIds, 'text')}`);
+    params.push(...countryIds);
   }
 
   if (permissionGroups) {
@@ -89,70 +94,63 @@ export const buildPermissionsBasedMeditrakSyncQuery = async (
 
   query.append(selectFromClause(select));
   query.append(`WHERE (
-  `);
+   `);
 
   query.append('(');
   query.append(permissionsFreeChanges(since));
   query.append(')');
 
-  if (unsynced.countries) {
-    // Never before synced countries due to new country permissions
+  const addPermissionsOrClause = clause => {
     query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(unsynced.countryIds, null, 0));
+     OR (`);
+    query.append(clause);
     query.append(')');
+  };
+
+  if (unsynced.countries) {
+    // Never before synced countries due to new country permissions
+    addPermissionsOrClause(changesWithPermissions(unsynced.countryIds, null, 0));
   }
 
   if (unsynced.countries && unsynced.permissionGroups) {
     // Never before synced surveys due to new permission groups and country permissions
-    query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(unsynced.countryIds, unsynced.permissionGroups, 0));
-    query.append(')');
+    addPermissionsOrClause(
+      changesWithPermissions(unsynced.countryIds, unsynced.permissionGroups, 0),
+    );
   }
 
   if (unsynced.countries && synced.permissionGroups) {
     // Never before synced surveys due to new country permissions
-    query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(unsynced.countryIds, synced.permissionGroups, 0));
-    query.append(')');
+    addPermissionsOrClause(changesWithPermissions(unsynced.countryIds, synced.permissionGroups, 0));
   }
 
   if (synced.countries && unsynced.permissionGroups) {
     // Never before synced surveys due to new permission groups
-    query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(synced.countryIds, unsynced.permissionGroups, 0));
-    query.append(')');
+    addPermissionsOrClause(changesWithPermissions(synced.countryIds, unsynced.permissionGroups, 0));
   }
 
   if (synced.countries) {
     // Previously synced countries
-    query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(synced.countryIds, null, since));
-    query.append(')');
+    addPermissionsOrClause(changesWithPermissions(synced.countryIds, null, since));
   }
 
   if (synced.countries && synced.permissionGroups) {
     // Previously synced surveys
-    query.append(` 
-    OR (`);
-    query.append(changesWithPermissions(synced.countryIds, synced.permissionGroups, since));
-    query.append(')');
+    addPermissionsOrClause(
+      changesWithPermissions(synced.countryIds, synced.permissionGroups, since),
+    );
   }
 
   query.append(`
-    OR (`);
+     OR (`);
   query.append(deletesSinceLastSync(since));
   query.append(`
-    ))`);
+     ))`);
 
   const filter = await recordTypeFilter(req);
   if (filter) {
     query.append(`
-      AND `);
+       AND `);
     query.append(filter);
   }
 

diff --git a/packages/central-server/src/database/createPermissionsBasedMeditrakSyncQueue.js b/packages/central-server/src/database/createPermissionsBasedMeditrakSyncQueue.js
@@ -24,8 +24,9 @@ SELECT msq.*,
 	max(e."type") AS entity_type,
 	
 	COALESCE(
+		${groupToArrayOrNull('co.id')}, 
 		${groupToArrayOrNull('e_co.id')}, 
-		${groupToArrayOrNull('c.id')}, 
+		${groupToArrayOrNull('c.country_id')}, 
 		${groupToArrayOrNull('ga.country_id')}, 
 		${groupToFlatArrayOrNull('s.country_ids')},
 		${groupToFlatArrayOrNull('sg_s.country_ids')},
@@ -47,6 +48,7 @@ SELECT msq.*,
 		${groupToArrayOrNull('o_os_q_ssc_ss_s_pg."name"')}
 	) as permission_groups
 FROM meditrak_sync_queue msq 
+LEFT JOIN country co ON msq.record_id = co.id
 LEFT JOIN entity e ON msq.record_id = e.id
 LEFT JOIN country e_co ON e_co.code = e.country_code 
 LEFT JOIN clinic c ON msq.record_id = c.id

diff --git a/packages/central-server/src/tests/apiV2/changes/getChangesMetadata.test.js b/packages/central-server/src/tests/apiV2/changes/getChangesMetadata.test.js
@@ -98,7 +98,7 @@ describe('GET /changes/metadata', async () => {
         });
 
         const { changeCount, countries, permissionGroups } = response.body;
-        expect(changeCount).to.equal(18);
+        expect(changeCount).to.equal(20);
         expect(countries.sort()).to.eql([PERM_SYNC_COUNTRY_1.code]);
         expect(permissionGroups.sort()).to.eql([PERM_SYNC_PG_PUBLIC.name]);
       });
@@ -117,7 +117,7 @@ describe('GET /changes/metadata', async () => {
         });
 
         const { changeCount, countries, permissionGroups } = response.body;
-        expect(changeCount).to.equal(27);
+        expect(changeCount).to.equal(29);
         expect(countries.sort()).to.eql([PERM_SYNC_COUNTRY_1.code, PERM_SYNC_COUNTRY_2.code]);
         expect(permissionGroups.sort()).to.eql([PERM_SYNC_PG_ADMIN.name, PERM_SYNC_PG_PUBLIC.name]);
       });
@@ -140,7 +140,7 @@ describe('GET /changes/metadata', async () => {
         });
 
         const { changeCount, countries } = response.body;
-        expect(changeCount).to.equal(8);
+        expect(changeCount).to.equal(10);
         expect(countries.sort()).to.eql([PERM_SYNC_COUNTRY_1.code, PERM_SYNC_COUNTRY_2.code]);
       });
 

diff --git a/packages/central-server/src/tests/apiV2/changes/permissionsBasedSync.fixtures.js b/packages/central-server/src/tests/apiV2/changes/permissionsBasedSync.fixtures.js
@@ -74,6 +74,12 @@ const SURVEYS = [
     permissionGroup: PERM_SYNC_PG_ADMIN.name,
     countries: [PERM_SYNC_COUNTRY_1.code, PERM_SYNC_COUNTRY_2.code],
   },
+  {
+    code: 'PERM_SYNC_SURVEY_5',
+    name: 'Permission based sync survey 5',
+    permissionGroup: PERM_SYNC_PG_PUBLIC.name,
+    countries: [], // No countries means available to all countries
+  },
 ];
 
 const QUESTIONS = [