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

[NFC] Extract function to get component permissions #12994

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Function extraction as part of code simplification

Before

Code harder to read

After

Code easier to read

Technical Details

Just an extraction

Comments

I will follow this with fixes described in https://lab.civicrm.org/dev/core/issues/454 with the end goal of making getcount work with permissions

@civibot
Copy link

civibot bot commented Oct 24, 2018

(Standard links)

@civibot civibot bot added the master label Oct 24, 2018
@@ -2772,7 +2730,7 @@ public static function checkPermission($activityId, $action) {
$targetID = CRM_Utils_Array::key('Activity Targets', $activityContacts);

// Check for source contact.
if (!$componentId || $allow) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton this seems to be a bit odd given that what if there is no component id?

Copy link
Contributor Author

@eileenmcnaughton eileenmcnaughton Oct 24, 2018

Choose a reason for hiding this comment

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

@seamuslee001 yeah I remove componentId there because I return TRUE when componentId if NULL from the hasPermission function. $allow is reset 2 lines after that point so the change doesn't carry on past there

@seamuslee001
Copy link
Contributor

I am satisfied that nothing is changing here and it is in deed NFC

@seamuslee001
Copy link
Contributor

Merging

@seamuslee001 seamuslee001 merged commit 12e4974 into civicrm:master Oct 24, 2018
@eileenmcnaughton eileenmcnaughton deleted the activity_extract branch October 24, 2018 02:54
@eileenmcnaughton
Copy link
Contributor Author

thanks @seamuslee001 !

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