[REF] Remove calls to fatal() #16746
Conversation
Also other minor cleanup on - comment blocks - using strict comparison - don't use elseif where the prior if threw an exceptionn
|
(Standard links)
|
| @@ -88,7 +90,7 @@ public static function add(&$params) { | |||
| if (!empty($params['workflow_id']) && !CRM_Core_Permission::check('edit system workflow message templates')) { | |||
| throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $systemWorkflowPermissionDeniedMessage])); | |||
| } | |||
| elseif (!CRM_Core_Permission::check('edit user-driven message templates')) { | |||
| if (!CRM_Core_Permission::check('edit user-driven message templates')) { | |||
There was a problem hiding this comment.
elseif is meaningless when previous if throws an exception
There was a problem hiding this comment.
@eileenmcnaughton i think it might have some meaning, If you have an end user that is able to edit system workflow templates but not the user driven ones (why i have no idea). The change here would mean that the check for user driven message templates permission is now no longer dependant on if you have a workflow_id or not.
There was a problem hiding this comment.
Right but if the first IF is true then it throws an exception & never reaches the second - which is what elseif achieves when no exception is thrown
There was a problem hiding this comment.
My point is if the user is able to satisfy the permission check i.e. the if fails (we have a workflow id and they have the correct permission) changing from an elseif to an if will mean the 2nd permission check will run no matter if there is a workflow_id in the params or not. With elseif it will only run if there is no workflow id in the params array.
(edited comment)
There was a problem hiding this comment.
So we have 2 checks
If check 1 is true then an exception is thrown and the process does not proceed. The elseif is never reached
If not then the second check is performed, as it would be if the elseif were there
The second check will still only run if the first check is false because of the early exit
Overview
Before
use of CRM_Core_Fatal()
After
throwing exceptions
Technical Details
Comments