From b7f31137d673665b5732020829d53afd3d11e30e Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Wed, 18 Sep 2024 14:23:11 -0700 Subject: [PATCH 01/12] refactor: header component --- .../components/header/header.component.html | 21 ++- .../components/header/header.component.ts | 137 +++++++++--------- 2 files changed, 80 insertions(+), 78 deletions(-) diff --git a/src/app/shared/components/header/header.component.html b/src/app/shared/components/header/header.component.html index 4211224799..a66a230875 100644 --- a/src/app/shared/components/header/header.component.html +++ b/src/app/shared/components/header/header.component.html @@ -1,17 +1,26 @@ - - + + - + @if (showMenuButton()) { + + } - - {{ headerConfig.title }} + + {{ headerConfig().title }} diff --git a/src/app/shared/components/header/header.component.ts b/src/app/shared/components/header/header.component.ts index 755835bd17..5ce984e9f1 100644 --- a/src/app/shared/components/header/header.component.ts +++ b/src/app/shared/components/header/header.component.ts @@ -1,14 +1,12 @@ import { Location } from "@angular/common"; -import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; +import { ChangeDetectionStrategy, Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; +import { computed, effect, signal } from "@angular/core"; +import { toSignal } from "@angular/core/rxjs-interop"; import { NavigationEnd, NavigationStart, Router } from "@angular/router"; import { App } from "@capacitor/app"; import { Capacitor, PluginListenerHandle } from "@capacitor/core"; import { Subscription, fromEvent, map } from "rxjs"; -import type { - IAppConfig, - IHeaderColourOptions, - IHeaderVariantOptions, -} from "data-models/appConfig"; +import type { IHeaderVariantOptions } from "data-models/appConfig"; import { AppConfigService } from "../../services/app-config/app-config.service"; import { IonHeader, ScrollBaseCustomEvent, ScrollDetail } from "@ionic/angular"; import { _wait } from "packages/shared/src/utils/async-utils"; @@ -27,24 +25,28 @@ const heightsMap = { selector: "plh-main-header", templateUrl: "./header.component.html", styleUrls: ["./header.component.scss"], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class headerComponent implements OnInit, OnDestroy { @ViewChild(IonHeader) headerElement: IonHeader & { el: HTMLElement }; - showMenuButton = false; - showBackButton = false; - appConfigChanges$: Subscription; - headerConfig: IAppConfig["APP_HEADER_DEFAULTS"]; - routeChanges$: Subscription; + showMenuButton = signal(false); + showBackButton = signal(false); + headerConfig = computed(() => this.appConfigService.appConfig().APP_HEADER_DEFAULTS); + + /** + * Listen to router events to handle route change effects + * NOTE - use events instead of route as header sits outside ion-router (limited access) + **/ + private routeChanges = toSignal(this.router.events); + /** listen to hardware back button presses (on android device only) */ - hardwareBackButton$: PluginListenerHandle; + private hardwareBackButton$: PluginListenerHandle; /** track if navigation has been used to handle back button click behaviour */ - hasBackHistory = false; - colour: IHeaderColourOptions; - variant: IHeaderVariantOptions; + private hasBackHistory = false; /** Modify margin to move off-screen when using collapsed mode */ - public marginTop = "0px"; + public marginTop = signal(0); /** Track scroll events when using header collapse mode */ private scrollEvents$: Subscription; @@ -53,19 +55,35 @@ export class headerComponent implements OnInit, OnDestroy { private router: Router, private location: Location, private appConfigService: AppConfigService - ) {} + ) { + effect( + () => { + // when header config changes set the height and collapse properties + const { collapse, variant } = this.headerConfig(); + this.setHeaderHeightCSS(variant); + if (collapse !== undefined) { + this.setHeaderCollapse(collapse); + } + }, + { allowSignalWrites: true } + ); + effect( + () => { + // when route changes handle side-effects + const e = this.routeChanges(); + if (e instanceof NavigationEnd) { + this.handleRouteChange(); + } + if (e instanceof NavigationStart) { + this.hasBackHistory = true; + } + }, + { allowSignalWrites: true } + ); + } async ngOnInit() { - this.subscribeToAppConfigChanges(); - // subscribe to and handle route changes - this.routeChanges$ = this.router.events.subscribe((e) => { - if (e instanceof NavigationEnd) { - this.handleRouteChange(); - } - if (e instanceof NavigationStart) { - this.hasBackHistory = true; - } - }); + // trigger route change for initial configuration this.handleRouteChange(); // subscribe to and handle hardware back button press if (Capacitor.getPlatform() === "android") { @@ -73,24 +91,6 @@ export class headerComponent implements OnInit, OnDestroy { this.handleHardwareBackPress(); }); } - // HACK - uncomment to test collapse - // this.appConfigService.setAppConfig({ APP_HEADER_DEFAULTS: { collapse: true } }); - } - - subscribeToAppConfigChanges() { - this.appConfigChanges$ = this.appConfigService.changesWithInitialValue$.subscribe( - (changes: IAppConfig) => { - if (changes.APP_HEADER_DEFAULTS) { - const headerConfig = this.appConfigService.appConfig().APP_HEADER_DEFAULTS; - this.headerConfig = headerConfig; - this.updateHeaderConfig(); - // handle collapse config changes - if (changes.APP_HEADER_DEFAULTS?.collapse !== undefined) { - this.handleHeaderCollapseConfigChange(changes.APP_HEADER_DEFAULTS?.collapse); - } - } - } - ); } public handleBackButtonClick() { @@ -101,54 +101,48 @@ export class headerComponent implements OnInit, OnDestroy { } } - /** - * Optional methods that can respond to route changes from within the header component - * It cannot subscribe to standard router methods as sits outside ion-router-outlet - */ + /** Determine whether to show back and menu buttons based on location */ private handleRouteChange() { - const { should_show_back_button, should_show_menu_button } = this.headerConfig; - this.showBackButton = should_show_back_button(location); - this.showMenuButton = should_show_menu_button(location); - this.marginTop = "0px"; + const { should_show_back_button, should_show_menu_button } = this.headerConfig(); + this.showBackButton.set(should_show_back_button(location)); + this.showMenuButton.set(should_show_menu_button(location)); + this.marginTop.set(0); } /** When device back button evaluate conditions to handle app minimise */ private handleHardwareBackPress() { - const { should_minimize_app_on_back } = this.headerConfig; + const { should_minimize_app_on_back } = this.headerConfig(); if (should_minimize_app_on_back(location)) { App.minimizeApp(); } } - private updateHeaderConfig() { - this.colour = this.headerConfig.colour; - const { variant } = this.headerConfig; - this.variant = variant; + private setHeaderHeightCSS(variant: IHeaderVariantOptions) { // Set CSS property dynamically in component so that it can be exposed to deployment config/skins - document.documentElement.style.setProperty( - "--header-height", - `${heightsMap[this.variant] || heightsMap.default}px` - ); + const targetHeight = heightsMap[variant] || heightsMap.default; + document.documentElement.style.setProperty("--header-height", `${targetHeight}px`); } /** * When enabling header collapse listen add scroll event listeners to detect when to collapse * If disabling dispose of previous scroll event listeners */ - private handleHeaderCollapseConfigChange(shouldCollapse: boolean) { + private setHeaderCollapse(shouldCollapse: boolean) { // TODO: current header collapse implementation does not work on ios, so do not enable on this platform if (Capacitor.getPlatform() === "ios") return; + this.removeScrollEventListeners(); + if (shouldCollapse) { + this.listenToScrollEvents(); + } + } + private removeScrollEventListeners() { // If previously scroll events were subscribed then should be able to unsubscribe. // If initial config change undefined->false can ignore if (this.scrollEvents$) { this.scrollEvents$.unsubscribe(); this.scrollEvents$ = undefined; - this.marginTop = "0px"; - // unset height - } - if (shouldCollapse) { - this.listenToScrollEvents(); + this.marginTop.set(0); } } @@ -161,7 +155,7 @@ export class headerComponent implements OnInit, OnDestroy { const { deltaY, currentY } = detail; // bring header into view when scrolling in reverse or at top of page if (deltaY <= 0 || currentY < headerHeight) { - this.marginTop = "0px"; + this.marginTop.set(0); } else { // hide header after scrolled further than header height if (currentY >= headerHeight) { @@ -169,7 +163,7 @@ export class headerComponent implements OnInit, OnDestroy { // as that would trigger negative scroll direction this.scrollEvents$.unsubscribe(); // set negative margin to move header off-screen - this.marginTop = `-${headerHeight}px`; + this.marginTop.set(headerHeight * -1); await _wait(500); this.listenToScrollEvents(); } @@ -178,8 +172,7 @@ export class headerComponent implements OnInit, OnDestroy { } ngOnDestroy() { - this.appConfigChanges$.unsubscribe(); - this.routeChanges$.unsubscribe(); + this.removeScrollEventListeners(); if (this.hardwareBackButton$) { this.hardwareBackButton$.remove(); } From f357220ba119e1642bee6257ad8fb05f001634f6 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Wed, 18 Sep 2024 14:33:46 -0700 Subject: [PATCH 02/12] refactor: app component --- src/app/app.component.html | 124 ++++++++++++++++++------------------- src/app/app.component.ts | 34 +++------- 2 files changed, 71 insertions(+), 87 deletions(-) diff --git a/src/app/app.component.html b/src/app/app.component.html index 27bacab8d8..1b16ac041d 100644 --- a/src/app/app.component.html +++ b/src/app/app.component.html @@ -1,67 +1,67 @@ - - - - - - {{ sideMenuDefaults.title }} -
- @if (sideMenuDefaults.should_show_version) { - @if (deploymentConfig._content_version; as CONTENT_VERSION) { - - - {{ CONTENT_VERSION }} - - - } @else { - {{ deploymentConfig._app_builder_version }} - } - } - ({{ deploymentConfig.name }}) -
- - - - - - - - -
- -
- -
- - + + @if (sideMenuDefaults().enabled) { + + + + {{ sideMenuDefaults().title }} +
+ @if (sideMenuDefaults().should_show_version) { + @if (deploymentConfig._content_version; as CONTENT_VERSION) { + + + {{ CONTENT_VERSION }} + + + } @else { + {{ deploymentConfig._app_builder_version }} + } + } + @if (sideMenuDefaults().should_show_deployment_name) { + ({{ deploymentConfig.name }}) + } +
+ + + - - -
- - -
- + + + } + + + +
+ +
+ +
+ @if (footerDefaults().templateName; as footerTemplateName) { + + + + + + }
- - - + + +
+ +
+ + + +} diff --git a/src/app/app.component.ts b/src/app/app.component.ts index b16b1adf05..b0dbd0d5fd 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectorRef, Component } from "@angular/core"; +import { ChangeDetectionStrategy, Component, computed, signal } from "@angular/core"; import { Platform, MenuController } from "@ionic/angular"; import { Router } from "@angular/router"; import { Capacitor } from "@capacitor/core"; @@ -30,7 +30,6 @@ import { AppDataService } from "./shared/services/data/app-data.service"; import { AuthService } from "./shared/services/auth/auth.service"; import { LifecycleActionsService } from "./shared/services/lifecycle-actions/lifecycle-actions.service"; import { AppConfigService } from "./shared/services/app-config/app-config.service"; -import { IAppConfig } from "./shared/model"; import { TaskService } from "./shared/services/task/task.service"; import { AppUpdateService } from "./shared/services/app-update/app-update.service"; import { RemoteAssetService } from "./shared/services/remote-asset/remote-asset.service"; @@ -47,14 +46,13 @@ import { DeploymentService } from "./shared/services/deployment/deployment.servi selector: "app-root", templateUrl: "app.component.html", styleUrls: ["app.component.scss"], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class AppComponent { - appConfig: IAppConfig; - appAuthenticationDefaults: IAppConfig["APP_AUTHENTICATION_DEFAULTS"]; - sideMenuDefaults: IAppConfig["APP_SIDEMENU_DEFAULTS"]; - footerDefaults: IAppConfig["APP_FOOTER_DEFAULTS"]; + sideMenuDefaults = computed(() => this.appConfigService.appConfig().APP_SIDEMENU_DEFAULTS); + footerDefaults = computed(() => this.appConfigService.appConfig().APP_FOOTER_DEFAULTS); /** Track when app ready to render sidebar and route templates */ - public renderAppTemplates = false; + public renderAppTemplates = signal(false); public get deploymentConfig() { return this.deploymentService.config; @@ -74,7 +72,6 @@ export class AppComponent { // 3rd Party Services private platform: Platform, - private cdr: ChangeDetectorRef, private menuController: MenuController, private router: Router, // App services @@ -119,7 +116,6 @@ export class AppComponent { private async initializeApp() { this.platform.ready().then(async () => { this.platforms = this.platform.platforms().join(" "); - this.subscribeToAppConfigChanges(); await this.populateAppInitFields(); await this.initialiseCoreServices(); this.hackSetDeveloperOptions(); @@ -146,9 +142,7 @@ export class AppComponent { await SplashScreen.hide(); } // Show main template - this.renderAppTemplates = true; - // Detect changes in case expression changed prior to render (e.g. feedback sidebar) - this.cdr.detectChanges(); + this.renderAppTemplates.set(true); this.scheduleReinitialisation(); }); } @@ -172,14 +166,14 @@ export class AppComponent { */ private async loadAuthConfig() { const { firebase } = this.deploymentService.config; - const { enforceLogin } = this.appAuthenticationDefaults; + const { enforceLogin, signInTemplate } = + this.appConfigService.appConfig().APP_AUTHENTICATION_DEFAULTS; const ensureLogin = firebase.config && enforceLogin && Capacitor.isNativePlatform(); if (ensureLogin) { this.authService.ready(); const authUser = await this.authService.getCurrentUser(); if (!authUser) { - const templatename = this.appAuthenticationDefaults.signInTemplate; - const { modal } = await this.templateService.runStandaloneTemplate(templatename, { + const { modal } = await this.templateService.runStandaloneTemplate(signInTemplate, { showCloseButton: false, waitForDismiss: false, }); @@ -189,16 +183,6 @@ export class AppComponent { } } - /** Initialise appConfig and set dependent properties */ - private subscribeToAppConfigChanges() { - this.appConfigService.changesWithInitialValue$.subscribe((changes: IAppConfig) => { - this.appConfig = { ...this.appConfig, ...changes }; - this.sideMenuDefaults = this.appConfig.APP_SIDEMENU_DEFAULTS; - this.footerDefaults = this.appConfig.APP_FOOTER_DEFAULTS; - this.appAuthenticationDefaults = this.appConfig.APP_AUTHENTICATION_DEFAULTS; - }); - } - /** * Various services set core app data which may be used in templates such as current app day, * user id etc. Make sure these services have run their initialisation logic before proceeding. From 8f1acb3dd903e91adf14a6b95f48cc54db63bec4 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Thu, 7 Nov 2024 15:14:33 -0800 Subject: [PATCH 03/12] feat: set_data action standalone --- .../instance/template-action.registry.ts | 6 +- .../dynamic-data/dynamic-data.actions.spec.ts | 178 ++++++++++++++++++ .../dynamic-data/dynamic-data.actions.ts | 107 +++++++++++ .../dynamic-data.service.mock.spec.ts | 16 ++ .../dynamic-data/dynamic-data.service.spec.ts | 61 +----- .../dynamic-data/dynamic-data.service.ts | 19 +- .../dynamic-data/dynamic-data.utils.spec.ts | 24 +++ .../dynamic-data/dynamic-data.utils.ts | 33 ++++ 8 files changed, 382 insertions(+), 62 deletions(-) create mode 100644 src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts create mode 100644 src/app/shared/services/dynamic-data/dynamic-data.actions.ts create mode 100644 src/app/shared/services/dynamic-data/dynamic-data.service.mock.spec.ts create mode 100644 src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts create mode 100644 src/app/shared/services/dynamic-data/dynamic-data.utils.ts diff --git a/src/app/shared/components/template/services/instance/template-action.registry.ts b/src/app/shared/components/template/services/instance/template-action.registry.ts index 0369129867..a97c3e3e64 100644 --- a/src/app/shared/components/template/services/instance/template-action.registry.ts +++ b/src/app/shared/components/template/services/instance/template-action.registry.ts @@ -3,10 +3,8 @@ import clone from "clone"; import { FlowTypes } from "data-models"; export type IActionId = FlowTypes.TemplateRowAction["action_id"]; -export type IActionHandlers = Record< - IActionId, - (action: FlowTypes.TemplateRowAction) => Promise ->; +export type IActionHandler = (action: FlowTypes.TemplateRowAction) => Promise; +export type IActionHandlers = Record; @Injectable({ providedIn: "root" }) /** diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts new file mode 100644 index 0000000000..92f5c46d15 --- /dev/null +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts @@ -0,0 +1,178 @@ +import { TestBed } from "@angular/core/testing"; + +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { MockAppDataService } from "../data/app-data.service.spec"; +import { AppDataService } from "../data/app-data.service"; + +import { IActionSetDataParams } from "./dynamic-data.actions"; +import { DynamicDataService } from "./dynamic-data.service"; +import ActionFactory from "./dynamic-data.actions"; +import { firstValueFrom } from "rxjs"; +import { FlowTypes } from "packages/data-models"; +import { DeploymentService } from "../deployment/deployment.service"; +import { MockDeploymentService } from "../deployment/deployment.service.spec"; +import { TemplateActionRegistry } from "../../components/template/services/instance/template-action.registry"; + +const TEST_DATA_ROWS = [ + { id: "id_0", number: 0, string: "hello", _meta: "original" }, + { id: "id_1", number: 1, string: "hello", boolean: true, _meta: "original" }, +]; + +/******************************************************************************** + * Test Utilities + *******************************************************************************/ + +/** Generate a rows to trigger set_data action with included params */ +function getTestActionRow(params: IActionSetDataParams) { + params._list_id = "test_flow"; + const actionRow: FlowTypes.TemplateRowAction = { + action_id: "set_data", + trigger: "click", + args: [], + params, + }; + return actionRow; +} + +/** + * Trigger the set_data action with included params and return first update + * to corresponding data_list + * * */ +async function triggerTestSetDataAction(service: DynamicDataService, params: IActionSetDataParams) { + const actionRow = getTestActionRow(params); + const actions = new ActionFactory(service); + await actions.set_data(actionRow); + const obs = await service.query$("data_list", "test_flow"); + const data = await firstValueFrom(obs); + return data; +} + +/******************************************************************************** + * Tests + * yarn ng test --include src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts + *******************************************************************************/ +describe("DynamicDataService Actions", () => { + let service: DynamicDataService; + let actions: ActionFactory; + + beforeEach(async () => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DynamicDataService, + { + provide: AppDataService, + useValue: new MockAppDataService({ + data_list: { + test_flow: { + flow_name: "test_flow", + flow_type: "data_list", + // Make deep clone of data to avoid data overwrite issues + rows: JSON.parse(JSON.stringify(TEST_DATA_ROWS)), + }, + }, + }), + }, + { + provide: DeploymentService, + useValue: new MockDeploymentService({ name: "test" }), + }, + { + provide: TemplateActionRegistry, + useValue: { register: () => null }, + }, + ], + }); + + // HACK - polyfill not loaded for rxdb dev plugin so manually fill global before running tests + window.global = window; + + service = TestBed.inject(DynamicDataService); + await service.ready(); + // Ensure any data previously persisted is cleared + await service.resetFlow("data_list", "test_flow"); + actions = new ActionFactory(service); + }); + + /************************************************************* + * Main Tests + ************************************************************/ + it("set_data by _id", async () => { + const params: IActionSetDataParams = { _id: "id_1", string: "updated string" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].string).toEqual("hello"); + expect(data[1].string).toEqual("updated string"); + }); + + it("set_data by _index", async () => { + const params: IActionSetDataParams = { _index: 1, string: "updated string" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].string).toEqual("hello"); + expect(data[1].string).toEqual("updated string"); + }); + + it("set_data bulk", async () => { + const params: IActionSetDataParams = { string: "updated string" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].string).toEqual("updated string"); + expect(data[1].string).toEqual("updated string"); + }); + + it("set_data with item ref (single)", async () => { + const params: IActionSetDataParams = { _id: "id_1", number: "@item.number + 100" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].number).toEqual(0); + expect(data[1].number).toEqual(101); + }); + + it("set_data with item ref (bulk)", async () => { + const params: IActionSetDataParams = { number: "@item.number + 100" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].number).toEqual(100); + expect(data[1].number).toEqual(101); + }); + + it("set_data ignores updates for unchanged data", async () => { + const { _updates } = await actions["parseParams"]({ _list_id: "test_flow", number: 1 }); + expect(_updates).toEqual([{ id: "id_0", number: 1 }]); + }); + + it("set_data prevents update to metadata fields", async () => { + const params: IActionSetDataParams = { _meta: "updated", string: "updated" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].string).toEqual("updated"); + expect(data[0]._meta).toEqual("original"); + }); + + it("set_data ignores evaluation when _updates provided", async () => { + const params: IActionSetDataParams = { _updates: [{ id: "id_0", number: "@item.number" }] }; + const data = await triggerTestSetDataAction(service, params); + // test case illustrative only of not parsing data (would have been parsed independently) + expect(data[0].number).toEqual("@item.number"); + expect(data[1].number).toEqual(1); + }); + + /************************************************************* + * Quality Control + ************************************************************/ + + it("throws error if provided _id does not exist", async () => { + const params = getTestActionRow({ + _id: "missing_id", + string: "sets an item correctly a given id", + }); + await expectAsync(actions.set_data(params)).toBeRejectedWithError( + `[Update Fail] no doc exists\ndata_list: test_flow\n_id: missing_id` + ); + }); + + it("throws error if provided _index does not exist", async () => { + const params = getTestActionRow({ + _index: 10, + string: "sets an item correctly a given id", + }); + await expectAsync(actions.set_data(params)).toBeRejectedWithError( + `[Update Fail] no doc exists\ndata_list: test_flow\n_index: 10` + ); + }); +}); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts new file mode 100644 index 0000000000..3b8d5e41a9 --- /dev/null +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -0,0 +1,107 @@ +import { IActionHandler } from "../../components/template/services/instance/template-action.registry"; +import type { DynamicDataService } from "./dynamic-data.service"; +import { firstValueFrom } from "rxjs"; +import { isEqual, isObjectLiteral } from "packages/shared/src/utils/object-utils"; +import { FlowTypes } from "packages/data-models"; +import { MangoQuery } from "rxdb"; +import { evaluateDynamicDataUpdate } from "./dynamic-data.utils"; + +/** Metadata passed to set_data action to specify data for update **/ +interface IActionSetDataParamsMeta { + /** Reference to source data_list id. All rows in list will be updated */ + _list_id?: string; + + /** ID of data_list row for single update */ + _id?: string; + + /** Index of data_list row for single update */ + _index?: number; + + /** List of compiled data updates if computed outside of action (e.g. data_items) */ + _updates?: FlowTypes.Data_listRow[]; +} + +/** Key-value pairs to update. These support reference to self `@item` context */ +export type IActionSetDataParams = IActionSetDataParamsMeta & Record; + +class DynamicDataActionFactory { + constructor(private service: DynamicDataService) {} + + /** + * Similar syntax can be used for multiple items. + * + * or from outside a loop can specify + * click | set_data | _list: @data.example_list, completed:true; + */ + public set_data: IActionHandler = async ({ params }: { params?: IActionSetDataParams }) => { + const { _list_id, _updates } = await this.parseParams(params); + // Hack, no current method for bulk update so make successive (changes debounced in component) + for (const { id, ...writeableProps } of _updates) { + await this.service.update("data_list", _list_id, id, writeableProps); + } + }; + + private async parseParams(params: IActionSetDataParams) { + // util to log item params and add name prefix as part of thrown errors + function throwParseError(msg: string) { + console.error(params); + throw new Error("[set_data] " + msg); + } + if (isObjectLiteral(params)) { + let { _updates, _list_id } = params; + + // handle parse from item reference string + if (_list_id) { + if (!_updates) { + _updates = await this.generateUpdateList(params); + } + + return { _updates, _list_id }; + } + } + + // throw error if args not parsed correctly + throwParseError("invalid params"); + } + + private async generateUpdateList(params: IActionSetDataParams) { + // remove metadata from rest of update + const { _id, _index, _list_id, _updates, ...update } = params; + const query: MangoQuery = {}; + if (_id) { + query.selector = { id: _id }; + } + let ref = await this.service.query$("data_list", _list_id, query); + let items = await firstValueFrom(ref); + if (items.length === 0) { + const msg = `[Update Fail] no doc exists\ndata_list: ${_list_id}\n_id: ${_id}`; + throw new Error(msg); + } + // NOTE - RXDB doesn't support querying by index or pagination so still retrieve all and then reduce + if (typeof _index === "number") { + const targetItem = items[_index]; + if (!targetItem) { + const msg = `[Update Fail] no doc exists\ndata_list: ${_list_id}\n_index: ${_index}`; + throw new Error(msg); + } + items = [items[_index]]; + } + + const cleanedUpdate = this.removeUpdateMetadata(update); + + // Evaluate item updates for any `@item` self-references + const evaluated = evaluateDynamicDataUpdate(items, cleanedUpdate); + + // Filter to only include updates that will change original item + return evaluated.filter((data, i) => !isEqual(items[i], data)); + } + + private removeUpdateMetadata(update: Record) { + for (const key of Object.keys(update)) { + if (key.startsWith("_")) delete update[key]; + } + return update; + } +} + +export default DynamicDataActionFactory; diff --git a/src/app/shared/services/dynamic-data/dynamic-data.service.mock.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.service.mock.spec.ts new file mode 100644 index 0000000000..939e34393d --- /dev/null +++ b/src/app/shared/services/dynamic-data/dynamic-data.service.mock.spec.ts @@ -0,0 +1,16 @@ +import { FlowTypes } from "packages/data-models"; +import { DynamicDataService } from "./dynamic-data.service"; +import { of } from "rxjs"; + +/** Mock implementation used in other tests */ +export class MockDynamicDataService implements Partial { + constructor(private mockQueryData = {}) {} + // mock query just returns data provided as observable + public async query$(flow_type: FlowTypes.FlowType, flow_name: string) { + return of(this.mockQueryData[flow_type]?.[flow_name]); + } + + public async ready() { + return true; + } +} diff --git a/src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts index e6cde5bff1..95fafc4e6b 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts @@ -2,9 +2,11 @@ import { TestBed } from "@angular/core/testing"; import { HttpClientTestingModule } from "@angular/common/http/testing"; import { firstValueFrom } from "rxjs"; -import { DynamicDataService, ISetItemContext } from "./dynamic-data.service"; +import { DynamicDataService } from "./dynamic-data.service"; import { AppDataService } from "../data/app-data.service"; import { MockAppDataService } from "../data/app-data.service.spec"; +import { DeploymentService } from "../deployment/deployment.service"; +import { MockDeploymentService } from "../deployment/deployment.service.spec"; const TEST_DATA_ROWS = [ { id: "id1", number: 1, string: "hello", boolean: true, _meta_field: { test: "hello" } }, @@ -13,12 +15,6 @@ const TEST_DATA_ROWS = [ ]; type ITestRow = (typeof TEST_DATA_ROWS)[number]; -const SET_ITEM_CONTEXT: ISetItemContext = { - flow_name: "test_flow", - itemDataIDs: ["id1", "id2"], - currentItemId: "id1", -}; - /** * Call standalone tests via: * yarn ng test --include src/app/shared/services/dynamic-data/dynamic-data.service.spec.ts @@ -44,6 +40,10 @@ describe("DynamicDataService", () => { }, }), }, + { + provide: DeploymentService, + useValue: new MockDeploymentService({ name: "test" }), + }, ], }); @@ -51,7 +51,6 @@ describe("DynamicDataService", () => { window.global = window; service = TestBed.inject(DynamicDataService); - TestBed.inject(AppDataService); await service.ready(); // Ensure any data previously persisted is cleared await service.resetFlow("data_list", "test_flow"); @@ -109,56 +108,12 @@ describe("DynamicDataService", () => { expect(res.length).toEqual(20); }); - it("sets an item correctly for current item", async () => { - await service.setItem({ - context: SET_ITEM_CONTEXT, - writeableProps: { string: "sets an item correctly for current item" }, - }); - const obs = await service.query$("data_list", "test_flow"); - const data = await firstValueFrom(obs); - expect(data[0].string).toEqual("sets an item correctly for current item"); - expect(data[1].string).toEqual("goodbye"); - }); - - it("sets an item correctly for a given _id", async () => { - await service.setItem({ - context: SET_ITEM_CONTEXT, - _id: "id2", - writeableProps: { string: "sets an item correctly for a given _id" }, - }); - const obs = await service.query$("data_list", "test_flow"); - const data = await firstValueFrom(obs); - expect(data[0].string).toEqual("hello"); - expect(data[1].string).toEqual("sets an item correctly for a given _id"); - }); - - it("sets an item correctly for a given _index", async () => { - await service.setItem({ - context: SET_ITEM_CONTEXT, - _index: 1, - writeableProps: { string: "sets an item correctly for a given _index" }, - }); - const obs = await service.query$("data_list", "test_flow"); - const data = await firstValueFrom(obs); - expect(data[0].string).toEqual("hello"); - expect(data[1].string).toEqual("sets an item correctly for a given _index"); - }); - it("supports reading data with protected fields", async () => { const obs = await service.query$("data_list", "test_flow"); const data = await firstValueFrom(obs); expect(data[0]["_meta_field"]).toEqual({ test: "hello" }); }); - it("ignores writes to protected fields", async () => { - await service.setItem({ - context: SET_ITEM_CONTEXT, - writeableProps: { _meta_field: "updated", string: "updated" }, - }); - const obs = await service.query$("data_list", "test_flow"); - const data = await firstValueFrom(obs); - expect(data[0]["string"]).toEqual("updated"); - expect(data[0]["_meta_field"]).toEqual({ test: "hello" }); - }); + it("adds metadata (row_index) to docs", async () => { const obs = await service.query$("data_list", "test_flow"); const data = await firstValueFrom(obs); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.service.ts b/src/app/shared/services/dynamic-data/dynamic-data.service.ts index 356ccfd645..3829806015 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.service.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.service.ts @@ -11,6 +11,7 @@ import { PersistedMemoryAdapter } from "./adapters/persistedMemory"; import { ReactiveMemoryAdapter, REACTIVE_SCHEMA_BASE } from "./adapters/reactiveMemory"; import { TemplateActionRegistry } from "../../components/template/services/instance/template-action.registry"; import { TopLevelProperty } from "rxdb/dist/types/types"; +import ActionsFactory from "./dynamic-data.actions"; import { DeploymentService } from "../deployment/deployment.service"; type IDocWithMeta = { id: string; APP_META?: Record }; @@ -52,6 +53,10 @@ export class DynamicDataService extends AsyncServiceBase { ) { super("Dynamic Data"); this.registerInitFunction(this.initialise); + // register action handlers + const { set_data } = new ActionsFactory(this); + this.templateActionRegistry.register({ set_data }); + // HACK - Legacy `set_item` action still managed here (will be removed in #2454) this.registerTemplateActionHandlers(); } @@ -140,12 +145,16 @@ export class DynamicDataService extends AsyncServiceBase { const existingDoc = await this.db.getDoc(collectionName, row_id); if (existingDoc) { const data = existingDoc.toMutableJSON(); - update = deepMergeObjects(data, update); + const mergedUpdate = deepMergeObjects(data, update); + // update memory db + await this.db.updateDoc({ collectionName, id: row_id, data: mergedUpdate }); + // update persisted db + this.writeCache.update({ flow_name, flow_type, id: row_id, data: mergedUpdate }); + } else { + throw new Error( + `[Update Fail] no doc exists for ${flow_type}:${flow_name} with row_id: ${row_id}` + ); } - // update memory db - await this.db.updateDoc({ collectionName, id: row_id, data: update }); - // update persisted db - this.writeCache.update({ flow_name, flow_type, id: row_id, data: update }); } } diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts new file mode 100644 index 0000000000..7302507734 --- /dev/null +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts @@ -0,0 +1,24 @@ +import { FlowTypes } from "packages/data-models"; +import { evaluateDynamicDataUpdate } from "./dynamic-data.utils"; + +const itemRows: FlowTypes.Data_listRow[] = [ + { id: "id_0", number: 0, string: "hello" }, + { id: "id_1", number: 1, string: "hello", boolean: true }, +]; + +/******************************************************************************** + * Tests + * yarn ng test --include src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts + *******************************************************************************/ +describe("DynamicDataService Utils", () => { + /************************************************************* + * Main Tests + ************************************************************/ + it("evaluateDynamicDataUpdate", async () => { + const res = evaluateDynamicDataUpdate(itemRows, { number: "@item.number+10" }); + expect(res).toEqual([ + { id: "id_0", number: 10 }, + { id: "id_1", number: 11 }, + ]); + }); +}); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts new file mode 100644 index 0000000000..6692d7e07d --- /dev/null +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts @@ -0,0 +1,33 @@ +import type { FlowTypes } from "packages/data-models/flowTypes"; +import { TemplatedData } from "packages/shared/src/models/templatedData/templatedData"; +import { AppDataEvaluator } from "packages/shared/src/models/appDataEvaluator/appDataEvaluator"; + +/** + * Given an update to apply to a list of items check whether the update self-references + * `@item.some_field` within any part of the update, and if so evaluated for each + * individual item context. Return list of updates to apply, evaluated for item context + */ +export function evaluateDynamicDataUpdate( + items: FlowTypes.Data_listRow[], + update: Record +) { + if (hasDynamicDataItemReferences(update)) { + const evaluator = new AppDataEvaluator(); + return items.map((item) => { + evaluator.setExecutionContext({ item }); + const evaluated = evaluator.evaluate(update); + // ensure target item id also included in update + return { ...evaluated, id: item.id }; + }); + } else { + return items.map((item) => { + return { ...update, id: item.id }; + }); + } +} + +/** Check whether the update does include any `@item` references */ +function hasDynamicDataItemReferences(data: any) { + const contextVariables = new TemplatedData().listContextVariables(data, ["item"]); + return contextVariables.item; +} From 82ba9ed48a1799eb9ca539e76d75ed65fa30d269 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Thu, 7 Nov 2024 15:15:45 -0800 Subject: [PATCH 04/12] refactor: mock error handler service --- .../template/services/template-field.service.spec.ts | 2 +- src/app/shared/services/data/app-data.service.spec.ts | 2 +- .../error-handler/error-handler.service.mock.spec.ts | 11 +++++++++++ .../error-handler/error-handler.service.spec.ts | 10 ---------- 4 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 src/app/shared/services/error-handler/error-handler.service.mock.spec.ts diff --git a/src/app/shared/components/template/services/template-field.service.spec.ts b/src/app/shared/components/template/services/template-field.service.spec.ts index dcfd73f763..8006345abe 100644 --- a/src/app/shared/components/template/services/template-field.service.spec.ts +++ b/src/app/shared/components/template/services/template-field.service.spec.ts @@ -4,7 +4,7 @@ import { TemplateFieldService } from "./template-field.service"; import type { PromiseExtended } from "dexie"; import { booleanStringToBoolean } from "src/app/shared/utils"; import { ErrorHandlerService } from "src/app/shared/services/error-handler/error-handler.service"; -import { MockErrorHandlerService } from "src/app/shared/services/error-handler/error-handler.service.spec"; +import { MockErrorHandlerService } from "src/app/shared/services/error-handler/error-handler.service.mock.spec"; /** Mock calls for field values from the template field service to return test data */ export class MockTemplateFieldService implements Partial { diff --git a/src/app/shared/services/data/app-data.service.spec.ts b/src/app/shared/services/data/app-data.service.spec.ts index 8394190d5d..2ea55d24cf 100644 --- a/src/app/shared/services/data/app-data.service.spec.ts +++ b/src/app/shared/services/data/app-data.service.spec.ts @@ -7,7 +7,7 @@ import { AppDataService, IAppDataCache } from "./app-data.service"; import { FlowTypes } from "../../model"; import { MockAppDataVariableService } from "./app-data-variable.service.spec"; import { ErrorHandlerService } from "../error-handler/error-handler.service"; -import { MockErrorHandlerService } from "../error-handler/error-handler.service.spec"; +import { MockErrorHandlerService } from "../error-handler/error-handler.service.mock.spec"; import { DbService } from "../db/db.service"; import { MockDbService } from "../db/db.service.spec"; import { Injectable } from "@angular/core"; diff --git a/src/app/shared/services/error-handler/error-handler.service.mock.spec.ts b/src/app/shared/services/error-handler/error-handler.service.mock.spec.ts new file mode 100644 index 0000000000..7f3d020570 --- /dev/null +++ b/src/app/shared/services/error-handler/error-handler.service.mock.spec.ts @@ -0,0 +1,11 @@ +import { ErrorHandlerService } from "./error-handler.service"; + +/** Mock calls for sheets from the appData service to return test data */ +export class MockErrorHandlerService implements Partial { + public logError(error: Error): Promise { + throw error; + } + public handleError(error: Error): Promise { + throw error; + } +} diff --git a/src/app/shared/services/error-handler/error-handler.service.spec.ts b/src/app/shared/services/error-handler/error-handler.service.spec.ts index 96afdbf7f1..f377e0c6b3 100644 --- a/src/app/shared/services/error-handler/error-handler.service.spec.ts +++ b/src/app/shared/services/error-handler/error-handler.service.spec.ts @@ -3,16 +3,6 @@ import { TestBed } from "@angular/core/testing"; import { ErrorHandlerService } from "./error-handler.service"; import { FirebaseService } from "../firebase/firebase.service"; -/** Mock calls for sheets from the appData service to return test data */ -export class MockErrorHandlerService implements Partial { - public logError(error: Error): Promise { - throw error; - } - public handleError(error: Error): Promise { - throw error; - } -} - describe("ErrorHandlerService", () => { let service: ErrorHandlerService; From f5611bffcf12ee61968c0176819fe7d19e27ee5d Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Thu, 7 Nov 2024 15:31:30 -0800 Subject: [PATCH 05/12] fix: item change checks --- packages/shared/src/utils/object-utils.ts | 3 +-- .../dynamic-data/dynamic-data.actions.spec.ts | 5 +++-- .../services/dynamic-data/dynamic-data.actions.ts | 6 +++--- .../dynamic-data/dynamic-data.utils.spec.ts | 9 ++++++++- .../services/dynamic-data/dynamic-data.utils.ts | 13 +++++++++++++ 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/shared/src/utils/object-utils.ts b/packages/shared/src/utils/object-utils.ts index 11a2d0a9c4..2174bb77d4 100644 --- a/packages/shared/src/utils/object-utils.ts +++ b/packages/shared/src/utils/object-utils.ts @@ -100,8 +100,7 @@ export function isEqual(a: any, b: any) { if (!isEqual(Object.keys(aSorted), Object.keys(bSorted))) return false; return isEqual(Object.values(aSorted), Object.values(bSorted)); } - // could not compare (e.g. symbols, date objects, functions, buffers etc) - console.warn(`[isEqual] could not compare`, a, b); + // NOTE - does not compare symbols, date objects, functions, buffers etc. return false; } diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts index 92f5c46d15..6d5c18258e 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts @@ -133,8 +133,9 @@ describe("DynamicDataService Actions", () => { }); it("set_data ignores updates for unchanged data", async () => { - const { _updates } = await actions["parseParams"]({ _list_id: "test_flow", number: 1 }); - expect(_updates).toEqual([{ id: "id_0", number: 1 }]); + const params: IActionSetDataParams = { _list_id: "test_flow", number: 1 }; + const updates = await actions["generateUpdateList"](params); + expect(updates).toEqual([{ id: "id_0", number: 1 }]); }); it("set_data prevents update to metadata fields", async () => { diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts index 3b8d5e41a9..33de9830b4 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -1,10 +1,10 @@ import { IActionHandler } from "../../components/template/services/instance/template-action.registry"; import type { DynamicDataService } from "./dynamic-data.service"; import { firstValueFrom } from "rxjs"; -import { isEqual, isObjectLiteral } from "packages/shared/src/utils/object-utils"; +import { isObjectLiteral } from "packages/shared/src/utils/object-utils"; import { FlowTypes } from "packages/data-models"; import { MangoQuery } from "rxdb"; -import { evaluateDynamicDataUpdate } from "./dynamic-data.utils"; +import { evaluateDynamicDataUpdate, isItemChanged } from "./dynamic-data.utils"; /** Metadata passed to set_data action to specify data for update **/ interface IActionSetDataParamsMeta { @@ -93,7 +93,7 @@ class DynamicDataActionFactory { const evaluated = evaluateDynamicDataUpdate(items, cleanedUpdate); // Filter to only include updates that will change original item - return evaluated.filter((data, i) => !isEqual(items[i], data)); + return evaluated.filter((data, i) => isItemChanged(items[i], data)); } private removeUpdateMetadata(update: Record) { diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts index 7302507734..23bf6a1215 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts @@ -1,5 +1,5 @@ import { FlowTypes } from "packages/data-models"; -import { evaluateDynamicDataUpdate } from "./dynamic-data.utils"; +import { evaluateDynamicDataUpdate, isItemChanged } from "./dynamic-data.utils"; const itemRows: FlowTypes.Data_listRow[] = [ { id: "id_0", number: 0, string: "hello" }, @@ -21,4 +21,11 @@ describe("DynamicDataService Utils", () => { { id: "id_1", number: 11 }, ]); }); + + it("isItemChanged true", async () => { + const res1 = isItemChanged({ id: "id_1", number: 1, string: "hello" }, { number: 2 }); + expect(res1).toEqual(true); + const res2 = isItemChanged({ id: "id_1", number: 1 }, { number: 1 }); + expect(res2).toEqual(false); + }); }); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts index 6692d7e07d..e8b8ad0e82 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.utils.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts @@ -1,6 +1,7 @@ import type { FlowTypes } from "packages/data-models/flowTypes"; import { TemplatedData } from "packages/shared/src/models/templatedData/templatedData"; import { AppDataEvaluator } from "packages/shared/src/models/appDataEvaluator/appDataEvaluator"; +import { isEqual } from "packages/shared/src/utils/object-utils"; /** * Given an update to apply to a list of items check whether the update self-references @@ -31,3 +32,15 @@ function hasDynamicDataItemReferences(data: any) { const contextVariables = new TemplatedData().listContextVariables(data, ["item"]); return contextVariables.item; } + +/** + * Compare an item with update snapshot and determine whether the update contains + * any changes. As updates are partials only compare properties in update against item equivalent + */ +export function isItemChanged( + item: FlowTypes.Data_listRow, + update: Partial +) { + const isChanged = Object.entries(update).find(([key, value]) => !isEqual(item[key], value)); + return isChanged ? true : false; +} From e99ab487c15bb0a94b43c05aaef1262c62b2d8ec Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Fri, 8 Nov 2024 09:54:17 -0800 Subject: [PATCH 06/12] feat: add reset_data action --- packages/data-models/flowTypes.ts | 1 + .../dynamic-data/adapters/persistedMemory.ts | 2 +- .../dynamic-data/dynamic-data.actions.ts | 5 +++ .../dynamic-data/dynamic-data.service.ts | 32 ++++++++++--------- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/data-models/flowTypes.ts b/packages/data-models/flowTypes.ts index 51f8f6fabf..e7b0a95a96 100644 --- a/packages/data-models/flowTypes.ts +++ b/packages/data-models/flowTypes.ts @@ -432,6 +432,7 @@ export namespace FlowTypes { "pop_up", "process_template", "reset_app", + "reset_data", "save_to_device", "screen_orientation", "set_field", diff --git a/src/app/shared/services/dynamic-data/adapters/persistedMemory.ts b/src/app/shared/services/dynamic-data/adapters/persistedMemory.ts index 4378dd7a81..523fdfa694 100644 --- a/src/app/shared/services/dynamic-data/adapters/persistedMemory.ts +++ b/src/app/shared/services/dynamic-data/adapters/persistedMemory.ts @@ -135,7 +135,7 @@ export class PersistedMemoryAdapter { this.persistStateToDB(); } - public async delete(flow_type: FlowTypes.FlowType, flow_name: string) { + public delete(flow_type: FlowTypes.FlowType, flow_name: string) { if (this.get(flow_type, flow_name)) { delete this.state[flow_type][flow_name]; this.persistStateToDB(); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts index 33de9830b4..44004bf739 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -41,6 +41,11 @@ class DynamicDataActionFactory { } }; + public reset_data: IActionHandler = async ({ params }: { params?: IActionSetDataParams }) => { + const { _list_id } = await this.parseParams(params); + return this.service.resetFlow("data_list", _list_id); + }; + private async parseParams(params: IActionSetDataParams) { // util to log item params and add name prefix as part of thrown errors function throwParseError(msg: string) { diff --git a/src/app/shared/services/dynamic-data/dynamic-data.service.ts b/src/app/shared/services/dynamic-data/dynamic-data.service.ts index 3829806015..310bcfb2a8 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.service.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.service.ts @@ -54,8 +54,8 @@ export class DynamicDataService extends AsyncServiceBase { super("Dynamic Data"); this.registerInitFunction(this.initialise); // register action handlers - const { set_data } = new ActionsFactory(this); - this.templateActionRegistry.register({ set_data }); + const { set_data, reset_data } = new ActionsFactory(this); + this.templateActionRegistry.register({ set_data, reset_data }); // HACK - Legacy `set_item` action still managed here (will be removed in #2454) this.registerTemplateActionHandlers(); } @@ -166,14 +166,20 @@ export class DynamicDataService extends AsyncServiceBase { await lastValueFrom(this.collectionCreators[collectionName]); } // Ensure any persisted data deleted - await this.writeCache.delete(flow_type, flow_name); + this.writeCache.delete(flow_type, flow_name); // Remove in-memory db if exists const existingCollection = this.db.getCollection(collectionName); if (existingCollection) { - await this.db.removeCollection(collectionName); + // Empty existing data and re-seed initial data + const docs = await existingCollection.find().exec(); + await existingCollection.bulkRemove(docs.map((d) => d.id)); + // Re-seed initial data + const initialData = await this.getInitialData(flow_type, flow_name); + await existingCollection.bulkInsert(initialData); + } else { + await this.createCollection(flow_type, flow_name); } - await this.createCollection(flow_type, flow_name); } /** Access full state of all persisted data layers */ @@ -205,17 +211,10 @@ export class DynamicDataService extends AsyncServiceBase { if (initialData.length === 0) { throw new Error(`No data exists for collection [${flow_name}], cannot initialise`); } - // add index property to each element before insert, for sorting queried data by original order - const initialDataWithMeta = initialData.map((el) => { - return { - ...el, - row_index: initialData.indexOf(el), - }; - }); - const schema = this.inferSchema(initialDataWithMeta[0]); + const schema = this.inferSchema(initialData[0]); await this.db.createCollection(collectionName, schema); - await this.db.bulkInsert(collectionName, initialDataWithMeta); + await this.db.bulkInsert(collectionName, initialData); // notify any observers that collection has been created this.collectionCreators[collectionName].next(collectionName); this.collectionCreators[collectionName].complete(); @@ -233,7 +232,10 @@ export class DynamicDataService extends AsyncServiceBase { const mergedData = this.mergeData(flowData?.rows, writeDataArray); // HACK - rxdb can't write any fields prefixed with `_` so extract all to top-level APP_META key const cleaned = mergedData.map((el) => this.extractMeta(el)); - return cleaned; + + // add index property to each element before insert, for sorting queried data by original order + const initialDataWithMeta = cleaned.map((el) => ({ ...el, row_index: cleaned.indexOf(el) })); + return initialDataWithMeta; } /** When working with rxdb collections only alphanumeric lower case names allowed */ From 8a681c670ebf2cb6a372c65330080c8216debfb3 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Fri, 8 Nov 2024 11:48:50 -0800 Subject: [PATCH 07/12] fix: template parsed param handling --- .../dynamic-data/dynamic-data.actions.ts | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts index 44004bf739..5f6a0e7758 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -46,27 +46,24 @@ class DynamicDataActionFactory { return this.service.resetFlow("data_list", _list_id); }; + /** Parse action parameters to generate a list of updates to apply */ private async parseParams(params: IActionSetDataParams) { - // util to log item params and add name prefix as part of thrown errors - function throwParseError(msg: string) { - console.error(params); - throw new Error("[set_data] " + msg); - } if (isObjectLiteral(params)) { - let { _updates, _list_id } = params; - + const parsed = this.hackParseTemplatedParams(params); + let { _updates, _list_id } = parsed; + console.log("params", { params, parsed }); // handle parse from item reference string if (_list_id) { if (!_updates) { - _updates = await this.generateUpdateList(params); + _updates = await this.generateUpdateList(parsed); } - return { _updates, _list_id }; } } // throw error if args not parsed correctly - throwParseError("invalid params"); + console.error(params); + throw new Error("[set_data] could not parse params"); } private async generateUpdateList(params: IActionSetDataParams) { @@ -107,6 +104,31 @@ class DynamicDataActionFactory { } return update; } + + /** + * Any params provided by templating system will already be partially parsed to replace dynamic references + * such as @local or @field. This also converts @item references to `this.item` + * Revert the item changes and also convert string parameter values to number where required + * + * TODO - this method should be removed and `items` made to update their own `this` context + **/ + private hackParseTemplatedParams(params: IActionSetDataParams) { + const parsed: IActionSetDataParams = {}; + // HACK - un-parse @item references that the templating system converts to `this.item` + for (const [key, value] of Object.entries(params)) { + if (typeof value === "string") { + parsed[key] = value.replace(/this\.item/g, "@item"); + } else { + parsed[key] = value; + } + } + // convert _index param which may be passed as string from template if defined inline + // NOTE - RXDB will automatically cast all other string values to correct type due to inferred schema + if (typeof params._index === "string") { + parsed._index = Number(params._index); + } + return parsed; + } } export default DynamicDataActionFactory; From 2cab98bbfecfd12ad028138e847448631eaa2037 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Sat, 9 Nov 2024 08:20:51 -0800 Subject: [PATCH 08/12] feat: add support for data type coercion --- .../dynamic-data/dynamic-data.utils.spec.ts | 38 ++++++++++++- .../dynamic-data/dynamic-data.utils.ts | 56 ++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts index 23bf6a1215..4fb0c7e5e7 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.spec.ts @@ -1,5 +1,10 @@ import { FlowTypes } from "packages/data-models"; -import { evaluateDynamicDataUpdate, isItemChanged } from "./dynamic-data.utils"; +import { + coerceDataUpdateTypes, + evaluateDynamicDataUpdate, + isItemChanged, +} from "./dynamic-data.utils"; +import { JsonSchema } from "rxdb"; const itemRows: FlowTypes.Data_listRow[] = [ { id: "id_0", number: 0, string: "hello" }, @@ -28,4 +33,35 @@ describe("DynamicDataService Utils", () => { const res2 = isItemChanged({ id: "id_1", number: 1 }, { number: 1 }); expect(res2).toEqual(false); }); + + it("coerceDataUpdateTypes", () => { + const schemaMapping: Record = { + number: { type: "number" }, + boolean: { type: "boolean" }, + text: { type: "string" }, + }; + const res1 = coerceDataUpdateTypes(schemaMapping, [ + { + number: "1", + boolean: "true", + text: "hello", + additional: "string", + }, + ]); + expect(res1).toEqual([ + { + number: 1, + boolean: true, + text: "hello", + additional: "string", + }, + ]); + // does not coerce missing or undefined properties + const res2 = coerceDataUpdateTypes(schemaMapping, [ + { + number: undefined, + }, + ]); + expect(res2).toEqual([{ number: undefined }]); + }); }); diff --git a/src/app/shared/services/dynamic-data/dynamic-data.utils.ts b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts index e8b8ad0e82..695ec62a36 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.utils.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.utils.ts @@ -1,7 +1,9 @@ import type { FlowTypes } from "packages/data-models/flowTypes"; import { TemplatedData } from "packages/shared/src/models/templatedData/templatedData"; import { AppDataEvaluator } from "packages/shared/src/models/appDataEvaluator/appDataEvaluator"; -import { isEqual } from "packages/shared/src/utils/object-utils"; +import { isEqual, isObjectLiteral } from "packages/shared/src/utils/object-utils"; +import { JsonSchema, JsonSchemaTypes } from "rxdb"; +import { booleanStringToBoolean } from "../../utils"; /** * Given an update to apply to a list of items check whether the update self-references @@ -44,3 +46,55 @@ export function isItemChanged( const isChanged = Object.entries(update).find(([key, value]) => !isEqual(item[key], value)); return isChanged ? true : false; } + +/** + * Use rxdb schema to cast data updates to the correct data types + * Used to convert parameter_list values stored as strings to number or boolean where required + * */ +export function coerceDataUpdateTypes(schemaProperties: Record, data: any[]) { + // create a general mapping to convert any listed schema property from a string value + // to the correct schema value type + const propertyMapping: Record any> = {}; + for (const [key, { type }] of Object.entries(schemaProperties)) { + // do not map metadata fields (can't be set by user input) + if (!key.startsWith("_")) { + propertyMapping[key] = typeMappings[type as JsonSchemaTypes]; + } + } + // coerce data values using mapping + const coerced = []; + for (const el of data) { + if (isObjectLiteral(el)) { + for (const [key, value] of Object.entries(el)) { + // only map values if they are strings and have a defined mapping function + if (propertyMapping[key] && typeof value === "string") { + el[key] = propertyMapping[key](value); + } + } + } + coerced.push(el); + } + return coerced; +} + +/** + * Mapping functions used to coerce string values to RXDB schema types + * This is a subset of mappings that could alternatively be provided by + * https://ajv.js.org/coercion.html + * **/ +const typeMappings: Record any | undefined> = { + // do not attempt to coerce arrays (data_list does not store array values) + array: undefined, + // convert boolean string to boolean + boolean: (v) => booleanStringToBoolean(v), + // convert string integer to integer + integer: (v) => parseInt(v, 10), + // do not attempt to coerce null schema + null: undefined, + // convert string number to number + number: (v) => Number(v), + // do not attempt to coerce objects (data_list does not store object values) + object: undefined, + // as incoming values will be formatted as strings do not provide any additional mapping + string: undefined, +}; From 7184921617b60082dacb7a98e3e9d458062f6961 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Sat, 9 Nov 2024 08:22:40 -0800 Subject: [PATCH 09/12] test: reset action and type coercion --- .../dynamic-data/dynamic-data.actions.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts index 6d5c18258e..cbc797763f 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts @@ -153,6 +153,27 @@ describe("DynamicDataService Actions", () => { expect(data[1].number).toEqual(1); }); + it("reset_data action restores data to initial", async () => { + const updatedData = await triggerTestSetDataAction(service, { string: "updated string" }); + expect(updatedData[0].string).toEqual("updated string"); + const resetActionBase = getTestActionRow({}); + await actions.reset_data({ ...resetActionBase, action_id: "reset_data" }); + const obs = await service.query$("data_list", "test_flow"); + const resetData = await firstValueFrom(obs); + expect(resetData[0].string).toEqual("hello"); + }); + + /************************************************************* + * Misc + ************************************************************/ + + it("Coerces string params to correct data type", async () => { + // NOTE - there is no specific code that casts variables, but RXDB will infer from schema + const params: IActionSetDataParams = { number: "300" }; + const data = await triggerTestSetDataAction(service, params); + expect(data[0].number).toEqual(300); + }); + /************************************************************* * Quality Control ************************************************************/ From afe1034e79eef4479277e9a79ef1f73b3e917649 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Sat, 9 Nov 2024 08:23:03 -0800 Subject: [PATCH 10/12] chore: code tidying --- .../services/dynamic-data/dynamic-data.actions.ts | 12 ++++++++++-- .../services/dynamic-data/dynamic-data.service.ts | 5 +++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts index 5f6a0e7758..f7d994aeae 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -4,7 +4,11 @@ import { firstValueFrom } from "rxjs"; import { isObjectLiteral } from "packages/shared/src/utils/object-utils"; import { FlowTypes } from "packages/data-models"; import { MangoQuery } from "rxdb"; -import { evaluateDynamicDataUpdate, isItemChanged } from "./dynamic-data.utils"; +import { + coerceDataUpdateTypes, + evaluateDynamicDataUpdate, + isItemChanged, +} from "./dynamic-data.utils"; /** Metadata passed to set_data action to specify data for update **/ interface IActionSetDataParamsMeta { @@ -94,8 +98,12 @@ class DynamicDataActionFactory { // Evaluate item updates for any `@item` self-references const evaluated = evaluateDynamicDataUpdate(items, cleanedUpdate); + // Coerce updates to correct data types (inline parameter_list values parsed as strings) + const schema = this.service.getSchema("data_list", _list_id); + const coerced = coerceDataUpdateTypes(schema?.jsonSchema?.properties, evaluated); + // Filter to only include updates that will change original item - return evaluated.filter((data, i) => isItemChanged(items[i], data)); + return coerced.filter((data, i) => isItemChanged(items[i], data)); } private removeUpdateMetadata(update: Record) { diff --git a/src/app/shared/services/dynamic-data/dynamic-data.service.ts b/src/app/shared/services/dynamic-data/dynamic-data.service.ts index 310bcfb2a8..d8df8ab769 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.service.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.service.ts @@ -189,6 +189,11 @@ export class DynamicDataService extends AsyncServiceBase { return this.writeCache.state; } + public getSchema(flow_type: FlowTypes.FlowType, flow_name: string) { + const collectionName = this.normaliseCollectionName(flow_type, flow_name); + return this.db.getCollection(collectionName)?.schema; + } + /** Ensure a collection exists, creating if not and populating with corresponding list data */ private async ensureCollection(flow_type: FlowTypes.FlowType, flow_name: string) { const collectionName = this.normaliseCollectionName(flow_type, flow_name); From 90a4567e89333fa0c993f946c972e1867e911a64 Mon Sep 17 00:00:00 2001 From: Johnny McQuade Date: Tue, 12 Nov 2024 14:56:08 +0000 Subject: [PATCH 11/12] ci: bump version number to 0.17.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8f751d1deb..a7fb4dfefa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "frontend", - "version": "0.16.38", + "version": "0.17.0", "author": "IDEMS International", "license": "See LICENSE", "homepage": "https://idems.international/", From a75b533f0ef9813f69f800a2d5d1262ef67374d3 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Mon, 18 Nov 2024 16:10:18 -0800 Subject: [PATCH 12/12] chore: remove console log --- src/app/shared/services/dynamic-data/dynamic-data.actions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts index f7d994aeae..3ead2ccf0e 100644 --- a/src/app/shared/services/dynamic-data/dynamic-data.actions.ts +++ b/src/app/shared/services/dynamic-data/dynamic-data.actions.ts @@ -55,7 +55,6 @@ class DynamicDataActionFactory { if (isObjectLiteral(params)) { const parsed = this.hackParseTemplatedParams(params); let { _updates, _list_id } = parsed; - console.log("params", { params, parsed }); // handle parse from item reference string if (_list_id) { if (!_updates) {