galovics commented on code in PR #2577:
URL: https://github.com/apache/fineract/pull/2577#discussion_r966392914
##########
fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java:
##########
@@ -69,6 +71,20 @@ public class ConfigureBusinessStepResource {
private final ConfigJobParameterService configJobParameterService;
private final PortfolioCommandSourceWritePlatformService
commandWritePlatformService;
+ @GET
+ @Path("/names")
+ @Produces({ MediaType.APPLICATION_JSON })
+ @Operation(summary = "List Business Jobs", description = "Returns the
configured Business Jobs")
Review Comment:
I'm not sure what you mean by "Business Jobs". Probably you wanna write
something like "List of jobs with configurable business steps".
##########
fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java:
##########
@@ -69,6 +71,20 @@ public class ConfigureBusinessStepResource {
private final ConfigJobParameterService configJobParameterService;
private final PortfolioCommandSourceWritePlatformService
commandWritePlatformService;
+ @GET
+ @Path("/names")
+ @Produces({ MediaType.APPLICATION_JSON })
+ @Operation(summary = "List Business Jobs", description = "Returns the
configured Business Jobs")
+ @ApiResponses({
+ @ApiResponse(responseCode = "200", description = "OK", content =
@Content(array = @ArraySchema(schema = @Schema(implementation =
ConfigureBusinessStepResourceSwagger.GetBusinessJobConfigResponse.class)))) })
+ public String retrieveAllConfiguredBusinessJobs(@Context final UriInfo
uriInfo) {
Review Comment:
Method naming is wrong - see above.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResourceSwagger.java:
##########
@@ -27,6 +27,15 @@ private ConfigureBusinessStepResourceSwagger() {
}
+ @Schema(description = "GetBusinessJobConfigResponse")
+ public static final class GetBusinessJobConfigResponse {
+
+ private GetBusinessJobConfigResponse() {}
+
+ public List<String> businessJobs;
Review Comment:
Naming.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java:
##########
@@ -69,6 +71,20 @@ public class ConfigureBusinessStepResource {
private final ConfigJobParameterService configJobParameterService;
private final PortfolioCommandSourceWritePlatformService
commandWritePlatformService;
+ @GET
+ @Path("/names")
+ @Produces({ MediaType.APPLICATION_JSON })
+ @Operation(summary = "List Business Jobs", description = "Returns the
configured Business Jobs")
+ @ApiResponses({
+ @ApiResponse(responseCode = "200", description = "OK", content =
@Content(array = @ArraySchema(schema = @Schema(implementation =
ConfigureBusinessStepResourceSwagger.GetBusinessJobConfigResponse.class)))) })
+ public String retrieveAllConfiguredBusinessJobs(@Context final UriInfo
uriInfo) {
+
+ List<String> businessJobNames =
configJobParameterService.getAllJobNames();
+ final Gson gson = new Gson();
Review Comment:
We're not using GSON directly to serialize API data. Please look at existing
other API implementations.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStepRepository.java:
##########
@@ -21,8 +21,13 @@
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
+import org.springframework.data.jpa.repository.Query;
public interface BatchBusinessStepRepository extends
JpaRepository<BatchBusinessStep, Long>,
JpaSpecificationExecutor<BatchBusinessStep> {
List<BatchBusinessStep> findAllByJobName(String jobName);
+
+ @Query("SELECT DISTINCT bbs.jobName FROM BatchBusinessStep bbs")
+ List<String> findJobNames();
Review Comment:
Don't like the naming here, probably like `findConfiguredJobNames`
##########
fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java:
##########
@@ -69,6 +71,20 @@ public class ConfigureBusinessStepResource {
private final ConfigJobParameterService configJobParameterService;
private final PortfolioCommandSourceWritePlatformService
commandWritePlatformService;
+ @GET
+ @Path("/names")
Review Comment:
Definitely not the proper naming for the API (`/jobs/names`). Please figure
out a business step related API naming here.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/service/ConfigJobParameterService.java:
##########
@@ -30,4 +31,7 @@ public interface ConfigJobParameterService {
CommandProcessingResult updateStepConfigByJobName(JsonCommand command,
String jobName);
JobBusinessStepDetail getAvailableBusinessStepsByJobName(String jobName);
+
+ List<String> getAllJobNames();
Review Comment:
Naming I don't like, see other comments.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
