Merge pull request #349 from getodk/edit-epic/edit-instance-interface

[Edits] Engine I/O support for editing submitted instances

Author: eyelidlessness

.changeset/violet-apricots-collect.md

@@ -0,0 +1,8 @@
+---
+'@getodk/xforms-engine': minor
+---
+
+Partial support for editing submitted instances:
+
+- Introduce `editInstance` entrypoints, intended for editing previously submitted instance state
+- Implement resource resolution for `editInstance` entrypoints, intended for supporting I/O-bound submission edit workflows

packages/common/src/fixtures/import-glob-helper.ts

@@ -1,11 +1,10 @@
+import type { Awaitable } from '../../types/helpers.d.ts';
 import { IS_NODE_RUNTIME } from '../env/detection.ts';
 
 interface GlobURLFetchResponse {
 	text(): Promise<string>;
 }
 
-type Awaitable<T> = Promise<T> | T;
-
 type FetchGlobURL = (globURL: string) => Awaitable<GlobURLFetchResponse>;
 
 let fetchGlobURL: FetchGlobURL;

packages/common/types/helpers.d.ts

@@ -43,3 +43,7 @@ export type AnyConstructor = ConstructorOf<any>;
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export type AnyFunction = (this: any, ...args: any[]) => any;
+
+export type Awaitable<T> = Promise<T> | T;
+
+export type Thunk<T> = () => T;

packages/scenario/src/client/editInstance.ts

@@ -0,0 +1,90 @@
+import { getBlobText } from '@getodk/common/lib/web-compat/blob.ts';
+import type {
+	EditedFormInstance,
+	InstancePayload,
+	ResolvableFormInstanceInput,
+	ResolvableInstanceAttachmentsMap,
+	ResolveFormInstanceResource,
+	RootNode,
+} from '@getodk/xforms-engine';
+import { constants as ENGINE_CONSTANTS } from '@getodk/xforms-engine';
+import { assert, expect } from 'vitest';
+import type { InitializableForm } from './init.ts';
+
+/**
+ * @todo This type should probably:
+ *
+ * - Be exported from the engine
+ * - With a name like this
+ * - With the `status` enum updated to replace "ready" with "submittable"
+ *
+ * Doing so is deferred for now, to avoid a late breaking change to the engine's
+ * client interface which will also affect integrating host applications (e.g.
+ * Central, whose release is blocked awaiting edit functionality).
+ *
+ * Similarly, exporting a type of the same name with the existing "ready" enum
+ * value is deferred for now to avoid a confusing mismatch between names.
+ */
+type SubmittableInstancePayload = Extract<
+	InstancePayload<'monolithic'>,
+	{ readonly status: 'ready' }
+>;
+
+type AssertSubmittable = (
+	payload: InstancePayload<'monolithic'>
+) => asserts payload is SubmittableInstancePayload;
+
+/**
+ * @todo Can Vitest assertion extensions do this type refinement directly?
+ * Normally we'd use {@link assert} for this, but we already have
+ * `toBeReadyForSubmission`, with much clearer intent and semantics.
+ */
+const assertSubmittable: AssertSubmittable = (payload) => {
+	expect(payload).toBeReadyForSubmission();
+};
+
+const mockSubmissionIO = (payload: SubmittableInstancePayload): ResolvableFormInstanceInput => {
+	const instanceFile = payload.data[0].get(ENGINE_CONSTANTS.INSTANCE_FILE_NAME);
+	const resolveInstance = () => getBlobText(instanceFile);
+	const attachmentFiles = Array.from(payload.data)
+		.flatMap((data) => Array.from(data.values()))
+		.filter((value): value is File => value !== instanceFile && value instanceof File);
+	const attachments: ResolvableInstanceAttachmentsMap = new Map(
+		attachmentFiles.map((file) => {
+			const resolveAttachment: ResolveFormInstanceResource = () => {
+				return Promise.resolve(new Response(file));
+			};
+
+			return [file.name, resolveAttachment];
+		})
+	);
+
+	return {
+		inputType: 'FORM_INSTANCE_INPUT_RESOLVABLE',
+		resolveInstance,
+		attachments,
+	};
+};
+
+/**
+ * Creates a new {@link EditedFormInstance} from an existing
+ * {@link instanceRoot}:
+ *
+ * 1. Prepare an {@link InstancePayload | instance payload} from the existing
+ *    instance
+ * 2. Assert that the payload is
+ *    {@link SubmittableInstancePayload | submittable}
+ * 3. Wrap the payload's data to satisfy the {@link ResolvableFormInstanceInput}
+ *    interface (effectively {@link mockSubmissionIO | mocking submission I/O})
+ * 4. Create an {@link EditedFormInstance} from that I/O-mocked input
+ */
+export const editInstance = async (
+	form: InitializableForm,
+	instanceRoot: RootNode
+): Promise<EditedFormInstance> => {
+	const payload = await instanceRoot.prepareInstancePayload();
+
+	assertSubmittable(payload);
+
+	return form.editInstance(mockSubmissionIO(payload));
+};

packages/scenario/src/client/init.ts

@@ -37,7 +37,7 @@ export type InitializableForm =
 	| LoadFormWarningResult;
 
 interface InitializedTestForm {
-	readonly formResult: InitializableForm;
+	readonly form: InitializableForm;
 	readonly instanceRoot: RootNode;
 	readonly owner: Owner;
 	readonly dispose: VoidFunction;
@@ -50,7 +50,7 @@ export const initializeTestForm = async (
 	return createRoot(async (dispose) => {
 		const owner = getAssertedOwner();
 
-		const { formResult, root: instanceRoot } = await runInSolidScope(owner, async () => {
+		const { formResult: form, root: instanceRoot } = await runInSolidScope(owner, async () => {
 			return createInstance(formResource, {
 				form: {
 					...defaultConfig,
@@ -64,7 +64,7 @@ export const initializeTestForm = async (
 		});
 
 		return {
-			formResult,
+			form,
 			instanceRoot,
 			owner,
 			dispose,

packages/scenario/src/jr/Scenario.ts

@@ -1,6 +1,7 @@
 import type { XFormsElement } from '@getodk/common/test/fixtures/xform-dsl/XFormsElement.ts';
 import { xmlElement } from '@getodk/common/test/fixtures/xform-dsl/index.ts';
 import type {
+	AnyFormInstance,
 	AnyNode,
 	FormResource,
 	MonolithicInstancePayload,
@@ -19,6 +20,7 @@ import { RankValuesAnswer } from '../answer/RankValuesAnswer.ts';
 import { SelectValuesAnswer } from '../answer/SelectValuesAnswer.ts';
 import type { ValueNodeAnswer } from '../answer/ValueNodeAnswer.ts';
 import { answerOf } from '../client/answerOf.ts';
+import { editInstance } from '../client/editInstance.ts';
 import type { InitializableForm, TestFormOptions } from '../client/init.ts';
 import { initializeTestForm } from '../client/init.ts';
 import { isRepeatRange } from '../client/predicates.ts';
@@ -67,14 +69,15 @@ import { JRTreeReference } from './xpath/JRTreeReference.ts';
  */
 const nonReactiveIdentityStateFactory = <T extends object>(value: T): T => value;
 
-export interface ScenarioConstructorOptions {
-	readonly owner: Owner;
-	readonly dispose: VoidFunction;
+interface ScenarioFormMeta {
 	readonly formName: string;
 	readonly formElement: XFormsElement;
 	readonly formOptions: TestFormOptions;
-	readonly formResult: InitializableForm;
-	readonly instanceRoot: RootNode;
+}
+
+export interface ScenarioConfig extends ScenarioFormMeta {
+	readonly owner: Owner;
+	readonly dispose: VoidFunction;
 }
 
 type FormFileName = `${string}.xml`;
@@ -128,7 +131,7 @@ const isAnswerItemCollectionParams = (
 type ScenarioClass = typeof Scenario;
 
 export interface ScenarioConstructor<T extends Scenario = Scenario> extends ScenarioClass {
-	new (options: ScenarioConstructorOptions): T;
+	new (meta: ScenarioConfig, form: InitializableForm, instanceRoot: RootNode): T;
 }
 
 /**
@@ -171,75 +174,60 @@ export class Scenario {
 		this: This,
 		...args: ScenarioStaticInitParameters
 	): Promise<This['prototype']> {
-		let formElement: XFormsElement;
-		let formName: string;
-		let formOptions: TestFormOptions;
+		let formMeta: ScenarioFormMeta;
 
 		if (isFormFileName(args[0])) {
 			return this.init(r(args[0]));
 		} else if (args.length === 1) {
 			const [resource] = args;
 
-			formElement = xmlElement(resource.textContents);
-			formName = resource.formName;
-			formOptions = this.getTestFormOptions();
+			formMeta = {
+				formElement: xmlElement(resource.textContents),
+				formName: resource.formName,
+				formOptions: this.getTestFormOptions(),
+			};
 		} else {
-			const [name, form, overrideOptions] = args;
+			const [formName, formElement, overrideOptions] = args;
 
-			formName = name;
-			formElement = form;
-			formOptions = this.getTestFormOptions(overrideOptions);
+			formMeta = {
+				formName,
+				formElement,
+				formOptions: this.getTestFormOptions(overrideOptions),
+			};
 		}
 
-		const formResource = formElement.asXml() satisfies FormResource;
-		const { dispose, owner, formResult, instanceRoot } = await initializeTestForm(
-			formResource,
-			formOptions
+		const { dispose, owner, form, instanceRoot } = await initializeTestForm(
+			formMeta.formElement.asXml() satisfies FormResource,
+			formMeta.formOptions
 		);
 
 		return runInSolidScope(owner, () => {
-			return new this({
-				owner,
-				dispose,
-				formName,
-				formElement,
-				formOptions,
-				formResult,
-				instanceRoot,
-			});
+			return new this(
+				{
+					...formMeta,
+					owner,
+					dispose,
+				},
+				form,
+				instanceRoot
+			);
 		});
 	}
 
 	declare readonly ['constructor']: ScenarioConstructor<this>;
 
-	private readonly owner: Owner;
-	private readonly dispose: VoidFunction;
-	private readonly formElement: XFormsElement;
-	private readonly formOptions: TestFormOptions;
-	private readonly formResult: InitializableForm;
-
-	readonly formName: string;
-	readonly instanceRoot: RootNode;
-
 	protected readonly getPositionalEvents: Accessor<PositionalEvents>;
 
 	protected readonly getEventPosition: Accessor<number>;
 	private readonly setEventPosition: Setter<number>;
 
 	protected readonly getSelectedPositionalEvent: Accessor<AnyPositionalEvent>;
 
-	protected constructor(options: ScenarioConstructorOptions) {
-		const { owner, dispose, formName, formElement, formOptions, formResult, instanceRoot } =
-			options;
-
-		this.owner = owner;
-		this.dispose = dispose;
-		this.formName = formName;
-		this.formElement = formElement;
-		this.formOptions = formOptions;
-		this.formResult = formResult;
-		this.instanceRoot = instanceRoot;
-
+	protected constructor(
+		private readonly config: ScenarioConfig,
+		private readonly form: InitializableForm,
+		readonly instanceRoot: RootNode
+	) {
 		const [getEventPosition, setEventPosition] = createSignal(0);
 
 		this.getPositionalEvents = () => getPositionalEvents(instanceRoot);
@@ -260,7 +248,7 @@ export class Scenario {
 
 		afterEach(() => {
 			PositionalEvent.cleanup();
-			dispose();
+			config.dispose();
 		});
 	}
 
@@ -768,21 +756,7 @@ export class Scenario {
 	 * will remain) unaffected by those calls.
 	 */
 	newInstance(): this {
-		return runInSolidScope(this.owner, () => {
-			const { dispose, owner, formName, formElement, formOptions, formResult } = this;
-			const instance = formResult.createInstance();
-			const instanceRoot = instance.root;
-
-			return new this.constructor({
-				owner,
-				dispose,
-				formName,
-				formElement,
-				formOptions,
-				formResult,
-				instanceRoot,
-			});
-		});
+		return this.fork(this.form.createInstance());
 	}
 
 	getValidationOutcome(): ValidateOutcome {
@@ -1086,28 +1060,18 @@ export class Scenario {
 	}
 
 	/**
-	 * @todo We may also want a conceptually equivalent static method, composing
-	 * `loadForm`/`restoreInstance` behavior.
+	 * @todo Naming? The name here was chosen to indicate this creates a "fork" of various aspects of a {@link Scenario} instance (most of which are internal/class-private) with a new {@link RootNode | form instance root} (derived from the current {@link Scenario} instance's {@link })
 	 */
-	async restoreWebFormsInstanceState(payload: RestoreFormInstanceInput): Promise<this> {
-		const { dispose, owner, formName, formElement, formOptions, formResult } = this;
-
-		const instance = await runInSolidScope(owner, () => {
-			return this.formResult.restoreInstance(payload, formOptions);
+	private fork(instance: AnyFormInstance): this {
+		return runInSolidScope(this.config.owner, () => {
+			return new this.constructor(this.config, this.form, instance.root);
 		});
-		const instanceRoot = instance.root;
+	}
 
-		return runInSolidScope(owner, () => {
-			return new this.constructor({
-				owner,
-				dispose,
-				formName,
-				formElement,
-				formOptions,
-				formResult,
-				instanceRoot,
-			});
-		});
+	async restoreWebFormsInstanceState(payload: RestoreFormInstanceInput): Promise<this> {
+		const instance = await this.form.restoreInstance(payload, this.config.formOptions);
+
+		return this.fork(instance);
 	}
 
 	// TODO: consider adapting tests which use the following interfaces to use
@@ -1186,7 +1150,7 @@ export class Scenario {
 		expect(
 			form.asXml(),
 			'Attempted to serialize instance with unexpected form XML. Is instance from an unrelated form?'
-		).toBe(this.formElement.asXml());
+		).toBe(this.config.formElement.asXml());
 
 		return this.proposed_serializeAndRestoreInstanceState();
 	}
@@ -1214,6 +1178,13 @@ export class Scenario {
 
 		return this.restoreWebFormsInstanceState(payload);
 	}
+
+	/** @see {@link editInstance} */
+	async proposed_editCurrentInstanceState(): Promise<this> {
+		const instance = await editInstance(this.form, this.instanceRoot);
+
+		return this.fork(instance);
+	}
 }
 
 /**