From c8a7f13ad0452a90cb10f438557ebeb04dad9391 Mon Sep 17 00:00:00 2001 From: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:38:18 +0300 Subject: [PATCH] fix(#9552): handle rules-engine stale state after upgrade for 4.13.x (#9555) #9552 --- .../rules-engine/src/rules-state-store.js | 8 +++-- shared-libs/rules-engine/src/target-state.js | 2 ++ .../test/rules-state-store.spec.js | 33 +++++++++++++++++++ .../rules-engine/test/target-state.spec.js | 14 ++++++++ .../targets/target-accuracy.wdio-spec.js | 25 ++++++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) diff --git a/shared-libs/rules-engine/src/rules-state-store.js b/shared-libs/rules-engine/src/rules-state-store.js index f3187ac799..a48915753c 100644 --- a/shared-libs/rules-engine/src/rules-state-store.js +++ b/shared-libs/rules-engine/src/rules-state-store.js @@ -36,12 +36,16 @@ const self = { currentUserSettings = settings.user; setOnChangeState(stateChangeCallback); + if (!state) { + return true; + } + const rulesConfigHash = hashRulesConfig(settings); - if (state && state.rulesConfigHash !== rulesConfigHash) { + if (state.rulesConfigHash !== rulesConfigHash || targetState.isStale(state.targetState)) { state.stale = true; } - return !state || state.stale; + return state.stale; }, /** diff --git a/shared-libs/rules-engine/src/target-state.js b/shared-libs/rules-engine/src/target-state.js index 8f3d7214c6..20e4f66986 100644 --- a/shared-libs/rules-engine/src/target-state.js +++ b/shared-libs/rules-engine/src/target-state.js @@ -118,6 +118,8 @@ module.exports = { return state; }, + isStale: (state) => !state || !state.targets || !state.aggregate, + storeTargetEmissions: (state, contactIds, targetEmissions) => { let isUpdated = false; if (!Array.isArray(targetEmissions)) { diff --git a/shared-libs/rules-engine/test/rules-state-store.spec.js b/shared-libs/rules-engine/test/rules-state-store.spec.js index 9ec8558d5a..46ec07d526 100644 --- a/shared-libs/rules-engine/test/rules-state-store.spec.js +++ b/shared-libs/rules-engine/test/rules-state-store.spec.js @@ -1,5 +1,7 @@ const { expect } = require('chai'); const { RestorableRulesStateStore } = require('./mocks'); +const md5 = require('md5'); + const sinon = require('sinon'); const moment = require('moment'); @@ -431,4 +433,35 @@ describe('rules-state-store', () => { }); }); }); + + describe('load', () => { + it('should mark as stale when rules config hash has changed', () => { + const staleState = {}; + const stale = rulesStateStore.load(staleState, { config: '123' }); + expect(stale).to.equal(true); + }); + + it('should mark as stale when target-state is stale', () => { + const settings = { config: '123' }; + const staleState = { + targetState: {}, + rulesConfigHash: md5(JSON.stringify(settings)) + }; + const stale = rulesStateStore.load(staleState, settings); + expect(stale).to.equal(true); + }); + + it('should not mark as stale when not stale', () => { + const settings = { config: '123' }; + const staleState = { + targetState: { + targets: {}, + aggregate: {} + }, + rulesConfigHash: md5(JSON.stringify(settings)) + }; + const stale = rulesStateStore.load(staleState, settings); + expect(stale).to.equal(undefined); + }); + }); }); diff --git a/shared-libs/rules-engine/test/target-state.spec.js b/shared-libs/rules-engine/test/target-state.spec.js index 464dc4045f..625f1b9c3f 100644 --- a/shared-libs/rules-engine/test/target-state.spec.js +++ b/shared-libs/rules-engine/test/target-state.spec.js @@ -275,5 +275,19 @@ describe('target-state', () => { ]); }); }); + + describe('isStale', () => { + it('should return true if state is invalid', () => { + expect(targetState.isStale()).to.equal(true); + expect(targetState.isStale({})).to.equal(true); + expect(targetState.isStale({ oldTarget: { emissions: [] } })).to.equal(true); + expect(targetState.isStale({ targets: {} })).to.equal(true); + expect(targetState.isStale({ aggregate: {} })).to.equal(true); + }); + + it('should return false if state is valid', () => { + expect(targetState.isStale({ targets: {}, aggregate: {} })).to.equal(false); + }); + }); }); diff --git a/tests/e2e/default/targets/target-accuracy.wdio-spec.js b/tests/e2e/default/targets/target-accuracy.wdio-spec.js index 4efe1a3d4a..171c10b7c8 100644 --- a/tests/e2e/default/targets/target-accuracy.wdio-spec.js +++ b/tests/e2e/default/targets/target-accuracy.wdio-spec.js @@ -1,10 +1,12 @@ const path = require('path'); const moment = require('moment'); const chtConfUtils = require('@utils/cht-conf'); +const chtDbUtils = require('@utils/cht-db'); const utils = require('@utils'); const loginPage = require('@page-objects/default/login/login.wdio.page'); const commonPage = require('@page-objects/default/common/common.wdio.page'); const contactsPage = require('@page-objects/default/contacts/contacts.wdio.page'); +const analyticsPage = require('@page-objects/default/analytics/analytics.wdio.page'); const userFactory = require('@factories/cht/users/users'); const placeFactory = require('@factories/cht/contacts/place'); const personFactory = require('@factories/cht/contacts/person'); @@ -170,4 +172,27 @@ describe('Target accuracy', () => { const docs = await utils.db.allDocs({ startkey: 'target', endkey: 'target\ufff0' }); expect(docs.rows.length).to.equal(1); }); + + it('should handle old format of the rules-state-store', async () => { + const docId = '_local/rulesStateStore'; + const localRulesStateStore = await chtDbUtils.getDoc(docId); + await chtDbUtils.updateDoc('_local/rulesStateStore', { + rulesStateStore: { + ...localRulesStateStore.rulesStateStore, + targetState: localRulesStateStore.rulesStateStore.targetState.targets + } + }); + + await commonPage.refresh(); + await analyticsPage.goToTargets(); + await commonPage.waitForPageLoaded(); + + const targets = await analyticsPage.getTargets(); + expect(targets.length).to.equal(1); + expect(targets[0]).to.deep.include({ + title: 'targets.new_persons.title', + goal: '0', + count: '20', + }); + }); });