You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "Aboya1417 (via GitHub)" <gi...@apache.org> on 2024/03/28 15:46:59 UTC
[PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Aboya1417 opened a new pull request, #3842:
URL: https://github.com/apache/fineract/pull/3842
…ded integration test and swagger documentation
## Description
We want the loan account summary data to contain currency as we will like to know the currencies tied to different loan products during setup.
The client account endpoint https://baseurl/fineract-provider/api/v1/clients/23/accounts returns a payload containing both savings and loan account, the savings account summary contains currency while the loan account summary does not contain currency.
The aim is to replicate what already exist on the savings account summary for the loan account summary by bringing in currency.
This Jira ticket [FINERACT-2020](https://issues.apache.org/jira/browse/FINERACT-2020) contains more vivid description.
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2025749731
Commits has been squashed.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on code in PR #3842:
URL: https://github.com/apache/fineract/pull/3842#discussion_r1543289876
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java:
##########
@@ -474,6 +479,7 @@ public String loanAccountSummarySchema() {
.append(" l.charged_off_on_date as chargedOffOnDate, cobu.username as chargedOffByUsername, ")
.append(" cobu.firstname as chargedOffByFirstname, cobu.lastname as chargedOffByLastname ").append(" from m_loan l ")
.append("LEFT JOIN m_product_loan AS lp ON lp.id = l.product_id")
+ .append(" join m_currency curr on curr.code = l.currency_code")
Review Comment:
Done
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2026766505
After i updated my branch with the changes from the remote develop branch, i observed that delinquency buckets Avro scheme throws an error on the first build but on a second attempt it was successful.
I did a fresh fork and i still observed the same behaviour, i'll check to see why this is happening.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2025654338
@Aboya1417 Please squash your commits.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "jdailey (via GitHub)" <gi...@apache.org>.
jdailey commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2026136703
I see a failure in the build, might be unrelated. But, need to have you also check the boxes on the commit. See list and make sure all are covered.
Why is the delinquency buckets Avro scheme throwing an error on build?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2025546339
We want the loan account summary data to contain currency as we will like to know the currencies tied to different loan products during setup.
The client account endpoint https://baseurl/fineract-provider/api/v1/clients/23/accounts returns a payload containing both savings and loan account, the savings account summary contains currency while the loan account summary does not contain currency.
The aim is to replicate what already exist on the savings account summary for the loan account summary by bringing in currency.
This Jira ticket [FINERACT-2020](https://issues.apache.org/jira/browse/FINERACT-2020) contains more vivid description.
There was a pull request i initiated before now but that became stale because i didn't have time to add the integration test and the swagger documentation but that has been done in this PR.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2035272618
> @Aboya1417 Please fix the conflicts and squash your commits!
@adamsaghy commits has been been squashed and conflicts has been resolved.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2037298197
Hi @Aboya1417
As far as i can tell from this PR, you have 9 commits (please squash them) and this PR is conflicting with the apache/develop branch (please rebase your branch). After that i can trigger a build for this PR and we can check what exactly is failing and why.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3842:
URL: https://github.com/apache/fineract/pull/3842#discussion_r1543273267
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java:
##########
@@ -474,6 +479,7 @@ public String loanAccountSummarySchema() {
.append(" l.charged_off_on_date as chargedOffOnDate, cobu.username as chargedOffByUsername, ")
.append(" cobu.firstname as chargedOffByFirstname, cobu.lastname as chargedOffByLastname ").append(" from m_loan l ")
.append("LEFT JOIN m_product_loan AS lp ON lp.id = l.product_id")
+ .append(" join m_currency curr on curr.code = l.currency_code")
Review Comment:
Please use left join
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2032948841
> I see a failure in the build, might be unrelated. But, need to have you also check the boxes on the commit. See list and make sure all are covered.
>
> Why is the delinquency buckets Avro scheme throwing an error on build?
@jdailey i made a push to address the build failure
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2043228390
> @Aboya1417 Hi
>
> It was fixed in a separate PR. I am triggering new build
Everything seems fine now, can you please share what the issues was and the fixes you made?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2037283147
@adamsaghy, @jdailey the pipeline build keeps failing because there are other integration tests that were not written by me that fails whenever integration test is being run, however the test i wrote for this issue has passed.
Is there something else i am suppose to do for us to move forward with this?
Here is a result of the Build & Test on the pipeline
org.apache.fineract.integrationtests.LoanAccountsContainsCurrencyFieldTest
Test testGetClientLoanAccountsUsingExternalIdContainsCurrency() PASSED
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2037620603
Hi @adamsaghy,
The Fineract Build & Test still failed, is there something i did wrong?
or it's something we need to investigate why it's happening?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2034387127
@Aboya1417 Please fix the conflicts and squash your commits!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy closed pull request #3842: FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad…
URL: https://github.com/apache/fineract/pull/3842
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 closed pull request #3842: FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad…
URL: https://github.com/apache/fineract/pull/3842
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2037402058
> Hi @Aboya1417
>
> As far as i can tell from this PR, you have 9 commits (please squash them) and this PR is conflicting with the apache/develop branch (please rebase your branch). After that i can trigger a build for this PR and we can check what exactly is failing and why.
Hi @adamsaghy,
This has been done.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2038036182
@Aboya1417 Feel free to give a try...i will check also why it could happen...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3842:
URL: https://github.com/apache/fineract/pull/3842
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "Aboya1417 (via GitHub)" <gi...@apache.org>.
Aboya1417 commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2043108171
> @Aboya1417 Feel free to give a try...i will check also why it could happen...
Hi @adamsaghy, any development on this because on my end the test gets to build successfully.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2043111882
@Aboya1417 Hi
It was fixed in a separate PR. I am triggering new build
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2020:Added currency field to LoanAccountSummaryData, also ad… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #3842:
URL: https://github.com/apache/fineract/pull/3842#issuecomment-2043243552
https://github.com/apache/fineract/pull/3851
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org