Skip to content

FINERACT-1724: Approval limit calculation modification#3286

Merged
adamsaghy merged 1 commit intoapache:developfrom
abraham-menyhart:FINERACT-1724-approval-limit-calculation-modification
Jul 5, 2023
Merged

FINERACT-1724: Approval limit calculation modification#3286
adamsaghy merged 1 commit intoapache:developfrom
abraham-menyhart:FINERACT-1724-approval-limit-calculation-modification

Conversation

@abraham-menyhart
Copy link
Copy Markdown
Contributor

Description

Describe the changes made and why they were made.

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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure the totalDisbursed is containing this "newly" disbursed amount already?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I just double checked it, and it does contain the newly disbursed amount as well.

Copy link
Copy Markdown
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 and please add test cases which validate your fix!

@abraham-menyhart abraham-menyhart force-pushed the FINERACT-1724-approval-limit-calculation-modification branch from f8635f1 to 563aee4 Compare July 5, 2023 10:54
@abraham-menyhart
Copy link
Copy Markdown
Contributor Author

Please see my comments and please add test cases which validate your fix!

@adamsaghy can you please recheck? I added an integration test to validate this case.

@abraham-menyhart abraham-menyhart force-pushed the FINERACT-1724-approval-limit-calculation-modification branch 2 times, most recently from 0953675 to 4b125b7 Compare July 5, 2023 11:36
@abraham-menyhart abraham-menyhart force-pushed the FINERACT-1724-approval-limit-calculation-modification branch from 4b125b7 to 22a4e97 Compare July 5, 2023 11:37
Copy link
Copy Markdown
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 28747fe into apache:develop Jul 5, 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