ptuomola commented on a change in pull request #1368:
URL: https://github.com/apache/fineract/pull/1368#discussion_r499273863
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java
##########
@@ -124,12 +125,13 @@ public Response runReport(@PathParam("reportName")
@Parameter(description = "rep
parameterTypeValue = "report";
String reportType =
this.readExtraDataAndReportingService.getReportType(reportName,
isSelfServiceUserReport);
ReportingProcessService reportingProcessService =
this.reportingProcessServiceProvider.findReportingProcessService(reportType);
- if (reportingProcessService != null) {
- return reportingProcessService.processRequest(reportName,
queryParams);
+ if (reportingProcessService == null) {
+ throw new
PlatformServiceUnavailableException("err.msg.report.service.implementation.missing",
+ ReportingProcessServiceProvider.SERVICE_MISSING +
reportType, reportType);
Review comment:
Unfortunately based on my read of the code, I don't think this is quite
right:
After this change, we are bailing out if we can't find a reporting process
service for the report type.
Previously, if we couldn't find one, we would continue to execute the
"hardcoded" reporting process service in the same function below.
I think continuing execution is the expected behaviour: for any report that
is not of type Pentaho or SMS, the report_type is set to Table. But there is no
ReportingProcessService with type Table, so therefore the logic in this
function below is used to simply prepare the CSV / PDF file.
In other words: if we check this in, as far as I can see it would break any
report that doesn't have a specific report handler configured.
Now having said that, a good idea would be to create a "Table" type report
handler and move the code from this function to that report handler. And after
that we could assume that there's a handler for each type of report, and bail
out if we can't find one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
