From 930bc3b9c348cf848c73fa009bad5b090595de38 Mon Sep 17 00:00:00 2001 From: Anilcan Cakir Date: Tue, 21 Jul 2026 22:36:09 +0300 Subject: [PATCH 1/2] feat(database): expose 422 validation errors from Model.save() Model.save() returned only a bool and discarded the server's 422 error body, so callers could not surface per-field validation messages. Capture the Laravel-shape errors into validationErrors (with a validationError(field) convenience), cleared at the start of each save and populated on a non-successful response. The bool return is unchanged (backward compatible); a thrown transport error leaves the map empty so callers can distinguish a field-validation failure from a network failure. --- .../concerns/interacts_with_persistence.dart | 52 +++++++++++- test/database/eloquent/model_test.dart | 82 +++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/lib/src/database/eloquent/concerns/interacts_with_persistence.dart b/lib/src/database/eloquent/concerns/interacts_with_persistence.dart index 4e8d5f3..f8a996b 100644 --- a/lib/src/database/eloquent/concerns/interacts_with_persistence.dart +++ b/lib/src/database/eloquent/concerns/interacts_with_persistence.dart @@ -45,6 +45,38 @@ import '../../events/model_events.dart'; /// } /// ``` mixin InteractsWithPersistence on Model { + // --------------------------------------------------------------------------- + // Validation State + // --------------------------------------------------------------------------- + + /// Per-field validation errors from the most recent remote [save]. + /// + /// Populated when a remote save receives a Laravel validation response + /// (`{message: ..., errors: {field: [msg, ...]}}`, typically a 422) and reset + /// on every remote save attempt. Read through [validationErrors]. + Map> _validationErrors = {}; + + /// The per-field validation errors from the most recent [save]. + /// + /// A remote save that fails with the Laravel validation shape + /// (`{message: ..., errors: {field: [msg, ...]}}`, typically a 422) fills this + /// map so the caller can render the messages under the matching form fields + /// instead of a generic failure. It is cleared at the start of each remote + /// save, so a successful save (or a save with no field errors) leaves it + /// empty; a thrown transport error also leaves it empty, letting the caller + /// treat that as a non-field failure. The returned map is unmodifiable. + Map> get validationErrors => + Map.unmodifiable(_validationErrors); + + /// The first validation message for [field], or `null` when [field] has none. + /// + /// A convenience over [validationErrors] for the common form case of showing + /// a single message per field. + String? validationError(String field) => + _validationErrors[field]?.isNotEmpty == true + ? _validationErrors[field]!.first + : null; + // --------------------------------------------------------------------------- // Static Factory Methods // --------------------------------------------------------------------------- @@ -222,6 +254,8 @@ mixin InteractsWithPersistence on Model { // Save to remote if (useRemote) { + // Drop any field errors from a prior save before the round trip. + _validationErrors = {}; try { MagicResponse response; if (exists) { @@ -239,9 +273,15 @@ mixin InteractsWithPersistence on Model { id = responseData[primaryKey]; } } + } else { + // A non-2xx (a 422 validation failure and the like) carries the + // per-field error shape; surface it for the caller without changing + // the bool return contract. + _validationErrors = _extractValidationErrors(response); } } catch (_) { - // Remote failed, continue to local + // Remote failed (transport). Leave _validationErrors empty so the + // caller treats this as a non-field failure. } } @@ -409,6 +449,16 @@ mixin InteractsWithPersistence on Model { } } + /// Extract the Laravel validation error shape from a failed [response]. + /// + /// Reads `response.data`'s `{errors: {field: [msg, ...]}}` block (the shape + /// Laravel returns on a 422) and returns `{}` when that shape is absent, so a + /// non-validation failure yields no field errors. Delegates to + /// [MagicResponse.errors], the framework's canonical parser for this shape. + Map> _extractValidationErrors(MagicResponse response) { + return response.errors; + } + /// Extract model data from API response. /// /// Handles both direct data and nested `data` key. diff --git a/test/database/eloquent/model_test.dart b/test/database/eloquent/model_test.dart index de4324e..a51cdca 100644 --- a/test/database/eloquent/model_test.dart +++ b/test/database/eloquent/model_test.dart @@ -427,6 +427,88 @@ void main() { expect(map.containsKey('email'), isFalse); }); }); + + group('InteractsWithPersistence validation errors', () { + setUp(() { + MagicApp.reset(); + Magic.flush(); + }); + + tearDown(() { + Http.unfake(); + MagicApp.reset(); + Magic.flush(); + }); + + test('save exposes 422 field errors and clears them on success', () async { + // 1. A remote 422 populates validationErrors and keeps save() false. + Http.fake( + (MagicRequest request) => Http.response({ + 'message': 'The given data was invalid.', + 'errors': { + 'name': ['The name field is required.'], + }, + }, 422), + ); + + final user = _RemoteUser()..fill({'email': 'new@example.com'}); + final failed = await user.save(); + + expect(failed, isFalse); + expect( + user.validationErrors, + containsPair('name', ['The name field is required.']), + ); + expect(user.validationError('name'), 'The name field is required.'); + + // 2. A subsequent successful save clears the prior field errors. + Http.fake( + (MagicRequest request) => Http.response({ + 'data': {'id': 7, 'name': 'Ok'}, + }, 201), + ); + user.setAttribute('name', 'Ok'); + final ok = await user.save(); + + expect(ok, isTrue); + expect(user.validationErrors, isEmpty); + expect(user.validationError('name'), isNull); + }); + + test('save leaves validationErrors empty on a non-field failure', () async { + // A 500 with no `errors` block is a non-validation failure: save() is + // false but no per-field errors are surfaced. + Http.fake( + (MagicRequest request) => + Http.response({'message': 'Server error.'}, 500), + ); + + final user = _RemoteUser()..fill({'name': 'Boom'}); + final failed = await user.save(); + + expect(failed, isFalse); + expect(user.validationErrors, isEmpty); + expect(user.validationError('name'), isNull); + }); + }); +} + +/// A remote-only model for testing the validation-error surface on [save]. +class _RemoteUser extends Model with InteractsWithPersistence { + @override + String get table => 'remote_users'; + + @override + String get resource => 'remote_users'; + + @override + List get fillable => ['name', 'email']; + + @override + bool get useLocal => false; + + @override + bool get useRemote => true; } /// A model with hidden attributes for testing serialization. From 464f044821f1571c121d43a998056d2ec04471eb Mon Sep 17 00:00:00 2001 From: Anilcan Cakir Date: Sat, 25 Jul 2026 20:05:51 +0300 Subject: [PATCH 2/2] fix(database): freeze validationErrors at both levels and correct its contract Map.unmodifiable is shallow: MagicResponse.errors builds a fresh map of growable lists, so a caller could still mutate the per-field messages the getter handed out. _extractValidationErrors now freezes the map and every list inside it, the field holds that frozen map (const {} when empty), and the getter returns it directly instead of re-wrapping on each read. The doc comment also claimed a successful save always leaves the map empty. It does not: the map tracks the REMOTE leg, so a hybrid model whose remote save 422s while its local write succeeds returns true with the errors filled. Say so, and drop the '?.isNotEmpty == true' plus bang from validationError() for a plain null-and-empty guard. Adds an unmodifiable-at-both-levels test, the CHANGELOG entry the contributing checklist asks for, and the doc + skill pages for the new public members. --- CHANGELOG.md | 2 + doc/eloquent/getting-started.md | 17 ++++++ .../concerns/interacts_with_persistence.dart | 53 +++++++++++++------ .../references/forms-validation.md | 27 ++++++++++ test/database/eloquent/model_test.dart | 22 ++++++++ 5 files changed, 106 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62900ad..664f96e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ All notable changes to this project will be documented in this file. ### Added +- **`Model.save()` now exposes the backend's per-field 422 errors instead of discarding them.** `save()` returned only a `bool`, so a form that wrote through the ORM could tell that a remote save failed but not why, and every 422 collapsed into a generic "something went wrong" toast. A failed remote save now captures the Laravel validation shape (`{"message": ..., "errors": {"field": ["message"]}}`) into two new members on `InteractsWithPersistence`: `validationErrors` (`Map>`) and `validationError(field)` (the first message for one field). The map is cleared at the start of every remote save, so it stays empty after a save that succeeded or carried no field errors, and it also stays empty when the remote leg throws (a transport failure), which is how a caller distinguishes a field-validation failure from a network failure: an empty map plus a `false` return means "render a generic error". It is deeply unmodifiable (both the map and each message list), and it tracks the REMOTE leg rather than `save()`'s return value, so a hybrid model (`useRemote` and `useLocal`) whose remote save 422s while its local write succeeds returns `true` with the errors filled. The `bool` return contract is unchanged, so this is purely additive for existing callers. Touches `lib/src/database/eloquent/concerns/interacts_with_persistence.dart`; covered by the `InteractsWithPersistence validation errors` group in `test/database/eloquent/model_test.dart`; documented in `doc/eloquent/getting-started.md` (Inserting & Updating -> Validation Errors) and `skills/magic-framework/references/forms-validation.md` (Server Error Mapping). + - **`MagicRouter` now re-runs its redirect chain when auth state changes, not only on navigation.** The router evaluated its guards (the `'auth'` / `'guest'` redirects) only while resolving a route, so a login or logout that happened while the user was already sitting on a page (a token expiry, a background sign-out, a successful login on the auth screen) did not move them off a now-forbidden route until the next manual navigation. The router now listens to the auth guard's state notifier and refreshes `routerConfig` on a change, so an auth transition re-evaluates redirects immediately (an expired session bounces to login; a login leaves the guest-only auth screen). Consumers with no bound `auth` guard are unaffected (the notifier is absent and the listener is a no-op). Touches `lib/src/routing/magic_router.dart`; covered by `test/routing/router_auth_refresh_test.dart`. ### Fixed diff --git a/doc/eloquent/getting-started.md b/doc/eloquent/getting-started.md index 7cb9ad1..11e3780 100644 --- a/doc/eloquent/getting-started.md +++ b/doc/eloquent/getting-started.md @@ -249,6 +249,23 @@ if (user != null) { } ``` +### Validation Errors + +`save()` returns a `bool`, which tells you *that* a remote save failed but not *why*. When the backend answers with Laravel's validation shape (`{"message": ..., "errors": {"field": ["message"]}}`, typically a 422), the per-field messages are captured on the model so a form can render them under the matching fields: + +```dart +final monitor = Monitor()..fill({'name': '', 'url': 'not-a-url'}); + +if (!await monitor.save()) { + monitor.validationErrors; // {'name': ['The name field is required.'], ...} + monitor.validationError('name'); // 'The name field is required.' (first message) +} +``` + +`validationErrors` is cleared at the start of every remote save, so it stays empty after a save that succeeded or returned no field errors. A save that failed on the transport (no response at all) also leaves it empty, which is how you tell a field-validation failure from a network failure: an empty map plus a `false` return means "show a generic error". + +The map is deeply unmodifiable, and it tracks the remote leg rather than the return value. A [hybrid](#hybrid-persistence) model whose remote save 422s while its local write succeeds returns `true` and still carries the errors, so check the map even after a successful save when local persistence is on. + ### Dirty Checking Track which attributes have changed: diff --git a/lib/src/database/eloquent/concerns/interacts_with_persistence.dart b/lib/src/database/eloquent/concerns/interacts_with_persistence.dart index f8a996b..1a537ed 100644 --- a/lib/src/database/eloquent/concerns/interacts_with_persistence.dart +++ b/lib/src/database/eloquent/concerns/interacts_with_persistence.dart @@ -53,29 +53,44 @@ mixin InteractsWithPersistence on Model { /// /// Populated when a remote save receives a Laravel validation response /// (`{message: ..., errors: {field: [msg, ...]}}`, typically a 422) and reset - /// on every remote save attempt. Read through [validationErrors]. - Map> _validationErrors = {}; + /// on every remote save attempt. Always holds a deeply unmodifiable map (see + /// [_extractValidationErrors]), so [validationErrors] hands it out directly. + Map> _validationErrors = const {}; /// The per-field validation errors from the most recent [save]. /// /// A remote save that fails with the Laravel validation shape /// (`{message: ..., errors: {field: [msg, ...]}}`, typically a 422) fills this /// map so the caller can render the messages under the matching form fields - /// instead of a generic failure. It is cleared at the start of each remote - /// save, so a successful save (or a save with no field errors) leaves it - /// empty; a thrown transport error also leaves it empty, letting the caller - /// treat that as a non-field failure. The returned map is unmodifiable. - Map> get validationErrors => - Map.unmodifiable(_validationErrors); + /// instead of a generic failure. + /// + /// It is cleared at the start of every remote save, so it stays empty after a + /// remote save that succeeded or returned no field errors, and after a thrown + /// transport error (which the caller treats as a non-field failure). A model + /// that never saves remotely never fills it. + /// + /// It tracks the REMOTE leg, not [save]'s return value: a hybrid model + /// (`useRemote` and `useLocal` both true) whose remote leg returns a 422 while + /// its local write succeeds returns `true` from [save] with this map filled. + /// Check it even after a save reported success when local persistence is on. + /// + /// Deeply unmodifiable: neither the map nor the message lists inside it can be + /// mutated through this getter. + Map> get validationErrors => _validationErrors; /// The first validation message for [field], or `null` when [field] has none. /// /// A convenience over [validationErrors] for the common form case of showing /// a single message per field. - String? validationError(String field) => - _validationErrors[field]?.isNotEmpty == true - ? _validationErrors[field]!.first - : null; + String? validationError(String field) { + final List? messages = _validationErrors[field]; + + if (messages == null || messages.isEmpty) { + return null; + } + + return messages.first; + } // --------------------------------------------------------------------------- // Static Factory Methods @@ -255,7 +270,7 @@ mixin InteractsWithPersistence on Model { // Save to remote if (useRemote) { // Drop any field errors from a prior save before the round trip. - _validationErrors = {}; + _validationErrors = const {}; try { MagicResponse response; if (exists) { @@ -453,10 +468,18 @@ mixin InteractsWithPersistence on Model { /// /// Reads `response.data`'s `{errors: {field: [msg, ...]}}` block (the shape /// Laravel returns on a 422) and returns `{}` when that shape is absent, so a - /// non-validation failure yields no field errors. Delegates to + /// non-validation failure yields no field errors. Delegates the parsing to /// [MagicResponse.errors], the framework's canonical parser for this shape. + /// + /// The result is frozen at both levels: [MagicResponse.errors] builds a fresh + /// mutable map of mutable lists, and a shallow `Map.unmodifiable` would still + /// let a caller mutate the per-field lists it hands out. Map> _extractValidationErrors(MagicResponse response) { - return response.errors; + return Map>.unmodifiable({ + for (final MapEntry> entry + in response.errors.entries) + entry.key: List.unmodifiable(entry.value), + }); } /// Extract model data from API response. diff --git a/skills/magic-framework/references/forms-validation.md b/skills/magic-framework/references/forms-validation.md index 53091dc..bf22749 100644 --- a/skills/magic-framework/references/forms-validation.md +++ b/skills/magic-framework/references/forms-validation.md @@ -300,6 +300,33 @@ Magic handles Laravel-style 422 error responses end-to-end: 5. **`rules()` / `FormValidator.rules()`** checks `controller.hasError(field)` first; if found, returns the server message directly. 6. Error appears automatically under the form field with no extra widget code. +When the write goes through the ORM instead of a hand-rolled `Http` call, the model captures the same shape, so step 2 reads from the model: + +| Member | Type | Notes | +|---|---|---| +| `model.validationErrors` | `Map>` | Per-field messages from the most recent remote `save()`. Deeply unmodifiable, cleared at the start of every remote save | +| `model.validationError(field)` | `String?` | First message for `field`, or `null` | + +```dart +if (!await monitor.save()) { + final Map> errors = monitor.validationErrors; + + // Empty map + false return = transport failure, not a field failure. + if (errors.isEmpty) { + setError(trans('common.error_occurred')); + return; + } + + validationErrors = { + for (final MapEntry> entry in errors.entries) + entry.key: entry.value.first, + }; + notifyListeners(); +} +``` + +A hybrid model (`useRemote` and `useLocal`) whose remote leg 422s while its local write succeeds returns `true` from `save()` with the map filled, so check the map even on success when local persistence is on. + ## Message i18n diff --git a/test/database/eloquent/model_test.dart b/test/database/eloquent/model_test.dart index a51cdca..3419d85 100644 --- a/test/database/eloquent/model_test.dart +++ b/test/database/eloquent/model_test.dart @@ -475,6 +475,28 @@ void main() { expect(user.validationError('name'), isNull); }); + test('validationErrors is unmodifiable at both levels', () async { + Http.fake( + (MagicRequest request) => Http.response({ + 'message': 'The given data was invalid.', + 'errors': { + 'name': ['The name field is required.'], + }, + }, 422), + ); + + final user = _RemoteUser()..fill({'email': 'new@example.com'}); + await user.save(); + + final Map> errors = user.validationErrors; + + // A shallow Map.unmodifiable would still hand out growable message + // lists, so the per-field list has to be frozen too. + expect(errors, containsPair('name', ['The name field is required.'])); + expect(() => errors['email'] = ['injected'], throwsUnsupportedError); + expect(() => errors['name']?.add('injected'), throwsUnsupportedError); + }); + test('save leaves validationErrors empty on a non-field failure', () async { // A 500 with no `errors` block is a non-validation failure: save() is // false but no per-field errors are surfaced.