RN-400: Meditrak permissions based sync (app-side changes) #3995
Conversation
| @@ -16,7 +16,7 @@ const HomeToolbar = ({ onNavigateToSurveysMenu, onNavigateToTupaiaWebsite, style | |||
| textStyle={localStyles.buttonText} | |||
| /> | |||
| <BlueButton | |||
| title="SURVEY FACILITIES" | |||
| title="SURVEY" | |||
There was a problem hiding this comment.
@edmofro 2 months of refactors and now I'm finally able to make this change 😉
| @@ -28,7 +31,7 @@ const MEASURE_BATCH_IN_DATA = 'kilobytes'; | |||
| const KILOBYTE = 1024; // bytes | |||
|
|
|||
| const MIN_BATCH_RECORDS = 1; | |||
| const MAX_BATCH_RECORDS = 500; | |||
| const MAX_BATCH_RECORDS = 2000; | |||
There was a problem hiding this comment.
Based on the api_request_log, the majority of our syncs manage to hit this 500 max, so I bumped it to 2000. This also gets hit pretty easily on my local, but the app write speed is balanced with the query time more effeciently 👍
| @@ -246,61 +250,50 @@ export class Synchroniser { | |||
| */ | |||
| async pushChanges(changesSyncJson) { | |||
| const startTime = new Date().getTime(); | |||
| try { | |||
There was a problem hiding this comment.
Removed all the redundant try/catches. ESlint told me to
| @@ -17,11 +17,11 @@ | |||
| <key>CFBundlePackageType</key> | |||
| <string>APPL</string> | |||
| <key>CFBundleShortVersionString</key> | |||
| <string>1.11.121</string> | |||
| <string>1.12.123</string> | |||
There was a problem hiding this comment.
Funny new app number here, it seems like we track the version just by the last number? But semver suggests we should bump the middle number for new features...
There was a problem hiding this comment.
Yes it is funny! Have just updated the slab doc here to explain the reason, but to repost:
Don't reset or wrap patch number, instead ALWAYS just increment it by 1 with every release. This is because we use a shortcut where the patch number doubles as the android build number, which needs to increase monotonically.
| const countriesSynced = this.database.getSetting(COUNTRIES_SYNCED); | ||
| const permissionGroupsSynced = this.database.getSetting(PERMISSION_GROUPS_SYNCED); | ||
| if (countriesSynced) { | ||
| // eslint-disable-next-line no-param-reassign |
There was a problem hiding this comment.
I don't think there's much waste in creating a clone of the incoming parameter to avoid the eslint disabling (and discouraging the practice of the unsafe pattern)
There was a problem hiding this comment.
Yeah... you're right huh? I get a bit too cowboy with this pattern from time to time, will fix this 👍
| * Returns the number of changes left to pull | ||
| * @return {Promise} Resolves with the change count, or passes up any error thrown | ||
| */ | ||
| async getIncomingChangeCount(since, filters = {}) { | ||
| try { | ||
| const responseJson = await this.api.get(`${API_ENDPOINT}/count`, { since, ...filters }); | ||
| if (responseJson.error && responseJson.error.length > 0) { | ||
| throw new Error(responseJson.error); | ||
| } | ||
| if (typeof responseJson.changeCount !== 'number') { | ||
| throw new Error('Unexpected response from server'); | ||
| } | ||
| return responseJson.changeCount; | ||
| } catch (error) { | ||
| throw error; // Throw error up | ||
| async getIncomingChangeMetadata(since, filters = {}) { | ||
| const countriesSynced = this.database.getSetting(COUNTRIES_SYNCED); |
There was a problem hiding this comment.
Would be nice somewhere around here to add a comment briefly describing the way sync is filtered by permissions, so the reader doesn't need to look into the server code to understand what's going on
There was a problem hiding this comment.
Great point! Will do 😄
rohan-bes
force-pushed
the
rn-400-permissions-based-sync-p1
branch
from
adf82df to
2e35a56
Compare
rohan-bes
force-pushed
the
rn-400-permissions-based-sync-p2
branch
2 times, most recently
from
7d5888d to
f81308c
Compare
rohan-bes
force-pushed
the
rn-400-permissions-based-sync-p1
branch
from
5eac074 to
a9979fe
Compare
- Track countries and permissions groups that are synced to the database - Only fetch data for countries and permissions groups that the user has access to or the device has already synced
- Added comment explaining why we track and pass through the countries/permission groups synced - Stopped doing naughty param reassign business
rohan-bes
force-pushed
the
rn-400-permissions-based-sync-p2
branch
from
f81308c to
e266ffb
Compare
Issue RN-400:
Changes to meditrak-app to support permissions based synchronization. Much simpler changes here, refer to the other PR for the logic