RN-423: Add 'Sync Groups' to the Admin Panel (kobo sync link part 1) #4088
Conversation
| const syncGroupChecker = accessPolicy => | ||
| assertSyncGroupEditPermissions(accessPolicy, this.models, this.recordId); | ||
|
|
||
| await this.assertPermissions(assertAnyPermissions([assertBESAdminAccess, syncGroupChecker])); |
There was a problem hiding this comment.
We may need to consider how to change this permission check... I suspect there will be some lesmis users who want to configure the sync group but don't have BES Admin...
| `DROP TRIGGER IF EXISTS data_service_sync_group_trigger ON data_service_sync_group`, | ||
| ); | ||
|
|
||
| await db.runSql(`ALTER TABLE data_service_sync_group RENAME COLUMN code TO data_group_code`); |
There was a problem hiding this comment.
Actually I don't understand how this works. We can't change code here without updating DataBroker.
There was a problem hiding this comment.
Yeah honestly I found this rename a bit confusing as well.
It seems like a dssg has a data_group_code which corresponds to a survey code in Tupaia. But should the syncGroupCode in the config actually just be the code of this dssg? I made that mistake a couple times when writing logic around this, as it seems really weird to me that dssg's don't have a code of their own atm...
I'd suggest changing so that:
current:
id | data_group_code | service_type | config | sync_cursor
-----------------------------------------------------------------------------------
123 | LEMIS_FQS1 | kobo | { syncGroupCode: FQS1, ...} | 10-10-2020
becomes:
id | code | data_group_code | service_type | config | sync_cursor
------------------------------------------------------------------------
123 | FQS1 | LEMIS_FQS1 | kobo | { ...} | 10-10-2020
Then when pulling from data-broker, you do:
const koboData = await dataBroker.pull(
{
code: 'FQS1',
type: 'SYNC_GROUP',
}
);
Thoughts?
You could possibly argue that the koboSurveyCode (which exists in the config) should also have its own column, but I think given that this table is meant to be agnostic in regards to the service_type its appropriate for it to be in the config.
There was a problem hiding this comment.
Also, just wanted to flag a slight concern I have around how often we rely on pseudo foriegn key relationships. Here the data_group_code must correspond to a data_group code. But we don't enforce that in any way via foreign key constraints. It's quiet common for us to do this in Tupaia, but it always feels a bit risky to me... Do you think I ought to add a constraint here?
There was a problem hiding this comment.
Discussed, like the code column, FK not necessary but worth a chat in the future.
| data_group_code = trim(both '"' from config->>'internalSurveyCode'), | ||
| code = trim(both '"' from config->>'internalSurveyCode'), | ||
| config = (config || jsonb_build_object('syncGroupCode', code)) - 'internalSurveyCode' | ||
| `); |
- give it the same value as the 'data_group_code' column as a convention
- Made clearer that the value isn't used for anything but kept purely for posterity's sake
…p (kobo sync link part 2) (#4089)
rohan-bes
force-pushed
the
rn-423-kobo-sync-link-p1
branch
from
81215d5 to
dca2df8
Compare
Issue RN-423:
In this PR, we've added 'Sync Groups' to the Admin Panel. This is where the user can configure the mapping between Tupaia<->Kobo in the
Configsection.The mapping is just a JSON configuration which we accept for now is good enough for the user.
Subsequent PRs will add the functionality to view the Sync Group logs, and perform manual syncs via the Admin Panel, which should help the end user test out configuring a new Sync Group.
All credit to @IgorNadj on this one, I'm just ferrying it over.