-
Notifications
You must be signed in to change notification settings - Fork 236
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
REPORT-848: added try catch statements to catch NullPointerException #170
base: master
Are you sure you want to change the base?
Conversation
…n different forms
@@ -148,5 +151,10 @@ else if (successUrl.startsWith(pathToRemove)) { | |||
} | |||
design = rs.saveReportDesign(design); | |||
return "redirect:" + successUrl; | |||
}catch(PropertyValueException e) { |
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.
@mozzy11 , what happens when a NullPointerException
is thrown instead?
@@ -213,5 +215,9 @@ else if (successUrl.startsWith(pathToRemove)) { | |||
} | |||
design = rs.saveReportDesign(design); | |||
return "redirect:" + successUrl; | |||
}catch(PropertyValueException e) { |
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.
Same applies here and anywhere else in this PR
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 did several testing of that , and instead of a Null Pointer Exception being throen when an empty parameter is passed to the controller, its a PropertyValueEception that is thrown . i think the NPE is already manged by the util methods used there in that specific controller where i was catching the propertValueException
@@ -137,6 +140,10 @@ public String editReportDesign(ModelMap model, | |||
url.append("&returnUrl=" + returnUrl); | |||
} | |||
return "redirect:"+url.toString(); | |||
}catch(NullPointerException e){ |
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.
it was only here , where an empty parameter passed to the controller throws a NPE
@dkayiwa can this be merged? Screenshot provided in comments here: https://issues.openmrs.org/browse/REPORT-848 |
@gracepotma cannot be merged with merge conflicts. 😊 |
@mozzy11 can you address the merge conflicts? |
Ticket: https://issues.openmrs.org/browse/REPORT-848