diff --git a/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.spec.tsx b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.spec.tsx new file mode 100644 index 0000000..7034403 --- /dev/null +++ b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.spec.tsx @@ -0,0 +1,193 @@ +import { + act, + type CSSProperties, + type ReactElement, + type ReactNode, +} from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { + ActivityLogRecord, + ApprovalInstanceRecord, +} from '@rytass/bpm-core-client/workflow'; +import { InstanceHistorySection } from './InstanceHistorySection'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +// `@mezzanine-ui/core/stepper` and `@mezzanine-ui/react` are ESM-only, which +// the jest environment cannot load; the rest of this lib's component specs +// stub them the same way. The stubs keep `style` and `className` so what this +// spec asserts — the inline styles the section hands to Typography — still +// reaches the DOM. +jest.mock('@mezzanine-ui/core/stepper', () => ({ + stepClasses: new Proxy( + {}, + { get: (_target, key): string => `mzn-step-${String(key)}` }, + ), +})); + +jest.mock('@mezzanine-ui/react', () => { + function MockTypography( + props: Readonly>, + ): ReactElement { + return ( + + {props.children as ReactNode} + + ); + } + + function MockStepper(props: Readonly>): ReactElement { + return
{props.children as ReactNode}
; + } + + function MockTooltip(props: Readonly>): ReactElement { + return {props.children as ReactNode}; + } + + return { + Stepper: MockStepper, + Tooltip: MockTooltip, + Typography: MockTypography, + }; +}); + +const ERROR_COLOR = 'var(--mzn-color-text-error)'; + +function createDecisionActivityLog( + id: string, + action: string, + comment: string, +): ActivityLogRecord { + return { + actorMemberId: 'member-approver', + createdAt: '2026-09-07T08:44:45.000Z', + eventType: 'TASK_DECIDED', + id, + instanceId: 'instance-1', + nodeId: 'userTask_1', + payloadJson: JSON.stringify({ action, comment }), + taskId: `task-${id}`, + } as unknown as ActivityLogRecord; +} + +function renderHistory( + container: HTMLElement, + activityLogs: readonly ActivityLogRecord[], +): Root { + const root = createRoot(container); + + act((): void => { + root.render( + ( + + ) as ReactElement, + ); + }); + + return root; +} + +/** + * The deepest element whose whole text is `text`. Reading the deepest one + * matters: a comment line is a `` label followed by a text node, so an + * ancestor could match the same string while carrying different styles. + */ +function findDeepestByExactText( + container: HTMLElement, + text: string, +): HTMLElement | null { + const matches = Array.from( + container.querySelectorAll('*'), + ).filter((element): boolean => element.textContent === text); + + return matches.length > 0 ? matches[matches.length - 1] : null; +} + +describe('InstanceHistorySection comment rendering', () => { + let container: HTMLElement; + let root: Root | null = null; + + beforeEach((): void => { + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach((): void => { + if (root) { + act((): void => { + root?.unmount(); + }); + + root = null; + } + + container.remove(); + }); + + it('separates a comment label from its text with a full-width colon', () => { + root = renderHistory(container, [ + createDecisionActivityLog('a1', 'APPROVED', '核准,請依核定條件執行'), + ]); + + // Without the separator the timeline reads "同意說明核准,請依核定條件執行", + // which is both unreadable and inconsistent with every other part of the + // step, all of which are rendered as "X:Y". + expect( + findDeepestByExactText(container, '同意說明:核准,請依核定條件執行'), + ).not.toBeNull(); + }); + + it('keeps a rejection reason in the error colour', () => { + root = renderHistory(container, [ + createDecisionActivityLog('r1', 'REJECTED', '資料不足,請補件'), + ]); + + const rejectionReason = findDeepestByExactText( + container, + '拒絕原因:資料不足,請補件', + ); + + expect(rejectionReason).not.toBeNull(); + // The colour is the only cue that separates a rejection reason from an + // approval note once both are rendered as their own comment line. + expect(rejectionReason?.style.color).toBe(ERROR_COLOR); + expect(rejectionReason?.style.borderLeftColor).toBe(ERROR_COLOR); + }); + + it('does not grey out the label of a rejection reason', () => { + root = renderHistory(container, [ + createDecisionActivityLog('r2', 'REJECTED', '資料不足,請補件'), + ]); + + const label = findDeepestByExactText(container, '拒絕原因:'); + + expect(label).not.toBeNull(); + expect(label?.style.color).toBe(''); + }); + + it('leaves an approval note in the default text colour', () => { + root = renderHistory(container, [ + createDecisionActivityLog('a2', 'APPROVED', '核准,請依核定條件執行'), + ]); + + const approvalNote = findDeepestByExactText( + container, + '同意說明:核准,請依核定條件執行', + ); + + expect(approvalNote?.style.color).toBe(''); + }); +}); diff --git a/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.tsx b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.tsx index 511677d..28a6da2 100644 --- a/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.tsx +++ b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceHistorySection.tsx @@ -40,7 +40,31 @@ const HISTORY_MEMBER_NAME_STYLE: CSSProperties = { textUnderlineOffset: 3, }; -const HISTORY_DANGER_TEXT_STYLE: CSSProperties = { +// An approval comment is the one part of a timeline entry a person wrote, so +// it gets its own line and a readable size instead of being appended to the +// metadata run. +// The border is written as longhands so the danger tone can override only its +// colour; mixing `borderLeft` with a `borderLeftColor` override in one style +// object makes the result depend on key order and warns in React dev mode. +const HISTORY_COMMENT_STYLE: CSSProperties = { + borderLeftColor: 'var(--mzn-color-border)', + borderLeftStyle: 'solid', + borderLeftWidth: 2, + marginTop: 4, + paddingLeft: 8, + whiteSpace: 'pre-wrap', +}; + +const HISTORY_COMMENT_LABEL_STYLE: CSSProperties = { + color: 'var(--mzn-color-text-secondary)', +}; + +// A rejection reason has to read as a rejection, so it carries the error colour +// itself — label included, which is why the label drops its own colour in this +// tone. +const HISTORY_COMMENT_DANGER_STYLE: CSSProperties = { + ...HISTORY_COMMENT_STYLE, + borderLeftColor: 'var(--mzn-color-text-error)', color: 'var(--mzn-color-text-error)', }; @@ -78,6 +102,14 @@ const ActivityHistoryStep = forwardRef< ref, ): ReactElement { const displayStatus = forcePending ? 'pending' : status; + const inlineDescriptionParts = descriptionParts.filter( + (part): part is Exclude => + part.type !== 'comment', + ); + const commentDescriptionParts = descriptionParts.filter( + (part): part is Extract => + part.type === 'comment', + ); return (
- {descriptionParts.length > 0 ? ( + {inlineDescriptionParts.length > 0 ? ( - {descriptionParts.map((part, partIndex) => ( + {inlineDescriptionParts.map((part, partIndex) => ( {partIndex > 0 ? ' · ' : null} {renderActivityDescriptionPart(part)} @@ -127,22 +159,40 @@ const ActivityHistoryStep = forwardRef< ))} ) : null} + {commentDescriptionParts.map((part, partIndex) => ( + + + {part.label}: + + {part.text} + + ))}
); }); +// Comment parts are rendered on their own line by the caller, so they never +// reach this inline renderer. function renderActivityDescriptionPart( - part: ActivityStepDescriptionPart, + part: Exclude, ): ReactElement | string { if (part.type === 'text') { return part.text; } - if (part.type === 'dangerText') { - return {part.text}; - } - if (!part.email) { return `${part.prefix}:${part.label}`; } diff --git a/libs/bpm-core-react/src/views/instances/detail/sections/InstanceTasksSection.tsx b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceTasksSection.tsx index 20cf112..0bf2317 100644 --- a/libs/bpm-core-react/src/views/instances/detail/sections/InstanceTasksSection.tsx +++ b/libs/bpm-core-react/src/views/instances/detail/sections/InstanceTasksSection.tsx @@ -43,6 +43,8 @@ import { readErrorMessage, readMemberDisplayText, readMemberOption, + isPresentMemberOption, + readAdhocTargetDraft, readMemberOptionFromValue, readNodeDisplayLabel, readReturnTargetOptions, @@ -228,7 +230,9 @@ export const InstanceTasksSection = forwardRef< // Ad-hoc modal (countersign / pre-approval / stage & completion notify) const [adhocComment, setAdhocComment] = useState(''); - const [adhocMember, setAdhocMember] = useState(null); + const [adhocMembers, setAdhocMembers] = useState( + [], + ); const [adhocMemberLoading, setAdhocMemberLoading] = useState(false); const [adhocMemberOptions, setAdhocMemberOptions] = useState< readonly MemberOption[] @@ -665,7 +669,7 @@ export const InstanceTasksSection = forwardRef< function resetAdhocModalState(): void { setAdhocComment(''); - setAdhocMember(null); + setAdhocMembers([]); setAdhocOnReject('REJECT_INSTANCE'); setAdhocTargetKind('MEMBER'); setAdhocWebhookUrl(''); @@ -709,25 +713,19 @@ export const InstanceTasksSection = forwardRef< const isNotifyMode = adhocMode === 'STAGE_NOTIFY' || adhocMode === 'COMPLETION_NOTIFY'; const useWebhookTarget = isNotifyMode && adhocTargetKind === 'WEBHOOK'; - const trimmedWebhookUrl = adhocWebhookUrl.trim(); - const selectedMember = adhocMember; - - if (useWebhookTarget && !trimmedWebhookUrl) { - setError('請輸入 Webhook URL'); - - return; - } + const targetDraft = readAdhocTargetDraft({ + memberIds: adhocMembers.map((member) => member.id), + useWebhookTarget, + webhookUrl: adhocWebhookUrl, + }); - if (!useWebhookTarget && !selectedMember) { - setError('請選擇對象成員'); + if (!targetDraft.valid) { + setError(targetDraft.error); return; } - const target = - useWebhookTarget || !selectedMember - ? { kind: 'WEBHOOK' as const, webhookUrl: trimmedWebhookUrl } - : { kind: 'MEMBER' as const, memberIds: [selectedMember.id] }; + const { target } = targetDraft; const trimmedAdhocComment = adhocComment.trim() || null; setAdhocSubmitting(true); @@ -816,7 +814,7 @@ export const InstanceTasksSection = forwardRef< const adhocConfirmDisabled = isAdhocNotifyMode && adhocTargetKind === 'WEBHOOK' ? !adhocWebhookUrl.trim() - : !adhocMember; + : adhocMembers.length === 0; const selectedAdhocOnRejectOption = ADHOC_ON_REJECT_OPTIONS.find((option) => option.id === adhocOnReject) ?? ADHOC_ON_REJECT_OPTIONS[0]; @@ -1160,25 +1158,26 @@ export const InstanceTasksSection = forwardRef< }} loading={adhocMemberLoading} loadingText="搜尋成員中..." - mode="single" - onChange={(option): void => - setAdhocMember(readMemberOptionFromValue(option)) - } - onSearch={handleSearchAdhocMembers} - onSearchTextChange={(searchText): void => - setAdhocMember( - readUniqueMemberOption(searchText, adhocMemberOptions), + menuMaxHeight={320} + mode="multiple" + onChange={(options): void => + setAdhocMembers( + options + .map(readMemberOptionFromValue) + .filter(isPresentMemberOption), ) } + onSearch={handleSearchAdhocMembers} onVisibilityChange={(open): void => { if (open) { void handleSearchAdhocMembers(''); } }} options={[...adhocMemberOptions]} - placeholder="搜尋姓名或信箱" + overflowStrategy="wrap" + placeholder="搜尋姓名或信箱,可加入多位" searchDebounceTime={300} - value={adhocMember} + value={[...adhocMembers]} /> )} diff --git a/libs/bpm-core-react/src/views/instances/detail/sections/shared.spec.ts b/libs/bpm-core-react/src/views/instances/detail/sections/shared.spec.ts new file mode 100644 index 0000000..18f8229 --- /dev/null +++ b/libs/bpm-core-react/src/views/instances/detail/sections/shared.spec.ts @@ -0,0 +1,147 @@ +import type { ActivityLogRecord } from '@rytass/bpm-core-client/workflow'; + +import { readActivityDetailParts, readAdhocTargetDraft } from './shared'; + +function createDecisionActivityLog(): ActivityLogRecord { + return { + actorMemberId: 'member-approver', + createdAt: '2026-09-07T08:44:45.000Z', + eventType: 'TASK_DECIDED', + id: 'activity-1', + instanceId: 'instance-1', + nodeId: 'userTask_1', + payloadJson: '{}', + taskId: 'task-1', + } as unknown as ActivityLogRecord; +} + +function readParts( + payload: Readonly>, +): readonly ReturnType[number][] { + return readActivityDetailParts( + createDecisionActivityLog(), + payload, + null, + new Map(), + new Map(), + null, + new Map(), + ); +} + +describe('readActivityDetailParts', () => { + it('keeps an approval comment out of the inline metadata run', () => { + const parts = readParts({ action: 'APPROVED', comment: '請注意交期' }); + + // The comment must not be folded into a `text` part: those are joined with + // " · " alongside the node, actor, timestamp and signature hash, which is + // what made a written comment unreadable in the timeline. + expect(parts).toContainEqual({ + label: '同意說明', + text: '請注意交期', + tone: 'neutral', + type: 'comment', + }); + expect( + parts.filter((part) => part.type === 'text' && part.text.includes('請注意交期')), + ).toHaveLength(0); + }); + + it('marks a rejection reason as a danger comment', () => { + const parts = readParts({ action: 'REJECTED', comment: '金額有誤' }); + + expect(parts).toContainEqual({ + label: '拒絕原因', + text: '金額有誤', + tone: 'danger', + type: 'comment', + }); + }); + + it('keeps the decision label inline', () => { + const parts = readParts({ action: 'APPROVED', comment: '請注意交期' }); + + expect(parts).toContainEqual({ text: '決議:同意', type: 'text' }); + }); + + it('emits no comment part when an approval carries no comment', () => { + const parts = readParts({ action: 'APPROVED', comment: null }); + + expect(parts.filter((part) => part.type === 'comment')).toHaveLength(0); + }); + + it('still shows a placeholder for a rejection with no comment', () => { + const parts = readParts({ action: 'REJECTED', comment: null }); + + expect(parts).toContainEqual({ + label: '拒絕原因', + text: '-', + tone: 'danger', + type: 'comment', + }); + }); +}); + +describe('readAdhocTargetDraft', () => { + it('sends every selected member, not just the first', () => { + expect( + readAdhocTargetDraft({ + memberIds: ['member-a', 'member-b', 'member-c'], + useWebhookTarget: false, + webhookUrl: '', + }), + ).toEqual({ + target: { + kind: 'MEMBER', + memberIds: ['member-a', 'member-b', 'member-c'], + }, + valid: true, + }); + }); + + it('refuses an empty member selection', () => { + expect( + readAdhocTargetDraft({ + memberIds: [], + useWebhookTarget: false, + webhookUrl: '', + }), + ).toEqual({ error: '請選擇對象成員', valid: false }); + }); + + it('trims a webhook url', () => { + expect( + readAdhocTargetDraft({ + memberIds: [], + useWebhookTarget: true, + webhookUrl: ' https://example.com/hook ', + }), + ).toEqual({ + target: { kind: 'WEBHOOK', webhookUrl: 'https://example.com/hook' }, + valid: true, + }); + }); + + it('refuses a blank webhook url', () => { + expect( + readAdhocTargetDraft({ + memberIds: ['member-a'], + useWebhookTarget: true, + webhookUrl: ' ', + }), + ).toEqual({ error: '請輸入 Webhook URL', valid: false }); + }); + + it('ignores selected members once the webhook target is chosen', () => { + expect( + readAdhocTargetDraft({ + memberIds: ['member-a'], + useWebhookTarget: true, + webhookUrl: 'https://example.com/hook', + }), + ).toEqual({ + target: { kind: 'WEBHOOK', webhookUrl: 'https://example.com/hook' }, + valid: true, + }); + }); +}); diff --git a/libs/bpm-core-react/src/views/instances/detail/sections/shared.ts b/libs/bpm-core-react/src/views/instances/detail/sections/shared.ts index 9e33170..9b547ca 100644 --- a/libs/bpm-core-react/src/views/instances/detail/sections/shared.ts +++ b/libs/bpm-core-react/src/views/instances/detail/sections/shared.ts @@ -67,13 +67,23 @@ export type MemberOption = Readonly<{ export type ActivityStepDescriptionPart = | Readonly<{ text: string; type: 'text' }> - | Readonly<{ text: string; type: 'dangerText' }> | Readonly<{ email: string | null; label: string; memberId: string | null; prefix: string; type: 'member'; + }> + /** + * What an approver actually wrote. Kept apart from the `text` parts so the + * timeline can give it its own line instead of burying it mid-sentence + * between the node, the actor, the timestamp and the signature hash. + */ + | Readonly<{ + label: string; + text: string; + tone: 'danger' | 'neutral'; + type: 'comment'; }>; export interface ActivityStepRecord { @@ -368,6 +378,52 @@ export function readMemberOption(profile: MemberProfileRecord): MemberOption { }; } +/** + * The target an ad-hoc directive is sent to, or the reason it cannot be built + * yet. Kept out of the component so the "which shape do we send" decision is + * testable on its own — it is the part that changed when the member picker + * went from one selection to many. + */ +export type AdhocTargetDraft = + | Readonly<{ + target: + | Readonly<{ kind: 'MEMBER'; memberIds: readonly string[] }> + | Readonly<{ kind: 'WEBHOOK'; webhookUrl: string }>; + valid: true; + }> + | Readonly<{ error: string; valid: false }>; + +export function readAdhocTargetDraft({ + memberIds, + useWebhookTarget, + webhookUrl, +}: { + readonly memberIds: readonly string[]; + readonly useWebhookTarget: boolean; + readonly webhookUrl: string; +}): AdhocTargetDraft { + if (useWebhookTarget) { + const trimmedWebhookUrl = webhookUrl.trim(); + + return trimmedWebhookUrl + ? { + target: { kind: 'WEBHOOK', webhookUrl: trimmedWebhookUrl }, + valid: true, + } + : { error: '請輸入 Webhook URL', valid: false }; + } + + return memberIds.length > 0 + ? { target: { kind: 'MEMBER', memberIds }, valid: true } + : { error: '請選擇對象成員', valid: false }; +} + +export function isPresentMemberOption( + option: MemberOption | null, +): option is MemberOption { + return Boolean(option); +} + export function readMemberOptionFromValue(value: unknown): MemberOption | null { if (!isRecord(value)) { return null; @@ -411,10 +467,12 @@ export function readTextDescriptionPart( return isPresentText(text) ? { text, type: 'text' } : null; } -export function readDangerTextDescriptionPart( +export function readCommentDescriptionPart( + label: string, text: string | null, + tone: 'danger' | 'neutral' = 'neutral', ): ActivityStepDescriptionPart | null { - return isPresentText(text) ? { text, type: 'dangerText' } : null; + return isPresentText(text) ? { label, text, tone, type: 'comment' } : null; } export function readMemberDescriptionPart( @@ -664,13 +722,13 @@ export function readActivityDetailParts( return [ readTextDescriptionPart(decisionLabel), action === 'REJECTED' - ? readDangerTextDescriptionPart(`拒絕原因:${comment ?? '-'}`) + ? readCommentDescriptionPart('拒絕原因', comment ?? '-', 'danger') : null, - action === 'APPROVED' && comment - ? readTextDescriptionPart(`同意說明:${comment}`) + action === 'APPROVED' + ? readCommentDescriptionPart('同意說明', comment) : null, action === 'RETURNED' - ? readTextDescriptionPart(`退回說明:${comment ?? '-'}`) + ? readCommentDescriptionPart('退回說明', comment ?? '-') : null, action === 'TRANSFERRED' ? readTextDescriptionPart( @@ -681,7 +739,7 @@ export function readActivityDetailParts( ) : null, action === 'TRANSFERRED' - ? readTextDescriptionPart(`轉派說明:${comment ?? '-'}`) + ? readCommentDescriptionPart('轉派說明', comment ?? '-') : null, signature ? readTextDescriptionPart(