Realize Active Record models generator - #151
Conversation
- Add optional parameters to Command for property visibility, getters/setters, relations, and traits - Enhance Column class with default value handling and helper methods - Create Relation class to represent database relationships - Improve Generator with better type mapping (int, float, bool) and relation generation - Update default template to support all new features - Add comprehensive test coverage for new functionality - Create enhanced test database schema with various column types and relations Agent-Logs-Url: https://github.com/yiisoft/yii-gii/sessions/70800279-d97a-44c7-8b35-caa2159e0744 Co-authored-by: Tigrov <8563175+Tigrov@users.noreply.github.com>
Claude
AI
changed the title
|
@claude[agent] See my review comments |
…rface for DB expressions, and set() for relation columns - Use reflection on ColumnInterface::phpTypecast() return type to determine PHP type - Check default value using instanceof ExpressionInterface instead of string matching - Track columns used in relationships and use ActiveRecord::set() for their setters - Fix array default value handling to use var_export() for proper representation Agent-Logs-Url: https://github.com/yiisoft/yii-gii/sessions/37a0f7b8-949f-4180-8839-77ca52a68c64 Co-authored-by: Tigrov <8563175+Tigrov@users.noreply.github.com>
Claude
AI
changed the title
…type support - Remove fallback to $columnSchema->getPhpType() method which doesn't exist - Add support for ReflectionUnionType by extracting the first non-null type - Add support for ReflectionIntersectionType (returns 'mixed') - Default to 'string' if reflection fails completely Agent-Logs-Url: https://github.com/yiisoft/yii-gii/sessions/c185468b-b761-4cab-b415-6cf5df8a8fbd Co-authored-by: Tigrov <8563175+Tigrov@users.noreply.github.com>
- Add ColumnInterface import and use it for type hints in getPhpType() and hasDbDefaultExpression() - Replace isAllowNull() with !isNotNull() as isAllowNull() doesn't exist - Improve type safety with proper method signatures Agent-Logs-Url: https://github.com/yiisoft/yii-gii/sessions/c777132a-aa01-45a2-afb6-4369b20dcf39 Co-authored-by: Tigrov <8563175+Tigrov@users.noreply.github.com>
|
@claude[agent] Fix tests |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/Generator/ActiveRecord/Generator.php:107
getModelFile()always writes to@src/Model/regardless of thenamespaceprovided in the command/CLI. This makes the--namespaceoption (and the command hint about storing in the related directory) misleading and can produce files in the wrong PSR-4 path. Consider deriving the output directory from$command->namespace(or adding an explicit output directory option, like the controller generator does).
private function getModelFile(Command $command): string
{
$directory = '@src/Model/';
return $this->aliases->get(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Returns true if this is a hasOne relation. | ||
| */ | ||
| public function isHasOne(): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if this is a hasMany relation. | ||
| */ | ||
| public function isHasMany(): bool | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
isHasOne()/isHasMany() are currently hard-coded (true/false), so isHasMany() is dead code and relation generation can never produce hasMany. Either implement cardinality detection (e.g., based on unique indexes/PK on the FK columns) or remove these methods and the branching to avoid a misleading API.
| if ($command->generateRelations) { | ||
| foreach ($schema->getForeignKeys() as $foreignKey) { | ||
| $relations[] = new Relation($foreignKey, $command->getModelName()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Relation generation only iterates $schema->getForeignKeys() for the current table, which covers only outgoing FKs. That means models won’t get inverse relations (incoming FKs / hasMany side) unless the DB schema happens to define a reverse FK too. If Issue #147 expects inverse relations, consider also scanning other tables for foreign keys that reference the current table and generating those as hasMany/hasOne accordingly.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dels' into claude/generate-active-record-models
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/Generator/ActiveRecord/Generator.php:19
- The class-level docblock still says this generator creates a controller and action view files, but this is the ActiveRecord model generator. Updating the description would avoid misleading users and keep docs consistent with behavior.
/**
* This generator will generate a controller and one or a few action view files.
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/Generator/ActiveRecord/Generator.php:19
- The class-level docblock still states that this generator produces controllers and view files, which is misleading for an ActiveRecord model generator. Please update it to describe ActiveRecord model generation (properties, defaults, relations, etc.) so the documentation matches the actual behavior.
/**
* This generator will generate a controller and one or a few action view files.
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[Required] | ||
| #[Regex( | ||
| pattern: '/^[a-z\\\\]*$/i', | ||
| message: 'Only word characters and backslashes are allowed.', | ||
| pattern: '/^\\\\?[a-z_]\w*(?:\\\\[a-z_]\w*)*$/i', | ||
| message: 'Invalid base class name', | ||
| skipOnEmpty: true, | ||
| )] | ||
| #[ClassExistsRule] | ||
| private readonly string $baseClass = ActiveRecord::class, | ||
| #[Required(message: 'A code template must be selected.')] | ||
| #[TemplateRule] | ||
| protected string $template = 'default', | ||
| public readonly string $baseClass = ActiveRecord::class, | ||
| #[Required] | ||
| #[In(['private', 'protected', 'public'])] | ||
| public readonly string $propertyVisibility = 'protected', | ||
| public readonly bool $generateGettersSetters = true, | ||
| public readonly bool $generateRelations = true, | ||
| public readonly bool $useRepositoryTrait = false, | ||
| string $template = 'default', |
There was a problem hiding this comment.
The generated model code assumes the chosen $baseClass implements ActiveRecord APIs such as set(), relation(), relationQuery(), hasOne() / hasMany(). However, the command validation currently only checks that the class exists, not that it extends/implements the required ActiveRecord base/interface. This can lead to generated code that fatals at runtime if a user provides a different base class. Consider adding a validation rule to enforce a compatible base class (e.g., is_subclass_of($baseClass, ActiveRecordInterface::class)), or conditionally generating relation/setter code only when supported.
Issue #147