diff --git a/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.spec.ts b/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.spec.ts index dd2aff6..093825c 100644 --- a/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.spec.ts +++ b/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.spec.ts @@ -3189,6 +3189,72 @@ describe('WorkflowEngineService', () => { ); }); + it('returns a cancelled ad-hoc directive that still resolves its GraphQL getters', async (): Promise => { + const fixture = createServiceFixture({ + currentVersionId: 'template-version-1', + formVersionStatus: FormDefinitionVersionStatusEnum.PUBLISHED, + processAdhocDirectives: [ + createAdhocDirective({ + createdByMemberId: 'member-finance', + id: 'directive-cancel-1', + status: AdhocDirectiveStatusEnum.PENDING, + targetValue: { + kind: AdhocTargetKindEnum.MEMBER, + memberIds: ['member-x'], + }, + type: AdhocDirectiveTypeEnum.COUNTERSIGN, + }), + ], + processWorkflowSnapshot: createLinearUserTaskWorkflow(), + templateVersionStatus: ApprovalTemplateVersionStatusEnum.PUBLISHED, + }); + + const cancelled = await fixture.service.cancelAdhocDirective({ + cancelledByMemberId: 'member-finance', + directiveId: 'directive-cancel-1', + }); + + expect(cancelled.status).toBe(AdhocDirectiveStatusEnum.CANCELLED); + // `targetValueJson` is a non-nullable GraphQL field backed by a prototype + // getter, so the resolver only survives if the saved value is still an + // entity instance. Spreading the row into a plain object drops the getter + // and the mutation fails with an opaque INTERNAL_SERVER_ERROR *after* the + // cancellation has already been written, which reads to the approver as + // "the withdraw failed" even though it succeeded. + expect(cancelled).toBeInstanceOf(AdhocDirectiveEntity); + expect(cancelled.targetValueJson).toBe( + JSON.stringify({ + kind: AdhocTargetKindEnum.MEMBER, + memberIds: ['member-x'], + }), + ); + }); + + it('returns a cancelled instance that still resolves its GraphQL getters', async (): Promise => { + const fixture = createServiceFixture({ + currentVersionId: 'template-version-1', + formVersionStatus: FormDefinitionVersionStatusEnum.PUBLISHED, + processFormData: { amount: 1200 }, + processWorkflowSnapshot: createLinearUserTaskWorkflow(), + templateVersionStatus: ApprovalTemplateVersionStatusEnum.PUBLISHED, + }); + + const cancelled = await fixture.service.cancelApprovalInstance({ + cancelledByMemberId: 'member-001', + comment: null, + instanceId: 'instance-1', + }); + + expect(cancelled.state).toBe(ApprovalInstanceStateEnum.CANCELLED); + // `ApprovalInstance` exposes five getter-backed JSON fields, all of them + // non-nullable. Spreading the row into a plain object drops every one, so + // the withdrawal is committed and the mutation still fails with an opaque + // INTERNAL_SERVER_ERROR — the initiator sees "cancel failed" on a case + // that is already cancelled. + expect(cancelled).toBeInstanceOf(ApprovalInstanceEntity); + expect(cancelled.formDataJson).toBe(JSON.stringify({ amount: 1200 })); + }); + it('dispatches ad-hoc completion notifications on reject and cancels pending flow directives', async (): Promise => { const fixture = createServiceFixture({ currentVersionId: 'template-version-1', @@ -3623,19 +3689,35 @@ function createServiceFixture({ ), findOne: jest.fn(() => Promise.resolve( - savedInstance ?? - createApprovalInstance({ - formData: processFormData, - formDataOptionSnapshot: processOptionSnapshot, - formDefinitionSnapshot: processFormDefinitionSnapshot, - state: instanceState, - updatedAt: transactionalInstanceUpdatedAt, - workflowSnapshot: processWorkflowSnapshot, - }), + // A row read back from the database is an entity whatever shape the + // caller last handed to `save`, so it is rehydrated here. Only + // `save` reports the shape it was actually given. + savedInstance + ? Object.assign(createApprovalInstance(), savedInstance) + : createApprovalInstance({ + formData: processFormData, + formDataOptionSnapshot: processOptionSnapshot, + formDefinitionSnapshot: processFormDefinitionSnapshot, + state: instanceState, + updatedAt: transactionalInstanceUpdatedAt, + workflowSnapshot: processWorkflowSnapshot, + }), ), ), save: jest.fn((entity: ApprovalInstanceEntity) => { - savedInstance = Object.assign(createApprovalInstance(), entity); + // The saved row keeps the prototype it arrived with. Rebuilding it as + // `Object.assign(createApprovalInstance(), entity)` handed back an + // entity even when the caller had spread the row into a plain object, + // which hid exactly the regression the cancel paths guard against: + // the getter-backed `*Json` fields the GraphQL schema exposes live on + // the prototype, so a plain object fails the mutation at resolve time. + savedInstance = Object.assign( + Object.create( + Object.getPrototypeOf(entity) as object, + ) as ApprovalInstanceEntity, + createApprovalInstance(), + entity, + ); return Promise.resolve(savedInstance); }), diff --git a/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.ts b/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.ts index abad75e..e17a379 100644 --- a/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.ts +++ b/libs/bpm-core/src/lib/workflow-engine/workflow-engine.service.ts @@ -939,11 +939,15 @@ export class WorkflowEngineService { instanceId: instance.id, manager, }); - const cancelledInstance = await instanceRepository.save({ - ...instance, - completedAt: cancelledAt, - state: ApprovalInstanceStateEnum.CANCELLED, - }); + // Same prototype-preserving reason as `cancelAdhocDirective`: this + // entity is returned to the resolver and `ApprovalInstance` exposes + // five getter-backed JSON fields. + const cancelledInstance = await instanceRepository.save( + Object.assign(new ApprovalInstanceEntity(), instance, { + completedAt: cancelledAt, + state: ApprovalInstanceStateEnum.CANCELLED, + }), + ); await this.dispatchAdhocCompletionNotifications( manager, @@ -1317,10 +1321,16 @@ export class WorkflowEngineService { ); } - const cancelledDirective = await directiveRepository.save({ - ...directive, - status: AdhocDirectiveStatusEnum.CANCELLED, - }); + // Assigning onto a fresh entity keeps the prototype, so the row handed + // back to the resolver still carries the getters the GraphQL schema + // exposes (e.g. `targetValueJson`). Spreading it into a plain object + // drops them, and the mutation then fails with an opaque + // INTERNAL_SERVER_ERROR *after* the cancellation has been committed. + const cancelledDirective = await directiveRepository.save( + Object.assign(new AdhocDirectiveEntity(), directive, { + status: AdhocDirectiveStatusEnum.CANCELLED, + }), + ); await this.recordAdhocDirectiveActivity( manager,