Merge branch 'refactor/approval-ux-eng-review-fixes' into 'main'
refactor(approval-ux): eng-review fixes — extract helpers, add tests, register i18n See merge request 2-6/2-6-4/terravault/ordinis!187
This commit is contained in:
@@ -52,6 +52,42 @@ const SCOPE_OPTIONS = (t: (k: string) => string): SelectOption[] => [
|
|||||||
|
|
||||||
type EditorTab = 'metadata' | 'schema' | 'preview' | 'events'
|
type EditorTab = 'metadata' | 'schema' | 'preview' | 'events'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide whether the editor's Save button should be blocked because the
|
||||||
|
* change has to go through the schema-draft workflow instead of inline PUT.
|
||||||
|
* Mirrors the three backend gate clauses in DictionaryDefinitionService
|
||||||
|
* exactly so the UX matches the server contract.
|
||||||
|
*
|
||||||
|
* Returns false (don't block) for create-mode, for non-approval-required
|
||||||
|
* dicts, and for pure metadata edits (display name / description / locales
|
||||||
|
* etc.) that leave schema + approval config alone.
|
||||||
|
*
|
||||||
|
* Exported for unit testability — the three-clause logic is the load-bearing
|
||||||
|
* piece, easier to verify in isolation than mounting the full editor with
|
||||||
|
* its query and mutation deps.
|
||||||
|
*/
|
||||||
|
export function computeRequiresDraftFlow({
|
||||||
|
isEdit,
|
||||||
|
initialApprovalRequired,
|
||||||
|
initialMinRole,
|
||||||
|
currentApprovalRequired,
|
||||||
|
currentMinRole,
|
||||||
|
schemaChangeCount,
|
||||||
|
}: {
|
||||||
|
isEdit: boolean
|
||||||
|
initialApprovalRequired: boolean
|
||||||
|
initialMinRole: string | null
|
||||||
|
currentApprovalRequired: boolean
|
||||||
|
currentMinRole: string | null
|
||||||
|
schemaChangeCount: number
|
||||||
|
}): boolean {
|
||||||
|
if (!isEdit || !initialApprovalRequired) return false
|
||||||
|
const schemaChanged = schemaChangeCount > 0
|
||||||
|
const approvalToggledOff = !currentApprovalRequired
|
||||||
|
const minRoleChanged = initialMinRole !== currentMinRole
|
||||||
|
return schemaChanged || approvalToggledOff || minRoleChanged
|
||||||
|
}
|
||||||
|
|
||||||
export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props) => {
|
export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props) => {
|
||||||
const { t } = useTranslation()
|
const { t } = useTranslation()
|
||||||
const createMut = useCreateDictionary()
|
const createMut = useCreateDictionary()
|
||||||
@@ -150,17 +186,17 @@ export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props
|
|||||||
// 422 approval_required, but it's much better UX to disable Save up front
|
// 422 approval_required, but it's much better UX to disable Save up front
|
||||||
// and tell the user to open a draft instead. Mirror the three backend
|
// and tell the user to open a draft instead. Mirror the three backend
|
||||||
// gate clauses exactly so the UX matches the server contract.
|
// gate clauses exactly so the UX matches the server contract.
|
||||||
const wasApprovalRequired =
|
const requiresDraftFlow = computeRequiresDraftFlow({
|
||||||
Boolean((initial as { approvalRequired?: boolean } | null)?.approvalRequired)
|
isEdit,
|
||||||
const initialMinRole =
|
initialApprovalRequired: Boolean(
|
||||||
((initial as { approvalMinRole?: string } | null)?.approvalMinRole ?? '') || null
|
(initial as { approvalRequired?: boolean } | null)?.approvalRequired,
|
||||||
const currentMinRole = approvalMinRole.trim() === '' ? null : approvalMinRole.trim()
|
),
|
||||||
const schemaChangedFromInitial = isEdit && changes.length > 0
|
initialMinRole:
|
||||||
const approvalToggledOff = isEdit && wasApprovalRequired && !approvalRequired
|
((initial as { approvalMinRole?: string } | null)?.approvalMinRole ?? '') || null,
|
||||||
const minRoleChanged = isEdit && wasApprovalRequired && initialMinRole !== currentMinRole
|
currentApprovalRequired: approvalRequired,
|
||||||
const requiresDraftFlow =
|
currentMinRole: approvalMinRole.trim() === '' ? null : approvalMinRole.trim(),
|
||||||
isEdit && wasApprovalRequired
|
schemaChangeCount: changes.length,
|
||||||
&& (schemaChangedFromInitial || approvalToggledOff || minRoleChanged)
|
})
|
||||||
|
|
||||||
const activeMutation = isEdit ? updateMut : createMut
|
const activeMutation = isEdit ? updateMut : createMut
|
||||||
const serverError = serverErrorMessage(activeMutation.error)
|
const serverError = serverErrorMessage(activeMutation.error)
|
||||||
@@ -490,9 +526,13 @@ export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props
|
|||||||
detail={initial}
|
detail={initial}
|
||||||
initialProposedSchema={schemaJson}
|
initialProposedSchema={schemaJson}
|
||||||
onCreated={() => {
|
onCreated={() => {
|
||||||
|
// Cascade close: dismiss the draft modal AND the editor. Once
|
||||||
|
// the draft is submitted, the in-memory edit in the editor is
|
||||||
|
// stale by design — the draft is now the source of truth until
|
||||||
|
// a reviewer decides. Leaving the editor open would let the
|
||||||
|
// maker silently mutate further state that can't go anywhere
|
||||||
|
// (Save still disabled until the draft resolves).
|
||||||
setDraftHandoffOpen(false)
|
setDraftHandoffOpen(false)
|
||||||
// Close the editor too — the maker's done editing live, the draft
|
|
||||||
// is now the source of truth until a reviewer decides.
|
|
||||||
onClose()
|
onClose()
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -0,0 +1,71 @@
|
|||||||
|
import { describe, expect, it } from 'vitest'
|
||||||
|
import { computeRequiresDraftFlow } from './DictionaryEditorDialog'
|
||||||
|
|
||||||
|
const base = {
|
||||||
|
isEdit: true,
|
||||||
|
initialApprovalRequired: true,
|
||||||
|
initialMinRole: 'ordinis:reviewer' as string | null,
|
||||||
|
currentApprovalRequired: true,
|
||||||
|
currentMinRole: 'ordinis:reviewer' as string | null,
|
||||||
|
schemaChangeCount: 0,
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('computeRequiresDraftFlow', () => {
|
||||||
|
it('returns false in create mode (no initial dict)', () => {
|
||||||
|
expect(computeRequiresDraftFlow({ ...base, isEdit: false })).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns false for non-approval-required dicts even with schema changes', () => {
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({
|
||||||
|
...base,
|
||||||
|
initialApprovalRequired: false,
|
||||||
|
schemaChangeCount: 5,
|
||||||
|
}),
|
||||||
|
).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns false for pure metadata edits on approval-required dict', () => {
|
||||||
|
// Editing display name / description / locales doesn't touch schema or
|
||||||
|
// approval config; backend allows inline. UX should NOT block Save here.
|
||||||
|
expect(computeRequiresDraftFlow(base)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns true when schema changed on approval-required dict', () => {
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({ ...base, schemaChangeCount: 1 }),
|
||||||
|
).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns true when approvalRequired toggled off (backdoor block)', () => {
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({ ...base, currentApprovalRequired: false }),
|
||||||
|
).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns true when approvalMinRole changed', () => {
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({ ...base, currentMinRole: 'ordinis:admin' }),
|
||||||
|
).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns true when approvalMinRole cleared', () => {
|
||||||
|
// Edge case from MR !182 audit: clearing the min-role would let any
|
||||||
|
// generic reviewer approve restricted-dict drafts. Must route through
|
||||||
|
// draft workflow.
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({ ...base, currentMinRole: null }),
|
||||||
|
).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('handles all three clauses together (worst-case schema rewrite + governance flip)', () => {
|
||||||
|
expect(
|
||||||
|
computeRequiresDraftFlow({
|
||||||
|
...base,
|
||||||
|
schemaChangeCount: 3,
|
||||||
|
currentApprovalRequired: false,
|
||||||
|
currentMinRole: null,
|
||||||
|
}),
|
||||||
|
).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -55,6 +55,34 @@ type Props = {
|
|||||||
* вернёт detail в message.</li>
|
* вернёт detail в message.</li>
|
||||||
* </ul>
|
* </ul>
|
||||||
*/
|
*/
|
||||||
|
/**
|
||||||
|
* Resolve the schema JSON to seed the editor with. Three-tier priority:
|
||||||
|
*
|
||||||
|
* 1. edit-mode draft — `editDraft.proposedSchema`
|
||||||
|
* Maker is revising their own draft after CHANGES_REQUESTED; the
|
||||||
|
* draft itself is the source of truth.
|
||||||
|
*
|
||||||
|
* 2. caller pre-fill — `initialProposedSchema` prop
|
||||||
|
* DictionaryEditorDialog's inline CTA passes in-progress edits so
|
||||||
|
* the maker doesn't retype them.
|
||||||
|
*
|
||||||
|
* 3. live HEAD — `detailSchemaJson`
|
||||||
|
* First-time draft on this dict with no pre-fill; start from the
|
||||||
|
* current published schema.
|
||||||
|
*
|
||||||
|
* Exported so the priority can be unit-tested without rendering the
|
||||||
|
* whole modal (mutations + queries make the render test setup heavy).
|
||||||
|
*/
|
||||||
|
export function pickSeedSchema(
|
||||||
|
editDraft: SchemaDraft | undefined,
|
||||||
|
initialProposedSchema: unknown,
|
||||||
|
detailSchemaJson: unknown,
|
||||||
|
): unknown {
|
||||||
|
if (editDraft) return editDraft.proposedSchema
|
||||||
|
if (initialProposedSchema !== undefined) return initialProposedSchema
|
||||||
|
return detailSchemaJson
|
||||||
|
}
|
||||||
|
|
||||||
export function CreateSchemaDraftModal({
|
export function CreateSchemaDraftModal({
|
||||||
open,
|
open,
|
||||||
onClose,
|
onClose,
|
||||||
@@ -65,17 +93,10 @@ export function CreateSchemaDraftModal({
|
|||||||
}: Props) {
|
}: Props) {
|
||||||
const { t } = useTranslation()
|
const { t } = useTranslation()
|
||||||
const isEdit = Boolean(editDraft)
|
const isEdit = Boolean(editDraft)
|
||||||
// Seed order: edit-mode draft → caller-provided initialProposedSchema →
|
|
||||||
// live HEAD. Caller pre-fill wins over live HEAD so the maker's in-progress
|
|
||||||
// edits from DictionaryEditorDialog flow through.
|
|
||||||
const initialJson = useMemo(
|
const initialJson = useMemo(
|
||||||
() =>
|
() =>
|
||||||
JSON.stringify(
|
JSON.stringify(
|
||||||
editDraft
|
pickSeedSchema(editDraft, initialProposedSchema, detail.schemaJson),
|
||||||
? editDraft.proposedSchema
|
|
||||||
: initialProposedSchema !== undefined
|
|
||||||
? initialProposedSchema
|
|
||||||
: detail.schemaJson,
|
|
||||||
null,
|
null,
|
||||||
2,
|
2,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -0,0 +1,23 @@
|
|||||||
|
import { describe, expect, it } from 'vitest'
|
||||||
|
import type { SchemaDraft } from '@/api/client'
|
||||||
|
import { pickSeedSchema } from './CreateSchemaDraftModal'
|
||||||
|
|
||||||
|
const draft = (proposed: unknown): SchemaDraft =>
|
||||||
|
({ proposedSchema: proposed } as unknown as SchemaDraft)
|
||||||
|
|
||||||
|
describe('pickSeedSchema', () => {
|
||||||
|
it('edit-mode draft wins over pre-fill and live HEAD', () => {
|
||||||
|
const seed = pickSeedSchema(draft({ type: 'edit' }), { type: 'pre-fill' }, { type: 'head' })
|
||||||
|
expect(seed).toEqual({ type: 'edit' })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('caller pre-fill wins over live HEAD when no edit-mode draft', () => {
|
||||||
|
const seed = pickSeedSchema(undefined, { type: 'pre-fill' }, { type: 'head' })
|
||||||
|
expect(seed).toEqual({ type: 'pre-fill' })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('falls back to live HEAD when neither edit draft nor pre-fill provided', () => {
|
||||||
|
const seed = pickSeedSchema(undefined, undefined, { type: 'head' })
|
||||||
|
expect(seed).toEqual({ type: 'head' })
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -128,6 +128,9 @@ i18n
|
|||||||
'reviews.drawer.commentPlaceholder': 'Для approve опционально, для reject обязательно',
|
'reviews.drawer.commentPlaceholder': 'Для approve опционально, для reject обязательно',
|
||||||
'schema.approvalRequired.label': 'Требовать approval (maker-checker)',
|
'schema.approvalRequired.label': 'Требовать approval (maker-checker)',
|
||||||
'schema.approvalRequired.hint': 'Approval Workflow v2. Все CRUD операции пойдут через draft → review → approve. Защита от operator error на критичных справочниках. Существующие маршруты (POST/PUT/DELETE) вернут 409 draft_required с pointer на /drafts API.',
|
'schema.approvalRequired.hint': 'Approval Workflow v2. Все CRUD операции пойдут через draft → review → approve. Защита от operator error на критичных справочниках. Существующие маршруты (POST/PUT/DELETE) вернут 409 draft_required с pointer на /drafts API.',
|
||||||
|
'schema.approvalRequired.editorBlockTitle': 'Изменения требуют ревью',
|
||||||
|
'schema.approvalRequired.editorBlock': 'Этот словарь требует maker-checker review. Откройте черновик — ваши текущие изменения будут предзаполнены, вам нужно только указать причину и отправить на согласование.',
|
||||||
|
'schema.approvalRequired.openDraftCta': 'Создать черновик с моими изменениями',
|
||||||
'schema.approvalMinRole.label': 'Минимальная роль ревьюера (опционально)',
|
'schema.approvalMinRole.label': 'Минимальная роль ревьюера (опционально)',
|
||||||
'schema.approvalMinRole.hint': 'Например ordinis:internal или ordinis:restricted. Если задано — backend требует чтобы ревьюер имел эту роль (поверх стандартных ordinis:record:reviewer / ordinis:schema:reviewer) для approve / reject / publish. Пусто = достаточно стандартной reviewer role. ordinis:admin проходит всегда.',
|
'schema.approvalMinRole.hint': 'Например ordinis:internal или ordinis:restricted. Если задано — backend требует чтобы ревьюер имел эту роль (поверх стандартных ordinis:record:reviewer / ordinis:schema:reviewer) для approve / reject / publish. Пусто = достаточно стандартной reviewer role. ordinis:admin проходит всегда.',
|
||||||
'search.title': 'Smart search',
|
'search.title': 'Smart search',
|
||||||
@@ -577,8 +580,8 @@ i18n
|
|||||||
// === Schema review queue panel ===
|
// === Schema review queue panel ===
|
||||||
'reviews.tab.records': 'Записи',
|
'reviews.tab.records': 'Записи',
|
||||||
'reviews.tab.schemas': 'Схемы',
|
'reviews.tab.schemas': 'Схемы',
|
||||||
'reviews.schema.empty': 'Нет схем на ревью',
|
'reviews.schema.empty': 'Очередь пуста',
|
||||||
'reviews.schema.emptyDescription': 'Когда maker отправит schema draft на ревью, он появится здесь.',
|
'reviews.schema.emptyDescription': 'Схемы на ревью появятся здесь.',
|
||||||
'reviews.schema.title': 'Изменения схем',
|
'reviews.schema.title': 'Изменения схем',
|
||||||
'reviews.schema.label': 'Schema workflow',
|
'reviews.schema.label': 'Schema workflow',
|
||||||
'reviews.schema.queueTotal_one': '{{count}} draft ждёт ревью',
|
'reviews.schema.queueTotal_one': '{{count}} draft ждёт ревью',
|
||||||
@@ -843,6 +846,9 @@ i18n
|
|||||||
'reviews.drawer.commentPlaceholder': 'Optional for approve, required for reject',
|
'reviews.drawer.commentPlaceholder': 'Optional for approve, required for reject',
|
||||||
'schema.approvalRequired.label': 'Require approval (maker-checker)',
|
'schema.approvalRequired.label': 'Require approval (maker-checker)',
|
||||||
'schema.approvalRequired.hint': 'Approval Workflow v2. All CRUD goes through draft → review → approve. Protects critical dictionaries from operator error. Existing endpoints (POST/PUT/DELETE) return 409 draft_required pointing to /drafts API.',
|
'schema.approvalRequired.hint': 'Approval Workflow v2. All CRUD goes through draft → review → approve. Protects critical dictionaries from operator error. Existing endpoints (POST/PUT/DELETE) return 409 draft_required pointing to /drafts API.',
|
||||||
|
'schema.approvalRequired.editorBlockTitle': 'Changes require review',
|
||||||
|
'schema.approvalRequired.editorBlock': 'This dictionary requires maker-checker review. Open a draft — your current edits will be pre-filled, you only need to add a reason and submit for review.',
|
||||||
|
'schema.approvalRequired.openDraftCta': 'Create draft with my changes',
|
||||||
'schema.approvalMinRole.label': 'Minimum reviewer role (optional)',
|
'schema.approvalMinRole.label': 'Minimum reviewer role (optional)',
|
||||||
'schema.approvalMinRole.hint': 'e.g. ordinis:internal or ordinis:restricted. When set, backend requires reviewer to hold this role (on top of standard ordinis:record:reviewer / ordinis:schema:reviewer) for approve / reject / publish. Empty = standard reviewer role suffices. ordinis:admin always passes.',
|
'schema.approvalMinRole.hint': 'e.g. ordinis:internal or ordinis:restricted. When set, backend requires reviewer to hold this role (on top of standard ordinis:record:reviewer / ordinis:schema:reviewer) for approve / reject / publish. Empty = standard reviewer role suffices. ordinis:admin always passes.',
|
||||||
'search.title': 'Smart search',
|
'search.title': 'Smart search',
|
||||||
@@ -1272,8 +1278,8 @@ i18n
|
|||||||
// === Schema review queue panel ===
|
// === Schema review queue panel ===
|
||||||
'reviews.tab.records': 'Records',
|
'reviews.tab.records': 'Records',
|
||||||
'reviews.tab.schemas': 'Schemas',
|
'reviews.tab.schemas': 'Schemas',
|
||||||
'reviews.schema.empty': 'No schemas under review',
|
'reviews.schema.empty': 'Queue is empty',
|
||||||
'reviews.schema.emptyDescription': 'Once a maker submits a schema draft for review, it will appear here.',
|
'reviews.schema.emptyDescription': 'Schemas under review will appear here.',
|
||||||
'reviews.schema.title': 'Schema changes',
|
'reviews.schema.title': 'Schema changes',
|
||||||
'reviews.schema.label': 'Schema workflow',
|
'reviews.schema.label': 'Schema workflow',
|
||||||
'reviews.schema.queueTotal_one': '{{count}} draft awaiting review',
|
'reviews.schema.queueTotal_one': '{{count}} draft awaiting review',
|
||||||
|
|||||||
@@ -0,0 +1,63 @@
|
|||||||
|
import { describe, expect, it } from 'vitest'
|
||||||
|
import { shallowDiffSummary, isEmptyDiffSummary } from './diff'
|
||||||
|
|
||||||
|
describe('shallowDiffSummary', () => {
|
||||||
|
it('returns null when both inputs are null', () => {
|
||||||
|
expect(shallowDiffSummary(null, null)).toBeNull()
|
||||||
|
expect(shallowDiffSummary(undefined, undefined)).toBeNull()
|
||||||
|
expect(shallowDiffSummary(null, undefined)).toBeNull()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('reports added keys when before is empty/null', () => {
|
||||||
|
const s = shallowDiffSummary(null, { code: 'A', name: 'Alpha' })
|
||||||
|
expect(s).toEqual({ added: ['code', 'name'], removed: [], changed: [] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('reports removed keys when after is empty/null', () => {
|
||||||
|
const s = shallowDiffSummary({ code: 'A', name: 'Alpha' }, null)
|
||||||
|
expect(s).toEqual({ added: [], removed: ['code', 'name'], changed: [] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('reports changed keys when values differ', () => {
|
||||||
|
const s = shallowDiffSummary({ code: 'A', count: 1 }, { code: 'A', count: 2 })
|
||||||
|
expect(s).toEqual({ added: [], removed: [], changed: ['count'] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('handles mixed +/-/~ in one call', () => {
|
||||||
|
const s = shallowDiffSummary(
|
||||||
|
{ code: 'A', count: 1, oldField: 'x' },
|
||||||
|
{ code: 'A', count: 2, newField: 'y' },
|
||||||
|
)
|
||||||
|
expect(s).toEqual({
|
||||||
|
added: ['newField'],
|
||||||
|
removed: ['oldField'],
|
||||||
|
changed: ['count'],
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
it('treats nested-object change as one changed entry on the outer key', () => {
|
||||||
|
const s = shallowDiffSummary(
|
||||||
|
{ code: 'A', meta: { lat: 55, lon: 37 } },
|
||||||
|
{ code: 'A', meta: { lat: 55, lon: 38 } },
|
||||||
|
)
|
||||||
|
expect(s).toEqual({ added: [], removed: [], changed: ['meta'] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('reports empty buckets when payloads are deeply equal', () => {
|
||||||
|
const s = shallowDiffSummary({ code: 'A', count: 1 }, { code: 'A', count: 1 })
|
||||||
|
expect(s).toEqual({ added: [], removed: [], changed: [] })
|
||||||
|
expect(s && isEmptyDiffSummary(s)).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('isEmptyDiffSummary', () => {
|
||||||
|
it('returns true when all three buckets are empty', () => {
|
||||||
|
expect(isEmptyDiffSummary({ added: [], removed: [], changed: [] })).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns false when any bucket has entries', () => {
|
||||||
|
expect(isEmptyDiffSummary({ added: ['x'], removed: [], changed: [] })).toBe(false)
|
||||||
|
expect(isEmptyDiffSummary({ added: [], removed: ['x'], changed: [] })).toBe(false)
|
||||||
|
expect(isEmptyDiffSummary({ added: [], removed: [], changed: ['x'] })).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -0,0 +1,61 @@
|
|||||||
|
/**
|
||||||
|
* Shallow JSON-record diff utilities for UI summary chips.
|
||||||
|
*
|
||||||
|
* <p>Lives in `src/lib` (not in a route file) so that callers beyond the
|
||||||
|
* record-draft ReviewDrawer can reuse it — RecordHistoryDrawer, audit row
|
||||||
|
* summary, future audit-trail tooltips. Pre-emptive DRY: surfaced during
|
||||||
|
* the 2026-05-14 eng review of MR !186.
|
||||||
|
*
|
||||||
|
* <p>Shallow on purpose: ordinis business records are flat-ish at the top
|
||||||
|
* level (business fields, no deep object trees that would require a real
|
||||||
|
* structural diff). Nested-object changes are reported as a single
|
||||||
|
* "changed" entry on the outer key. Good enough for "what's the gist" UI
|
||||||
|
* chips. For full schema-level diff with line-by-line highlighting, see
|
||||||
|
* `ChangelogDiffModal` + its backend `summary.added/removed/changed`
|
||||||
|
* payload.
|
||||||
|
*
|
||||||
|
* <p>Equality is checked via `JSON.stringify`. Order-sensitive — relies on
|
||||||
|
* V8/JSC preserving insertion order for plain objects (true on every
|
||||||
|
* modern runtime ordinis ships against). Sufficient for record data which
|
||||||
|
* is always object-literal serialized; not safe for Map/Set/Date/RegExp,
|
||||||
|
* none of which appear in record payloads.
|
||||||
|
*/
|
||||||
|
|
||||||
|
export type ShallowDiffSummary = {
|
||||||
|
added: string[]
|
||||||
|
removed: string[]
|
||||||
|
changed: string[]
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @returns null when both inputs are null/undefined (e.g. CLOSE op draft
|
||||||
|
* with no live record). Caller can skip rendering entirely.
|
||||||
|
* Otherwise: per-bucket key lists, never null.
|
||||||
|
*/
|
||||||
|
export function shallowDiffSummary(
|
||||||
|
before: Record<string, unknown> | null | undefined,
|
||||||
|
after: Record<string, unknown> | null | undefined,
|
||||||
|
): ShallowDiffSummary | null {
|
||||||
|
if (!before && !after) return null
|
||||||
|
const b = before ?? {}
|
||||||
|
const a = after ?? {}
|
||||||
|
const added: string[] = []
|
||||||
|
const removed: string[] = []
|
||||||
|
const changed: string[] = []
|
||||||
|
for (const k of Object.keys(a)) {
|
||||||
|
if (!(k in b)) {
|
||||||
|
added.push(k)
|
||||||
|
} else if (JSON.stringify(a[k]) !== JSON.stringify(b[k])) {
|
||||||
|
changed.push(k)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (const k of Object.keys(b)) {
|
||||||
|
if (!(k in a)) removed.push(k)
|
||||||
|
}
|
||||||
|
return { added, removed, changed }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Convenience: empty when all three buckets are empty. */
|
||||||
|
export function isEmptyDiffSummary(s: ShallowDiffSummary): boolean {
|
||||||
|
return s.added.length === 0 && s.removed.length === 0 && s.changed.length === 0
|
||||||
|
}
|
||||||
@@ -29,6 +29,7 @@ import { useDraft, useLiveRecord, useReviewQueue, useSchemaReviewQueue } from '@
|
|||||||
import { useApproveDraft, useRejectDraft } from '@/api/mutations'
|
import { useApproveDraft, useRejectDraft } from '@/api/mutations'
|
||||||
import type { DraftOperation, DraftResponse } from '@/api/client'
|
import type { DraftOperation, DraftResponse } from '@/api/client'
|
||||||
import { UserCell } from '@/lib/useUserDisplay'
|
import { UserCell } from '@/lib/useUserDisplay'
|
||||||
|
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
|
||||||
|
|
||||||
export const Route = createFileRoute('/reviews')({
|
export const Route = createFileRoute('/reviews')({
|
||||||
component: ReviewsPage,
|
component: ReviewsPage,
|
||||||
@@ -41,43 +42,57 @@ const operationVariant = (op: DraftOperation): 'info' | 'success' | 'warning' =>
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Shallow diff summary between two record JSON payloads. Reviewer sees three
|
* Diff summary chips for the ReviewDrawer — "+N added (fields), −M removed,
|
||||||
* counts above the side-by-side dump — "+N added, −M removed, ~K changed" —
|
* ~K changed" above the side-by-side JSON panes. Reviewer sees the gist
|
||||||
* so they don't have to eyeball-diff long records to find the actual edit.
|
* without eyeball-diffing long records.
|
||||||
*
|
*
|
||||||
* Shallow on purpose: ordinis records are flat-ish dicts at the top level
|
* Backed by `shallowDiffSummary` from `@/lib/diff` (extracted there during
|
||||||
* (business fields, no deep object trees). Nested object changes count as
|
* the 2026-05-14 eng review of MR !186 for reuse — RecordHistoryDrawer and
|
||||||
* one "changed" entry on the outer key. Good enough for reviewer at-a-glance.
|
* audit row summaries will plug into the same helper). Memo'd against the
|
||||||
*
|
* three actual inputs so it doesn't recompute on unrelated state changes
|
||||||
* Returns null when before/after both null (CLOSE op, or first CREATE) —
|
* (textarea typing, drawer resize, etc.).
|
||||||
* caller can skip the summary row entirely.
|
|
||||||
*/
|
*/
|
||||||
type DraftDiffSummary = {
|
function DraftDiffSummary({
|
||||||
added: string[]
|
live,
|
||||||
removed: string[]
|
proposed,
|
||||||
changed: string[]
|
operation,
|
||||||
}
|
}: {
|
||||||
function shallowDiffSummary(
|
live: Record<string, unknown> | null | undefined
|
||||||
before: Record<string, unknown> | null | undefined,
|
proposed: Record<string, unknown> | null | undefined
|
||||||
after: Record<string, unknown> | null | undefined,
|
operation: DraftOperation
|
||||||
): DraftDiffSummary | null {
|
}) {
|
||||||
if (!before && !after) return null
|
const summary = useMemo(() => {
|
||||||
const b = before ?? {}
|
// CLOSE op has no proposed payload by design — skip without computing.
|
||||||
const a = after ?? {}
|
if (operation === 'CLOSE') return null
|
||||||
const added: string[] = []
|
return shallowDiffSummary(live, proposed)
|
||||||
const removed: string[] = []
|
}, [live, proposed, operation])
|
||||||
const changed: string[] = []
|
|
||||||
for (const k of Object.keys(a)) {
|
if (!summary || isEmptyDiffSummary(summary)) return null
|
||||||
if (!(k in b)) {
|
return (
|
||||||
added.push(k)
|
<div className="flex flex-wrap items-center gap-3 text-mono text-cap tabular-nums">
|
||||||
} else if (JSON.stringify(a[k]) !== JSON.stringify(b[k])) {
|
{summary.added.length > 0 && (
|
||||||
changed.push(k)
|
<span className="inline-flex items-center gap-1 text-aurora">
|
||||||
}
|
<span className="font-bold">+</span>
|
||||||
}
|
<span>{summary.added.length}</span>
|
||||||
for (const k of Object.keys(b)) {
|
<span className="text-mute lowercase">({summary.added.join(', ')})</span>
|
||||||
if (!(k in a)) removed.push(k)
|
</span>
|
||||||
}
|
)}
|
||||||
return { added, removed, changed }
|
{summary.removed.length > 0 && (
|
||||||
|
<span className="inline-flex items-center gap-1 text-danger">
|
||||||
|
<span className="font-bold">−</span>
|
||||||
|
<span>{summary.removed.length}</span>
|
||||||
|
<span className="text-mute lowercase">({summary.removed.join(', ')})</span>
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
|
{summary.changed.length > 0 && (
|
||||||
|
<span className="inline-flex items-center gap-1 text-warn">
|
||||||
|
<span className="font-bold">~</span>
|
||||||
|
<span>{summary.changed.length}</span>
|
||||||
|
<span className="text-mute lowercase">({summary.changed.join(', ')})</span>
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -591,55 +606,11 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Diff summary chips above the side-by-side dump — reviewer sees
|
<DraftDiffSummary
|
||||||
"+1 added, ~2 changed" before eyeballing the JSON. Skipped for
|
live={liveQ.data?.data as Record<string, unknown> | null | undefined}
|
||||||
CLOSE op (no proposed) and when both sides are null. */}
|
proposed={draft.data as Record<string, unknown> | null | undefined}
|
||||||
{(() => {
|
operation={draft.operation}
|
||||||
const summary =
|
/>
|
||||||
draft.operation === 'CLOSE'
|
|
||||||
? null
|
|
||||||
: shallowDiffSummary(
|
|
||||||
liveQ.data?.data as Record<string, unknown> | null | undefined,
|
|
||||||
draft.data as Record<string, unknown> | null | undefined,
|
|
||||||
)
|
|
||||||
if (!summary) return null
|
|
||||||
const empty =
|
|
||||||
summary.added.length === 0 &&
|
|
||||||
summary.removed.length === 0 &&
|
|
||||||
summary.changed.length === 0
|
|
||||||
if (empty) return null
|
|
||||||
return (
|
|
||||||
<div className="flex flex-wrap items-center gap-3 text-mono text-cap tabular-nums">
|
|
||||||
{summary.added.length > 0 && (
|
|
||||||
<span className="inline-flex items-center gap-1 text-aurora">
|
|
||||||
<span className="font-bold">+</span>
|
|
||||||
<span>{summary.added.length}</span>
|
|
||||||
<span className="text-mute lowercase">
|
|
||||||
({summary.added.join(', ')})
|
|
||||||
</span>
|
|
||||||
</span>
|
|
||||||
)}
|
|
||||||
{summary.removed.length > 0 && (
|
|
||||||
<span className="inline-flex items-center gap-1 text-danger">
|
|
||||||
<span className="font-bold">−</span>
|
|
||||||
<span>{summary.removed.length}</span>
|
|
||||||
<span className="text-mute lowercase">
|
|
||||||
({summary.removed.join(', ')})
|
|
||||||
</span>
|
|
||||||
</span>
|
|
||||||
)}
|
|
||||||
{summary.changed.length > 0 && (
|
|
||||||
<span className="inline-flex items-center gap-1 text-warn">
|
|
||||||
<span className="font-bold">~</span>
|
|
||||||
<span>{summary.changed.length}</span>
|
|
||||||
<span className="text-mute lowercase">
|
|
||||||
({summary.changed.join(', ')})
|
|
||||||
</span>
|
|
||||||
</span>
|
|
||||||
)}
|
|
||||||
</div>
|
|
||||||
)
|
|
||||||
})()}
|
|
||||||
|
|
||||||
<div className="grid grid-cols-1 md:grid-cols-2 gap-3">
|
<div className="grid grid-cols-1 md:grid-cols-2 gap-3">
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
Reference in New Issue
Block a user