Merge branch 'refactor/reviews-interaction-polish' into 'main'
refactor(reviews): interaction state polish — 5 UX fixes See merge request 2-6/2-6-4/terravault/ordinis!189
This commit is contained in:
@@ -17,40 +17,14 @@ record rows reuse ReviewDrawer with status-gated footer.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 2. Interaction state pass — reviews page polish
|
## 2. Interaction state pass — reviews page polish ⏳ MR !189 pending
|
||||||
|
|
||||||
**What.** One MR covering five small fixes:
|
Closed in [MR !189](https://git.nstart.cloud/2-6/2-6-4/terravault/ordinis/-/merge_requests/189) — all 5 fixes in one MR:
|
||||||
|
1. Records empty state — icon + description + audit link
|
||||||
- Records empty state: add description + primary action ("Queue is clear, see
|
2. Reject reason inline validation — aria-invalid + role=alert
|
||||||
/audit for past decisions"). Current `<EmptyState title={t('reviews.empty')} />`
|
3. SchemaDraftDrawer empty state — QueryErrorState для errors + dashed block для no-draft
|
||||||
is just a heading.
|
4. Approve/reject success flash — 600ms green/pink band before close
|
||||||
- Reject reason inline validation: today `handleReject` does
|
5. Bulk failed reasons humanized — extracted `humanizeBulkReason()` lib + 7 unit tests
|
||||||
`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.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -206,13 +206,24 @@ export function SchemaDraftDrawer({
|
|||||||
>
|
>
|
||||||
<div className="space-y-4">
|
<div className="space-y-4">
|
||||||
{isLoading && <LoadingBlock size="md" label={t('loading')} />}
|
{isLoading && <LoadingBlock size="md" label={t('loading')} />}
|
||||||
|
{/* QueryErrorState уже сам обрабатывает 403/404 с human messages —
|
||||||
|
больше не надо guessить здесь. Раньше один blank empty покрывал
|
||||||
|
"not loaded / not found / forbidden" одинаково. */}
|
||||||
{error && <QueryErrorState error={error} onRetry={() => refetch()} />}
|
{error && <QueryErrorState error={error} onRetry={() => refetch()} />}
|
||||||
{!isLoading && !draft && (
|
{!isLoading && !error && !draft && (
|
||||||
<p className="text-body text-mute">
|
<div className="flex flex-col items-center justify-center text-center py-10 px-4 gap-2 rounded-md border border-dashed border-line bg-surface-2/30">
|
||||||
|
<p className="text-body text-ink-2">
|
||||||
{t('workflow.schemaDraft.empty', {
|
{t('workflow.schemaDraft.empty', {
|
||||||
defaultValue: 'Активного черновика схемы нет.',
|
defaultValue: 'Активного черновика на этом словаре нет.',
|
||||||
})}
|
})}
|
||||||
</p>
|
</p>
|
||||||
|
<p className="text-cell text-mute max-w-md">
|
||||||
|
{t('workflow.schemaDraft.emptyHint', {
|
||||||
|
defaultValue:
|
||||||
|
'Создайте новый черновик через editor словаря — кнопка «Создать черновик схемы» появится если ваши изменения требуют ревью.',
|
||||||
|
})}
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{draft && (
|
{draft && (
|
||||||
|
|||||||
@@ -581,6 +581,14 @@ i18n
|
|||||||
'reviews.tab.records': 'Записи',
|
'reviews.tab.records': 'Записи',
|
||||||
'reviews.tab.schemas': 'Схемы',
|
'reviews.tab.schemas': 'Схемы',
|
||||||
'reviews.tab.mine': 'Мои',
|
'reviews.tab.mine': 'Мои',
|
||||||
|
// Interaction state polish (TODO 2)
|
||||||
|
'reviews.emptyDescription':
|
||||||
|
'Очередь чистая. Прошлые решения можно посмотреть в журнале аудита.',
|
||||||
|
'reviews.emptyAuditLink': 'Открыть /audit',
|
||||||
|
'reviews.drawer.rejectReasonRequired':
|
||||||
|
'Укажите причину отклонения — comment обязателен для reject (compliance).',
|
||||||
|
'workflow.schemaDraft.emptyHint':
|
||||||
|
'Создайте новый черновик через editor словаря — кнопка «Создать черновик схемы» появится если ваши изменения требуют ревью.',
|
||||||
'reviews.schema.empty': 'Очередь пуста',
|
'reviews.schema.empty': 'Очередь пуста',
|
||||||
'reviews.schema.emptyDescription': 'Схемы на ревью появятся здесь.',
|
'reviews.schema.emptyDescription': 'Схемы на ревью появятся здесь.',
|
||||||
// === /reviews → "Мои" tab (maker self-tracking) ===
|
// === /reviews → "Мои" tab (maker self-tracking) ===
|
||||||
@@ -1315,6 +1323,14 @@ i18n
|
|||||||
'reviews.tab.records': 'Records',
|
'reviews.tab.records': 'Records',
|
||||||
'reviews.tab.schemas': 'Schemas',
|
'reviews.tab.schemas': 'Schemas',
|
||||||
'reviews.tab.mine': 'Mine',
|
'reviews.tab.mine': 'Mine',
|
||||||
|
// Interaction state polish (TODO 2)
|
||||||
|
'reviews.emptyDescription':
|
||||||
|
'Queue is clear. Past decisions live in the audit log.',
|
||||||
|
'reviews.emptyAuditLink': 'Open /audit',
|
||||||
|
'reviews.drawer.rejectReasonRequired':
|
||||||
|
'Reject requires a comment (compliance) — fill the comment field first.',
|
||||||
|
'workflow.schemaDraft.emptyHint':
|
||||||
|
'Create a new draft via the dictionary editor — the "Create schema draft" button appears when your changes need review.',
|
||||||
'reviews.schema.empty': 'Queue is empty',
|
'reviews.schema.empty': 'Queue is empty',
|
||||||
'reviews.schema.emptyDescription': 'Schemas under review will appear here.',
|
'reviews.schema.emptyDescription': 'Schemas under review will appear here.',
|
||||||
// === /reviews → "Mine" tab (maker self-tracking) ===
|
// === /reviews → "Mine" tab (maker self-tracking) ===
|
||||||
|
|||||||
@@ -0,0 +1,44 @@
|
|||||||
|
import { describe, expect, it } from 'vitest'
|
||||||
|
import { humanizeBulkReason } from './bulkReason'
|
||||||
|
|
||||||
|
describe('humanizeBulkReason', () => {
|
||||||
|
it('maps self_approve_forbidden to russian title', () => {
|
||||||
|
expect(humanizeBulkReason('self_approve_forbidden')).toBe(
|
||||||
|
'Нельзя одобрить свой draft (maker-checker)',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('maps draft_not_pending to russian title', () => {
|
||||||
|
expect(humanizeBulkReason('draft_not_pending')).toBe(
|
||||||
|
'Draft уже не pending — обнови очередь',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('maps forbidden_role to russian title', () => {
|
||||||
|
expect(humanizeBulkReason('forbidden_role')).toBe(
|
||||||
|
'Недостаточно прав (нужна reviewer role)',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('maps reject_reason_required to russian title', () => {
|
||||||
|
expect(humanizeBulkReason('reject_reason_required')).toBe(
|
||||||
|
'Reject требует comment (compliance)',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('maps self_reject_forbidden to russian title', () => {
|
||||||
|
expect(humanizeBulkReason('self_reject_forbidden')).toBe(
|
||||||
|
'Нельзя отклонить свой draft',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns unknown reason as-is (backend message fallback)', () => {
|
||||||
|
expect(humanizeBulkReason('Some raw backend message')).toBe(
|
||||||
|
'Some raw backend message',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns empty string for empty input', () => {
|
||||||
|
expect(humanizeBulkReason('')).toBe('')
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -0,0 +1,27 @@
|
|||||||
|
/**
|
||||||
|
* Bulk-action failure reason → human-readable label.
|
||||||
|
*
|
||||||
|
* <p>Lives in `src/lib` (а не в `routes/reviews.tsx`) потому что bulk runner
|
||||||
|
* captures axios errors как plain string `reason` (либо `response.data.code`,
|
||||||
|
* либо `error.message`) и хранит в `BulkResult.failed[].reason`. Раньше
|
||||||
|
* reviewer видел сырой code типа `self_approve_forbidden` в bulk result
|
||||||
|
* alert'е — теперь маппит на russain title из {@code extractReviewError}
|
||||||
|
* mapping (single source of truth).
|
||||||
|
*
|
||||||
|
* <p>Если reason нет в known codes — возвращает as-is (backend message
|
||||||
|
* вероятно уже human-readable, либо unknown server error который оставляем
|
||||||
|
* сырым чтобы reviewer мог скопировать в bug report).
|
||||||
|
*
|
||||||
|
* <p>Surfaced during TODO 2 (`/plan-design-review` interaction state pass
|
||||||
|
* 2026-05-14). Pure function — unit tested через `bulkReason.test.ts`.
|
||||||
|
*/
|
||||||
|
export function humanizeBulkReason(reason: string): string {
|
||||||
|
const known: Record<string, string> = {
|
||||||
|
self_approve_forbidden: 'Нельзя одобрить свой draft (maker-checker)',
|
||||||
|
self_reject_forbidden: 'Нельзя отклонить свой draft',
|
||||||
|
draft_not_pending: 'Draft уже не pending — обнови очередь',
|
||||||
|
forbidden_role: 'Недостаточно прав (нужна reviewer role)',
|
||||||
|
reject_reason_required: 'Reject требует comment (compliance)',
|
||||||
|
}
|
||||||
|
return known[reason] ?? reason
|
||||||
|
}
|
||||||
@@ -38,6 +38,7 @@ import { useApproveDraft, useRejectDraft, useWithdrawDraft } from '@/api/mutatio
|
|||||||
import type { DraftOperation, DraftResponse, DraftStatus } from '@/api/client'
|
import type { DraftOperation, DraftResponse, DraftStatus } from '@/api/client'
|
||||||
import { UserCell } from '@/lib/useUserDisplay'
|
import { UserCell } from '@/lib/useUserDisplay'
|
||||||
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
|
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
|
||||||
|
import { humanizeBulkReason } from '@/lib/bulkReason'
|
||||||
import {
|
import {
|
||||||
MyDraftsPanel,
|
MyDraftsPanel,
|
||||||
classifyRecordStatus,
|
classifyRecordStatus,
|
||||||
@@ -368,7 +369,7 @@ function ReviewsPage() {
|
|||||||
<ul className="ml-4 mt-1 list-disc">
|
<ul className="ml-4 mt-1 list-disc">
|
||||||
{bulkResult.failed.map((f) => (
|
{bulkResult.failed.map((f) => (
|
||||||
<li key={f.id}>
|
<li key={f.id}>
|
||||||
<span className="font-mono">{f.bk}</span>: {f.reason}
|
<span className="font-mono">{f.bk}</span>: {humanizeBulkReason(f.reason)}
|
||||||
</li>
|
</li>
|
||||||
))}
|
))}
|
||||||
</ul>
|
</ul>
|
||||||
@@ -379,7 +380,19 @@ function ReviewsPage() {
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{activeTab === 'records' && queue.data && queue.data.totalElements === 0 && (
|
{activeTab === 'records' && queue.data && queue.data.totalElements === 0 && (
|
||||||
<EmptyState title={t('reviews.empty')} />
|
<EmptyState
|
||||||
|
icon={<CheckCircleIcon weight="duotone" size={32} />}
|
||||||
|
title={t('reviews.empty')}
|
||||||
|
description={t('reviews.emptyDescription', {
|
||||||
|
defaultValue:
|
||||||
|
'Очередь чистая. Прошлые решения можно посмотреть в журнале аудита.',
|
||||||
|
})}
|
||||||
|
action={
|
||||||
|
<Link to="/audit" className="text-accent hover:underline text-cell">
|
||||||
|
{t('reviews.emptyAuditLink', { defaultValue: 'Открыть /audit' })} →
|
||||||
|
</Link>
|
||||||
|
}
|
||||||
|
/>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{activeTab === 'records' && queue.data && queue.data.totalElements > 0 && (
|
{activeTab === 'records' && queue.data && queue.data.totalElements > 0 && (
|
||||||
@@ -609,6 +622,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
const rejectMut = useRejectDraft()
|
const rejectMut = useRejectDraft()
|
||||||
const withdrawMut = useWithdrawDraft()
|
const withdrawMut = useWithdrawDraft()
|
||||||
const [comment, setComment] = useState('')
|
const [comment, setComment] = useState('')
|
||||||
|
/** Inline error для reject reason — show when reviewer кликнул Reject с
|
||||||
|
* пустым comment'ом. Реально cleared автоматически когда user печатает. */
|
||||||
|
const [rejectError, setRejectError] = useState<string | null>(null)
|
||||||
|
/** Brief success affordance после approve/reject mutation — drawer
|
||||||
|
* остаётся 600ms с зелёной/красной полосой, чтобы reviewer почувствовал
|
||||||
|
* что click registered. Без этого drawer закрывается мгновенно, click
|
||||||
|
* feedback теряется. */
|
||||||
|
const [decisionFlash, setDecisionFlash] = useState<'approve' | 'reject' | null>(null)
|
||||||
|
|
||||||
// Identity gates — определяют что показать в footer'е drawer'а.
|
// Identity gates — определяют что показать в footer'е drawer'а.
|
||||||
//
|
//
|
||||||
@@ -627,8 +648,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
{ id: draftId, comment: comment.trim() || undefined },
|
{ id: draftId, comment: comment.trim() || undefined },
|
||||||
{
|
{
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
|
setDecisionFlash('approve')
|
||||||
|
// Hold drawer 600ms так чтобы reviewer увидел зелёную полосу;
|
||||||
|
// потом reset state и close. Без флэша approve было instant.
|
||||||
|
setTimeout(() => {
|
||||||
setComment('')
|
setComment('')
|
||||||
|
setDecisionFlash(null)
|
||||||
onClose()
|
onClose()
|
||||||
|
}, 600)
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@@ -636,13 +663,29 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
|
|
||||||
const handleReject = () => {
|
const handleReject = () => {
|
||||||
if (!draftId) return
|
if (!draftId) return
|
||||||
if (!comment.trim()) return // backend will 400, but fail fast in UI
|
if (!comment.trim()) {
|
||||||
|
// Раньше return был silent — reviewer кликал Reject и ничего не
|
||||||
|
// происходило. Backend всё равно вернёт 400 missing_reject_reason,
|
||||||
|
// но user-friendly inline error лучше.
|
||||||
|
setRejectError(
|
||||||
|
t('reviews.drawer.rejectReasonRequired', {
|
||||||
|
defaultValue:
|
||||||
|
'Укажите причину отклонения — comment обязателен для reject (compliance).',
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
setRejectError(null)
|
||||||
rejectMut.mutate(
|
rejectMut.mutate(
|
||||||
{ id: draftId, comment: comment.trim() },
|
{ id: draftId, comment: comment.trim() },
|
||||||
{
|
{
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
|
setDecisionFlash('reject')
|
||||||
|
setTimeout(() => {
|
||||||
setComment('')
|
setComment('')
|
||||||
|
setDecisionFlash(null)
|
||||||
onClose()
|
onClose()
|
||||||
|
}, 600)
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@@ -828,7 +871,16 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{isPending && !isMineAndPending && (
|
{isPending && !isMineAndPending && (
|
||||||
<div className="border-t border-line pt-3 space-y-2">
|
<div
|
||||||
|
className={
|
||||||
|
'border-t pt-3 space-y-2 transition-colors duration-300 -mx-4 px-4 ' +
|
||||||
|
(decisionFlash === 'approve'
|
||||||
|
? 'border-green bg-green-bg/30'
|
||||||
|
: decisionFlash === 'reject'
|
||||||
|
? 'border-pink bg-pink-bg/30'
|
||||||
|
: 'border-line')
|
||||||
|
}
|
||||||
|
>
|
||||||
<label
|
<label
|
||||||
htmlFor="review-comment"
|
htmlFor="review-comment"
|
||||||
className="text-cap text-ink-2"
|
className="text-cap text-ink-2"
|
||||||
@@ -838,9 +890,24 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
<TextInput
|
<TextInput
|
||||||
id="review-comment"
|
id="review-comment"
|
||||||
value={comment}
|
value={comment}
|
||||||
onChange={(e) => setComment(e.target.value)}
|
onChange={(e) => {
|
||||||
|
setComment(e.target.value)
|
||||||
|
// Stale validation error gone как только user печатает.
|
||||||
|
if (rejectError && e.target.value.trim()) setRejectError(null)
|
||||||
|
}}
|
||||||
placeholder={t('reviews.drawer.commentPlaceholder')}
|
placeholder={t('reviews.drawer.commentPlaceholder')}
|
||||||
|
aria-invalid={Boolean(rejectError)}
|
||||||
|
aria-describedby={rejectError ? 'review-comment-error' : undefined}
|
||||||
/>
|
/>
|
||||||
|
{rejectError && (
|
||||||
|
<p
|
||||||
|
id="review-comment-error"
|
||||||
|
role="alert"
|
||||||
|
className="text-cell text-pink"
|
||||||
|
>
|
||||||
|
{rejectError}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
<div className="flex items-center justify-end gap-2 pt-2">
|
<div className="flex items-center justify-end gap-2 pt-2">
|
||||||
<Button
|
<Button
|
||||||
variant="secondary"
|
variant="secondary"
|
||||||
@@ -853,7 +920,7 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
|||||||
variant="danger"
|
variant="danger"
|
||||||
leftIcon={<XCircleIcon weight="bold" size={14} />}
|
leftIcon={<XCircleIcon weight="bold" size={14} />}
|
||||||
onClick={handleReject}
|
onClick={handleReject}
|
||||||
disabled={!comment.trim() || rejectMut.isPending || approveMut.isPending}
|
disabled={rejectMut.isPending || approveMut.isPending}
|
||||||
loading={rejectMut.isPending}
|
loading={rejectMut.isPending}
|
||||||
>
|
>
|
||||||
{t('reviews.action.reject')}
|
{t('reviews.action.reject')}
|
||||||
|
|||||||
Reference in New Issue
Block a user