Preparing v6

[drbyte]

Some of the pending PR's involve breaking or nearly-breaking changes, so I'm considering tagging v6 for them.
Should some of these be kept in v5 anyway?

• Fix Role::withCount if belongsToMany declared #2280 - fix withCount support -- affects Role model.

• Hint model properties #2230 - hint model properties -- affects Role and Permission models

• Added types #2266 - add types to function signatures -- affects a lot of files (needs merge conflicts fixed, and patterns applied to all files added by other PRs)

• Custom wildcard verification/separators support #2252 - custom wildcard verification support - not seriously breaking, but extended classes should be redone.

Probably not breaking, but touch main Registrar file:

• fix: Lazily bind dependencies #2309 - lazily bind dependencies (performance improvement)
• Extract query to getPermissionsWithRoles method. #2316 - extract getPermissionsWithRoles method

Teams -- @erikn69 should both of these be merged as-is? I know some time has passed without much comment on them, so does that mean they're less critical because they're easily addressed in userland instead of here?

• [V5] Teams trait for adding relations and added team deleting logic #1935 - teams trait and team deletion logic
• Fix detaching user models on teams feature #2220 #2221 - detaching user models on teams

[erikn69]

Hi, I also think that Php8 syntax should be adopted for v6, so we leave support for 7.3, 7.4

Teams

• Fix detaching user models on teams feature #2220 #2221
  It's a bug fix, I added a test to demonstrate the problem, it can be merged as-is on V5(On Teams when delete a user without soft delete trait, not all data gets detached #2220)
• [V5] Teams trait for adding relations and added team deleting logic #1935 it is opcional trait, if you want to keep the package as light as possible, you can ignore it, because it can be handled manually in each implementation. Add it in case someone has that problem, the PR can be tagged(Maybe a discussion should be added)

Others

• Custom wildcard verification/separators support #2252 It makes no difference to me in the version that is added or if it is not added, it was only to help Ability to change wildcard permission dot (.) to colon (:) #2110,
  Also it avoids calling if(config('permission.enable_wildcard_permission', false)) on every hasPermissionTo
• fix: Lazily bind dependencies #2309 Looks great to me, it could fixes Multitenancy cache #2310 (comment), but I don't know if it affects any scenario outside of my knowledge
• Extract query to getPermissionsWithRoles method. #2316 It is not a breaking change for V5, but looks like helps only on specific cases,
• Hint model properties #2230 It is only hints, it doesn't change functionality, it could be added on V5
• Added types #2266 V6

[drbyte]

Your comments about v5 are noted. I'll probably merge these today or tomorrow:

• Fix detaching user models on teams feature #2220 #2221
• Custom wildcard verification/separators support #2252
• fix: Lazily bind dependencies #2309
• Extract query to getPermissionsWithRoles method. #2316
• Hint model properties #2230
• perhaps [V5] Teams trait for adding relations and added team deleting logic #1935 after additional review

[erikn69]

looks good to me 👍

• [V5] Teams trait for adding relations and added team deleting logic #1935 i think useless on most of the cases, that's why I put it optional

[drbyte]

@erikn69 thanks for the feedback! Greatly appreciated!

[erikn69]

#2309 Looks great to me, it could fixes #2310 (comment), but I don't know if it affects any scenario outside of my knowledge

It was a breaking change #2318, #2319, i did make a commit for fix that(erikn69@d0f799c) but i don't know if is better revert the PR

[parallels999]

• [V6] Full uuid/guid/ulid support #2089 UUIDs/ULIDs/GUIDs support on V6 - change data type on findById is a breaking change for somebody overwriting that function, if that is true, maybe for V6 release
• create_permission_tables.php use anonymous classes in migrations on V6

[erikn69]

If necessary UUID support I can do a rebase
For anonymous classes Laravel 7.x, 8.x should be dropped

[drbyte]

I'm reluctant to completely drop all Laravel 7 and 8, and PHP7 support if some of the open PRs should be applied for sites still running there.
Obviously, supporting legacy dependencies has a limit.
So, that begs the question, what open PRs should be merged into v5 before we tag v6? (ie: avoiding the need to backport)
EDIT: I'm working through Eric's comments now.

[parallels999]

what open PRs should be merged into v5 before we tag v6?

From the list i think

• Hint model properties #2230
• Fix detaching user models on teams feature #2220 #2221
• Extract query to getPermissionsWithRoles method. #2316
• fix: Lazily bind dependencies #2309

[drbyte]

@parallels999 thanks for the feedback! Greatly appreciated!

[marcoraddatz]

In the last few months there have been two changes in minor updates that needed to be reverted. Although I love this package and I am grateful to everyone who contributes to it, I would suggest to be more careful regarding new features.

So I would suggest better to release more major versions whenever there is even the slightest chance of breaking something, or even start with pre-release versions, that can be tested within test environments. Since this package is not in line with Laravel versioning, is there anything against releasing 1+ major versions per year?

I'd put following PRs in v6:

• fix: Lazily bind dependencies #2309 (the follow up)
• Added types #2266
• Add multiple guards support to givePermissionTo method #2291 (should require tests?!)

[parallels999]

• Add multiple guards support to givePermissionTo method #2291
  i think these must be closed and look for another approach

[drbyte]

#2280 and #2321 are now merged into 6.x / dev-main for testing.

[parallels999]

@drbyte what about #2089

#2317 (comment)

[parallels999]

@drbyte what about spatie/laravel-package-tools for the service provider?

[drbyte]

Perhaps. I wonder if it just creates more overhead, potentially slowing things down?

What specific benefit do you see it providing?

[parallels999]

yes you could be right, never mind

[drbyte]

I did explore whether the "install" command might be beneficial, but it's not really much different than simply publishing assets. Most of the rest of the "install" process actually requires programmer intervention. So, I'm still leaning away from it, to reduce complexity.

[systemsolutionweb]

@drbyte hi, any release date for this? any RC?

[drbyte]

At this point it's essentially stable. Just not officially released.
You can use it now as version ^6.0-dev.
Do consult the upgrading.md file for things related to migrations and tests.

[drbyte]

I'm looking for arguments for-and-against defaulting to UUID in v6.

• FOR: simpler for those who need it, because it's already done.
• FOR: a lot of systems use UUID by default
• AGAINST: it's overkill if you don't have a lot of permissions/roles
• AGAINST: takes up more database space and RAM and cache
• AGAINST: it's a lot harder to use tinker and specify a role/permission id in bespoke commands
• AGAINST: converting old data to uuid is a bit of a nuisance, but probably a minor issue

I'm on the fence.

[drbyte]

@erikn69 do you mind reviewing the latest updates to the uuid.md documentation file, to be sure nothing's been missed in the edits I've merged and tweaked?

[erikn69]

Everything looks in order to me

[drbyte]

@erikn69 what are your thoughts on defaulting to using uuid on the pivot table morph key column, in model_has_roles and model_has_permissions?
Like, if we defaulted to uuid, then it would support uuid/ulid/int out-of-the-box with no changes required.
One potential "downside" is that uuid sets the field as varchar instead of as bigUnsignedInt ... but does that actually cause any performance degradation by the lookup when matching on foreign key models?
In the simple test I've modeled, MariaDB 10.6 isn't complaining about mismatched column types in the foreign relationship, and lookups function fine, but it's a very small dataset I'm testing with.
Curious your thoughts.

[erikn69]

I have always maintained the idea of ​​having the data types as they should be, I have them as big int, and I would not change them, I think it should adapt to the needs of the majority, if it does not affect performance I do not see problems

#2513 is incomplete and the complexity is increasing.

[drbyte]

#2513 is incomplete and the complexity is increasing.

agreed.

[drbyte]

I have always maintained the idea of ​​having the data types as they should be, I have them as big int, and I would not change them

Ya, me too.

I think it should adapt to the needs of the majority, if it does not affect performance I do not see problems

I see the value in uuid, but I think int will always be more performant.

I prefer ulid over uuid in my apps, but uuid takes up more chars so any package should specify uuid, which will then accommodate both uuid and ulid.

[drbyte]

Update on v6 release:
I had hoped to battle-test v6 on a complex new project I've been helping build ... but that project got half-done and the stakeholders decided to put the brakes on for several months, for various reasons, some of which are sensible, others are not. But I digress.

So, I hope to release v6 very soon. Within a couple weeks.