feat: deactivation, two-factor administration, list price and history - #5
Merged
Merged
Conversation
Archiving instead of deleting for master data and vehicles, 2FA controls in user management, gross list price behind the cost gate, 'Termine' renamed to 'Historie' with accident and damage types, and the image upload fix for images below 2000 pixels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven tasks across two branches. The archiving approach changed during planning: the codebase already carries the state (vehicles.status with Aktiv/Ausgemustert, employees.active), so deletion is bound to that state instead of adding SoftDeletes on top, which would have produced three overlapping states on a vehicle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cost an hour and a wrong password hash in the dev database: quotes are already gone by the time the command reaches the container, so $argv in a php -r one-liner expands on the way in rather than inside PHP. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The state "active/inactive" existed in only two places: employees.active, where nothing acted on it, and vehicles.status as Aktiv/Ausgemustert. Deletion is about to depend on that state, so dealerships, leasing companies, fuel cards and fuel card providers need the same field. Deliberately not SoftDeletes. An extra deleted_at would give a vehicle three states (active, retired, deleted) and an employee two that sound alike. It would also mean guarding Vehicle::deleting(), which today wipes the image and documents off disk and fires on a soft delete just the same. Defaults to true so existing rows stay usable, mirrored in $attributes so a freshly instantiated model matches a freshly written row. The form test is not redundant with the model test: a factory bypasses mass assignment, so the model test would pass even with 'active' missing from $fillable. Table columns, the status filter and the actions follow in the next commit, where all six lists change together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deletion now depends on state: an active record offers "Deaktivieren", and only an inactive one can be deleted at all. Deactivating is always allowed, including for an employee who still has a vehicle assigned — someone leaves the company and the car stays with them until a successor turns up. The assignment survives. The two conditions on deletion are independent and both stay: the state decides whether deleting is offered, the foreign key decides whether it can succeed. GuardedDeleteAction still explains which vehicle is in the way, now under the heading "Endgültig löschen nicht möglich". The state column differs per model — vehicles carry an enum in `status`, master data a boolean `active` — so the three actions share a TracksActiveState trait rather than each reimplementing the comparison. Two things worth knowing about the vehicle: VehiclesTable has no delete action at all; a vehicle is deleted from the footer of its edit page, via a local deleteAction() on EditVehicle. Guarding only the tables would have left the rule bypassable at the one place where a vehicle actually gets deleted. "Ausmustern" replaces "Deaktivieren" there, because that is the word used for a vehicle leaving the fleet, and the list already filtered on VehicleStatus::Active by default — the five master data lists now mirror that with a TernaryFilter defaulting to active. The unique indexes on vehicles.license_plate and fuel_card_providers.name stay as they are; loosening them would allow the same plate twice in one table. Instead the validation message says the value may be held by a retired or inactive record, which is not visible in the default view. Eight existing tests deleted active records and now fail correctly. They deactivate first — the rule was not weakened to keep them green. One test in MasterDataActiveFlagTest failed intermittently (roughly one run in fifteen, always at assertHasNoFormErrors, so a validation error). Feeding 3000 factory rounds through the form's rules produced no invalid values, so the cause is not proven. The test now uses fixed field values instead of faker output and reports the actual error bag, which removes the only plausible source of randomness and makes any future failure name the field. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pickers on the vehicle form filter each master-data relation on active, but keep the currently assigned value in the option list even when it has turned inactive. Without that exception the form would not find the stored value in its options and quietly clear the assignment on the next save — a failure mode the test suite pins down. The option label carries a "(inaktiv)" suffix on such kept values, so the person editing sees why an unusual entry appears among otherwise active ones. The vehicle list mirrors this on the driver column. Global search grows a getGlobalSearchEloquentQuery() on each of the five master-data resources (filtering on active) and on VehicleResource (filtering on status). Otherwise search would remain the one way to re-surface an inactive record without meaning to. The fuel card provider select on the fuel card form gets the same treatment — an inactive provider does not belong on a new card, but the one already set must stay pickable for the same reason as the vehicle pickers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vehicles.gross_list_price, decimal(10, 2) nullable, sits inside the "Kosten" section of the vehicle form — the same hidden-section mechanism protects it that protects the other cost fields (CLAUDE.md, "Kostendaten"). A gross list price submitted by a user without canViewCosts is discarded on save; the test proves this by sending 1.00 and expecting the pre-existing 30000 to survive. The value appears in the cost overview table and the exporter between the running cost columns and the annual total, and on the vehicle detail page inside the same Kosten section. It is deliberately absent from annual_total_cost and from any summarizer — a list price is not a running position and does not make sense as a fleet-wide sum. Two column-order tests in VehicleCostExportVerificationTest picked up a column offset from the new export column; both now reference index 7 for Jahreskosten with a comment explaining why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The section that lived under "Termine" now shows as "Historie" — a planned service, a workshop visit, an inspection, an accident and a minor damage all share the same list, and "Termin" fits an accident poorly. Only labels change; VehicleAppointment, vehicle_appointments and AppointmentType stay as they are, since renaming them would cost a migration and change nothing users see. Accident maps to 'danger' (the panel's Rose): it should stand out among the calmer entry types. Damage maps to 'orange', freshly registered in AdminPanelProvider because 'warning' is already the inspection colour and every type should carry a distinct meaning through its badge. The two existing tests that pinned "Termine" as the visible label were updated to "Historie". The colour-uniqueness test now enforces both distinctness across all five types and the ban on 'primary' (which remains the brand red). The ban on 'danger' had to go — that is exactly where the accident belongs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two things an admin can do that they could not before, plus a note on what they still cannot. Reset — a per-row action on the user list, visible only when the target has 2FA set up and only for admins (UserPolicy::resetTwoFactor). It clears app_authentication_secret and app_authentication_recovery_codes on that one user, so someone who lost their phone can enroll again. What an admin cannot do here is set up another user's 2FA in their place: the secret must only ever exist in the user's own authenticator app; if a second person knew it, the second factor would not be one. Require — a new users.requires_two_factor boolean, defaulted to false so the migration does not lock existing users out. Editable through a toggle in the user form, visible as a "2FA Pflicht" icon column in the user list. Enforcement runs through a dedicated EnsureTwoFactorWhenRequired middleware in the panel's authMiddleware, because Filament's built-in isRequired is evaluated when the routes are registered — before any user is known — and so cannot make a per-user decision. On a request from an affected user without a secret, the middleware redirects to the profile page. That page, its Livewire requests and the logout route are excluded so the user is not trapped in a redirect loop with no way to enroll or sign out. The test suite pins both directions down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four changes from the first real use of the app. Grouped as commits on one branch because they land together; a separate branch (#4) already ships the image-upload fix from the same round.
Deaktivieren statt Löschen
Die Anwendung führte den Zustand „aktiv/inaktiv" schon an zwei Stellen — beim Mitarbeiter unwirksam, beim Fahrzeug als
status. Statt SoftDeletes obendrauf bekommen die übrigen vier Stammdaten dasselbe Feld und die Regel wird an diesen Zustand geknüpft:GuardedDeleteActionerklärt weiterhin, welches Fahrzeug im Weg steht.Bewusst kein SoftDeletes: Ein
deleted_atobendrauf ergäbe beim Fahrzeug drei Zustände (aktiv, ausgemustert, gelöscht) und beim Mitarbeiter zwei gleich klingende, undVehicle::deleting— das heute Bild und Dokumente von der Platte räumt — würde beim Archivieren mitfeuern.Achtung: Die Fahrzeugliste hat gar keine Löschaktion — die hängt an
EditVehicle::deleteAction(). Guarding nur der Tabellen hätte die Regel an der einzigen Stelle offen gelassen, an der man ein Fahrzeug wirklich löscht.Inaktive aus Auswahl, Suche und Anzeige
active, ergänzt um den bereits gesetzten Wert — sonst leerte das nächste Speichern eine Zuordnung, deren Bezug inzwischen inaktiv geworden ist.getGlobalSearchEloquentQuery().vehicles.license_plateundfuel_card_providers.namebleibenunique; ein ausgemustertes Fahrzeug blockiert sein Kennzeichen weiterhin. Die Validierungsmeldung nennt den Grund.2FA in der Benutzerverwaltung
users.requires_two_factormit eigener Middleware, die einen betroffenen Benutzer auf die Profilseite umleitet. Profilseite, Livewire-Requests und Logout sind ausgenommen — sonst leitet die Middleware auf eine Seite um, die sie erneut umleitet.isRequiredkonnte die Pflicht nicht liefern, weil sie beim Registrieren der Routen ausgewertet wird und dort noch kein Benutzer bekannt ist.Bruttolistenpreis
vehicles.gross_list_price,decimal(10,2)nullable, in der Section „Kosten" — dort greift derviewCosts-Schutz durch die versteckte Section.annual_total_costoder einem Summarizer. Ein Listenpreis ist keine laufende Position.„Historie" statt „Termine"
Nur die Beschriftungen. Klassen- und Tabellennamen (
VehicleAppointment,vehicle_appointments,AppointmentType) bleiben. Zwei neue Typen: Unfall (danger, das Rose des Panels) und Schaden (orange, inAdminPanelProviderneu registriert).dangerwar bisher gemieden, damit Badges nicht mit dem Löschen verwechselt werden — für einen Unfall ist genau dieses Hervorstechen richtig.Verifikation
Volle Testsuite: 431 grün (1330 Assertions). Pint sauber. Drei neue Migrationen (nur
up/down, keine bestehenden Daten berührt).Beim Lauf trat einmal ein unzuverlässiger Test auf (
MasterDataActiveFlagTest > saves the toggle through the form, etwa 1 von 15 Läufen anassertHasNoFormErrors). 3000 Durchläufe der Factory-Werte gegen die Formularregeln waren alle gültig, die Ursache also nicht bewiesen. Der Test nutzt jetzt feste Werte statt Zufallsdaten und gibt im Fehlerfall den Fehlerbeutel aus, damit ein künftiger Fehlschlag das Feld benennt.🤖 Generated with Claude Code