Adding attribute name for BelongsToMany sometimes causes migration error

[LorenzoPapi]

• Laravel Version: 12.34.0
• PHP Version: 8.4.13
• Blueprint Version: 2.12.0
• Platform: Linux

Issue:

I have a many to many relationship between AppRoles and AppUsers: each user HasMany roles and each role BelongsToMany users. Both the models use ulids: however, adding users after the line belongsToMany: AppUser causes the Blueprint to generate a migration where, instead of the usual foreignUlid, a foreignId is generated. If I remove the name it works flawlessly. I also have foreign key constraints active.

draft.yaml:

models:
  AppRole:
    ulid

    slug: string:20:unique
    name: string:50

    relationships:
      belongsToMany: AppUser:users
  
  AppUser:
    ulid
    
    username: string:255:unique
    password: password
    firstName: string:255
    lastName: string:255
    studies: string:255 nullable
    room: string:20 nullable
    telephone: string:20 nullable
    description: longtext
    
    softDeletes
    timestamps: false

    relationships:
      hasMany: AppRole:role
      belongsToMany: Group:group

  Group:
    name: string
    color: string

    relationships:
      hasMany: AppUser:user

[jasonmccreary]

My guess is since you are using unconventional model names, Blueprint can not guess the table. I'd need to see what the output migration looks like to better understand the problem.

I'd also be curious if specifying the unconventional naming in your model meta data would resolve your issue. For example:

AppModel:
  meta:
    table: users

[LorenzoPapi]

I think I have actually pinpointed the issue and fixed it locally changing the blueprint source code. I'll try to explain my thought process.

So the problem was that, in the migration, the method called was $table->foreignId()..., instead of $table->foreignUlid()..., therefore I went into the MigrationGenerator and found the function buildPivotTableDefinition which was called when creating the many to many relationship table.

the problem was here:

$type = isset($models[$segment]) ? $models[$segment]->idType() : 'id';

since $segment was not found as a model key, it defaulted back to the normal id method. now, segment was equal to AppUser:users, which of course was wrong, so it meant that the name wasn't being understood. I solved it in the following way:

            $key = Str::before($segment, ":");
            $column = Str::snake($key);
            $references = 'id';
            $on = Str::plural($column);
            $foreign = Str::singular($column) . '_' . $references;

            if (config('blueprint.use_constraints')) {
                $this->hasForeignKeyConstraints = true;

                $type = isset($models[$key]) ? $models[$key]->idType() : 'id';
                $definition .= $this->buildForeignKey($foreign, $on, $type) . ';' . PHP_EOL;
            } else {
                $definition .= $this->generateForeignKeyDefinition($segment, $foreign, $models);
            }

this way, I consider as a possible models key everything before the :, which is usually the considered model name, instead of the full segment, I don't know if it counts as a fix but I have no problems for now

[jasonmccreary]

Thanks. If you are willing to open a PR with this change, that would be great. I'll refine it and get it merged.

[LorenzoPapi]

Alright, I'll try to open it today. Also, I added the ability to insert traits as a meta attribute, at least for models because that's what I needed. If I add it to controllers too and open another PR would it get merged?

[jasonmccreary]

Sure. Please make them separate PRs though. Also, double check adding traits as meta data doesn't already rexist. I felt like it did. But maybe that was extends only.

[LorenzoPapi]

Ok got it. I've opened the PR that solves this issue. When I'll have some times I'll try to make the other one for the traits: as far as I've seen, custom trait was going to be implemented via #746, which was closed without merging. Also looking around the code, the only added traits are the default ones (HasUlids, HasUuids and so on).

Anyhow, you can close this issue after you merge #757.

[jasonmccreary]

After looking at this more closely, setting meta when you use an unconventional table name is the appropriate solution, and works currently. Generally speaking, there should be no reason to set the table name when providing a relationship. Especially a conventional one.

Feel free to open a separate PR for the extends/use meta options.

[LorenzoPapi]

I understand your closing of the issue, however I wasn't using any unconventional table names. Since my app model was called AppUser, blueprint generated the table app_users. I then wanted the AppRole model to have a many to many relationship with the app_users table, and call that relationship inside the model with the name users. However, when I did this, it would incorrectly understand which table I was referring too.

[jasonmccreary]

If it's not respecting the relationship method name, then I am glad to reopen this. That wasn't what the PR description seemed to relay. There are also not any tests. So it's hard to see what the result of your change is.