Merge branch 'feat/approval-ux-editor-cta-and-drawer-diff' into 'main'
feat(approval-ux): inline draft CTA + reviewer diff summary + tightened drawer See merge request 2-6/2-6-4/terravault/ordinis!186
This commit is contained in:
Александр Зимин
@@ -0,0 +1,159 @@
|
||||
# TODOS
|
||||
|
||||
Design debt and follow-ups surfaced by `/plan-design-review` of the approval
|
||||
workflow UX (2026-05-14). Each entry is self-contained: pick one up and read
|
||||
the **Context** to understand the motivation without re-running the review.
|
||||
|
||||
---
|
||||
|
||||
## 1. `/reviews` "Мои" tab — maker visibility
|
||||
|
||||
**What.** Add a third tab "Мои" on the `/reviews` page with three sub-sections:
|
||||
Pending (drafts awaiting review), Decided (drafts that received a verdict),
|
||||
WIP (drafts the maker started but did not yet submit).
|
||||
|
||||
**Why.** Today the maker has no surface to see "where are my drafts?". MR !170
|
||||
added the backend (`listByMaker` + analogous schema endpoint), but the UI never
|
||||
got the entry point. Reviewer's queue mixes records and schemas — symmetric
|
||||
"my drafts" view is missing.
|
||||
|
||||
**Pros.** Reuses `/reviews` page chrome. Backend endpoints already exist. Closes
|
||||
the loop on the maker journey ("submit → ??? → ..." now has a destination).
|
||||
|
||||
**Cons.** Adds two more list queries to the page. Need to think about whether
|
||||
the existing reviewer "Записи / Схемы" toggle nests inside "Мои" too (so 6 panels
|
||||
total) or "Мои" is flat.
|
||||
|
||||
**Context.** Review pass 1 rated information architecture 6/10 specifically
|
||||
because maker visibility was missing. User chose "third tab on /reviews"
|
||||
over a separate `/my-drafts` route — keeps everything in one place.
|
||||
|
||||
**Depends on / blocked by.** Nothing — endpoints live (`/admin/dictionaries/*/drafts?makerId=...`
|
||||
and analogous for schemas).
|
||||
|
||||
---
|
||||
|
||||
## 2. Interaction state pass — reviews page polish
|
||||
|
||||
**What.** One MR covering five small fixes:
|
||||
|
||||
- Records empty state: add description + primary action ("Queue is clear, see
|
||||
/audit for past decisions"). Current `<EmptyState title={t('reviews.empty')} />`
|
||||
is just a heading.
|
||||
- Reject reason inline validation: today `handleReject` does
|
||||
`if (!comment.trim()) return` silently. Button click → nothing happens. Show
|
||||
inline error or set the TextArea into error state.
|
||||
- Drawer empty state (`workflow.schemaDraft.empty`): distinguish "not loaded",
|
||||
"not found" (deleted), "no permission". Today the same blank message covers
|
||||
all three.
|
||||
- Approve action: drawer closes immediately on success. Add a 500ms success
|
||||
state (or toast) so the reviewer gets visual confirmation. Currently feels
|
||||
like the click might not have registered.
|
||||
- Post-bulk-result polish: the `bulkResult` Alert lists business keys but the
|
||||
failed list shows raw `reason` codes (`self_approve_forbidden`,
|
||||
`draft_not_pending`). Map through `extractReviewError` already in the file.
|
||||
|
||||
**Why.** Pass 2 rated state coverage 5/10. These are the visible gaps. Each
|
||||
is 10-30 LOC; the value is interaction confidence.
|
||||
|
||||
**Pros.** All in one place — `reviews.tsx`, `SchemaDraftDrawer.tsx`. Low risk.
|
||||
|
||||
**Cons.** Five different surfaces in one MR means review surface is wider than
|
||||
a single-purpose change. Still worth grouping because they're all "state
|
||||
polish" semantically.
|
||||
|
||||
**Context.** Pass 2 finding. None of these block functionality; they're trust
|
||||
builders. Reject silent fail is the most user-visible — a reviewer clicks the
|
||||
button and nothing happens, then notices the TextArea is empty after a beat.
|
||||
|
||||
**Depends on / blocked by.** Nothing.
|
||||
|
||||
---
|
||||
|
||||
## 5. DESIGN.md for the admin UI
|
||||
|
||||
**What.** Run `/design-consultation` skill or stub DESIGN.md manually to lock
|
||||
the implicit design system: drawer-vs-modal usage rules, term glossary
|
||||
(maker / reviewer / publisher and their Russian counterparts), Badge variant
|
||||
mapping (CREATE → success, UPDATE → info, CLOSE → warning — already used
|
||||
consistently but undocumented), code panes pattern (live = `bg-line/30`,
|
||||
proposed = `bg-accent/4`).
|
||||
|
||||
**Why.** Pass 5 rated design system alignment 4/10. The system *exists* — design
|
||||
tokens are used consistently — but it's tribal knowledge. Next developer building
|
||||
a feature like "Phase 4 webhook approvals" will guess and either match by luck
|
||||
or drift.
|
||||
|
||||
**Pros.** Half-day work. Pays off on every new feature. Source of truth for
|
||||
`/plan-design-review` calibration on future plans.
|
||||
|
||||
**Cons.** Documentation has to be maintained or it lies. Mitigated by keeping
|
||||
DESIGN.md to invariants (tokens, primitives, semantic rules), not specific
|
||||
components.
|
||||
|
||||
**Context.** User chose "defer to /design-consultation sprint" instead of
|
||||
stubbing inline. The decision is to do it properly when the sprint happens,
|
||||
not piecemeal during this review.
|
||||
|
||||
**Depends on / blocked by.** Nothing.
|
||||
|
||||
---
|
||||
|
||||
## 6. A11y audit post-deploy via `/design-review`
|
||||
|
||||
**What.** After the in-flight approval workflow MRs deploy to staging, run
|
||||
`/design-review` skill against the live `/reviews` page + DictionaryEditorDialog
|
||||
flow. The skill does Lighthouse, axe, screen reader walk-through, contrast
|
||||
audit, keyboard navigation testing.
|
||||
|
||||
**Why.** Pass 6 rated responsive/a11y 5/10. Specific suspects:
|
||||
- Button heights not verified at 44px touch target.
|
||||
- `text-mute` contrast against `bg-surface` — might fail WCAG AA 4.5:1.
|
||||
- Reject reason TextArea has no specified `minHeight`.
|
||||
- Bulk action toolbar wrap behavior at 375px.
|
||||
- Diff `<pre>` panes have no aria-label — screen reader reads JSON literally.
|
||||
- Keyboard tab order through long diff panes — reviewer may have to tab 50 times
|
||||
to reach approve buttons.
|
||||
|
||||
**Pros.** Necessary for compliance contexts (gov contracts, accessibility laws).
|
||||
Catches things static review misses (real screen reader, real touch device).
|
||||
|
||||
**Cons.** Requires the in-flight MRs to be deployed first. Cannot do in
|
||||
plan mode.
|
||||
|
||||
**Context.** User chose "defer to /design-review post-deploy". The audit will
|
||||
produce its own punch list with severity ratings.
|
||||
|
||||
**Depends on / blocked by.** MRs !179, !180, !182, !183, !184, !185 deployed
|
||||
to staging.
|
||||
|
||||
---
|
||||
|
||||
## 7. Notifications service: draft decision toast + email
|
||||
|
||||
**What.** Notifications service subscribes to outbox `RecordDraftApproved /
|
||||
RecordDraftRejected / RecordDraftWithdrawn` events (now fired thanks to
|
||||
MR !184) and:
|
||||
|
||||
1. Fires email to maker with reviewer comment.
|
||||
2. Sets a flag (DB or Redis) so on next login the admin UI shows a toast:
|
||||
"Your draft for `test1/BK-42` was approved by reviewer X. The record is
|
||||
now live."
|
||||
3. Bell icon in the header gains a badge counter for unread decisions.
|
||||
|
||||
**Why.** Maker has no synchronous feedback. They submit, then either:
|
||||
- Tab "Мои" → Decided (covered by TODO 1) — but only when they come back
|
||||
- This TODO — push notification so they know without re-visiting
|
||||
|
||||
**Pros.** Closes the maker feedback loop. Reuses the outbox infra. Mirrors
|
||||
existing notification flows in `ordinis-notifications`.
|
||||
|
||||
**Cons.** New email template + new in-app toast surface + bell badge. Half-day
|
||||
to a day of work split between notifications service and admin-ui.
|
||||
|
||||
**Context.** Pass 7 unresolved decision. User chose "in-app toast + email via
|
||||
notifications service" over the lighter "tab badge polling" option. Backend
|
||||
events already fire (MR !184), so the gate is consumer side.
|
||||
|
||||
**Depends on / blocked by.** MR !184 deployed. TODO 1 ("Мои" tab) helps but
|
||||
is not a hard blocker — the toast and email work without that surface.
|
||||
@@ -20,6 +20,7 @@ import { dictionaryDetailQuery, useDictionaries } from '@/api/queries'
|
||||
import type { CreateDictionaryRequest, DataScope, DictionaryDetail } from '@/api/client'
|
||||
import { SchemaBuilder } from './SchemaBuilder'
|
||||
import { EventsPreviewTab } from './EventsPreviewTab'
|
||||
import { CreateSchemaDraftModal } from '@/components/workflow/CreateSchemaDraftModal'
|
||||
import {
|
||||
buildSchemaJson,
|
||||
diffSchemas,
|
||||
@@ -84,6 +85,11 @@ export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props
|
||||
const [properties, setProperties] = useState<PropertyDef[]>(parsed.properties)
|
||||
const [idSource, setIdSource] = useState<string>(parsed.idSource ?? '')
|
||||
const [nameError, setNameError] = useState<string | null>(null)
|
||||
/** Open state for the "Создать черновик с моими изменениями" CTA. The CTA
|
||||
* is the inline handoff from the disabled-Save path (requiresDraftFlow)
|
||||
* into the schema-draft modal with the maker's in-progress schemaJson
|
||||
* pre-loaded — no retyping. */
|
||||
const [draftHandoffOpen, setDraftHandoffOpen] = useState(false)
|
||||
|
||||
// Template (только в create-mode): admin выбирает существующий dict,
|
||||
// его schema + locale + scope копируются. Имя остаётся пустым (admin
|
||||
@@ -431,12 +437,29 @@ export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props
|
||||
{serverError && <Alert variant="error">{serverError}</Alert>}
|
||||
|
||||
{requiresDraftFlow && (
|
||||
<Alert variant="warning">
|
||||
<Alert
|
||||
variant="warning"
|
||||
title={t('schema.approvalRequired.editorBlockTitle', {
|
||||
defaultValue: 'Изменения требуют ревью',
|
||||
})}
|
||||
action={
|
||||
<Button
|
||||
type="button"
|
||||
variant="primary"
|
||||
size="sm"
|
||||
onClick={() => setDraftHandoffOpen(true)}
|
||||
>
|
||||
{t('schema.approvalRequired.openDraftCta', {
|
||||
defaultValue: 'Создать черновик с моими изменениями',
|
||||
})}
|
||||
</Button>
|
||||
}
|
||||
>
|
||||
{t('schema.approvalRequired.editorBlock', {
|
||||
defaultValue:
|
||||
'Этот словарь требует ревью изменений. Закройте редактор и нажмите ' +
|
||||
'«Создать черновик схемы» на странице справочника — там вы сможете ' +
|
||||
'отправить эти изменения на согласование.',
|
||||
'Этот словарь требует maker-checker review. Откройте черновик — ' +
|
||||
'ваши текущие изменения будут предзаполнены, вам нужно только ' +
|
||||
'указать причину и отправить на согласование.',
|
||||
})}
|
||||
</Alert>
|
||||
)}
|
||||
@@ -456,6 +479,24 @@ export const DictionaryEditorDialog = ({ open, mode, onClose, onSuccess }: Props
|
||||
</Button>
|
||||
</FormActions>
|
||||
</div>
|
||||
{/* Inline handoff: CreateSchemaDraftModal mounted with the in-progress
|
||||
schemaJson pre-loaded. Maker hits "Создать черновик с моими изменениями"
|
||||
→ modal opens with the same edits they had → they add a reason and
|
||||
submit. No retyping, no hunt for a button on the dict page. */}
|
||||
{isEdit && initial && (
|
||||
<CreateSchemaDraftModal
|
||||
open={draftHandoffOpen}
|
||||
onClose={() => setDraftHandoffOpen(false)}
|
||||
detail={initial}
|
||||
initialProposedSchema={schemaJson}
|
||||
onCreated={() => {
|
||||
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()
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
</Modal>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -24,6 +24,15 @@ type Props = {
|
||||
* CHANGES_REQUESTED — backend auto-transition CHANGES_REQUESTED → DRAFT.
|
||||
*/
|
||||
editDraft?: SchemaDraft
|
||||
/**
|
||||
* Optional pre-filled proposed schema. When passed (and not in edit mode),
|
||||
* the JSON editor seeds with this instead of `detail.schemaJson`. Used by
|
||||
* the "Создать черновик с моими изменениями" CTA in DictionaryEditorDialog:
|
||||
* the maker tweaks the schema, hits the disabled Save, then jumps straight
|
||||
* into the draft modal with their in-progress edits already loaded. No
|
||||
* retyping.
|
||||
*/
|
||||
initialProposedSchema?: unknown
|
||||
/** Callback после create — parent может закрыть modal + открыть drawer. */
|
||||
onCreated?: (draftId: string) => void
|
||||
}
|
||||
@@ -46,18 +55,31 @@ type Props = {
|
||||
* вернёт detail в message.</li>
|
||||
* </ul>
|
||||
*/
|
||||
export function CreateSchemaDraftModal({ open, onClose, detail, editDraft, onCreated }: Props) {
|
||||
export function CreateSchemaDraftModal({
|
||||
open,
|
||||
onClose,
|
||||
detail,
|
||||
editDraft,
|
||||
initialProposedSchema,
|
||||
onCreated,
|
||||
}: Props) {
|
||||
const { t } = useTranslation()
|
||||
const isEdit = Boolean(editDraft)
|
||||
// В edit mode seed'им из draft'овой proposedSchema, иначе из live HEAD.
|
||||
// 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(
|
||||
() =>
|
||||
JSON.stringify(
|
||||
editDraft ? editDraft.proposedSchema : detail.schemaJson,
|
||||
editDraft
|
||||
? editDraft.proposedSchema
|
||||
: initialProposedSchema !== undefined
|
||||
? initialProposedSchema
|
||||
: detail.schemaJson,
|
||||
null,
|
||||
2,
|
||||
),
|
||||
[editDraft, detail.schemaJson],
|
||||
[editDraft, initialProposedSchema, detail.schemaJson],
|
||||
)
|
||||
const initialReason = editDraft?.reason ?? ''
|
||||
const [json, setJson] = useState(initialJson)
|
||||
|
||||
@@ -40,6 +40,46 @@ const operationVariant = (op: DraftOperation): 'info' | 'success' | 'warning' =>
|
||||
return 'info'
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
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 }
|
||||
}
|
||||
|
||||
/**
|
||||
* Approval Workflow v2 — reviewer queue (D3=A pool model).
|
||||
*
|
||||
@@ -192,10 +232,10 @@ function ReviewsPage() {
|
||||
{schemasCount === 0 && !schemaQueue.isLoading && (
|
||||
<EmptyState
|
||||
title={t('reviews.schema.empty', {
|
||||
defaultValue: 'Нет схем на ревью',
|
||||
defaultValue: 'Очередь пуста',
|
||||
})}
|
||||
description={t('reviews.schema.emptyDescription', {
|
||||
defaultValue: 'Когда maker отправит schema draft на ревью, он появится здесь.',
|
||||
defaultValue: 'Схемы на ревью появятся здесь.',
|
||||
})}
|
||||
/>
|
||||
)}
|
||||
@@ -523,16 +563,23 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
|
||||
{draft && (
|
||||
<>
|
||||
<div className="flex flex-wrap items-center gap-2 text-cell">
|
||||
<Badge variant={operationVariant(draft.operation)}>{draft.operation}</Badge>
|
||||
<span className="font-mono">{draft.businessKey}</span>
|
||||
<span className="text-mute inline-flex items-center gap-1">
|
||||
{t('reviews.drawer.maker')}: <UserCell uuid={draft.makerId} />
|
||||
</span>
|
||||
<span className="text-mute">
|
||||
{t('reviews.drawer.submitted')}:{' '}
|
||||
{new Date(draft.submittedAt).toLocaleString()}
|
||||
</span>
|
||||
{/* Tightened header: top row is identity (op + bk), bottom row is
|
||||
provenance (who/when). Was a single flex-wrap line with four
|
||||
spans — visually busy, hard to scan. */}
|
||||
<div className="space-y-1 text-cell">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<Badge variant={operationVariant(draft.operation)}>{draft.operation}</Badge>
|
||||
<span className="font-mono">{draft.businessKey}</span>
|
||||
</div>
|
||||
<div className="flex flex-wrap items-center gap-x-4 gap-y-1 text-mute">
|
||||
<span className="inline-flex items-center gap-1">
|
||||
{t('reviews.drawer.maker')}: <UserCell uuid={draft.makerId} />
|
||||
</span>
|
||||
<span>
|
||||
{t('reviews.drawer.submitted')}:{' '}
|
||||
{new Date(draft.submittedAt).toLocaleString()}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{draft.makerComment && (
|
||||
@@ -544,6 +591,56 @@ 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>
|
||||
)
|
||||
})()}
|
||||
|
||||
<div className="grid grid-cols-1 md:grid-cols-2 gap-3">
|
||||
<div>
|
||||
<p className="text-cap text-mute mb-1">
|
||||
|
||||
Reference in New Issue
Block a user