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

dev/drupal#146 - Wrong link for Drupal 8 permissions page / Make CMS permissions url lookup more OO-ey #18986

Merged
merged 1 commit into from
Nov 19, 2020

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/drupal/-/issues/146

At Administer - Users and Permissions - Permissions there's a link to the CMS's permissions page. It's wrong for drupal 8/9.

This started smaller but then I noticed backdrop was completely missing so figured I might as well do it right. So this moves the hardcoded CMS names code into their respective System subclasses.

Before

Drupal 8/9 link is wrong. Backdrop also wrong.

After

Right

Technical Details

OO

Comments

@civibot
Copy link

civibot bot commented Nov 17, 2020

(Standard links)

@civibot civibot bot added the master label Nov 17, 2020
@demeritcowboy demeritcowboy force-pushed the drupal-permissions-page branch from e4aa153 to bc4824a Compare November 17, 2020 15:34
* @return array
*/
public function getCMSPermissionsUrlParams() {
return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@demeritcowboy should this be an empty array instead of NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking. Yep that would be more consistent. Have updated.

@seamuslee001
Copy link
Contributor

I tested this on Drupal 7 and WordPress and both seem to be fine. One minor issue as I have raised otherwise looks good to merge IMO

@demeritcowboy demeritcowboy force-pushed the drupal-permissions-page branch from bc4824a to 839834b Compare November 19, 2020 14:27
@demeritcowboy demeritcowboy changed the title dev/drupal#146 - Make CMS permissions url lookup more OO-ey dev/drupal#146 - Wrong link for Drupal 8 permissions page / Make CMS permissions url lookup more OO-ey Nov 19, 2020
@seamuslee001 seamuslee001 merged commit e50a017 into civicrm:master Nov 19, 2020
@demeritcowboy demeritcowboy deleted the drupal-permissions-page branch November 21, 2020 14:30
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