-
Notifications
You must be signed in to change notification settings - Fork 3
Nachtragshaushaltsplan (NHHP): change-set overlay, apply/revert, editor & view #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/4.5.0
Are you sure you want to change the base?
Changes from 33 commits
067b535
8120093
2ece216
658ec8e
ddf280b
e29844a
84663ca
3dda8f1
b6b6470
982a862
d426ef4
ed726ab
212f2d9
a15ef25
780ebf5
b9860b4
a8944c0
28ecb29
a4cf3c0
a7a9f7d
1722094
cbc13ee
5c3f7ee
2116de8
78cedfc
08f39dd
db60bdc
cb667fd
747c98c
6506cbb
1cc8c7e
b34d250
97be360
e0c48a8
c96f709
a397d10
e8be09e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| <?php | ||
|
|
||
| namespace App\Console\Commands\stufis; | ||
|
|
||
| use App\Models\BudgetPlan; | ||
| use App\States\BudgetPlan\Active; | ||
| use App\States\BudgetPlan\Approved; | ||
| use App\Support\Budget\AmendmentConflictException; | ||
| use Illuminate\Console\Command; | ||
| use Illuminate\Support\Facades\Log; | ||
| use Spatie\ModelStates\Exceptions\CouldNotPerformTransition; | ||
| use Throwable; | ||
|
|
||
| /** | ||
| * Scheduled effectiveness for Nachtragshaushaltspläne (amendments, OP#581): an approved amendment | ||
| * with an `effective_date` in the past should go live on its own, without someone manually | ||
| * clicking "aktivieren" on the day. Runs daily (see routes/console.php). | ||
| * | ||
| * Every due amendment is transitioned independently, so one amendment's conflict (e.g. a stale | ||
| * item, or its parent plan no longer being Active) doesn't block the others. Failures are logged | ||
| * and reported on stderr; the command exits non-zero when any amendment failed, so the run stays | ||
| * visible and the schedule's failure hooks can act on it — the run is safely re-triggerable, since | ||
| * only successfully-applied amendments leave Approved. | ||
| */ | ||
| class ApplyDueAmendments extends Command | ||
| { | ||
| /** | ||
| * The name and signature of the console command. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $signature = 'stufis:apply-due-amendments'; | ||
|
|
||
| /** | ||
| * The console command description. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $description = 'Activate approved Nachtragshaushaltspläne (amendments) whose effective_date has arrived'; | ||
|
|
||
| public function handle(): int | ||
| { | ||
| $due = BudgetPlan::query() | ||
| ->whereNotNull('parent_plan_id') | ||
| ->where('state', Approved::$name) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there should be a whereState() from spatie with a cleaner signature |
||
| ->whereNotNull('effective_date') | ||
| ->whereDate('effective_date', '<=', today()) | ||
| ->get() | ||
| ->filter(fn (BudgetPlan $amendment): bool => $amendment->parentPlan?->state instanceof Active); | ||
|
|
||
| if ($due->isEmpty()) { | ||
| $this->info('No due amendments to activate.'); | ||
|
|
||
| return self::SUCCESS; | ||
| } | ||
|
|
||
| $failed = 0; | ||
| foreach ($due as $amendment) { | ||
| try { | ||
| $amendment->state->transitionTo(Active::class); | ||
| $this->info("Activated amendment #{$amendment->id} ({$amendment->label()})."); | ||
| } catch (AmendmentConflictException|CouldNotPerformTransition $e) { | ||
| $failed++; | ||
| $this->error("Amendment #{$amendment->id} could not be activated: {$e->getMessage()}"); | ||
| Log::warning('stufis:apply-due-amendments failed for amendment', [ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. back to OP WP: how should a failed apply be notified. To whom, and when, and how often? |
||
| 'amendment_id' => $amendment->id, | ||
| 'message' => $e->getMessage(), | ||
| ]); | ||
| } catch (Throwable $e) { | ||
| $failed++; | ||
| $this->error("Amendment #{$amendment->id} could not be activated: {$e->getMessage()}"); | ||
| Log::error('stufis:apply-due-amendments unexpected failure for amendment', [ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
| 'amendment_id' => $amendment->id, | ||
| 'exception' => $e, | ||
| ]); | ||
| } | ||
| } | ||
|
|
||
| return $failed > 0 ? self::FAILURE : self::SUCCESS; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,19 @@ public function budgetPlan(): BelongsTo | |
| return $this->belongsTo(BudgetPlan::class, 'budget_plan_id'); | ||
| } | ||
|
|
||
| /** | ||
| * Every BudgetItemChange row currently pointing at this item, across however many amendments | ||
| * touch it in parallel — OP#581 explicitly allows that (the unique key is | ||
| * (budget_plan_id, budget_item_id), not budget_item_id alone). For modify/delete this is | ||
| * always the live item (see BudgetItemChange's class doc: nothing is copied, budget_item_id | ||
| * points straight at this row); for add it only resolves here once AmendmentApplier has | ||
| * rehomed the new item onto this row's plan. | ||
| */ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this comment more relevant for readers in the future. It explains very verbose why its HasMany, it explains the design very complicated |
||
| public function amendmentChanges(): HasMany | ||
| { | ||
| return $this->hasMany(BudgetItemChange::class, 'budget_item_id'); | ||
| } | ||
|
|
||
| /** The plan this item "mounts" (only set for mount items). */ | ||
| public function referencedPlan(): BelongsTo | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| <?php | ||
|
|
||
| namespace App\Models; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
| use Illuminate\Database\Eloquent\Relations\BelongsTo; | ||
| use Illuminate\Support\Carbon; | ||
|
|
||
| /** | ||
| * App\Models\BudgetItemChange | ||
| * | ||
| * One delta row of a Nachtragshaushaltsplan (amendment) against a single budget_item, keyed by | ||
| * (budget_plan_id, budget_item_id) — see the Architecture section of OP#581 for the full change-set | ||
| * design. `action` is one of: | ||
| * | ||
| * - modify: the item already existed on the parent plan; `diff` holds | ||
| * {field: {"from": ..., "to": ...}} for every touched field. | ||
| * - add: `budget_item_id` points at a real BudgetItem row created under the amendment plan; | ||
| * `diff` is typically empty (the item itself carries the new data). | ||
| * - delete: `budget_item_id` points at a live item slated for removal (only allowed when it has | ||
| * no bookings); `diff` is typically empty. | ||
| * | ||
| * The column is named `diff`, not `changes`: Eloquent's own HasAttributes trait already declares a | ||
| * `protected $changes` property for its dirty-tracking bookkeeping, and a `changes` column silently | ||
| * shadows it when read from INSIDE the model (magic `__get()` only kicks in for external access, so | ||
| * `$this->changes` there would read Eloquent's internal array instead of the cast attribute). This | ||
| * already caused a production bug once; renaming the column removes the trap entirely instead of | ||
| * routing around it. | ||
| * | ||
| * @property int $id | ||
| * @property int $budget_plan_id | ||
| * @property int $budget_item_id | ||
| * @property string $action | ||
| * @property array<string, array{from: mixed, to: mixed}>|null $diff | ||
| * @property string|null $reason | ||
| * @property Carbon $created_at | ||
| * @property Carbon $updated_at | ||
| * @property-read BudgetPlan $amendmentPlan | ||
| * @property-read BudgetItem $budgetItem | ||
| */ | ||
| class BudgetItemChange extends Model | ||
| { | ||
| public const string ACTION_MODIFY = 'modify'; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shall we have enums for that? |
||
|
|
||
| public const string ACTION_ADD = 'add'; | ||
|
|
||
| public const string ACTION_DELETE = 'delete'; | ||
|
|
||
| protected $table = 'budget_item_change'; | ||
|
|
||
| protected $fillable = ['budget_plan_id', 'budget_item_id', 'action', 'diff', 'reason']; | ||
|
|
||
| #[\Override] | ||
| protected function casts(): array | ||
| { | ||
| return [ | ||
| 'diff' => 'array', | ||
| ]; | ||
| } | ||
|
|
||
| public function amendmentPlan(): BelongsTo | ||
| { | ||
| return $this->belongsTo(BudgetPlan::class, 'budget_plan_id'); | ||
| } | ||
|
|
||
| public function budgetItem(): BelongsTo | ||
| { | ||
| return $this->belongsTo(BudgetItem::class, 'budget_item_id'); | ||
| } | ||
|
|
||
| /** | ||
| * The {from, to} pair recorded for a single field, or null when this change row doesn't | ||
| * (or no longer) touches that field. | ||
| * | ||
| * @return array{from: mixed, to: mixed}|null | ||
| */ | ||
| public function fieldChange(string $field): ?array | ||
| { | ||
| return $this->diff[$field] ?? null; | ||
| } | ||
|
|
||
| /** Whether this row currently touches any field at all (an empty `diff` should be pruned). */ | ||
| public function isEmpty(): bool | ||
| { | ||
| return $this->action === self::ACTION_MODIFY && blank($this->diff); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a more fluent selector to the model