fix(zfa make): always generate data repo impl + wire per-method DI for entity presets (#284) - #287
Merged
Conversation
…r entity presets (#284)
Contributor
📝 WalkthroughWalkthroughEntity-based generation now defaults DI and test methods, emits repository implementations, and uses ChangesEntity generation updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/plugins/controller/controller_plugin_utils.dart`:
- Around line 61-79: Update the entity import construction in the
!config.noEntity block to use config.nameSnake for both the entity directory and
Dart file name, ensuring generated controller signatures resolve the matching
entity types.
In `@lib/src/plugins/di/di_plugin.dart`:
- Around line 104-116: The default methods include toggle, but downstream
allowlists exclude it. In lib/src/plugins/di/di_plugin.dart lines 104-116,
update validMethods in _generateEntityUseCaseDIFiles to allow toggle; in
lib/src/plugins/test/test_plugin.dart lines 85-93, update validMethods in
generate likewise, so default get, update, and toggle methods produce per-method
DI and test files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1123d3f1-2c63-4cfa-8720-f64cdd13e268
📒 Files selected for processing (7)
lib/src/plugins/controller/controller_plugin_utils.dartlib/src/plugins/di/di_plugin.dartlib/src/plugins/presenter/presenter_plugin.dartlib/src/plugins/repository/repository_plugin.dartlib/src/plugins/test/test_plugin.dartlib/src/plugins/view/builders/adaptive_layout_scaffold_builder.dartlib/src/plugins/view/view_plugin.dart
Comment on lines
+61
to
+79
| // #284/#281: Presentation layer (controller/presenter/view) must import | ||
| // `zuraffa_flutter` (which re-exports `zuraffa` + Flutter-specific types | ||
| // like Controller/Presenter/CleanView) instead of `zuraffa` alone — | ||
| // generated Flutter apps depend on `zuraffa_flutter`, not `zuraffa`, | ||
| // and `Controller` only exists in the Flutter package. | ||
| 'package:zuraffa_flutter/zuraffa_flutter.dart', | ||
| '${config.nameSnake}_presenter.dart', | ||
| ]; | ||
|
|
||
| // #284: Always import the entity so ProductPatch/ProductFields and other | ||
| // generated entity types are in scope for the controller's method | ||
| // signatures (updateProduct takes ProductPatch, toggleProduct takes | ||
| // ProductFields). Previously this was only added inside the | ||
| // `withState && !noEntity` block, leaving entity-based controllers | ||
| // without the entity import when withState was false. | ||
| if (!config.noEntity) { | ||
| imports.add('../../../domain/entities/$domainSnake/$domainSnake.dart'); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Build the entity import from config.nameSnake.
Line 77 uses domainSnake for both the entity directory and file name. If domain differs from the entity name, a Product controller can import domain/entities/catalog/catalog.dart instead of domain/entities/product/product.dart. ProductPatch and ProductFields then remain unresolved.
Use config.nameSnake for the entity directory and file name.
Proposed fix
- imports.add('../../../domain/entities/$domainSnake/$domainSnake.dart');
+ imports.add(
+ '../../../domain/entities/${config.nameSnake}/${config.nameSnake}.dart',
+ );Based on learnings, the fixed v5 entity layout is lib/src/domain/entities/{entity_snake}/{entity_snake}.dart, for example lib/src/domain/entities/product/product.dart.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // #284/#281: Presentation layer (controller/presenter/view) must import | |
| // `zuraffa_flutter` (which re-exports `zuraffa` + Flutter-specific types | |
| // like Controller/Presenter/CleanView) instead of `zuraffa` alone — | |
| // generated Flutter apps depend on `zuraffa_flutter`, not `zuraffa`, | |
| // and `Controller` only exists in the Flutter package. | |
| 'package:zuraffa_flutter/zuraffa_flutter.dart', | |
| '${config.nameSnake}_presenter.dart', | |
| ]; | |
| // #284: Always import the entity so ProductPatch/ProductFields and other | |
| // generated entity types are in scope for the controller's method | |
| // signatures (updateProduct takes ProductPatch, toggleProduct takes | |
| // ProductFields). Previously this was only added inside the | |
| // `withState && !noEntity` block, leaving entity-based controllers | |
| // without the entity import when withState was false. | |
| if (!config.noEntity) { | |
| imports.add('../../../domain/entities/$domainSnake/$domainSnake.dart'); | |
| } | |
| // #284/#281: Presentation layer (controller/presenter/view) must import | |
| // `zuraffa_flutter` (which re-exports `zuraffa` + Flutter-specific types | |
| // like Controller/Presenter/CleanView) instead of `zuraffa` alone — | |
| // generated Flutter apps depend on `zuraffa_flutter`, not `zuraffa`, | |
| // and `Controller` only exists in the Flutter package. | |
| 'package:zuraffa_flutter/zuraffa_flutter.dart', | |
| '${config.nameSnake}_presenter.dart', | |
| ]; | |
| // #284: Always import the entity so ProductPatch/ProductFields and other | |
| // generated entity types are in scope for the controller's method | |
| // signatures (updateProduct takes ProductPatch, toggleProduct takes | |
| // ProductFields). Previously this was only added inside the | |
| // `withState && !noEntity` block, leaving entity-based controllers | |
| // without the entity import when withState was false. | |
| if (!config.noEntity) { | |
| imports.add( | |
| '../../../domain/entities/${config.nameSnake}/${config.nameSnake}.dart', | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/plugins/controller/controller_plugin_utils.dart` around lines 61 -
79, Update the entity import construction in the !config.noEntity block to use
config.nameSnake for both the entity directory and Dart file name, ensuring
generated controller signatures resolve the matching entity types.
Source: Learnings
Comment on lines
+104
to
+116
| // #284: Apply the same entity-methods default the usecase/repository | ||
| // plugins use, so DI sees `isEntityBased=true` for canonical | ||
| // `zfa make Product --preset=crud` invocations and routes to | ||
| // _generateEntityUseCaseDIFiles (per-method DI files matching the | ||
| // per-method usecases the usecase plugin emits). Without this default, | ||
| // DI falls into the _generateCustomUseCaseDI branch and emits | ||
| // `product_usecase_di.dart` referencing a `ProductUseCase` | ||
| // class that is never generated, breaking the build. | ||
| methods: | ||
| context.data['methods']?.cast<String>().toList() ?? | ||
| (context.get<bool>('no-entity') == true | ||
| ? [] | ||
| : ['get', 'update', 'toggle']), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the default method contract consistent with downstream allowlists.
Both changed defaults advertise toggle, but the downstream allowlists filter it out. Add toggle to both allowlists.
lib/src/plugins/di/di_plugin.dart#L104-L116: addtoggletovalidMethodsin_generateEntityUseCaseDIFilesat Lines 938-947.lib/src/plugins/test/test_plugin.dart#L85-L93: addtoggletovalidMethodsingenerateat Lines 155-165.
This is required for the PR objective that default get, update, and toggle methods generate per-method DI and test files.
📍 Affects 2 files
lib/src/plugins/di/di_plugin.dart#L104-L116(this comment)lib/src/plugins/test/test_plugin.dart#L85-L93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/plugins/di/di_plugin.dart` around lines 104 - 116, The default
methods include toggle, but downstream allowlists exclude it. In
lib/src/plugins/di/di_plugin.dart lines 104-116, update validMethods in
_generateEntityUseCaseDIFiles to allow toggle; in
lib/src/plugins/test/test_plugin.dart lines 85-93, update validMethods in
generate likewise, so default get, update, and toggle methods produce per-method
DI and test files.
arrrrny
added a commit
that referenced
this pull request
Aug 12, 2026
* docs(agents): herdr shared-terminal rule (pallet replacement) * fix(plugins): apply CodeRabbit fixes from #287 Apply the 2 actionable CodeRabbit comments from merged PR #287: 1. controller_plugin_utils.dart: build entity import from config.nameSnake instead of domainSnake, so a controller imports the correct entity file (domain/entities/product/product.dart) when domain differs from entity name — prevents unresolved ProductPatch/ProductFields. 2. di_plugin.dart + test_plugin.dart: add 'toggle' to validMethods allowlists so the default method contract (which advertises toggle) is consistent with downstream filtering — toggle now generates matching DI and test scaffolding instead of being silently dropped. Verified: dart analyze lib/ clean (no errors), dart test 1189 pass. * fix(pr 288): apply CodeRabbit review fixes
This was referenced Aug 12, 2026
arrrrny
added a commit
that referenced
this pull request
Aug 12, 2026
#289) (#290) PR #287 added the entity-methods default [get, update, toggle] to both di_plugin.dart and test_plugin.dart so canonical zfa make invocations route through per-method generation. Every other generator that switches on method already had a toggle case (usecase, controller, presenter, view, repository, datasource, di, mock) EXCEPT the per-method test builder (test_builder_entity.dart). Its switch covered get/getList/list /create/update/delete/watch/watchList but not toggle, so the test plugin crashed with ArgumentError: Unknown method: toggle before emitting any file. The two test-builder helpers (_getFallbackValues and _generateFutureTests in test_builder_helpers.dart) also had no toggle branch, so even if the switch were relaxed the emitted test file would have been empty. Fix: add case toggle to test_builder_entity.dart (className = Toggle${entityName}UseCase, returnTypeConstructor = t$entityName, isStream/isCompletable = false — mirrors the usecase generator) and add matching toggle branches to _getFallbackValues (registerFallbackValue gets a concrete ToggleParams<idType, ${entityName}Fields> instance) and _generateFutureTests (paramsExpr + arrangeCall/verifyCall as mockVarName.toggle(any())) in test_builder_helpers.dart. Also adds a regression test in test/integration/toggle_method_test.dart that exercises the exact failing config from the issue (methods: [get, update, toggle] + all generation flags + generateTest: true) and asserts the toggle usecase test file is emitted with the expected shape (ToggleTodoUseCase class, mockRepository.toggle call, ToggleParams<String, TodoFields> constructor, TodoFields.id field reference). Closes #289. Co-authored-by: arrrrny <arrrrny@users.noreply.github.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
1 participant
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.
Closes #284
Summary
Fixes #284 (REOPENED — PR #286 was a false fix that only touched AGENTS.md, no code changed).
The canonical
zfa makecommand (as printed byzfa setup) produced non-compiling code with 24dart analyzeerrors caused by two missing files:Missing
data_product_repository.dart—repository_plugin.dartonly emitted the repo implementation whenconfig.generateData || config.generateDataSource, but those flags are not reliably set for crud/vpc+di entities (the schema default offalsewins over plugin-activation sync). The DI plugin unconditionally emitsproduct_repository_di.dartreferencingDataProductRepository, so the impl was missing.Missing
product_usecase.dart—di_plugin.dartrouted entity configs to_generateOrchestratorUseCaseDI, which emitsproduct_usecase_di.dartreferencing aProductUseCaseclass that is never generated. Same root cause intest_plugin.dart(looked forproduct_usecase.dartfor test generation, skipped with a warning).Changes (7 files, +59/-7)
#284 core fixes
lib/src/plugins/repository/repository_plugin.dart: Always emit the data repository implementation for entity-based configs (config.isEntityBased ||added to the generation condition), regardless ofgenerateData/generateDataSourceflags.lib/src/plugins/di/di_plugin.dart: Apply the entity-methods default (['get', 'update', 'toggle']) so DI routes to_generateEntityUseCaseDIFiles(per-method DI files matching the per-method usecases) instead of_generateOrchestratorUseCaseDI(which referenced non-existentProductUseCase).lib/src/plugins/test/test_plugin.dart: Same entity-methods default so the test plugin generates per-method tests instead of looking for non-existentproduct_usecase.dart.Generator plumbing (needed for the 0-errors requirement)
The 19 remaining errors were in the presentation layer (controller/presenter/view): generated Flutter apps depend on
zuraffa_flutter(notzuraffa), but presentation files importedpackage:zuraffa/zuraffa.dartwhich lacks Flutter-specific types (Controller,Presenter,CleanView,CleanViewState,ControlledWidgetBuilder). Fixed by switching topackage:zuraffa_flutter/zuraffa_flutter.dart(which re-exportszuraffa+ all Flutter types). The controller also needed an unconditional entity import soProductPatch/ProductFieldsare in scope for method signatures.lib/src/plugins/controller/controller_plugin_utils.dart:zuraffa->zuraffa_flutterimport + unconditional entity import for!config.noEntity.lib/src/plugins/presenter/presenter_plugin.dart:zuraffa->zuraffa_flutterimport.lib/src/plugins/view/view_plugin.dart:zuraffa->zuraffa_flutterimport.lib/src/plugins/view/builders/adaptive_layout_scaffold_builder.dart:zuraffa->zuraffa_flutterin template string.Repro evidence (0 errors)
Full repro chain on a clean checkout of this branch:
Before this PR (clean
development): 24 errorsdata_product_repository.dart(2: uri_does_not_exist + undefined_function) + missingproduct_usecase.dart(3: uri_does_not_exist + non_type_as_type_argument + undefined_function)extends_non_class(Controller/Presenter/CleanView/CleanViewState undefined),registerUseCase/dispose/onDisposed/onInitState/ControlledWidgetBuilder/globalKeyundefined,ProductPatch/ProductFieldsundefined,super_formal_parameter_without_associated_*,type_argument_not_matching_bounds,return_of_invalid_typeAfter this PR: 0 errors
Generated files verified:
lib/src/data/repositories/data_product_repository.dartEXISTS (was missing before)product_usecase_di.dartreferencing non-existentProductUseCase(per-method DI files generated instead:get_product_usecase_di.dart,update_product_usecase_di.dart)package:zuraffa_flutter/zuraffa_flutter.dart../../../domain/entities/product/product.dart) forProductPatch/ProductFieldsThe 18 remaining
infolints are alldepend_on_referenced_packages(thezuraffapackage is available transitively viazuraffa_flutterbut not a direct dependency) — these are style lints, not compilation errors, anddart analyzeexits with code 0.Test results
dart analyze lib/-> 1 pre-existing info (unrelatedlibrary_private_types_in_public_apiindecorator_dispatcher.dart:109)dart test test/plugins/-> 176 passdart test test/commands/make_command_test.dart setup_command_test.dart test_command_test.dart-> 34 passdart test test/commands/build_command_unit_test.dart build_yaml_guard_test.dart-> 37 passCloses #284
Summary by CodeRabbit
New Features
Bug Fixes