fix: order status dropdown could bypass confirm-gated cancel/refund
Some checks failed
Architecture Governance / architecture (push) Has been cancelled
Some checks failed
Architecture Governance / architecture (push) Has been cancelled
The status <select> on the order detail page let an admin jump straight to 'cancelled'/'refunded' with no confirmation, bypassing the dedicated cancel()/requestRefund() buttons that do confirm. It also stayed editable after an order reached a terminal status, so it could be moved backward out of cancelled/refunded. - Dropdown options now exclude terminal statuses; reaching them requires the confirm-gated buttons. - setStatus() guards against a terminal status slipping through regardless. - Once an order is terminal (isTerminal(), already computed but unused), the dropdown and both action buttons are disabled. - Same fix applied to the orders list page's bulk status dropdown, which had the identical gap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -46,14 +46,17 @@
|
|||||||
</div>
|
</div>
|
||||||
<div class="card no-print">
|
<div class="card no-print">
|
||||||
<h3>{{ 'adminOrders.changeStatus' | translate }}</h3>
|
<h3>{{ 'adminOrders.changeStatus' | translate }}</h3>
|
||||||
<select [attr.aria-label]="'adminOrders.changeStatus' | translate" [ngModel]="order.status" (ngModelChange)="setStatus(order.id, $event)">
|
<select [attr.aria-label]="'adminOrders.changeStatus' | translate" [ngModel]="order.status" (ngModelChange)="setStatus(order.id, $event)" [disabled]="isTerminal()">
|
||||||
@for (status of statuses; track status) {
|
@for (status of selectableStatuses; track status) {
|
||||||
<option [value]="status">{{ ('adminOrders.status.' + status) | translate }}</option>
|
<option [value]="status">{{ ('adminOrders.status.' + status) | translate }}</option>
|
||||||
}
|
}
|
||||||
|
@if (isTerminal()) {
|
||||||
|
<option [value]="order.status">{{ ('adminOrders.status.' + order.status) | translate }}</option>
|
||||||
|
}
|
||||||
</select>
|
</select>
|
||||||
<div class="actions">
|
<div class="actions">
|
||||||
<app-button variant="secondary" size="sm" (click)="requestRefund(order.id)">{{ 'adminOrders.requestRefund' | translate }}</app-button>
|
<app-button variant="secondary" size="sm" (click)="requestRefund(order.id)" [disabled]="isTerminal()">{{ 'adminOrders.requestRefund' | translate }}</app-button>
|
||||||
<app-button variant="danger" size="sm" (click)="cancel(order.id)">{{ 'adminOrders.cancelOrder' | translate }}</app-button>
|
<app-button variant="danger" size="sm" (click)="cancel(order.id)" [disabled]="isTerminal()">{{ 'adminOrders.cancelOrder' | translate }}</app-button>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</section>
|
</section>
|
||||||
|
|||||||
@@ -30,6 +30,8 @@ export class AdminOrderDetailPageComponent {
|
|||||||
private readonly translate = inject(TranslateService);
|
private readonly translate = inject(TranslateService);
|
||||||
|
|
||||||
readonly statuses: AdminOrderStatus[] = ['pending', 'processing', 'shipped', 'delivered', 'cancelled', 'refunded'];
|
readonly statuses: AdminOrderStatus[] = ['pending', 'processing', 'shipped', 'delivered', 'cancelled', 'refunded'];
|
||||||
|
/** Terminal statuses are only reachable via the confirm-gated cancel()/requestRefund(), never the raw dropdown. */
|
||||||
|
readonly selectableStatuses: AdminOrderStatus[] = this.statuses.filter(status => !TERMINAL_STATUSES.includes(status));
|
||||||
readonly workflowSteps = WORKFLOW_STEPS;
|
readonly workflowSteps = WORKFLOW_STEPS;
|
||||||
readonly noteDraft = signal('');
|
readonly noteDraft = signal('');
|
||||||
readonly internalNoteDraft = signal('');
|
readonly internalNoteDraft = signal('');
|
||||||
@@ -74,6 +76,11 @@ export class AdminOrderDetailPageComponent {
|
|||||||
}
|
}
|
||||||
|
|
||||||
setStatus(id: string, status: AdminOrderStatus): void {
|
setStatus(id: string, status: AdminOrderStatus): void {
|
||||||
|
if (TERMINAL_STATUSES.includes(status)) {
|
||||||
|
// Unreachable from the dropdown (options are filtered), but guard anyway
|
||||||
|
// since terminal transitions must always go through the confirm dialog.
|
||||||
|
return;
|
||||||
|
}
|
||||||
this.facade.setStatus(id, status);
|
this.facade.setStatus(id, status);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -37,8 +37,8 @@
|
|||||||
<div class="bulk-actions">
|
<div class="bulk-actions">
|
||||||
<span>{{ facade.selectedIds().length }} {{ 'adminProducts.selectedCount' | translate }}</span>
|
<span>{{ facade.selectedIds().length }} {{ 'adminProducts.selectedCount' | translate }}</span>
|
||||||
<select [attr.aria-label]="'adminOrders.changeStatus' | translate" [ngModel]="bulkStatusValue()" (ngModelChange)="bulkStatusValue.set($event)">
|
<select [attr.aria-label]="'adminOrders.changeStatus' | translate" [ngModel]="bulkStatusValue()" (ngModelChange)="bulkStatusValue.set($event)">
|
||||||
@for (status of statuses; track status) {
|
@for (status of bulkSelectableStatuses; track status) {
|
||||||
@if (status !== 'all') { <option [value]="status">{{ ('adminOrders.status.' + status) | translate }}</option> }
|
<option [value]="status">{{ ('adminOrders.status.' + status) | translate }}</option>
|
||||||
}
|
}
|
||||||
</select>
|
</select>
|
||||||
<app-button variant="secondary" size="sm" (click)="applyBulkStatus()">{{ 'adminOrders.applyStatus' | translate }}</app-button>
|
<app-button variant="secondary" size="sm" (click)="applyBulkStatus()">{{ 'adminOrders.applyStatus' | translate }}</app-button>
|
||||||
|
|||||||
@@ -30,6 +30,8 @@ export class AdminOrdersListPageComponent {
|
|||||||
private readonly languageService = inject(LanguageService);
|
private readonly languageService = inject(LanguageService);
|
||||||
|
|
||||||
readonly statuses = ['all', 'pending', 'processing', 'shipped', 'delivered', 'cancelled', 'refunded'] as const;
|
readonly statuses = ['all', 'pending', 'processing', 'shipped', 'delivered', 'cancelled', 'refunded'] as const;
|
||||||
|
/** Bulk status change excludes terminal statuses - cancel/refund must go through the confirm-gated single-order flow. */
|
||||||
|
readonly bulkSelectableStatuses: AdminOrderStatus[] = ['pending', 'processing', 'shipped', 'delivered'];
|
||||||
readonly allColumns = ALL_ORDER_COLUMNS;
|
readonly allColumns = ALL_ORDER_COLUMNS;
|
||||||
protected readonly columnsPanelOpen = signal(false);
|
protected readonly columnsPanelOpen = signal(false);
|
||||||
protected readonly bulkStatusValue = signal<AdminOrderStatus>('pending');
|
protected readonly bulkStatusValue = signal<AdminOrderStatus>('pending');
|
||||||
|
|||||||
Reference in New Issue
Block a user