Skip to content

Commit

Permalink
fix(#9552): migrate stale target state for interval turnover
Browse files Browse the repository at this point in the history
Co-authored-by: Diana Barsan <[email protected]>
  • Loading branch information
garethbowen and dianabarsan authored Oct 21, 2024
1 parent c8a7f13 commit e5a6b9d
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 6 deletions.
1 change: 1 addition & 0 deletions shared-libs/rules-engine/src/rules-state-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const self = {

const rulesConfigHash = hashRulesConfig(settings);
if (state.rulesConfigHash !== rulesConfigHash || targetState.isStale(state.targetState)) {
state.targetState = targetState.migrateStaleState(state.targetState);
state.stale = true;
}

Expand Down
13 changes: 9 additions & 4 deletions shared-libs/rules-engine/src/target-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,28 @@ const mergeEmissions = (state, contactIds, targetEmissions) => {
return isUpdated;
};

const createState = (existentStaleState) => {
return {
targets: existentStaleState ? existentStaleState : {},
aggregate: {}
};
};

module.exports = {
/**
* Builds an empty target-state.
*
* @param {Object[]} targets An array of target definitions
*/
createEmptyState: (targets=[]) => {
const state = {
targets: {},
aggregate: {}
};
const state = createState();

targets.forEach(definition => state.targets[definition.id] = { ...definition, emissions: {} });
return state;
},

isStale: (state) => !state || !state.targets || !state.aggregate,
migrateStaleState: (state) => module.exports.isStale(state) ? createState(state) : state,

storeTargetEmissions: (state, contactIds, targetEmissions) => {
let isUpdated = false;
Expand Down
45 changes: 45 additions & 0 deletions shared-libs/rules-engine/test/provider-wireup.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,51 @@ describe('provider-wireup integration tests', () => {
]);
});

it('should work with old format of the rules state store', async () => {
clock.setSystemTime(moment('2020-04-14').valueOf());
const rules = simpleNoolsTemplate('');
const settings = {
rules,
enableTargets: true,
targets: [{
id: 'uhc',
}],
monthStartDate: 15, // the target doc will be calculated using the current month start date value
};

// with monthStartDate = 15, and today being April 28th,
// the current interval is Apr 15 - May 14 and the previous interval is Mar 15 - Apr 14
const emissions = [
mockTargetEmission('uhc', 'doc4', moment('2020-02-23').valueOf(), true), // passes outside interval
mockTargetEmission('uhc', 'doc2', moment('2020-03-29').valueOf(), true), // passes within interval
mockTargetEmission('uhc', 'doc1', moment('2020-04-12').valueOf(), true), // passes within interval
mockTargetEmission('uhc', 'doc3', moment('2020-04-16').valueOf(), true), // passes outside interval
];

const staleState = {
rulesConfigHash: 'not the same hash!!',
contactState: {},
targetState: { uhc: { id: 'uhc', emissions: {} }},
calculatedAt: moment('2020-04-14').valueOf(),
monthStartDate: 1,
};

await prepareExistentState(staleState);
await loadState(settings);
await rulesStateStore.storeTargetEmissions([], emissions);
rulesStateStore.restore();

clock.setSystemTime(moment('2020-04-28').valueOf()); // next interval
await wireup.initialize(provider, settings, {});
expect(provider.commitTargetDoc.callCount).to.equal(1);
expect(provider.commitTargetDoc.args[0]).to.deep.equal([
[{ id: 'uhc', value: { pass: 2, total: 2 } }],
'2020-04',
{ userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' } },
true,
]);
});

it('should use inclusive operator when comparing dates (left)', async () => {
clock.setSystemTime(moment('2020-05-28').valueOf());
const rules = simpleNoolsTemplate('');
Expand Down
7 changes: 5 additions & 2 deletions shared-libs/rules-engine/test/rules-state-store.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,14 +441,17 @@ describe('rules-state-store', () => {
expect(stale).to.equal(true);
});

it('should mark as stale when target-state is stale', () => {
it('should mark as stale and migrate when target-state is stale', () => {
const settings = { config: '123' };
const targets = { t1: { emissions: [] }, t2: { emissions: [] } };
Object.freeze(targets);
const staleState = {
targetState: {},
targetState: targets,
rulesConfigHash: md5(JSON.stringify(settings))
};
const stale = rulesStateStore.load(staleState, settings);
expect(stale).to.equal(true);
expect(staleState.targetState).to.deep.equal({ targets, aggregate: { } });
});

it('should not mark as stale when not stale', () => {
Expand Down
19 changes: 19 additions & 0 deletions shared-libs/rules-engine/test/target-state.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,5 +289,24 @@ describe('target-state', () => {
expect(targetState.isStale({ targets: {}, aggregate: {} })).to.equal(false);
});
});

describe('migrateStaleState', () => {
it('should migrate on stale state', () => {
expect(targetState.migrateStaleState({ })).to.deep.equal( { targets: {}, aggregate: {} });
const targets = { t1: { emissions: [] }, t2: { emissions: [] } };
Object.freeze(targets);
expect(targetState.migrateStaleState(targets)).to.deep.equal({ targets, aggregate: {}});
});

it('should not migrate on not stale state', () => {
const state = {
targets: { t1: { emissions: [] }, t2: { emissions: [] } },
aggregate: { targets: [], filterInterval: {} },
};
Object.freeze(state);
Object.freeze(state.targets);
expect(targetState.migrateStaleState(state)).to.equal(state);
});
});
});

0 comments on commit e5a6b9d

Please sign in to comment.