[GitHub] [fineract] vorburger commented on a change in pull request #1251: FINERACT-1095 Added status parameter in Loans API

[GitBox <gi...@apache.org>]

vorburger commented on a change in pull request #1251:
URL: https://github.com/apache/fineract/pull/1251#discussion_r496273507



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanStatus.java
##########
@@ -80,6 +82,42 @@ public static LoanStatus fromInt(final Integer statusValue) {
         return enumeration;
     }
 
+    public static LoanStatus fromString(final String statusString) {
+
+        LoanStatus enumeration = LoanStatus.INVALID;
+
+        if (StringUtils.isEmpty(statusString)) {
+            return enumeration;
+        }
+
+        if (statusString.equalsIgnoreCase(LoanStatus.SUBMITTED_AND_PENDING_APPROVAL.toString())) {

Review comment:
       @thesmallstar I'm probably missing something, but isn't this just `LoanStatus.valueOf(statusString)`? If you're doing this only because of `equalsIgnoreCase()` then wouldn't a `LoanStatus.valueOf(statusString.toUpperCase())` suffice and have the same effect? If you want an invalid status String to return LoanStatus.INVALID, you may have to `catch (IllegalArgumentException e)` .. (I think; not 100% sure, do check). Actually that is debatable.. wouldn't you rather that an invalid status filter passed through the REST API from a client lead to an error, than be silently ignored?




----------------------------------------------------------------
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:
users@infra.apache.org