We are making calculations, gathering cacheable metadata, checking for access, etc. based on group permissions all over the place. Let's centralize this so we can get rid of a lot of duplicate and ugly code. This issue is to discuss the overall changes, whereas the specific implementations will go into smaller patches in child issues.

CommentFileSizeAuthor
#5 group-3026897-5.patch78.89 KBkristiaanvandeneynde

Comments

kristiaanvandeneynde created an issue. See original summary.

kristiaanvandeneynde’s picture

Title: Centralize all permission calculations/operations » [META] Centralize all permission calculations/operations
Issue summary: View changes

Code's finished, turning this into a meta issue and adding smaller patches in child issues.

kristiaanvandeneynde’s picture

Issue tags: +Needs change record

Everything went green! So now all we need is a change record for this meta issue.

A potential follow-up that is now possible is the altering of permissions by firing a "calculating group permissions" event from GroupPermissionCalculator.

kristiaanvandeneynde’s picture

Assigned: kristiaanvandeneynde » Unassigned
kristiaanvandeneynde’s picture

Status: Active » Needs review
StatusFileSize
new78.89 KB

Attached is a final patch that cleans up the previous work.

Most notably, it moves away from using "magic arrays" to represents permissions per scope and actually introduces a CalculatedGroupPermissionsItem value object which represents an account's permissions within a given scope. This allowed for further simplifications in the permission checker, calculator, hash generator and context.

Will need a change record for the new API and for the hash generator refactoring (because BC, sadly).

kristiaanvandeneynde’s picture

Will write up change records after lunch and then this can be considered Fixed.

kristiaanvandeneynde’s picture

Status: Needs review » Fixed
Issue tags: -Needs change record

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.