-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
FINERACT-1135: Fix SQL error at loan repayment template #1296
FINERACT-1135: Fix SQL error at loan repayment template #1296
Conversation
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.
Looks OK to me.
Minor feedback for now and the future, Always use the issue number in all caps when using it in a commit message: |
...main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java
Show resolved
Hide resolved
sqlBuilder.append( | ||
" l.currency_code as currencyCode, l.currency_digits as currencyDigits, l.currency_multiplesof as inMultiplesOf, rc.`name` as currencyName, "); | ||
sqlBuilder.append(" rc.display_symbol as currencyDisplaySymbol, rc.internationalized_name_code as currencyNameCode "); | ||
sqlBuilder.append(" FROM m_loan l"); |
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.
Is there a reason for removing all these spaces from beginning of the strings? The reason for them, as far as I can see, is avoiding a situation where someone adds before a string that doesn't have a space at the end, and causes the next string to therefore generate broken SQL. I don't think they are harmful and can save us from some accidental illegal SQL...
thanks @awasum |
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.
LGTM
@francisguchie any chance to get this backported to the release branch 1.4.0? The Jira ticket for that is still open... would appreciate if we could do this soon to push the release over the finish line. Thanks. |
Description
https://issues.apache.org/jira/browse/FINERACT-1135
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm has been updated with details of any API changes.
Integration tests have been created/updated for verifying the changes made.
All Integrations tests are passing with the new commits.
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide