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

Update WP permission page for new string array keys #20045

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

colemanw
Copy link
Member

Overview

This fixes an undefined index notice for permissions that use the new-style 'label' and 'description' array keys instead of numeric keys.

Before

image

image

After

No notice and descriptions are visible:
image

Technical Details

It seems there is a new syntax supported by the permission system and this WordPress page didn't get the memo.

This fixes an undefined index notice for permissions that use the new-style
'label' and 'description' array keys instead of numeric keys.
@civibot
Copy link

civibot bot commented Apr 13, 2021

(Standard links)

@civibot civibot bot added the master label Apr 13, 2021
@colemanw
Copy link
Member Author

@eileenmcnaughton am I right in assuming that this new array syntax is what we're moving to and this page needs to support it?

@eileenmcnaughton
Copy link
Contributor

@colemanw yes - although the word 'new' is arguable since I think it has been quite a few years,

This makes sense to me - I'll just give @kcristiano / @christianwach a chance to comment

@christianwach
Copy link
Member

I honestly haven't looked at the code that generates that page since I fixed the table layout many moons ago. If this change fixes things, then I'm all for it!

@eileenmcnaughton
Copy link
Contributor

I think the change is trivial enough that that is enough endorsement :-)

@eileenmcnaughton eileenmcnaughton merged commit c7a6cc2 into civicrm:master Apr 13, 2021
@eileenmcnaughton eileenmcnaughton deleted the fixWpPermissionPage branch April 13, 2021 21:35
@colemanw
Copy link
Member Author

Just an aside, it bugs me when we add a new pattern but don't take the time to update the old ones; they sit around collecting technical debt and, in many cases, get replicated by unsuspecting devs who (correctly) observe them to be the prevalent pattern in our codebase and therefore the correct examples to follow.

@christianwach
Copy link
Member

@colemanw Not that I wrote it, but in defence of the code you fixed, it seems to be over six years old, so presumably predates the current pattern. 🤷 😄

@eileenmcnaughton
Copy link
Contributor

@colemanw I hear you & @christianwach I think the comment was directed at whoever fixed if for drupal & not wordpress (which I am totally not going to git blame in case it turns out to be me)

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.

3 participants