team_foreign_key column inconsistently nullable

[coolamit]

Description

The team_foreign_key column is nullable in roles table but not in model_has_permissions & model_has_roles.

So if an app uses some global roles (available to all teams) then it would set the team_foreign_key as NULL for those and assign Team ID to the roles specific to a team. But when a role is added where team_foreign_key is set to NULL and then assigned to a user, an error is thrown because team_foreign_key columns in model_has_permissions & model_has_roles are not nullable.

I can raise a PR to fix this in the migration stub if that would be acceptable.

Steps To Reproduce

1. Enable teams and set the team_foreign_key in the permission.php config.
2. Create a role but do not set the team_foreign_key to it.
3. Assign this role to a user.

Example Application

No response

Version of spatie/laravel-permission package:

6.17.0

Version of laravel/framework package:

11.44.2

PHP version:

8.3

Database engine and version:

PostgreSQL

OS: Windows/Mac/Linux version:

Ubuntu

[coolamit]

This can be fixed easily by adding nullable() in Lines 62 & 86 of the migration stub. Primary index creation on Lines 65 & 89 will have to be changed to unique index as well since nullable column cannot be part of primary key.

[drbyte]

This can be fixed easily by adding nullable() in Lines 62 & 86 of the migration stub. Primary index creation on Lines 65 & 89 will have to be changed to unique index as well since nullable column cannot be part of primary key.

That's already done in the teams migration file: https://github.com/spatie/laravel-permission/blob/ea7d29cf733705eba94b002462e42c6ff5585f4c/database/migrations/add_teams_fields.php.stub#L31-L77

[coolamit]

This can be fixed easily by adding nullable() in Lines 62 & 86 of the migration stub. Primary index creation on Lines 65 & 89 will have to be changed to unique index as well since nullable column cannot be part of primary key.

That's already done in the teams migration file:

laravel-permission/database/migrations/add_teams_fields.php.stub

Lines 31 to 77 in ea7d29c

Its there only for roles table but not in model_has_permissions & model_has_roles. So if a role has been added with team ID set to null & it is then assigned to a user, app encounters an error because model_has_permissions & model_has_roles tables don't accept null for team ID.

[drbyte]

Apologies. Yes, you're correct. I eagerly read that migration incorrectly, and conflated the point with something else.

There is a long history of it being this way. And by design it was intentionally left nullable when the teams feature was added. But that has caused a few others to raise the same point, and suggestion. Here are some of the related issues/discussions/scenarios, where some potential ripple-effect issues are also mentioned: #1980 where it was decided to leave it to each user to set it in their own migration if they needed it. https://github.com/spatie/laravel-permission/discussions/1840#discussioncomment-1277282 #1954

If we were to revisit the idea of making it nullable, we'd need to guard against the problems that creates. So we'd need to start with some "failing tests" that create the scenario that this change is intended to solve, and probably add some tests that are non-issues presently (so no tests exist for them) because the key is not nullable, to be sure things don't break when changing it.

[season886]

I think if team configuration is enabled, all team_foreign_keys should not be empty. If not, this field will not be created. I want to use this field as tenant mode. Is it feasible?