Problem/Motivation

There is a typo in src/Access/ChainGroupPermissionCalculator.php. The word calculator is misspelled as "calulator".

Proposed resolution

Fix the typo in the CID.

Issue fork group-3221785

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

devkinetic created an issue. See original summary.

devkinetic opened merge request !26

devkinetic’s picture

devkinetic commented
Status: Active » Needs review
kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Fix the CID, so it matches the rest of the calls for that specific cache.

As far as I can tell, that is the only place the CID is being used. So while it is a typo, it's not breaking anything, right?

devkinetic’s picture

devkinetic commented

I found this while integrating a patch I wrote over a year ago around group membership edit permissions back on alpha 4 or 5. I believe we came to the consensus that group wasn't going to implement permissions to edit the actual user? In either case, it was simply a minor find. I can't say if anything breaks, but the chained group permission calculator itself can be called.

I will remove that line from the description, it was based on an assumption not fact.

devkinetic’s picture

devkinetic commented
Issue summary: View changes
kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Right, in that case, I'm thinking it might be prudent to include a post_update hook that reads out the old cache ID and stores it in the new one. Group permissions are cached because they're really expensive to calculate and maybe some websites won't like having to recalculate something that was still fine.

Maybe this is providing too much of a service, though. But I've had performance complaints before so I'm trying to tread carefully when it comes to clearing people's caches :D

devkinetic’s picture

devkinetic commented

I think that is a good idea. When we have to rebuild one of our bigger sites, we schedule downtime. I'm sure it would be appreciated, and since this isn't breaking anything currently, there is no rush.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs review » Reviewed & tested by the community

Looking at the code again I was mistaken. The cache we're talking about here is a simple static cache. So it's perfectly fine to simply change the name of the cache ID.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Reviewed & tested by the community » Fixed

Trying merge button for the first time, let's see what that does.

devkinetic’s picture

devkinetic commented

Awesome!

Status: Fixed » Closed (fixed)

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