refactor(budget-plan): give editability one home and delete through the policy

Review feedback on PR #329: BudgetPlan::isEditable() sat between business logic
and authorization, with the rule re-derived at each call site.

The state now owns it — BudgetPlanState::isEditable() is false by default and
overridden to true in Draft and Resolved, beside isAdvancement() and
isCascadingArc(). BudgetPlanPolicy::update() and delete() delegate to it, and
BudgetPlan::isEditable() is a thin forwarder that folds in an amendment's
stricter Draft-only rule so ⚡amendment-edit stops keeping its own copy.

Plan deletion goes through the policy rather than around it. deletePlan() had
authorized 'admin' and then repeated the state check with its own
abort_unless(); it now authorizes 'delete' once, so the server-side rule and the
modal's checklist cannot drift apart. Deleting a plan consequently takes the
budget officer role (ref-finanzen-hv) instead of admin — admins still pass via
UserPolicy::before(). Nothing released changes, since the whole delete flow is
new in 4.5.0.

Two call sites needed care while narrowing 'update':

  - ⚡plan-edit::mount() now runs its state check before authorize(), so a
    budget officer opening a frozen plan still gets the redirect to the
    read-only view rather than a bare 403.
  - The "change state" menu item had been using @can('update') as a stand-in for
    "is budget officer". Left alone it would have vanished on every Approved,
    Active and Completed plan — exactly where transitioning matters — so it now
    checks the role directly, matching what transitionTo() enforces.

Coverage for the new rule: a budget officer may delete, a plain user may not and
never sees the modal, admins still may, and the state gate still holds. The
existing "forbids a non-admin from deleting the plan" case asserted the rule
this commit deliberately replaces and now covers a user without the role.

Refs: OP#581

Author: lukas-staab

app/Models/BudgetPlan.php

@@ -8,7 +8,6 @@
 use App\States\BudgetPlan\BudgetPlanState;
 use App\States\BudgetPlan\Completed;
 use App\States\BudgetPlan\Draft;
-use App\States\BudgetPlan\Resolved;
 use App\Support\Budget\AmendmentDeltaSummary;
 use Carbon\Carbon;
 use Cknow\Money\Money;
@@ -170,15 +169,13 @@ public function amendmentDeltaSummary(): array
     }
 
     /**
-     * Whether a normal (non-amendment) plan may still go through ⚡plan-edit (F8, OP#581):
-     * editable in Draft/Resolved, frozen from Approved onward — Approved is the point past which
-     * the plan is meant to be a stable, agreed-upon document, and Active/Completed plans are live
-     * or done. An amendment follows its own, stricter rule (Draft only) enforced directly in
-     * ⚡amendment-edit, not this method.
+     * Whether this plan may still go through ⚡plan-edit / ⚡amendment-edit (F8, OP#581). Delegates
+     * to the state (see BudgetPlanState::isEditable() for the Draft/Resolved rule and its
+     * rationale), plus an amendment's own stricter constraint: Draft only.
      */
     public function isEditable(): bool
     {
-        return $this->state instanceof Draft || $this->state instanceof Resolved;
+        return $this->state->isEditable() && (! $this->isAmendment() || $this->state instanceof Draft);
     }
 
     /** Whether this plan is an amendment (supplements another plan) rather than an original plan. */

app/Policies/BudgetPlanPolicy.php

@@ -28,12 +28,12 @@ public function create(User $user): bool
 
     public function update(User $user, BudgetPlan $budgetPlan): bool
     {
-        return $user->can('budget-officer', User::class);
+        return $user->can('budget-officer', User::class) && $budgetPlan->state->isEditable();
     }
 
     public function delete(User $user, BudgetPlan $budgetPlan): bool
     {
-        return $user->can('budget-officer', User::class);
+        return $user->can('budget-officer', User::class) && $budgetPlan->state->isEditable();
     }
 
     public function transitionTo(User $user, BudgetPlan $budgetPlan, BudgetPlanState $newState): bool

app/States/BudgetPlan/BudgetPlanState.php

@@ -115,6 +115,16 @@ public function isCascadingArc(BudgetPlanState $target): bool
             || ($this instanceof Completed && $target instanceof Active);
     }
 
+    /**
+     * Whether a plan may still be edited while in this state (F8, OP#581). Approved is the point
+     * past which the plan is meant to be a stable, agreed-upon document, so only Draft and
+     * Resolved override this to true — Active/Completed plans are live or done and stay frozen.
+     */
+    public function isEditable(): bool
+    {
+        return false;
+    }
+
     /**
      * Business-rule checks a plan's budget items must satisfy to legitimately BE in this state
      * (OP#584): short_name (Titelnummer) unique within scope, name non-empty, value non-negative.

app/States/BudgetPlan/Draft.php

@@ -18,6 +18,12 @@ public function color(): string
         return 'zinc';
     }
 
+    #[\Override]
+    public function isEditable(): bool
+    {
+        return true;
+    }
+
     // No itemRules() override: Draft is rank 0 in BudgetPlanState::order(), so no transition ever
     // advances INTO it — ⚡plan-view::changeState() only checks item rules on a forward step (see
     // BudgetPlanState::isAdvancement()), meaning Draft's own itemRules() is simply never consulted.

app/States/BudgetPlan/Resolved.php

@@ -17,4 +17,10 @@ public function color(): string
     {
         return 'sky';
     }
+
+    #[\Override]
+    public function isEditable(): bool
+    {
+        return true;
+    }
 }

lang/de/budget-plan.php

@@ -119,7 +119,7 @@
     'view.delete-confirm' => 'Diesen Haushaltsplan und alle seine Titel unwiderruflich löschen?',
     'view.delete-modal.heading' => 'Haushaltsplan löschen',
     'view.delete-modal.intro' => 'Bevor der Haushaltsplan gelöscht werden kann, müssen folgende Bedingungen erfüllt sein:',
-    'view.delete-modal.conditions.admin' => 'Du bist als Administrator/in zum Löschen berechtigt.',
+    'view.delete-modal.conditions.role' => 'Du bist als Haushaltsverantwortliche*r zum Löschen berechtigt.',
     'view.delete-modal.conditions.editable-state' => 'Der Plan befindet sich in einem löschbaren Status (Entwurf oder Beschlossen) – aktuell: :state.',
     'view.delete-modal.cancel' => 'Abbrechen',
     'view.delete-modal.confirm' => 'Endgültig löschen',

resources/views/pages/budget-plan/⚡amendment-edit/amendment-edit.php

@@ -6,7 +6,6 @@
 use App\Models\BudgetPlan;
 use App\Models\Enums\BudgetItemChangeAction;
 use App\Models\Enums\BudgetType;
-use App\States\BudgetPlan\Draft;
 use App\Support\Budget\TitleNumberer;
 use App\Support\Money\DefaultMoneyFormater;
 use Cknow\Money\Money;
@@ -51,8 +50,10 @@ public function mount(int $plan_id, int $amendment_id): void
         abort_unless($amendment->parent_plan_id === $plan_id, 404);
 
         // the editor is only available while the amendment is still a draft — once it has moved
-        // on in its own workflow, plan-view's diff view is the read-only place to look at it
-        if (! ($amendment->state instanceof Draft)) {
+        // on in its own workflow, plan-view's diff view is the read-only place to look at it. Goes
+        // through the model's isEditable() (BudgetPlan::isEditable() folds in this amendment-only
+        // Draft-only rule) rather than re-checking the state directly.
+        if (! $amendment->isEditable()) {
             $this->redirect(route('budget-plan.view', $amendment->id), navigate: true);
 
             return;

resources/views/pages/budget-plan/⚡plan-edit/plan-edit.php

@@ -45,7 +45,6 @@
     public function mount(int $plan_id): void
     {
         $plan = BudgetPlan::findOrFail($plan_id);
-        $this->authorize('update', $plan);
 
         // an amendment is never edited here — its edits must go through the change-tracking
         // amendment editor, which lets the live parent-plan items stay untouched while drafting
@@ -56,13 +55,18 @@ public function mount(int $plan_id): void
         }
 
         // F8 (OP#581): once Approved (or beyond), the plan is a stable, agreed-upon document —
-        // direct route access is refused the same way an out-of-state amendment redirects away
+        // direct route access is refused the same way an out-of-state amendment redirects away.
+        // Checked before authorize() so a stale/bookmarked link degrades to this friendly redirect
+        // rather than a 403 — BudgetPlanPolicy::update() also enforces this same state rule, but
+        // only ever to refuse a non-officer, since by this point the plan is already editable.
         if (! $plan->isEditable()) {
             $this->redirect(route('budget-plan.view', $plan->id), navigate: true);
 
             return;
         }
 
+        $this->authorize('update', $plan);
+
         $this->organization = $plan->organization;
         $this->fiscal_year_id = $plan->fiscal_year_id;
 

resources/views/pages/budget-plan/⚡plan-view/plan-view.blade.php

@@ -41,8 +41,8 @@
                         <flux:menu.separator/>
                     @endif
                     @if($plan->isAmendment())
-                        {{-- an amendment is only editable through its dedicated editor, and only while Draft --}}
-                        @if($plan->state instanceof \App\States\BudgetPlan\Draft)
+                        {{-- an amendment is only editable through its dedicated editor, and only while Draft (see BudgetPlan::isEditable()) --}}
+                        @if($plan->isEditable())
                             <flux:menu.item icon="pencil"
                                             :href="route('budget-plan.amendment.edit', [$plan->parent_plan_id, $plan->id])" wire:navigate>{{ __('budget-plan.view.edit') }}</flux:menu.item>
                         @else
@@ -60,7 +60,10 @@
                             </div>
                         </flux:tooltip>
                     @endif
-                    @can('update', $plan)
+                    {{-- the officer role directly, not the 'update' ability: 'update' now also
+                         requires the plan to still be editable (Draft/Resolved), but a workflow
+                         transition (e.g. Approved -> Active) must stay reachable past that point too --}}
+                    @can('budget-officer', \App\Models\User::class)
                         <flux:menu.item icon="arrow-path" x-on:click="$flux.modal('state-modal').show()">
                             {{ __('budget-plan.view.change-state') }}
                         </flux:menu.item>
@@ -87,7 +90,7 @@
                             @endcan
                         @endif
                     </flux:menu.submenu>
-                    @can('admin', \App\Models\User::class)
+                    @can('budget-officer', \App\Models\User::class)
                         <flux:menu.separator/>
                         {{-- a native window.confirm() would be the only non-Flux dialog left in the
                              app (and is unstyleable), so this goes through a flux:modal like every
@@ -122,7 +125,7 @@
                                 {{ $amendment->label() }} — {{ $amendment->state->label() }}
                             </flux:link>
                             @can('update', $amendment)
-                                @if($amendment->state instanceof \App\States\BudgetPlan\Draft)
+                                @if($amendment->isEditable())
                                     ·
                                     <flux:link :href="route('budget-plan.amendment.edit', [$plan->id, $amendment->id])" wire:navigate>
                                         {{ __('budget-plan.amendment.continue-editing') }}
@@ -404,7 +407,7 @@ class="sm:px-6"
         </div>
     </flux:modal>
 
-    @can('admin', \App\Models\User::class)
+    @can('budget-officer', \App\Models\User::class)
         {{-- F5 (OP#589): same checklist pattern as ⚡show-project's delete-modal — a condition row
              per requirement, Confirm disabled until every one holds, rather than a bare
              heading + Cancel/Confirm. --}}
@@ -426,7 +429,7 @@ class="sm:px-6"
                             @else
                                 <x-fas-circle-xmark class="w-4 h-4 mt-0.5 shrink-0 fill-red-600"/>
                             @endif
-                            <span>{{ __('budget-plan.view.delete-modal.conditions.admin') }}</span>
+                            <span>{{ __('budget-plan.view.delete-modal.conditions.role') }}</span>
                         </li>
                         <li class="flex items-start gap-2">
                             @if($plan_deletable)

resources/views/pages/budget-plan/⚡plan-view/plan-view.php

@@ -115,7 +115,7 @@ public function with(): array
             // F5 (OP#589): the delete-plan-modal's checklist rows — surfaced here rather than
             // computed inline in the blade so deletePlan()'s server-side guard below reads
             // identically to what the user was shown.
-            'user_can_delete_plan' => Auth::user()?->can('admin', User::class) ?? false,
+            'user_can_delete_plan' => Auth::user()?->can('budget-officer', User::class) ?? false,
             'plan_deletable' => $plan->isEditable(),
         ];
     }
@@ -224,17 +224,14 @@ public function changeState(): void
     }
 
     /**
-     * Delete the whole plan and its items. Admin-only for now, and (F5, OP#589) only while the
-     * plan is still editable (Draft/Resolved) — past Approved it's meant to be a stable,
-     * agreed-upon document, so it may no longer be wiped outright. Mirrors the checklist rows
-     * shown in delete-plan-modal.
+     * Delete the whole plan and its items. Both conditions — budget officer, and a still-editable
+     * plan — live in BudgetPlanPolicy::delete(); the checklist rows in delete-plan-modal show the
+     * same two conditions separately so a blocked user can see which one fails.
      */
     public function deletePlan(): void
     {
-        $this->authorize('admin', User::class);
-
         $plan = $this->plan();
-        abort_unless($plan->isEditable(), 403);
+        $this->authorize('delete', $plan);
 
         DB::transaction(static function () use ($plan): void {
             // budget_item has a self-referencing parent_id FK and a plan FK without cascade;