Skip to content

#126 Has and belongs to many ignores class_name #127

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.

Sign up for GitHub

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

Merged
shmax merged 5 commits into from
Sep 16, 2025
Merged

#126 Has and belongs to many ignores class_name #127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9dbd9d7
Fix relationship handling in HasAndBelongsToMany and improve test cov…
EthannSchneider Sep 15, 2025
04ff908
Add is_string_this_relationship method to AbstractRelationship and up…
EthannSchneider Sep 15, 2025
3035774
Add test for HasAndBelongsToMany relationship with explicit class nam…
EthannSchneider Sep 15, 2025
ce9e788
Added join_table since the autogenerated class_name is custom_workers…
EthannSchneider Sep 15, 2025
c3cbb57
Fix HasAndBelongsToMany relationship to use join_table instead of cla…
EthannSchneider Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/Relationship/HasAndBelongsToMany.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ public function __construct(string $attribute, array $options = [])
{
parent::__construct($attribute, $options);

$this->set_class_name($this->inferred_class_name(Utils::singularize($attribute)));
if (!isset($this->class_name)) {
$this->set_class_name($this->inferred_class_name(Utils::singularize($attribute)));
}

$this->options['association_foreign_key'] ??= Inflector::keyify($this->class_name);
}
Expand All @@ -46,7 +48,7 @@ public function load(Model $model): mixed
* @var Relation<TModel>
*/
$rel = new Relation($this->class_name, [], []);
$rel->from($this->attribute_name);
$rel->from($this->get_table()->table);
$other_table = Table::load(get_class($model));
$other_table_name = $other_table->table;
$other_table_primary_key = $other_table->pk[0];
Expand Down
38 changes: 18 additions & 20 deletions lib/Table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,20 @@ public function create_joins(array|string $joins): string
$ret .= $space;

if (false === stripos($value, 'JOIN ')) {
if (array_key_exists($value, $this->relationships)) {
$rel = $this->get_relationship($value);

/**
* PHPStan seems to be getting confused about the usage of a class-string
* as an array string.
*
* @phpstan-ignore-next-line
*/
$alias = !empty($existing_tables[$rel->class_name]) ? $value : null;
/* @phpstan-ignore-next-line */
$existing_tables[$rel->class_name] = true;

/* @phpstan-ignore-next-line */
$ret .= $rel->construct_inner_join_sql($this, false, $alias);
} else {
throw new RelationshipException("Relationship named $value has not been declared for class: {$this->class->getName()}");
}
$rel = $this->get_relationship($value, true);

/**
* PHPStan seems to be getting confused about the usage of a class-string
* as an array string.
*
* @phpstan-ignore-next-line
*/
$alias = !empty($existing_tables[$rel->class_name]) ? $value : null;
/* @phpstan-ignore-next-line */
$existing_tables[$rel->class_name] = true;

/* @phpstan-ignore-next-line */
$ret .= $rel->construct_inner_join_sql($this, false, $alias);
} else {
$ret .= $value;
}
Expand Down Expand Up @@ -382,8 +378,10 @@ public function get_fully_qualified_table_name(): string
*/
public function get_relationship(string $name, bool $strict = false): ?AbstractRelationship
{
if ($this->has_relationship($name)) {
return $this->relationships[$name];
foreach ($this->relationships as $relationship) {

@shmax shmax Sep 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Giving a general class like Table knowledge of a specific relationship type seems a little fishy to me. Is attribute_name not resolving to what we expect it to?

@EthannSchneider EthannSchneider Sep 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct that not is buisness

And we can't simply use attribute_name in HasAndBelongsToMany the name will have for example "custom_workers" and he will search for a relationsship the other side named "custom_workers" and he will not find one.

if ($relationship->attribute_name === $name || ($relationship instanceof HasAndBelongsToMany && $relationship->class_name === ucfirst(Utils::singularize($name)))) {
return $relationship;
}
}

if ($strict) {
Expand Down
14 changes: 14 additions & 0 deletions test/RelationshipTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,20 @@ public function testHasAndBelongsToManyPrimaryKeyIsDifferentThanForeignKeyRevers
$this->assert_sql_includes('INNER JOIN tasks_workers ON (workers.id = tasks_workers.worker_id) INNER JOIN tasks ON tasks.id = tasks_workers.task_id', Table::load(Worker::class)->last_sql);
}

public function testHasAndBelongsToManyWithExplicitClassName()
{
Task::$has_and_belongs_to_many = [
'custom_workers' => [
'class_name' => 'Worker'
]
];

$task = Task::find(1);
$workers = $task->custom_workers;
$this->assertEquals(1, count($workers));
$this->assertInstanceOf(Worker::class, $workers[0]);
}

public function testBelongsToCreateAssociation()
{
$event = Event::find(5);
Expand Down