DRM-09 : startOfPeriod and endOfPeriod not there exception should be handled properly #61
Conversation
|
@dkayiwa I made the changes according to the document. Please review and give feedback. |
| dvTypeList.add( dvtype ); | ||
| } | ||
| catch ( NumberFormatException e ) | ||
| { |
There was a problem hiding this comment.
It is not recommended to catch exceptions and do nothing. You would rather leave them to bubble through.
There was a problem hiding this comment.
You seem to have nothing to do in the exception handler. Can't you just remove the try and catch to let it bubble up?
| query.setParameter( "endOfPeriod", period.getEndDate() ); | ||
| if ( parameters.contains( "startOfPeriod" ) ) | ||
| { | ||
| query.setParameter( "startOfPeriod", period.getStartDate() ); |
There was a problem hiding this comment.
Can you add some unit tests for these changes?
|
@dkayiwa I changed the exception handling part and added a test case. |
|
@Choxmi can you squash the commits into one as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips |
Update ReportController.java TestCase for evaluateDataValueTemplate
|
Will do @dkayiwa |
|
@dkayiwa can you please review it now. |
|
@Choxmi you do not seem to have addressed my review comments. |
|
@Choxmi i will reopen when you resume |
Link of the JIRA ticket : https://issues.openmrs.org/browse/DRM-9