Skip to content

Commit

Permalink
Expire all tasks and targets calculations at the end of reporting per…
Browse files Browse the repository at this point in the history
…iod (#6320)

All tasks and targets are recalculated when reporting interval turns over.
Moves CalendarInterval into a shared lib.

#6267

(cherry picked from commit 2337464)
  • Loading branch information
dianabarsan committed Mar 27, 2020
1 parent 1609ee1 commit c4f4807
Show file tree
Hide file tree
Showing 13 changed files with 1,186 additions and 102 deletions.
961 changes: 961 additions & 0 deletions shared-libs/calendar-interval/package-lock.json

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions shared-libs/calendar-interval/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "@medic/calendar-interval",
"version": "1.0.0",
"description": "Provides helper function when dealing with monthly calendar intervals",
"main": "src/index.js",
"scripts": {
"test": "mocha ./test"
},
"author": "",
"license": "Apache-2.0",
"devDependencies": {
"chai": "^4.2.0",
"mocha": "^6.1.4",
"sinon": "^7.3.2"
},
"dependencies": {
"moment": "^2.24.0"
}
}
60 changes: 60 additions & 0 deletions shared-libs/calendar-interval/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
const moment = require('moment');

const normalizeStartDate = function(intervalStartDate) {
intervalStartDate = parseInt(intervalStartDate);

if (isNaN(intervalStartDate) || intervalStartDate <= 0 || intervalStartDate > 31) {
intervalStartDate = 1;
}

return intervalStartDate;
};

const getMinimumStartDate = function(intervalStartDate, relativeDate) {
return moment
.min(
moment(relativeDate).subtract(1, 'month').date(intervalStartDate).startOf('day'),
moment(relativeDate).startOf('month')
)
.valueOf();
};

const getMinimumEndDate = function(intervalStartDate, nextMonth, relativeDate) {
return moment
.min(
moment(relativeDate).add(nextMonth ? 1 : 0, 'month').date(intervalStartDate - 1).endOf('day'),
moment(relativeDate).add(nextMonth ? 1 : 0, 'month').endOf('month')
)
.valueOf();
};

module.exports = {
// Returns the timestamps of the start and end of the current calendar interval
// @param {Number} [intervalStartDate=1] - day of month when interval starts (1 - 31)
//
// if `intervalStartDate` exceeds month's day count, the start/end of following/current month is returned
// f.e. `intervalStartDate` === 31 would generate next intervals :
// [12-31 -> 01-30], [01-31 -> 02-[28|29]], [03-01 -> 03-30], [03-31 -> 04-30], [05-01 -> 05-30], [05-31 - 06-30]
getCurrent: function(intervalStartDate) {
intervalStartDate = normalizeStartDate(intervalStartDate);

if (intervalStartDate === 1) {
return {
start: moment().startOf('month').valueOf(),
end: moment().endOf('month').valueOf()
};
}

if (intervalStartDate <= moment().date()) {
return {
start: moment().date(intervalStartDate).startOf('day').valueOf(),
end: getMinimumEndDate(intervalStartDate, true)
};
}

return {
start: getMinimumStartDate(intervalStartDate),
end: getMinimumEndDate(intervalStartDate)
};
}
};
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
describe('service module', () => {
let clock,
service;
const sinon = require('sinon');
const chai = require('chai');
const moment = require('moment');

beforeEach(function() {
module('inboxApp');
inject(function($injector) {
service = $injector.get('CalendarInterval');
});
});
const service = require('../src/index');

describe('CalendarInterval', () => {
let clock;

afterEach(() => {
if (clock) {
clock.restore();
}
clock && clock.restore();
sinon.restore();
});


describe('getCurrent', () => {
it('returns 1st of current month when month start is not set or incorrect', () => {
clock = sinon.useFakeTimers(moment('2018-01-20 21:10:01').valueOf());
Expand Down
3 changes: 3 additions & 0 deletions shared-libs/rules-engine/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions shared-libs/rules-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
},
"homepage": "https://github.com/medic/medic/tree/master/shared-libs/rules-engine#readme",
"dependencies": {
"@medic/calendar-interval": "file:../calendar-interval",
"@medic/registration-utils": "file:../registration-utils",
"md5": "^2.2.1",
"medic-nootils": "^4.0.1",
Expand Down
53 changes: 36 additions & 17 deletions shared-libs/rules-engine/src/rules-state-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2. Target emissions @see target-state
*/
const md5 = require('md5');

const calendarInterval = require('@medic/calendar-interval');
const targetState = require('./target-state');

const EXPIRE_CALCULATION_AFTER_MS = 7 * 24 * 60 * 60 * 1000;
Expand Down Expand Up @@ -48,6 +48,7 @@ const self = {
* @param {Object} settings Settings for the behavior of the rules store
* @param {Object} settings.contact User's hydrated contact document
* @param {Object} settings.user User's user-settings document
* @param {Object} settings.monthStartDate reporting interval start date
* @param {Object} stateChangeCallback Callback which is invoked whenever the state changes.
* Receives the updated state as the only parameter.
*/
Expand All @@ -60,6 +61,7 @@ const self = {
rulesConfigHash: hashRulesConfig(settings),
contactState: {},
targetState: targetState.createEmptyState(settings.targets),
monthStartDate: settings.monthStartDate,
};
currentUserContact = settings.contact;
currentUserSettings = settings.user;
Expand All @@ -86,15 +88,16 @@ const self = {
}

const now = Date.now();
const { calculatedAt, isDirty } = state.contactState[contactId];
return !calculatedAt ||
const { calculatedAt, expireAt, isDirty } = state.contactState[contactId];
return !expireAt ||
isDirty ||
/* user rewinded their clock */ calculatedAt > now ||
/* isExpired */ calculatedAt < now - EXPIRE_CALCULATION_AFTER_MS;
calculatedAt > now || /* system clock changed */
expireAt < now; /* isExpired */
},

/**
* Determines if either the settings document or user's hydrated contact document have changed in a way which will impact the result of rules calculations.
* Determines if either the settings document or user's hydrated contact document have changed in a way which
* will impact the result of rules calculations.
* If they have changed in a meaningful way, the calculation state of all contacts is reset
*
* @param {Object} settings Settings for the behavior of the rules store
Expand All @@ -107,6 +110,7 @@ const self = {
rulesConfigHash,
contactState: {},
targetState: targetState.createEmptyState(settings.targets),
monthStartDate: settings.monthStartDate,
};
currentUserContact = settings.contact;
currentUserSettings = settings.user;
Expand All @@ -132,8 +136,14 @@ const self = {
return;
}

for (let contactId of contactIds) {
state.contactState[contactId] = { calculatedAt };
const reportingInterval = calendarInterval.getCurrent(state.monthStartDate);
const defaultExpiry = calculatedAt + EXPIRE_CALCULATION_AFTER_MS;

for (const contactId of contactIds) {
state.contactState[contactId] = {
calculatedAt,
expireAt: Math.min(reportingInterval.end, defaultExpiry),
};
}

return onStateChange(state);
Expand Down Expand Up @@ -169,14 +179,17 @@ const self = {
getContactIds: () => Object.keys(state.contactState),

/**
* The rules system supports the concept of "headless" reports and "headless" task documents. In these scenarios, a report exists on a user's device while the associated
* contact document of that report is not on the device. A common scenario associated with this case is during supervisor workflows where supervisors sync reports with the
* The rules system supports the concept of "headless" reports and "headless" task documents. In these scenarios,
* a report exists on a user's device while the associated contact document of that report is not on the device.
* A common scenario associated with this case is during supervisor workflows where supervisors sync reports with the
* needs_signoff attribute but not the associated patient.
*
* In these cases, getting a list of "all the contacts with rules" requires us to look not just through contact docs, but also through reports. To avoid this costly operation,
* the rules-state-store maintains a flag which indicates if the contact ids in the store can serve as a trustworthy authority.
* In these cases, getting a list of "all the contacts with rules" requires us to look not just through contact
* docs, but also through reports. To avoid this costly operation, the rules-state-store maintains a flag which
* indicates if the contact ids in the store can serve as a trustworthy authority.
*
* markAllFresh should be called when the list of contact ids within the store is the complete set of contacts with rules
* markAllFresh should be called when the list of contact ids within the store is the complete set of contacts with
* rules
*/
markAllFresh: (calculatedAt, contactIds) => {
state.allContactIds = true;
Expand All @@ -201,7 +214,8 @@ const self = {
/**
* Store a set of target emissions which were emitted by refreshing a set of contacts
*
* @param {string[]} contactIds An array of contact ids which produced these targetEmissions by being refreshed. If undefined, all contacts are updated.
* @param {string[]} contactIds An array of contact ids which produced these targetEmissions by being refreshed.
* If undefined, all contacts are updated.
* @param {Object[]} targetEmissions An array of target emissions (the result of the rules-emitter).
*/
storeTargetEmissions: (contactIds, targetEmissions) => {
Expand All @@ -214,16 +228,21 @@ const self = {
/**
* Aggregates the stored target emissions into target models
*
* @param {Function(emission)=} targetEmissionFilter Filter function to filter which target emissions should be aggregated
* @param {Function(emission)=} targetEmissionFilter Filter function to filter which target emissions should
* be aggregated
* @example aggregateStoredTargetEmissions(emission => emission.date > moment().startOf('month').valueOf())
*
* @returns {Object[]} result
* @returns {string} result[n].* All attributes of the target as defined in the settings doc
* @returns {Integer} result[n].total The total number of unique target emission ids matching instanceFilter
* @returns {Integer} result[n].pass The number of unique target emission ids matching instanceFilter with the latest emission with truthy "pass"
* @returns {Integer} result[n].pass The number of unique target emission ids matching instanceFilter with the
* latest emission with truthy "pass"
* @returns {Integer} result[n].percent The percentage of pass/total
*/
aggregateStoredTargetEmissions: targetEmissionFilter => targetState.aggregateStoredTargetEmissions(state.targetState, targetEmissionFilter),
aggregateStoredTargetEmissions: targetEmissionFilter => targetState.aggregateStoredTargetEmissions(
state.targetState,
targetEmissionFilter
),
};

const hashRulesConfig = (settings) => {
Expand Down
1 change: 1 addition & 0 deletions shared-libs/rules-engine/test/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ rule GenerateEvents {
enableTargets: true,
user: userSettingsDoc,
contact: userContactDoc,
monthStartDate: 1,
}, assign);
},

Expand Down
29 changes: 19 additions & 10 deletions shared-libs/rules-engine/test/provider-wireup.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ chai.use(chaiExclude);

const rulesStateStore = RestorableRulesStateStore();
const NOW = 50000;
const DEFAULT_EXPIRE = 7 * 24 * 60 * 60 * 1000;

const reportConnectedByPlace = {
_id: 'reportByPlace',
Expand Down Expand Up @@ -96,22 +97,29 @@ describe('provider-wireup integration tests', () => {
_id: pouchdbProvider.RULES_STATE_DOCID,
rulesStateStore: {
contactState: {},
monthStartDate: 1,
},
}]);

await wireup.fetchTasksFor(provider, ['abc']);
await provider.stateChangeCallback.returnValues[0];
expect(db.put.args[db.put.callCount - 1]).excludingEvery(['rulesConfigHash', 'targetState']).excluding('_rev').to.deep.eq([{
_id: pouchdbProvider.RULES_STATE_DOCID,
rulesStateStore: {
contactState: {
'abc': {
calculatedAt: NOW,
expect(db.put.args[db.put.callCount - 1])
.excludingEvery(['rulesConfigHash', 'targetState'])
.excluding('_rev')
.to.deep.eq([{
_id: pouchdbProvider.RULES_STATE_DOCID,
rulesStateStore: {
contactState: {
'abc': {
expireAt: NOW + DEFAULT_EXPIRE,
calculatedAt: NOW,
},
},
monthStartDate: 1,
},
},
}]);
expect(db.put.args[0][0].rulesStateStore.rulesConfigHash).to.eq(db.put.args[db.put.callCount - 1][0].rulesStateStore.rulesConfigHash);
}]);
expect(db.put.args[0][0].rulesStateStore.rulesConfigHash)
.to.eq(db.put.args[db.put.callCount - 1][0].rulesStateStore.rulesConfigHash);

// simulate restarting the app. the database is the same, but the taskFetcher is uninitialized
rulesEmitter.shutdown();
Expand Down Expand Up @@ -164,7 +172,8 @@ describe('provider-wireup integration tests', () => {
it('many', async () => {
sinon.stub(rulesStateStore, 'markDirty').resolves();
await wireup.updateEmissionsFor(provider, ['headless', 'patient', 'patient_id']);
expect(rulesStateStore.markDirty.args).to.deep.eq([[['patient', 'headless', 'patient']]]); // dupes don't matter here
// dupes don't matter here
expect(rulesStateStore.markDirty.args).to.deep.eq([[['patient', 'headless', 'patient']]]);
});
});

Expand Down
Loading

0 comments on commit c4f4807

Please sign in to comment.