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

[REF] getCorePermissions cleanup #19789

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 12, 2021

Overview

  • Stop passing paramter not recognised by getCorePermissions.
  • Standardise description of administer multiple organizations

Before

administer multiple organizations set up differently to all the other core permissions
Parameter passed to getCorePermissions even though the function signaure does not accept one

After

poof

Technical Details

I'm pretty sure this is just 'I don't want to touch it' that led to this permission being different. Either it's in core & is a core permission & should be subject to the same formatting etc or it should be moved to the multisite extension - while I can see the appeal of the latter it might be too big a lift for right now.

I need to rationalise getCorePermissions vs getBasicPermissions in order to extend the implied permissions concept

Comments

@seamuslee001

getCorePermissions does not accept any parameters so this stops passing them
@civibot
Copy link

civibot bot commented Mar 12, 2021

(Standard links)

@civibot civibot bot added the master label Mar 12, 2021
@eileenmcnaughton eileenmcnaughton changed the title [REF] Stop passing paramter not recognised by getCorePermissions [REF] getCorePermissions cleanup Mar 12, 2021
@colemanw
Copy link
Member

Code cleanup looks sensible & tests pass.

@colemanw colemanw merged commit 6ebc8f4 into civicrm:master Mar 12, 2021
@colemanw colemanw deleted the permc branch March 12, 2021 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants