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.

Sign up for GitHub

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

CRM-19739 : New Account Relationship Option Screen does not show excep… #10651

Merged
merged 3 commits into from
Aug 21, 2017
Merged

CRM-19739 : New Account Relationship Option Screen does not show excep… #10651

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74c3274
CRM-19739: New Account Relationship Option Screen does not show excep…
Jul 13, 2017
a48a4a2
replace setStatus with statusBounce
Aug 14, 2017
6bef1cd
Style fix and move success msg
Aug 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CRM/Financial/BAO/FinancialTypeAccount.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ public static function createDefaultFinancialAccounts($financialType) {
*
* @param obj $financialTypeAccount of CRM_Financial_DAO_EntityFinancialAccount
*
* @throws CRM_Core_Exception
*/
public static function validateRelationship($financialTypeAccount) {
$financialAccountLinks = CRM_Financial_BAO_FinancialAccount::getfinancialAccountRelations();
Expand All @@ -282,7 +283,7 @@ public static function validateRelationship($financialTypeAccount) {
$params = array(
1 => $accountRelationships[$financialTypeAccount->account_relationship],
);
throw new Exception(ts("This financial account cannot have '%1' relationship.", $params));
throw new CRM_Core_Exception(ts("This financial account cannot have '%1' relationship.", $params));
}
}

Expand Down
7 changes: 6 additions & 1 deletion CRM/Financial/Form/FinancialTypeAccount.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,12 @@ public function postProcess() {
if ($this->_action & CRM_Core_Action::ADD) {
$params['entity_id'] = $this->_aid;
}
$financialTypeAccount = CRM_Financial_BAO_FinancialTypeAccount::add($params, $ids);
try {
$financialTypeAccount = CRM_Financial_BAO_FinancialTypeAccount::add($params, $ids);
}
catch (CRM_Core_Exception $e) {
CRM_Core_Error::statusBounce($e->getMessage());
}
CRM_Core_Session::setStatus(ts('The financial type Account has been saved.'));
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton Aug 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

So, what I was alluding to above is I think this could be

try {
  $financialTypeAccount = CRM_Financial_BAO_FinancialTypeAccount::add($params, $ids);
}
catch (CRM_Core_Exception $e) {
  CRM_Core_Session::statusBounce($e->getMessage());
}

Copy link
Contributor Author

@jitendrapurohit jitendrapurohit Aug 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Tested and updated the code with statusBounce(). Thanks for the suggestion 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

L308 should be moved after line 303?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @pradpnayak - the statusBounce will mean the code does not reach that line, but that will be much clear if you change it as suggested @jitendrapurohit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the success message to the specified line for clarity.

}

Expand Down