From 083a99192544da427f06c5d6c7c5a208353cf988 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Sun, 12 Jul 2026 07:35:16 +0000 Subject: [PATCH] fix(notification-trigger): seed default collaborators onto the parent doc, not the attached tx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getCollaborators resolved the default collaborators of the parent doc (loaded via the triggering tx's attachedTo/attachedToClass) but seeded them onto tx.objectId — the attached child item (a comment/message create/update/remove tx). The parent's collaborators therefore landed on the child record instead of the parent. Seed onto doc._id/doc._class/doc.space instead. Same class of bug as the chunter seed-target fix. The returned collaborator list is unchanged; only the target of the newly created Collaborator records changes. Adds a unit test that drives getCollaborators directly: real getAddCollaboratTxes/getDocCollaborators, only getClassCollaborators stubbed. Parent and child are given distinct collaborators and spaces; it asserts the seed targets the parent (attachedTo/attachedToClass/objectSpace), never the child, while the returned collaborator list stays identical. Signed-off-by: Michael Uray --- .../src/__tests__/seedCollaborators.test.ts | 153 ++++++++++++++++++ .../notification-resources/src/index.ts | 4 +- 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 server-plugins/notification-resources/src/__tests__/seedCollaborators.test.ts diff --git a/server-plugins/notification-resources/src/__tests__/seedCollaborators.test.ts b/server-plugins/notification-resources/src/__tests__/seedCollaborators.test.ts new file mode 100644 index 00000000000..0526949e358 --- /dev/null +++ b/server-plugins/notification-resources/src/__tests__/seedCollaborators.test.ts @@ -0,0 +1,153 @@ +import { type Doc, type Tx, type TxCreateDoc } from '@hcengineering/core' + +// Imported AFTER the mocks (jest.mock is hoisted by ts-jest, so even these +// top-level ESM imports see the mocked modules). Avoid `require()` so the file +// passes `tsc --noEmit` (the package's tsconfig ships @types/jest). +import { getCollaborators } from '../index' +import coreDefault from '@hcengineering/core' + +// ------------------------------------------------------------------ +// jest.mock for @hcengineering/core +// +// getCollaborators bails immediately unless getClassCollaborators(...) returns a +// mixin for the (parent) doc's class. That helper reads the model DB, which is +// empty in this unit harness, so we override it to return a truthy mixin whose +// `fields` drive the REAL getDocCollaborators over a single TypeAccountUuid +// attribute. Everything else (default export `core`, TxProcessor, ...) stays +// real so the REAL getAddCollaboratTxes builds genuine Collaborator txes. +// +// jest.requireActual('@hcengineering/core') cannot be spread ({...actual}) in +// ts-jest (eager getters trip a circular-dependency crash in the compiled lib); +// delegate through a Proxy and override ONLY the one helper. +// ------------------------------------------------------------------ +jest.mock('@hcengineering/core', () => { + const actual = jest.requireActual('@hcengineering/core') + const mockGetClassCollaborators = jest.fn(() => ({ allFields: false, fields: ['user'] })) + return new Proxy(actual, { + get (target: any, prop: string) { + if (prop === 'getClassCollaborators') return mockGetClassCollaborators + return target[prop] + } + }) +}) + +// server-contact pulls a heavy dependency graph; getValueCollaborators only +// touches it on Ref/PersonId paths, which the TypeAccountUuid fixture avoids. +jest.mock('@hcengineering/server-contact', () => ({ + getAccountBySocialId: jest.fn(async () => undefined), + getEmployeesBySocialIds: jest.fn(async () => ({})) +})) + +// ------------------------------------------------------------------ +// Fixtures. +// +// PARENT is the doc loaded via tx.attachedTo/attachedToClass in the real caller +// (createCollaboratorNotifications). Its default collaborator resolves to +// 'parent-only-acc' via the TypeAccountUuid `user` field. +// +// CHILD_TX is the create-tx of the attached item (e.g. a comment). It carries a +// DIFFERENT default collaborator ('child-only-acc') and a DIFFERENT space; the +// bug seeded the parent's collaborators onto THIS record. +// ------------------------------------------------------------------ +const PARENT: Doc & { user: string } = { + _id: 'parent-doc' as any, + _class: 'test:class:Parent' as any, + space: 'parent-space' as any, + user: 'parent-only-acc', + modifiedOn: 0, + modifiedBy: 'someone' as any +} + +const CHILD_ID = 'child-item' +const CHILD_SPACE = 'child-space' +// The child's OWN default collaborator — deliberately distinct from the parent's. +// It must never surface here (getCollaborators seeds the parent). +const CHILD_OWN_ACC = 'child-only-acc' + +// Typed `any` on purpose: the real caller passes a TxCUD, but the create-tx +// of an attached item also carries `attributes` (TxCreateDoc), which we set so +// the fixture models a genuine child create-tx. +const CHILD_TX: any = { + _id: 'tx-1', + _class: coreDefault.class.TxCreateDoc, + objectId: CHILD_ID, + objectClass: 'test:class:Child', + objectSpace: CHILD_SPACE, + space: 'tx-space', + modifiedOn: 0, + modifiedBy: 'author', + attachedTo: PARENT._id, + attachedToClass: PARENT._class, + attributes: { user: CHILD_OWN_ACC } +} + +function makeControl (): any { + const ctx: any = { + with: async (_name: string, _params: any, fn: any) => fn(ctx), + error: () => {}, + contextData: {} + } + return { + hierarchy: { + // getDocCollaborators -> getKeyCollaborators -> findAttribute; a + // TypeAccountUuid attr makes getValueCollaborators return [value] directly. + findAttribute: (_cls: string, field: string) => + field === 'user' ? { type: { _class: coreDefault.class.TypeAccountUuid } } : undefined + }, + modelDb: {}, + txFactory: { + createTxCreateDoc: (_class: string, space: string, attributes: any) => ({ + _class: coreDefault.class.TxCreateDoc, + objectClass: _class, + objectSpace: space, + attributes + }) + }, + findAll: jest.fn(async (_ctx: any, _class: string, _query: any) => { + // No pre-existing Collaborator rows for the parent -> the seed branch fires. + if (_class === coreDefault.class.Collaborator) return [] + return [] + }), + ctx + } +} + +function collaboratorSeeds (res: Tx[]): Array> { + return res.filter( + (t): t is TxCreateDoc => + t._class === coreDefault.class.TxCreateDoc && (t as any).objectClass === coreDefault.class.Collaborator + ) +} + +describe('getCollaborators — seed default collaborators onto the parent doc', () => { + test('seeds the parent doc as target, never the attached child tx', async () => { + const control = makeControl() + const res: Tx[] = [] + + const recipients = await getCollaborators(control.ctx, PARENT, control, CHILD_TX, res) + + // (5) The returned collaborator list is independent of the seed target and + // remains identical before and after the fix. + expect(recipients).toEqual(['parent-only-acc']) + + // (1) parent default ('parent-only-acc') differs from child default + // ('child-only-acc'); (2) parent space differs from child space. + expect(PARENT.user).not.toBe(CHILD_OWN_ACC) + expect(PARENT.space).not.toBe(CHILD_SPACE) + + const seeds = collaboratorSeeds(res) + expect(seeds).toHaveLength(1) + const seed = seeds[0] + + // (3) The seed lands on the PARENT doc. + expect(seed.attributes.attachedTo).toBe(PARENT._id) + expect(seed.attributes.attachedToClass).toBe(PARENT._class) + expect(seed.objectSpace).toBe(PARENT.space) + expect(seed.attributes.collaborator).toBe('parent-only-acc') + + // (4) The parent-only collaborator is NOT attached to the child item. + const onChild = seeds.filter((s) => s.attributes.attachedTo === CHILD_ID) + expect(onChild).toHaveLength(0) + expect(seeds.some((s) => s.objectSpace === CHILD_SPACE)).toBe(false) + }) +}) diff --git a/server-plugins/notification-resources/src/index.ts b/server-plugins/notification-resources/src/index.ts index 4a29f177819..9f7f272f6dd 100644 --- a/server-plugins/notification-resources/src/index.ts +++ b/server-plugins/notification-resources/src/index.ts @@ -1538,7 +1538,9 @@ export async function getCollaborators ( } else { const collaborators = await getDocCollaborators(ctx, doc, mixin, control) - res.push(...getAddCollaboratTxes(tx.objectId, tx.objectClass, tx.objectSpace, control, collaborators)) + // The defaults were resolved for `doc`; seed that document rather than + // the originating transaction for its attached child. + res.push(...getAddCollaboratTxes(doc._id, doc._class, doc.space, control, collaborators)) return collaborators } }