refactor(approval-ux): eng-review fixes — extract helpers, add tests, register i18n
This commit is contained in:
@@ -29,6 +29,7 @@ import { useDraft, useLiveRecord, useReviewQueue, useSchemaReviewQueue } from '@
|
||||
import { useApproveDraft, useRejectDraft } from '@/api/mutations'
|
||||
import type { DraftOperation, DraftResponse } from '@/api/client'
|
||||
import { UserCell } from '@/lib/useUserDisplay'
|
||||
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
|
||||
|
||||
export const Route = createFileRoute('/reviews')({
|
||||
component: ReviewsPage,
|
||||
@@ -41,43 +42,57 @@ const operationVariant = (op: DraftOperation): 'info' | 'success' | 'warning' =>
|
||||
}
|
||||
|
||||
/**
|
||||
* Shallow diff summary between two record JSON payloads. Reviewer sees three
|
||||
* counts above the side-by-side dump — "+N added, −M removed, ~K changed" —
|
||||
* so they don't have to eyeball-diff long records to find the actual edit.
|
||||
* Diff summary chips for the ReviewDrawer — "+N added (fields), −M removed,
|
||||
* ~K changed" above the side-by-side JSON panes. Reviewer sees the gist
|
||||
* without eyeball-diffing long records.
|
||||
*
|
||||
* Shallow on purpose: ordinis records are flat-ish dicts at the top level
|
||||
* (business fields, no deep object trees). Nested object changes count as
|
||||
* one "changed" entry on the outer key. Good enough for reviewer at-a-glance.
|
||||
*
|
||||
* Returns null when before/after both null (CLOSE op, or first CREATE) —
|
||||
* caller can skip the summary row entirely.
|
||||
* Backed by `shallowDiffSummary` from `@/lib/diff` (extracted there during
|
||||
* the 2026-05-14 eng review of MR !186 for reuse — RecordHistoryDrawer and
|
||||
* 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
|
||||
* (textarea typing, drawer resize, etc.).
|
||||
*/
|
||||
type DraftDiffSummary = {
|
||||
added: string[]
|
||||
removed: string[]
|
||||
changed: string[]
|
||||
}
|
||||
function shallowDiffSummary(
|
||||
before: Record<string, unknown> | null | undefined,
|
||||
after: Record<string, unknown> | null | undefined,
|
||||
): DraftDiffSummary | 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 }
|
||||
function DraftDiffSummary({
|
||||
live,
|
||||
proposed,
|
||||
operation,
|
||||
}: {
|
||||
live: Record<string, unknown> | null | undefined
|
||||
proposed: Record<string, unknown> | null | undefined
|
||||
operation: DraftOperation
|
||||
}) {
|
||||
const summary = useMemo(() => {
|
||||
// CLOSE op has no proposed payload by design — skip without computing.
|
||||
if (operation === 'CLOSE') return null
|
||||
return shallowDiffSummary(live, proposed)
|
||||
}, [live, proposed, operation])
|
||||
|
||||
if (!summary || isEmptyDiffSummary(summary)) 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>
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -591,55 +606,11 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Diff summary chips above the side-by-side dump — reviewer sees
|
||||
"+1 added, ~2 changed" before eyeballing the JSON. Skipped for
|
||||
CLOSE op (no proposed) and when both sides are null. */}
|
||||
{(() => {
|
||||
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>
|
||||
)
|
||||
})()}
|
||||
<DraftDiffSummary
|
||||
live={liveQ.data?.data as Record<string, unknown> | null | undefined}
|
||||
proposed={draft.data as Record<string, unknown> | null | undefined}
|
||||
operation={draft.operation}
|
||||
/>
|
||||
|
||||
<div className="grid grid-cols-1 md:grid-cols-2 gap-3">
|
||||
<div>
|
||||
|
||||
Reference in New Issue
Block a user