From f1c19ae4a97370d1a4b2725ce86717b5d38b928d Mon Sep 17 00:00:00 2001 From: Jiann Date: Thu, 27 Aug 2026 11:26:21 +0800 Subject: [PATCH] fix(plugin-kanban): preserve dragged popup field layout --- .../__tests__/KanbanBlockModel.test.ts | 212 +++++++++++++++++- .../src/client-v2/models/KanbanBlockModel.tsx | 169 ++++++++++---- 2 files changed, 330 insertions(+), 51 deletions(-) diff --git a/packages/plugins/@nocobase/plugin-kanban/src/client-v2/__tests__/KanbanBlockModel.test.ts b/packages/plugins/@nocobase/plugin-kanban/src/client-v2/__tests__/KanbanBlockModel.test.ts index a09255855f1..7c296db6ead 100644 --- a/packages/plugins/@nocobase/plugin-kanban/src/client-v2/__tests__/KanbanBlockModel.test.ts +++ b/packages/plugins/@nocobase/plugin-kanban/src/client-v2/__tests__/KanbanBlockModel.test.ts @@ -13,7 +13,7 @@ import { createKanbanQuickCreatePopupTemplateShadowCtx, getKanbanQuickCreatePopupTemplateComponent, } from '../models/KanbanQuickCreatePopupTemplateSelect'; -import { FlowEngine } from '@nocobase/flow-engine'; +import { FlowEngine, type FlowModel } from '@nocobase/flow-engine'; describe('KanbanBlockModel.filterCollection', () => { test('defaults dragging to disabled for newly created blocks', () => { @@ -388,8 +388,11 @@ describe('KanbanBlockModel.filterCollection', () => { }); }); - test('persists hidden popup actions only when kanban popup settings are saved', async () => { - const save = vi.fn(); + test('does not overwrite a persisted card popup tree when the popup is reopened', async () => { + let persistedLayout = ['dragged-layout']; + const save = vi.fn(() => { + persistedLayout = ['stale-popup-tree']; + }); const saveStepParams = vi.fn(); const action = { uid: 'kanban-block-card-view-action', @@ -426,8 +429,163 @@ describe('KanbanBlockModel.filterCollection', () => { await model.ensureCardViewAction({ persist: true }); - expect(save).toHaveBeenCalledTimes(1); + expect(save).not.toHaveBeenCalled(); expect(saveStepParams).toHaveBeenCalledTimes(1); + expect(persistedLayout).toEqual(['dragged-layout']); + }); + + test('serializes popup synchronization and persists a new host once across concurrent opens', async () => { + let releaseFirstSync: (() => void) | undefined; + const firstSyncFinished = new Promise((resolve) => { + releaseFirstSync = resolve; + }); + let releaseSave: (() => void) | undefined; + const saveFinished = new Promise((resolve) => { + releaseSave = resolve; + }); + const action = { + uid: 'kanban-block-card-view-action', + getStepParams: vi.fn(() => ({})), + setStepParams: vi.fn(), + save: vi.fn(() => saveFinished), + saveStepParams: vi.fn(), + }; + const model = Object.create(KanbanBlockModel.prototype) as KanbanBlockModel; + Object.defineProperty(model, 'uid', { + value: 'kanban-block', + configurable: true, + }); + Object.defineProperty(model, 'subModels', { + value: {}, + configurable: true, + }); + Object.defineProperty(model, 'flowEngine', { + value: { + loadModel: vi.fn().mockResolvedValue(null), + createModel: vi.fn(() => action), + }, + configurable: true, + }); + Object.defineProperty(model, 'context', { + value: { + flowSettingsEnabled: true, + }, + configurable: true, + }); + Object.defineProperty(model, 'collection', { + value: { + name: 'tasks', + dataSourceKey: 'main', + }, + configurable: true, + }); + Object.defineProperty(model, 'props', { + value: {}, + writable: true, + configurable: true, + }); + model.setSubModel = vi.fn((key: string) => { + if (key !== 'cardViewAction') { + throw new Error(`Unexpected popup action key: ${key}`); + } + model.subModels.cardViewAction = action; + return action as unknown as FlowModel; + }) as unknown as typeof model.setSubModel; + model.syncCardViewAction = vi + .fn() + .mockImplementationOnce(() => firstSyncFinished) + .mockResolvedValue(undefined); + + const runtimeOpen = model.ensureCardViewAction(); + await vi.waitFor(() => expect(model.syncCardViewAction).toHaveBeenCalledTimes(1)); + const firstConfigOpen = model.ensureCardViewAction({ persist: true }); + const secondConfigOpen = model.ensureCardViewAction({ persist: true }); + await Promise.resolve(); + expect(action.save).not.toHaveBeenCalled(); + + if (!releaseFirstSync) { + throw new Error('Expected runtime popup synchronization to start'); + } + releaseFirstSync(); + await vi.waitFor(() => expect(action.save).toHaveBeenCalledTimes(1)); + if (!releaseSave) { + throw new Error('Expected the popup action save to start'); + } + releaseSave(); + await Promise.all([runtimeOpen, firstConfigOpen, secondConfigOpen]); + + expect(action.save).toHaveBeenCalledTimes(1); + expect(model.syncCardViewAction).toHaveBeenCalledTimes(3); + expect(model.flowEngine.loadModel).toHaveBeenCalledTimes(1); + expect(model.flowEngine.createModel).toHaveBeenCalledTimes(1); + expect(model.setSubModel).toHaveBeenCalledTimes(1); + }); + + test('does not replace a persisted popup action when loading it fails', async () => { + const loadError = new Error('popup action load failed'); + const model = Object.create(KanbanBlockModel.prototype) as KanbanBlockModel; + Object.defineProperty(model, 'uid', { + value: 'kanban-block', + configurable: true, + }); + Object.defineProperty(model, 'subModels', { + value: {}, + configurable: true, + }); + Object.defineProperty(model, 'flowEngine', { + value: { + loadModel: vi.fn().mockRejectedValue(loadError), + createModel: vi.fn(), + }, + configurable: true, + }); + model.setSubModel = vi.fn() as unknown as typeof model.setSubModel; + + await expect(model.ensureCardViewAction({ persist: true })).rejects.toBe(loadError); + + expect(model.flowEngine.createModel).not.toHaveBeenCalled(); + expect(model.setSubModel).not.toHaveBeenCalled(); + }); + + test('persists a transient popup action after it is hydrated into a new model instance', async () => { + const originalAction = { + uid: 'transient-card-popup', + save: vi.fn(), + }; + const hydratedAction = { + uid: originalAction.uid, + save: vi.fn(), + }; + const model = Object.create(KanbanBlockModel.prototype) as KanbanBlockModel; + Object.defineProperty(model, 'uid', { value: 'kanban-block', configurable: true }); + Object.defineProperty(model, 'subModels', { value: {}, configurable: true }); + Object.defineProperty(model, 'flowEngine', { + value: { + loadModel: vi.fn().mockResolvedValue(null), + createModel: vi.fn(() => originalAction), + }, + configurable: true, + }); + Object.defineProperty(model, 'context', { + value: { flowSettingsEnabled: false }, + configurable: true, + }); + model.setSubModel = vi.fn((_key: string, action: FlowModel) => { + model.subModels.cardViewAction = action; + return action; + }) as typeof model.setSubModel; + model.syncCardViewAction = vi.fn().mockResolvedValue(undefined); + + await model.ensureCardViewAction(); + model.subModels.cardViewAction = hydratedAction; + Object.defineProperty(model, 'context', { + value: { flowSettingsEnabled: true }, + configurable: true, + }); + + await model.ensureCardViewAction({ persist: true }); + + expect(hydratedAction.save).toHaveBeenCalledTimes(1); }); test('loads persisted kanban popup actions before creating hidden actions', async () => { @@ -480,7 +638,7 @@ describe('KanbanBlockModel.filterCollection', () => { expect(model.subModels.quickCreateAction).toBe(loadedAction); }); - test('keeps legacy kanban popup action uid usable when popup settings are saved', async () => { + test('keeps legacy kanban popup action uid usable without resaving its popup tree', async () => { const destroy = vi.fn(); const action = { uid: 'kanban-block-card-view-action', @@ -516,11 +674,6 @@ describe('KanbanBlockModel.filterCollection', () => { writable: true, configurable: true, }); - model.setSubModel = vi.fn(function (this: any, key, value) { - this.subModels[key] = value; - return value; - }) as any; - await model.ensureCardViewAction(); expect(action.clone).not.toHaveBeenCalled(); @@ -533,7 +686,7 @@ describe('KanbanBlockModel.filterCollection', () => { expect(action.clone).not.toHaveBeenCalled(); expect(model.subModels.cardViewAction).toBe(action); - expect(action.save).toHaveBeenCalledTimes(1); + expect(action.save).not.toHaveBeenCalled(); expect(action.saveStepParams).toHaveBeenCalledTimes(1); expect(destroy).not.toHaveBeenCalled(); }); @@ -2314,6 +2467,24 @@ describe('KanbanBlockModel.filterCollection', () => { ); }); + test('quick create falls back to an empty popup when loading the popup action fails', async () => { + const openEmptyPopupShell = vi.fn().mockResolvedValue(true); + + await KanbanBlockModel.prototype.openQuickCreate.call( + { + ensureQuickCreateAction: vi.fn().mockRejectedValue(new Error('load failed')), + getQuickCreateEnabled: () => true, + getPopupMode: () => 'drawer', + getPopupSize: () => 'medium', + translate: (value: string) => value, + openEmptyPopupShell, + }, + { value: 'todo' }, + ); + + expect(openEmptyPopupShell).toHaveBeenCalledWith({ mode: 'drawer', size: 'medium', title: 'Add new' }); + }); + test('card click falls back to an empty popup shell when the popup action open fails', async () => { const open = vi.fn().mockResolvedValue(undefined); const dispatchEvent = vi.fn().mockRejectedValue(new Error('open failed')); @@ -2389,6 +2560,25 @@ describe('KanbanBlockModel.filterCollection', () => { expect(dispatchEvent).toHaveBeenCalledTimes(1); }); + test('card click falls back to an empty popup when loading the popup action fails', async () => { + const openEmptyPopupShell = vi.fn().mockResolvedValue(true); + + await KanbanBlockModel.prototype.openCard.call( + { + context: { flowSettingsEnabled: true }, + ensureCardViewAction: vi.fn().mockRejectedValue(new Error('load failed')), + isCardClickable: () => true, + getCardOpenMode: () => 'drawer', + getCardPopupSize: () => 'medium', + translate: (value: string) => value, + openEmptyPopupShell, + }, + { id: 1 }, + ); + + expect(openEmptyPopupShell).toHaveBeenCalledWith({ mode: 'drawer', size: 'medium', title: 'Details' }); + }); + test('page size settings use the fixed dropdown options', () => { const flow: any = (KanbanBlockModel as any).globalFlowRegistry.getFlow('kanbanSettings'); const step: any = flow?.steps?.pageSize; diff --git a/packages/plugins/@nocobase/plugin-kanban/src/client-v2/models/KanbanBlockModel.tsx b/packages/plugins/@nocobase/plugin-kanban/src/client-v2/models/KanbanBlockModel.tsx index 9c0dc17c380..dbb06091c8f 100644 --- a/packages/plugins/@nocobase/plugin-kanban/src/client-v2/models/KanbanBlockModel.tsx +++ b/packages/plugins/@nocobase/plugin-kanban/src/client-v2/models/KanbanBlockModel.tsx @@ -19,6 +19,7 @@ import { FlowModelRenderer, FlowSettingsButton, MultiRecordResource, + type FlowModel, } from '@nocobase/flow-engine'; import { InputNumber, Space, Switch } from 'antd'; import React from 'react'; @@ -88,6 +89,12 @@ type KanbanPopupActionOptions = { persist?: boolean; }; +type KanbanPopupActionKey = 'cardViewAction' | 'quickCreateAction'; + +const unpersistedPopupActionUids = new Set(); +const popupActionInitializationPromises = new WeakMap>>>(); +const popupActionOperationPromises = new Map>(); + const POPUP_TEMPLATE_SETTING_KEYS = [ 'uid', 'dataSourceKey', @@ -936,12 +943,8 @@ export class KanbanBlockModel extends CollectionBlockModel<{ return this.subModels?.quickCreateAction?.uid; } - async loadPopupAction(actionKey: 'cardViewAction' | 'quickCreateAction') { - try { - return await this.flowEngine.loadModel({ parentId: this.uid, subKey: actionKey }); - } catch (error) { - return null; - } + async loadPopupAction(actionKey: KanbanPopupActionKey) { + return await this.flowEngine.loadModel({ parentId: this.uid, subKey: actionKey }); } async syncPopupAction( @@ -1035,25 +1038,97 @@ export class KanbanBlockModel extends CollectionBlockModel<{ ); } - async ensureCardViewAction(options: KanbanPopupActionOptions = {}) { - let action = this.subModels?.cardViewAction as any; - if (!action) { - const loadedAction = await this.loadPopupAction('cardViewAction'); - if (loadedAction) { - this.setSubModel('cardViewAction', loadedAction); - } else { - this.setSubModel('cardViewAction', createKanbanCardViewActionOptions()); + private async initializePopupAction(actionKey: KanbanPopupActionKey, modelOptions: { uid?: string; use: string }) { + const loadedAction = await this.loadPopupAction(actionKey); + if (this.subModels?.[actionKey]) { + return; + } + if (loadedAction) { + this.setSubModel(actionKey, loadedAction); + return; + } + const createdAction = this.flowEngine.createModel({ + ...modelOptions, + parentId: this.uid, + subKey: actionKey, + subType: 'object', + }); + unpersistedPopupActionUids.add(createdAction.uid); + this.setSubModel(actionKey, createdAction); + } + + private async getOrInitializePopupAction( + actionKey: KanbanPopupActionKey, + modelOptions: { uid?: string; use: string }, + ): Promise { + const currentAction = this.subModels?.[actionKey] as FlowModel | undefined; + if (currentAction) { + return currentAction; + } + + let initializationState = popupActionInitializationPromises.get(this); + if (!initializationState) { + initializationState = {}; + popupActionInitializationPromises.set(this, initializationState); + } + let initializationPromise = initializationState[actionKey]; + if (!initializationPromise) { + initializationPromise = this.initializePopupAction(actionKey, modelOptions); + initializationState[actionKey] = initializationPromise; + } + try { + await initializationPromise; + } finally { + if (initializationState[actionKey] === initializationPromise) { + delete initializationState[actionKey]; } - action = this.subModels?.cardViewAction as any; + } + return this.subModels?.[actionKey] as FlowModel | undefined; + } + + private async persistNewPopupAction( + action: FlowModel | undefined, + options: KanbanPopupActionOptions, + ): Promise { + if ( + !options.persist || + !this.context.flowSettingsEnabled || + !action || + !unpersistedPopupActionUids.has(action.uid) + ) { + return false; } - if (options.persist && this.context.flowSettingsEnabled && action?.save) { - await action.save(); + await action.save(); + return true; + } + + private async runPopupActionOperation(action: FlowModel, operation: () => Promise): Promise { + const previousOperation = popupActionOperationPromises.get(action.uid) || Promise.resolve(); + const operationPromise = previousOperation.catch(() => undefined).then(operation); + popupActionOperationPromises.set(action.uid, operationPromise); + try { + return await operationPromise; + } finally { + if (popupActionOperationPromises.get(action.uid) === operationPromise) { + popupActionOperationPromises.delete(action.uid); + } } + } - await this.syncCardViewAction(action, options); - - return action; + async ensureCardViewAction(options: KanbanPopupActionOptions = {}) { + const action = await this.getOrInitializePopupAction('cardViewAction', createKanbanCardViewActionOptions()); + if (!action) { + return action; + } + return await this.runPopupActionOperation(action, async () => { + const didPersist = await this.persistNewPopupAction(action, options); + await this.syncCardViewAction(action, options); + if (didPersist) { + unpersistedPopupActionUids.delete(action.uid); + } + return action; + }); } getQuickCreateAction() { @@ -1061,24 +1136,18 @@ export class KanbanBlockModel extends CollectionBlockModel<{ } async ensureQuickCreateAction(options: KanbanPopupActionOptions = {}) { - let action = this.subModels?.quickCreateAction as any; + const action = await this.getOrInitializePopupAction('quickCreateAction', createKanbanQuickCreateActionOptions()); if (!action) { - const loadedAction = await this.loadPopupAction('quickCreateAction'); - if (loadedAction) { - this.setSubModel('quickCreateAction', loadedAction); - } else { - this.setSubModel('quickCreateAction', createKanbanQuickCreateActionOptions()); + return action; + } + return await this.runPopupActionOperation(action, async () => { + const didPersist = await this.persistNewPopupAction(action, options); + await this.syncQuickCreateAction(action, options); + if (didPersist) { + unpersistedPopupActionUids.delete(action.uid); } - action = this.subModels?.quickCreateAction as any; - } - - if (options.persist && this.context.flowSettingsEnabled && action?.save) { - await action.save(); - } - - await this.syncQuickCreateAction(action, options); - - return action; + return action; + }); } async syncQuickCreateAction(action: any, options: KanbanPopupActionOptions = {}) { @@ -1145,7 +1214,17 @@ export class KanbanBlockModel extends CollectionBlockModel<{ return; } - const action = await this.ensureQuickCreateAction(); + let action: FlowModel | undefined; + try { + action = await this.ensureQuickCreateAction(); + } catch (error) { + await this.openEmptyPopupShell({ + mode: this.getPopupMode(), + size: this.getPopupSize(), + title: this.translate('Add new', { ns: 'kanban' }), + }); + return; + } if (!action?.uid) { await this.openEmptyPopupShell({ mode: this.getPopupMode(), @@ -1183,9 +1262,19 @@ export class KanbanBlockModel extends CollectionBlockModel<{ // The drawer content is stored under the hidden card action. Persist that // host before users add blocks in configuration mode so the content can be // loaded again after the drawer is destroyed. - const action = this.context?.flowSettingsEnabled - ? await this.ensureCardViewAction({ persist: true }) - : await this.ensureCardViewAction(); + let action: FlowModel | undefined; + try { + action = this.context?.flowSettingsEnabled + ? await this.ensureCardViewAction({ persist: true }) + : await this.ensureCardViewAction(); + } catch (error) { + await this.openEmptyPopupShell({ + mode: this.getCardOpenMode(), + size: this.getCardPopupSize(), + title: this.translate('Details'), + }); + return; + } if (!action || !record) { await this.openEmptyPopupShell({ mode: this.getCardOpenMode(),