Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: set data action standalone #2513

Merged
merged 12 commits into from
Nov 19, 2024
1 change: 1 addition & 0 deletions packages/data-models/flowTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ export namespace FlowTypes {
"pop_up",
"process_template",
"reset_app",
"reset_data",
"save_to_device",
"screen_orientation",
"set_field",
Expand Down
3 changes: 1 addition & 2 deletions packages/shared/src/utils/object-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@esmeetewinkel noticed that this warning has been appearing, see #2528. I can't understand what's causing this as the comparison appears to be between two strings. It doesn't seem to cause any problems for functionality, but just flagging here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the console log is wrong as up till this point we've checked:

  • strict equivalent (a===b) \ true
  • different types \ false
  • equivalent arrays and equivalent object literals \ true

so what we are left with are non-equivalent but same type (strings, numbers etc.), or additional primative types not already checked (e.g. date objects, bigints etc.).

So if we wanted we could add a type check for string, number and boolean types and confidently return false, and then add a log just to catch other types; but for now I've just dropped the log and added a note on the function limitation. I don't see this being of practical consequence as there's currently no way to construct other primitive types from the code.

// NOTE - does not compare symbols, date objects, functions, buffers etc.
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>
>;
export type IActionHandler = (action: FlowTypes.TemplateRowAction) => Promise<any>;
export type IActionHandlers = Record<IActionId, IActionHandler>;

@Injectable({ providedIn: "root" })
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TemplateFieldService> {
Expand Down
2 changes: 1 addition & 1 deletion src/app/shared/services/data/app-data.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
200 changes: 200 additions & 0 deletions src/app/shared/services/dynamic-data/dynamic-data.actions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
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$<any>("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 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 () => {
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the point of this test case is just to demonstrate that using the _updates param (which should only be considered from within the code) requires any dynamic values to be evaluated already?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, part of the idea with the parallel work on items loops is that the item loop handles all its own data update processing (it already has an active data_list query subscription, so doesn't to run a single db query like a regular set_data operation needs to).

Although part of me is thinking that this inefficiency might actually be better just to simplify the set_items handling, and given that the data_list query is only making a single call to in-memory db so really not an expensive operation to duplicate. So I might decide to remove this functionality in a follow-up PR, but for now it does act as a bypass.

expect(data[0].number).toEqual("@item.number");
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$<any>("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
************************************************************/

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`
);
});
});
Loading
Loading