-
Notifications
You must be signed in to change notification settings - Fork 841
CRM: Ensuring users assigned to contacts linked to transactions, invoices, quotes can view those objects #36239
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
Merged
Merged
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
2c23753
Ensuring users assigned to contacts linked to transactions, invoices,…
coder-karen 3354c68
changelog
coder-karen de65391
Cast settings checks to int after changing to trict comparison for ex…
coder-karen 958fd2c
Editing two comments to more correctly reflect changes below
coder-karen e986c52
Making sure we check if a company is assigned to an invoice before de…
coder-karen d91d8e1
Merge remote-tracking branch 'origin/trunk' into fix/crm-assign-owner…
coder-karen 2258f7c
Changing int casting and moving equality to be loose in two cases - s…
coder-karen a5b0ec9
Merge remote-tracking branch 'origin/trunk' into fix/crm-assign-owner…
coder-karen 0f0a197
Removing strict equality check
coder-karen 51168bf
Removing DAL3 check
coder-karen c05af7e
Merge remote-tracking branch 'origin/trunk' into fix/crm-assign-owner…
coder-karen 154bd2e
Update projects/plugins/crm/includes/ZeroBSCRM.Edit.php
coder-karen 0e8fe5b
Adding check for transactions and invoices with assigned contacts but…
coder-karen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
projects/plugins/crm/changelog/fix-crm-assign-ownership-viewobjects
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: patch | ||
| Type: fixed | ||
|
|
||
| Permissions: Allowing users assigned to contacts to view linked objects even if assign ownership is unchecked |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,41 +164,60 @@ public function preChecks(){ | |
|
|
||
| global $zbs; | ||
|
|
||
| // only do this stuff v3.0+ | ||
| if ($zbs->isDAL3()){ | ||
|
|
||
| $is_malformed_obj = false; | ||
|
|
||
| if (is_array($this->obj) && isset($this->obj['owner'])){ | ||
| $objOwner = (int)$this->obj['owner']; | ||
| $obj_owner = (int) $this->obj['owner']; | ||
|
|
||
| // Transactions can have a contact or company assigned, and quotes just a contact. This covers checking owners for both. | ||
| if ( isset( $this->obj['contact'][0]['owner'] ) ) { | ||
| $obj_owner = (int) $this->obj['contact'][0]['owner']; | ||
|
|
||
| } elseif ( isset( $this->obj['company'][0]['owner'] ) ) { | ||
| $obj_owner = (int) $this->obj['company'][0]['owner']; | ||
| // phpcs:disable Generic.WhiteSpace.ScopeIndent.IncorrectExact,Generic.WhiteSpace.ScopeIndent.Incorrect -- this sniff is incorrectly reporting spacing issues. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sometimes this old code makes the linter go haywire, I see no problem in disabling the error for this line. |
||
| } | ||
|
|
||
| // This covers checking owners for assigned contacts or companies in invoices. | ||
| if ( $this->objTypeID === ZBS_TYPE_INVOICE ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| $data = zeroBSCRM_invoicing_getInvoiceData( $this->objID ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| if ( ! empty( $data['invoiceObj']['contact'] ) ) { | ||
| $obj_owner = (int) $data['invoiceObj']['contact'][0]['owner']; | ||
| } elseif ( ! empty( $data['invoiceObj']['contact'] ) ) { | ||
| $obj_owner = (int) $data['invoiceObj']['company'][0]['owner']; | ||
| } | ||
| } | ||
| } else { | ||
| // phpcs:enable Generic.WhiteSpace.ScopeIndent.IncorrectExact,Generic.WhiteSpace.ScopeIndent.Incorrect | ||
| // if $this->obj is not an array, somehow it's not been loaded properly (probably perms) | ||
| // get owner info anyway | ||
| $is_malformed_obj = true; | ||
| $objOwner = $zbs->DAL->getObjectOwner(array( | ||
| 'objID' => $this->objID, | ||
| 'objTypeID' => $this->objTypeID | ||
| )); | ||
| $obj_owner = $zbs->DAL->getObjectOwner( // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| array( | ||
| 'objID' => $this->objID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| 'objTypeID' => $this->objTypeID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| ) | ||
| ); | ||
| } | ||
| // get current user | ||
| $currentUserID = get_current_user_id(); | ||
| $current_user_id = get_current_user_id(); | ||
|
|
||
| if ($objOwner > 0 && $objOwner != $currentUserID){ | ||
| // not current user | ||
| // does user have perms to edit? | ||
| $canEditAllContacts = current_user_can('admin_zerobs_customers') && $zbs->settings->get('perusercustomers') == 0; | ||
| $canGiveOwnership = $zbs->settings->get('usercangiveownership') == 1; | ||
| $canChangeOwner = ($canGiveOwnership || current_user_can('administrator') || $canEditAllContacts); | ||
| if ( $obj_owner > 0 && $obj_owner != $current_user_id ) { // phpcs:ignore Universal.Operators.StrictComparisons.LooseNotEqual -- as below, there is the chance the numbers could be strings here, as expected elsewhere in the plugin. | ||
| // not current user | ||
| // does user have perms to edit? | ||
| $can_edit_all_contacts = current_user_can( 'admin_zerobs_customers' ) && $zbs->settings->get( 'perusercustomers' ) == 0; // phpcs:ignore Universal.Operators.StrictComparisons.LooseEqual,WordPress.WP.Capabilities.Unknown -- this was defined in ZeroBSCRM.Permissions.php. | ||
| $can_give_ownership = $zbs->settings->get( 'usercangiveownership' ) == 1; // phpcs:ignore Universal.Operators.StrictComparisons.LooseEqual -- also above, there is the chance the numbers could be strings here, as expected elsewhere in the plugin. | ||
| $can_change_owner = ( $can_give_ownership || current_user_can( 'manage_options' ) || $can_edit_all_contacts ); | ||
|
|
||
| if (!$canChangeOwner){ | ||
| if ( ! $can_change_owner ) { | ||
|
|
||
| // owners can't be changed with user's perms, so denied msg | ||
| $this->preCheckFail( sprintf( __( 'You do not have permission to edit this %s.', 'zero-bs-crm' ), $zbs->DAL->typeStr( $this->objTypeID ) ) ); | ||
| return false; | ||
| // owners can't be changed with user's perms, so denied msg | ||
| // Translators: %s is the object type (for example transaction, quote, invoice). | ||
| $this->preCheckFail( sprintf( __( 'You do not have permission to edit this %s.', 'zero-bs-crm' ), $zbs->DAL->typeStr( $this->objTypeID ) ) ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| return false; | ||
|
|
||
| } | ||
| if ( !$this->has_permissions_to_edit ){ | ||
|
|
||
| // user does not have a role which can edit this object type | ||
| $this->preCheckFail( sprintf( __( 'You do not have permission to edit this %s.', 'zero-bs-crm' ), $zbs->DAL->typeStr( $this->objTypeID ) ) ); | ||
| return false; | ||
|
|
@@ -212,8 +231,6 @@ public function preChecks(){ | |
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| //load if is legit | ||
| return true; | ||
| } | ||
|
|
||
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.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!