REF - Normalize internal format of permissions #29173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before: Permissions might be in associative or non-associative array format. After: Always associative regardless of how they are passed in from hook_civicrm_permission(). This also improves efficiency by only calling the hook once, and improves memory use by caching the array once.
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
|
Cool - should we start adding deprecation notices for the oldest of the 3 formats? |
|
@eileenmcnaughton we just did! This PR adds noisy deprecation for both old formats & you merged it so now it's official :) |
|
@colemanw oh right - that makes me a bit nervous - if you have an extension can you already change the keys from 0,1 to 'label'? No older versions access them from the numbers? Cos if they do people can't update their extensions ? |
|
@eileenmcnaughton The associative format has been supported for a long time... at least since Because the previous code never actually cared about the array keys it would just shift the first item off the array and take it as the label, then shift the second item as the description. |
colemanw
added a commit
to colemanw/civicrm-drupal
that referenced
this pull request
Feb 7, 2024
colemanw
added a commit
to colemanw/civicrm-backdrop
that referenced
this pull request
Feb 7, 2024
colemanw
added a commit
to colemanw/civicrm-core
that referenced
this pull request
Feb 7, 2024
totten
added a commit
to totten/civicrm-core
that referenced
this pull request
Feb 7, 2024
There was a recent patch as part of civicrm#29173, so the version-number should be raised.
totten
reviewed
Feb 7, 2024
| @@ -402,6 +402,9 @@ public function getAllModulePermissions(): array { | |||
| // Convert them all to associative arrays. | |||
| foreach ($permissions as $name => $defn) { | |||
| $defn = (array) $defn; | |||
| if (!isset($defn['label'])) { | |||
| CRM_Core_Error::deprecatedWarning("Permission '$name' should be declared with 'label' and 'description' keys. See https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_permission/"); | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Note for extension authors: Switching your extension's hook_civicrm_permissions to return the preferred format (with 'label' & 'description' array keys) will avoid deprecation notices added by this PR. It is safe to make that change without requiring the latest version of CiviCRM, as the preferred format is supported for every 5.x.x. version.
Historically, there have been 3 formats for defining a permission. This consolidates the format internally, while maintaining backward-support for the other two.
Includes a documentation update: https://lab.civicrm.org/documentation/docs/dev/-/merge_requests/1148
Before
Permissions added via hook might be in associative or non-associative array format, or they might be a plain string.
After
Internally they are now always associative regardless of how they are passed in from
hook_civicrm_permission().Deprecation notice issued for extensions that pass them in the old ways.
Technical Details
Deprecation notice added to alert extension devs to update the format.
This also improves efficiency by only calling the hook once, and improves memory use by caching the array once.