Skip to content
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.

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

Issue when generating pivot models for tables with duplicated strings in their name #227

Closed
finiteinfinity opened this issue Dec 7, 2021 · 2 comments
Labels

Comments

@finiteinfinity
Copy link
Contributor

finiteinfinity commented Dec 7, 2021

I'll try and keep this issue as simple as possible, however, it's a rather complex issue nested deep within the generator code.
Given these 3 table definitions:

CREATE TABLE `employees` (
    `id` BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    `name` VARCHAR(191),
);

CREATE TABLE `employee_attributes` (
    `id` BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    `name` VARCHAR(191) NOT NULL,
)

CREATE TABLE `employees_employee_attributes` (
    `id` BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    `employeeId` BIGINT UNSIGNED NOT NULL,
    `employeeAttributeId` BIGINT UNSIGNED NOT NULL,
    CONSTRAINT `employees_employee_attributes_employeeid_foreign` FOREIGN KEY (employeeId) REFERENCES `employees` (id),
    CONSTRAINT `employees_employee_attributes_employeeattributeid_foreign` FOREIGN KEY (employeeAttributeId) REFERENCES `employee_attributes` (id),
)

When I run php artisan code:models --table employees, I would expect the generated Employee model to have a belongsToMany relation, which references the pivot table defined above. Instead I get a hasMany relation which completely ignores the pivot table. Below are the expected and actual outputs.

Expected

public function employeeAttributes()
{
    return $this->belongsToMany(
        EmployeeAttribute::class,
        'employees_employee_attributes',
        'employeeId',
        'employeeAttributeId'
    )->withPivot('id', 'companyId');
}

Actual

public function employeesEmployeeAttributes()
{
    return $this->hasMany(EmployeesEmployeeAttribute::class, 'employeeId');
}

As you can see, the generated relation is pointing directly at the pivot table instead of detecting the fact it's a pivot table and pointing to the referenced model instead - which in this case would be EmployeeAttribute.

The cause

I believe I've found the cause of this. In this method when it generates the pivot table name, all instances of the parent table name are stripped out of the pivot table name. In our case, it looks something like this:

$pivot = $this->getRelatedBlueprint()->table(); // employees_employee_attributes
$firstRecord = $this->parent->getRecordName(); // employee

...

$pivot = str_replace($firstRecord, '', $pivot); // $pivot = str_replace('employee', '', 'employees_employee_attributes');

So $pivot ends up being s__attributes. This happens because $firstRecord strips the pluralisation from the table, turning employees into just employee.

Fixes

I've not found any robust fixes yet, although the obvious solution would be to replace $firstRecord = $this->parent->getRecordName() with $firstRecord = $this->parent->getTable(), however this ends up breaking more than it fixes.

@finiteinfinity finiteinfinity changed the title Issue when generating pivot models for tables duplicated strings Issue when generating pivot models for tables with duplicated strings in their name Dec 7, 2021
antevgen added a commit to antevgen/laravel that referenced this issue Apr 12, 2024
@XedinUnknown
Copy link

XedinUnknown commented Apr 12, 2024

I have a similar problem; not sure if it relates.
The logic first removes one associated table name from the name of the pivot table, and checks whether what's left contains the other associated name.

Let's consider an example where one has to relate 2 tables many-to-many: goals and primary_goals. The pivot table should have a name that indicates that it relates 2 tables, and so a common approach would be to call it goals_primary_goals (I'll work with this example) or goals_to_primary_goals, but it seems that goals_to_goals_attributes would present the same problem.
This line $pivot = str_replace($firstRecord, '', $pivot);, given that $firstRecord is goals, will change goals_primary_goals to _primary_ (or sometimes s_primary_s, if there is a discrepancy between plural/singular table/model names etc).
Then, the check if (Str::contains($pivot, $target->getRecordName())) will fail, because _primary_ does not contain primary_goals (or goals, for that matter).

This leads to incorrect relationships via the pivot table, because the check if ($this->hasPivot()) will fail, and the OneToManyStrategy will be used, instead of many to many.

#276 by @antevgen fixes this.

@finiteinfinity
Copy link
Contributor Author

Fixed in #276

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants