belongsToMany Foreign UUID not working.

Question

belongsToMany Foreign UUID not working.

makzumi opened this issue a year ago · 18 comments

Laravel Version: 10.10
PHP Version: 8.1
Blueprint Version: 2.9
Platform: Windows

Issue:

The pivot table on a belongsToMany is not respecting UUID from the related models. Should be foreignUuid

draft.yaml:

models:
  Brand:
    uuid
    name: string index
    slug: string unique index
    logo: string nullable
    softDeletes
    relationships:
      belongsToMany: Product
  Product:
    uuid
    name: string index
    slug: unique index
    price: double:8,2
    softDeletes
    relationships:
      belongsToMany: Brand

jasonmccreary commented 8 months ago

Nope.

Answer 1 · 2023-12-19T18:37:31.000Z

Could you pull from master and see if it work. A PR was recently merged that may address this...

Answer 2 · 2023-12-19T18:38:20.000Z

Thanks Jason, will give it a try and thanks for Blueprint!

Answer 3 · 2023-12-19T18:46:40.000Z

sorry, nope, it is still not working, just tried master. :(

Answer 4 · 2023-12-19T18:54:04.000Z

Ok. @rcrosbourne is this something your PR might have fixed?

Answer 5 · 2023-12-20T15:44:00.000Z

I have found the issue (i think...) there's this method in the MigrationGenerator class that passes the type to the buildForeignKey method. BUT the type is hard coded, trying to figure out how to correct it:

Answer 6 · 2023-12-20T18:53:16.000Z

You definitely aren't pulling from master as that code has changed. So it'd be good to ensure you have set your composer constraint to dev-master and run composer update.

Try again and see if it generates the "correct" code. Also, if not, disable use_constraints and see if it does (without the constraints of course).

Answer 7 · 2023-12-20T19:00:38.000Z

I switched to dev-master again, but I'm still seeing no uuid on the pivot tables, I still think it has to do with that hardcoded "id" in that method.

Answer 8 · 2023-12-20T19:03:54.000Z

UPDATE:
It worked with the use_constraints set to false:

Answer 9 · 2023-12-20T19:04:39.000Z

I guess I can use it like this, thanks so much @jasonmccreary

Answer 10 · 2023-12-20T19:14:31.000Z

The other path should be supported.

@rcrosbourne, seems your PR missed the path for setting foreign UUID/ULID columns with constraints (i.e. use_constraints enabled). I'd appreciate if you could fix that path.

Answer 11 · 2023-12-20T19:36:04.000Z

Yea I will take a look at it.

Answer 12 · 2023-12-23T19:02:19.000Z

This PR addresses the issue.

Answer 13 · 2024-02-29T17:11:45.000Z

Any news about this?

Answer 14 · 2024-02-29T17:17:33.000Z

No plan to merge the #661?
It seems ready. It's not a big problem to edit migrations manually, but if it's fixed...

Answer 15 · 2024-02-29T17:19:47.000Z

As noted on the PR, it has some changes requested and like 46 commits which it difficult to review/trust. I plan to give more time to Blueprint after the release of Laravel 11. However, until then I don't have the time to wade through this PR unless the author can clean it up a bit.

Answer 16 · 2024-02-29T19:47:00.000Z

I understand, sorry I saw the commits, but didn't have time either to read all the comments. Anyway if this issue is easy to fix (I guess it is), would be opening a new PR just for it, be a solution?

Answer 17 · 2024-02-29T20:16:44.000Z

I welcome any PRs. It's easier on the maintainer if the changes are concise, have a well explained PR description, and contain passing tests.