-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Fix PHP Warnings. Replace fatal with statusBounce. Mark appendBreadCr… #10888
Fix PHP Warnings. Replace fatal with statusBounce. Mark appendBreadCr… #10888
Conversation
CRM/Financial/Form/Export.php
Outdated
@@ -97,7 +97,9 @@ public function preProcess() { | |||
|
|||
foreach ($batchStatus as $batchStatusId) { | |||
if ($batchStatusId == $this->_exportStatusId) { | |||
CRM_Core_Error::fatal(ts('You cannot exported the batches which were exported earlier.')); | |||
//CRM_Core_Session::singleton()->user |
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.
@mattwire any reason for leaving this commented out line here
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.
No.. :-)
…umbs parameter as array instead of string so editors don't give a warning
04e7228
to
0efdabe
Compare
@@ -97,7 +97,8 @@ public function preProcess() { | |||
|
|||
foreach ($batchStatus as $batchStatusId) { | |||
if ($batchStatusId == $this->_exportStatusId) { | |||
CRM_Core_Error::fatal(ts('You cannot exported the batches which were exported earlier.')); | |||
$url = CRM_Core_Session::singleton()->readUserContext(); |
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.
I don't think you need this $url bit here - check the start of the statusBounce fn
public static function statusBounce($status, $redirect = NULL, $title = NULL) {
$session = CRM_Core_Session::singleton();
if (!$redirect) {
$redirect = $session->readUserContext();
}
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.
That's what I thought... but it doesn't work properly without as the user context is not set otherwise in this case.
I think this is a good improvement. I have added merge-ready flag rather than merged it to give you a chance to respond to my comment. However, I don't think merging this is conditional on you making any further changes |
@eileenmcnaughton Thanks for reviewing. See response to comment :-) |
ok - I won't try to understand why then! |
…umbs parameter as array instead of string so editors don't give a warning
Overview
Fix some PHP Warnings - NFC.
Replace fatal with statusBounce (as the form doesn't redirect when exporting a second click on the export link causes a fatal error). With this change it redirects and shows a helpful error message.
Mark CRM_Utils_System appendBreadCrumbs parameter as array instead of string so editors don't give a warning.