Skip to content

FINERTACT-1981: Advanced payment allocation fixes for Loan Schedule#3611

Merged
adamsaghy merged 1 commit intoapache:developfrom
josehernandezfintecheandomx:enhancement/loan_product_adv_pment_alloc_validation
Dec 19, 2023
Merged

FINERTACT-1981: Advanced payment allocation fixes for Loan Schedule#3611
adamsaghy merged 1 commit intoapache:developfrom
josehernandezfintecheandomx:enhancement/loan_product_adv_pment_alloc_validation

Conversation

@josehernandezfintecheandomx
Copy link
Contributor

…essive loan disbursement handling

Description

Advanced payment allocation should only support Progressive loan disbursement handling, so we needed to add a validation in the API

Screenshot 2023-11-28 at 10 16 43 p m

Ignore if these details are present on the associated Apache Fineract JIRA ticket.

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.

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add testing!

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch from 5f556c5 to 711d89f Compare November 30, 2023 05:40
@josehernandezfintecheandomx
Copy link
Contributor Author

Please add testing!

Integration Test case added

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the description accordingly what the test is testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please throw unique error code supported.only.for.progressive.loan.schedule.handling is already been used elsewhere for other purposes!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments!

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch from 711d89f to 3417d61 Compare December 1, 2023 03:47
@josehernandezfintecheandomx
Copy link
Contributor Author

Please see my comments!

Comments resolved

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch 2 times, most recently from b55a57b to 2ccefdf Compare December 6, 2023 18:48
@josehernandezfintecheandomx josehernandezfintecheandomx changed the title FINERTACT-1724: Advanced payment allocation should only support Progr… FINERTACT-1724: Advanced payment allocation fixes for Loan Schedule Dec 6, 2023
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy
Copy link
Contributor

@josehernandezfintecheandomx Please check the failing tests!

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch 3 times, most recently from 96d7a0f to b69f97d Compare December 12, 2023 07:54
@adamsaghy adamsaghy changed the title FINERTACT-1724: Advanced payment allocation fixes for Loan Schedule FINERTACT-1981: Advanced payment allocation fixes for Loan Schedule Dec 12, 2023
@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch 3 times, most recently from 6d3face to 3cf4b53 Compare December 13, 2023 20:32
@adamsaghy
Copy link
Contributor

@josehernandezfintecheandomx Please check and fix the failing test cases!

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch from 3cf4b53 to 9392b05 Compare December 14, 2023 17:21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward compatibility, the loan schedule type is optional. So it might not be sent, in that case the CUMULATIVE is the fallback default value. You might wanna mimic that behaviour here as well.

@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch from 9392b05 to fb2774a Compare December 15, 2023 00:42
@josehernandezfintecheandomx josehernandezfintecheandomx force-pushed the enhancement/loan_product_adv_pment_alloc_validation branch from fb2774a to c15540c Compare December 15, 2023 14:46
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy adamsaghy merged commit aff009c into apache:develop Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants