fix(approvals): usertest-panel hardening (0.81.0)
Panel findings against the reworked approvals page, fixed in place: - Never fabricate the request time: ApprovalRecord.createdAt is nullable now; a missing created_at omits the line instead of rendering DateTime.now() (which drifted on refresh). Guard: approvals_origin_test pins the omit-on-null invariant. - Copyable errors on approve/reject/batch via showChainErrorSnack (the hard project rule) — batch surfaces the first real cause. - Reject requires a reason: ChainInlineHelp strip + confirm disabled while empty, no more silent close-and-nothing-happens. - Batch approve applies the same no-data confirmation as the single path, naming how many selected requests carry no show: data. - Plainer language: glossary "Vorgang (Flow)", history label FRAGE (was PROMPT), no-data hint drops developer jargon. - One-click copy of the run id; history payload pretty-prints like the card. Signed-off-by: flemming-it <stefan.a.flemming@googlemail.com>
This commit is contained in:
flemming-it
parent
28f6fe1a9a
commit
f7d7427d91
9 changed files with 411 additions and 122 deletions
|
|
@ -2,7 +2,9 @@ import 'dart:convert';
|
|||
import 'dart:io';
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
|
||||
import '../data/error_presentation.dart';
|
||||
import '../data/hub.dart';
|
||||
import '../data/workspace.dart';
|
||||
import '../l10n/app_localizations.dart';
|
||||
|
|
@ -129,7 +131,9 @@ class _ApprovalsPageState extends State<ApprovalsPage> {
|
|||
_toast(l.approvalsApprovedToast(a.flowName, a.stepId));
|
||||
_refresh();
|
||||
} catch (e) {
|
||||
_toast(l.approvalsApproveFailed(e.toString()));
|
||||
if (!mounted) return;
|
||||
// Copyable error — never a bare SnackBar(Text(e)).
|
||||
showChainErrorSnack(context, 'approvals.approve', e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -142,7 +146,8 @@ class _ApprovalsPageState extends State<ApprovalsPage> {
|
|||
_toast(l.approvalsRejectedToast(a.flowName, a.stepId));
|
||||
_refresh();
|
||||
} catch (e) {
|
||||
_toast(l.approvalsRejectFailed(e.toString()));
|
||||
if (!mounted) return;
|
||||
showChainErrorSnack(context, 'approvals.reject', e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -169,25 +174,54 @@ class _ApprovalsPageState extends State<ApprovalsPage> {
|
|||
/// drains as items succeed.
|
||||
Future<void> _batchApprove(List<ApprovalRecord> picked) async {
|
||||
final l = AppLocalizations.of(context)!;
|
||||
// Same conscious-confirmation guard as the single approve, but for
|
||||
// the whole batch: a one-click bulk approve must not silently
|
||||
// sweep through the data-less requests the single path stops on.
|
||||
final noData = picked.where((a) => a.payloadPreview == null).length;
|
||||
if (noData > 0) {
|
||||
final confirmed = await showDialog<bool>(
|
||||
context: context,
|
||||
builder: (ctx) => AlertDialog(
|
||||
title: Text(l.approvalsBatchNoDataTitle),
|
||||
content: Text(l.approvalsBatchNoDataBody(noData, picked.length)),
|
||||
actions: [
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(ctx, false),
|
||||
child: Text(l.buttonCancel),
|
||||
),
|
||||
FilledButton(
|
||||
onPressed: () => Navigator.pop(ctx, true),
|
||||
child: Text(l.approvalsNoDataConfirmAction),
|
||||
),
|
||||
],
|
||||
),
|
||||
);
|
||||
if (confirmed != true) return;
|
||||
}
|
||||
if (!mounted) return;
|
||||
setState(() => _batchInFlight = true);
|
||||
var ok = 0;
|
||||
var failed = 0;
|
||||
Object? firstError;
|
||||
for (final a in picked) {
|
||||
try {
|
||||
await HubService.instance.approve(a.id, _reviewer);
|
||||
ok += 1;
|
||||
_selectedIds.remove(a.id);
|
||||
} catch (_) {
|
||||
} catch (e) {
|
||||
failed += 1;
|
||||
firstError ??= e;
|
||||
}
|
||||
}
|
||||
if (!mounted) return;
|
||||
setState(() => _batchInFlight = false);
|
||||
_toast(
|
||||
failed == 0
|
||||
? l.approvalsBatchApproveDoneToast(ok)
|
||||
: l.approvalsBatchPartialFailure(ok, failed),
|
||||
);
|
||||
if (failed == 0) {
|
||||
_toast(l.approvalsBatchApproveDoneToast(ok));
|
||||
} else {
|
||||
// Partial failure: surface the copyable cause, not just a count.
|
||||
// The failed items stay selected so they remain visible.
|
||||
showChainErrorSnack(context, 'approvals.batchApprove', firstError!);
|
||||
}
|
||||
_refresh();
|
||||
}
|
||||
|
||||
|
|
@ -198,22 +232,24 @@ class _ApprovalsPageState extends State<ApprovalsPage> {
|
|||
setState(() => _batchInFlight = true);
|
||||
var ok = 0;
|
||||
var failed = 0;
|
||||
Object? firstError;
|
||||
for (final a in picked) {
|
||||
try {
|
||||
await HubService.instance.reject(a.id, _reviewer, reason);
|
||||
ok += 1;
|
||||
_selectedIds.remove(a.id);
|
||||
} catch (_) {
|
||||
} catch (e) {
|
||||
failed += 1;
|
||||
firstError ??= e;
|
||||
}
|
||||
}
|
||||
if (!mounted) return;
|
||||
setState(() => _batchInFlight = false);
|
||||
_toast(
|
||||
failed == 0
|
||||
? l.approvalsBatchRejectDoneToast(ok)
|
||||
: l.approvalsBatchPartialFailure(ok, failed),
|
||||
);
|
||||
if (failed == 0) {
|
||||
_toast(l.approvalsBatchRejectDoneToast(ok));
|
||||
} else {
|
||||
showChainErrorSnack(context, 'approvals.batchReject', firstError!);
|
||||
}
|
||||
_refresh();
|
||||
}
|
||||
|
||||
|
|
@ -229,26 +265,49 @@ class _ApprovalsPageState extends State<ApprovalsPage> {
|
|||
context: context,
|
||||
builder: (ctx) => AlertDialog(
|
||||
title: Text(l.approvalsRejectDialogTitle),
|
||||
content: TextField(
|
||||
controller: controller,
|
||||
autofocus: true,
|
||||
decoration: InputDecoration(
|
||||
labelText: l.approvalsRejectReasonLabel,
|
||||
border: const OutlineInputBorder(),
|
||||
),
|
||||
content: Column(
|
||||
mainAxisSize: MainAxisSize.min,
|
||||
crossAxisAlignment: CrossAxisAlignment.stretch,
|
||||
children: [
|
||||
// Explain the consequence at the point of input, and make
|
||||
// the reason a real required field: the button stays
|
||||
// disabled while it is empty instead of the dialog closing
|
||||
// silently and nothing happening (usertest finding).
|
||||
ChainInlineHelp(text: l.approvalsRejectDialogHelp),
|
||||
const SizedBox(height: ChainSpace.md),
|
||||
TextField(
|
||||
controller: controller,
|
||||
autofocus: true,
|
||||
minLines: 1,
|
||||
maxLines: 3,
|
||||
decoration: InputDecoration(
|
||||
labelText: l.approvalsRejectReasonLabel,
|
||||
helperText: l.approvalsRejectReasonHelper,
|
||||
border: const OutlineInputBorder(),
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
actions: [
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(ctx, null),
|
||||
child: Text(l.buttonCancel),
|
||||
),
|
||||
FilledButton(
|
||||
onPressed: () => Navigator.pop(ctx, controller.text),
|
||||
style: FilledButton.styleFrom(
|
||||
backgroundColor: Theme.of(ctx).colorScheme.error,
|
||||
foregroundColor: Theme.of(ctx).colorScheme.onError,
|
||||
),
|
||||
child: Text(l.approvalsRejectButton),
|
||||
ValueListenableBuilder<TextEditingValue>(
|
||||
valueListenable: controller,
|
||||
builder: (ctx, value, _) {
|
||||
final empty = value.text.trim().isEmpty;
|
||||
return FilledButton(
|
||||
onPressed: empty
|
||||
? null
|
||||
: () => Navigator.pop(ctx, controller.text.trim()),
|
||||
style: FilledButton.styleFrom(
|
||||
backgroundColor: Theme.of(ctx).colorScheme.error,
|
||||
foregroundColor: Theme.of(ctx).colorScheme.onError,
|
||||
),
|
||||
child: Text(l.approvalsRejectButton),
|
||||
);
|
||||
},
|
||||
),
|
||||
],
|
||||
),
|
||||
|
|
@ -687,23 +746,58 @@ class _ApprovalCard extends StatelessWidget {
|
|||
style: metaStyle,
|
||||
),
|
||||
),
|
||||
originLine(
|
||||
Icons.schedule,
|
||||
Text(
|
||||
'${l.approvalsOriginRequested}: '
|
||||
'${_formatTimestamp(approval.createdAt.toLocal())}',
|
||||
style: metaStyle,
|
||||
if (approval.createdAt != null)
|
||||
originLine(
|
||||
Icons.schedule,
|
||||
Text(
|
||||
'${l.approvalsOriginRequested}: '
|
||||
'${_formatTimestamp(approval.createdAt!.toLocal())}',
|
||||
style: metaStyle,
|
||||
),
|
||||
),
|
||||
),
|
||||
if (approval.flowExecution != null)
|
||||
originLine(
|
||||
Icons.tag_outlined,
|
||||
SelectableText(
|
||||
'${l.approvalsOriginRun}: ${approval.flowExecution}',
|
||||
style: ChainTheme.mono(
|
||||
size: 11,
|
||||
color: theme.colorScheme.onSurfaceVariant,
|
||||
),
|
||||
Row(
|
||||
children: [
|
||||
Expanded(
|
||||
child: SelectableText(
|
||||
'${l.approvalsOriginRun}: ${approval.flowExecution}',
|
||||
style: ChainTheme.mono(
|
||||
size: 11,
|
||||
color: theme.colorScheme.onSurfaceVariant,
|
||||
),
|
||||
),
|
||||
),
|
||||
// One-click copy so the run id can be pasted into the
|
||||
// Audit page's search to reach the event timeline
|
||||
// (a filtered deeplink is a larger shell change — see
|
||||
// the backlog).
|
||||
Tooltip(
|
||||
message: l.approvalsCopyRun,
|
||||
child: InkResponse(
|
||||
radius: 16,
|
||||
onTap: () async {
|
||||
await Clipboard.setData(
|
||||
ClipboardData(text: approval.flowExecution!),
|
||||
);
|
||||
if (context.mounted) {
|
||||
ScaffoldMessenger.of(context).showSnackBar(
|
||||
SnackBar(content: Text(l.approvalsRunCopied)),
|
||||
);
|
||||
}
|
||||
},
|
||||
child: Padding(
|
||||
padding: const EdgeInsets.all(2),
|
||||
child: Icon(
|
||||
Icons.content_copy,
|
||||
size: 13,
|
||||
color: theme.colorScheme.onSurfaceVariant,
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
const SizedBox(height: ChainSpace.md),
|
||||
|
|
@ -733,7 +827,7 @@ class _ApprovalCard extends StatelessWidget {
|
|||
),
|
||||
child: SingleChildScrollView(
|
||||
child: SelectableText(
|
||||
_prettyPreview(approval.payloadPreview!),
|
||||
_prettyJson(approval.payloadPreview!),
|
||||
style: ChainTheme.mono(
|
||||
size: 11,
|
||||
color: theme.colorScheme.onSurface,
|
||||
|
|
@ -813,16 +907,6 @@ class _ApprovalCard extends StatelessWidget {
|
|||
);
|
||||
}
|
||||
|
||||
String _prettyPreview(String raw) {
|
||||
if (raw.isEmpty) return raw;
|
||||
try {
|
||||
final dynamic parsed = const JsonDecoder().convert(raw);
|
||||
return const JsonEncoder.withIndent(' ').convert(parsed);
|
||||
} catch (_) {
|
||||
return raw;
|
||||
}
|
||||
}
|
||||
|
||||
String _expiresInLabel(BuildContext context, DateTime t) {
|
||||
final l = AppLocalizations.of(context)!;
|
||||
final remaining = t.difference(DateTime.now());
|
||||
|
|
@ -955,13 +1039,14 @@ class _HistoryDialog extends StatelessWidget {
|
|||
theme,
|
||||
AppLocalizations.of(context)!.approvalsDialogDecided,
|
||||
'${_formatTimestamp(record.decidedAt!.toLocal())} '
|
||||
'· ${record.decidedBy.isEmpty ? "(unknown)" : record.decidedBy}',
|
||||
'· ${record.decidedBy.isEmpty ? AppLocalizations.of(context)!.approvalsUnknownReviewer : record.decidedBy}',
|
||||
),
|
||||
if (record.createdAt != null)
|
||||
_kv(
|
||||
theme,
|
||||
AppLocalizations.of(context)!.approvalsDialogCreated,
|
||||
_formatTimestamp(record.createdAt!.toLocal()),
|
||||
),
|
||||
_kv(
|
||||
theme,
|
||||
AppLocalizations.of(context)!.approvalsDialogCreated,
|
||||
_formatTimestamp(record.createdAt.toLocal()),
|
||||
),
|
||||
if (record.project.isNotEmpty)
|
||||
_kv(
|
||||
theme,
|
||||
|
|
@ -1015,7 +1100,7 @@ class _HistoryDialog extends StatelessWidget {
|
|||
border: Border.all(color: theme.colorScheme.outlineVariant),
|
||||
),
|
||||
child: SelectableText(
|
||||
record.payloadPreview!,
|
||||
_prettyJson(record.payloadPreview!),
|
||||
style: ChainTheme.mono(size: 11),
|
||||
),
|
||||
),
|
||||
|
|
@ -1064,6 +1149,19 @@ class _HistoryDialog extends StatelessWidget {
|
|||
}
|
||||
}
|
||||
|
||||
/// Pretty-print a JSON payload preview; returns the raw string
|
||||
/// unchanged when it is empty or not valid JSON. Shared by the
|
||||
/// pending card + the history dialog so both render identically.
|
||||
String _prettyJson(String raw) {
|
||||
if (raw.isEmpty) return raw;
|
||||
try {
|
||||
final dynamic parsed = const JsonDecoder().convert(raw);
|
||||
return const JsonEncoder.withIndent(' ').convert(parsed);
|
||||
} catch (_) {
|
||||
return raw;
|
||||
}
|
||||
}
|
||||
|
||||
/// Locale-unambiguous YYYY-MM-DD HH:mm:ss formatter shared by
|
||||
/// the history row + dialog. Always renders in the operator's
|
||||
/// local time zone — ISO timestamps from the wire still appear
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue