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

Added Team-Knowledge to Permission-Model roles relation #2601

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

renky
Copy link

@renky renky commented Feb 6, 2024

In huge team-environments the Permission-Model eager loads always all roles of all teams independent of a current used team-id. This can lead to much ram usage due to multiple thousands loaded role-models.

This fix finally reduces amount of loaded role-models significantly.

@parallels999
Copy link
Contributor

Also you needs to add some tests, to avoid breaking it again in the future, and to show that a problem is fixed


I think this breaks cache feature, because all possible permission-role configurations are saved in cache

protected function getPermissionsWithRoles(): Collection
{
return $this->permissionClass::select()->with('roles')->get();
}

Maybe this works in your specific use case, you could have your own custom Permission model


In huge team-environments the Permission-Model eager loads always all role

Also Model-Role relation already has that code, because Model uses hasRoles trait
After cache feature(first load only), role models must be with teams id query on Model-Role and cached on Permission-Role
If that query is being executed, it is because you implemented something wrong, or the problem is somewhere else.

public function roles(): BelongsToMany
{
$relation = $this->morphToMany(
config('permission.models.role'),
'model',
config('permission.table_names.model_has_roles'),
config('permission.column_names.model_morph_key'),
app(PermissionRegistrar::class)->pivotRole
);
if (! app(PermissionRegistrar::class)->teams) {
return $relation;
}
$teamField = config('permission.table_names.roles').'.'.app(PermissionRegistrar::class)->teamsKey;
return $relation->wherePivot(app(PermissionRegistrar::class)->teamsKey, getPermissionsTeamId())
->where(fn ($q) => $q->whereNull($teamField)->orWhere($teamField, getPermissionsTeamId()));
}

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

Successfully merging this pull request may close these issues.

None yet

3 participants