FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

FINERACT-2793: migrate loan money movement tests to feign - #6355

Open
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign
Open

FINERACT-2793: migrate loan money movement tests to feign#6355
DeathGun44 wants to merge 15 commits into
apache:developfrom
DeathGun44:FINERACT-2793/migrate-loan-money-movement-tests-to-feign

Conversation

@DeathGun44

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 our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

LoanTransactionAuditingIntegrationTest reverses a repayment as a second
user to prove the audit fields record who made the change. Creating that
user was the last thing in the file still going through REST Assured's
UserHelper, and FeignUserHelper only knew how to build its own fixed
non-bypass user.
The generated UsersApi already exposes createUser; this just wraps it the
way the rest of the helpers do, returning the full response so callers can
read the generated id.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign
These three already extended FeignLoanTestBase but kept a parallel REST
Assured stack alive in @beforeeach: a RequestSpecification, a
ResponseSpecification and a legacy LoanTransactionHelper built on top of
them. Everything they called through that stack already had a typed
equivalent on the base, so the stack goes and the call sites move over.
Loan products now come from LoanProductTestBuilder.buildRequest() and loan
applications from LoanRequestBuilders, so neither goes through JSON.
Accounts move to the base's FeignAccountHelper and charge-off reason codes
to FeignCodeHelper.
The auditing test needs to reverse a repayment as a different user.
FineractFeignClientHelper.createNewFineractFeignClient already builds a
client for arbitrary credentials, so the second user gets its own
FeignTransactionHelper instead of a second RequestSpecification.
Its GET /internal/loan/{id}/transaction/{txId}/audit call stays on
FeignRawHttpHelper: the internal resource carries no @operation or
@apiresponse, so the endpoint is absent from the generated client. That is
the existing documented acceptance in the raw-HTTP audit, not a new usage.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…sement tests to feign
Same shape as the previous commit: both classes already extended
FeignLoanTestBase but kept a REST Assured stack for a legacy
LoanTransactionHelper. Disbursements, repayments, loan reads, business
dates and client creation all move to the base's typed equivalents, and
the loan product and application are built with buildRequest() and
LoanRequestBuilders.
LoanAccountRepaymentCalculationTest also carried a JournalEntryHelper
field that nothing referenced; it goes with the stack.
The disbursement-before-submission case used a second LoanTransactionHelper
wired to a 403 response spec and asserted on a parsed error list. It now
asserts the status the response spec pinned, the globalisation code and the
user message off CallFailedRuntimeException, so the expected 403 is still
checked rather than being reduced to "something threw".
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The last of the classes that extended FeignLoanTestBase while keeping a REST
Assured stack, and the one that leaned on it hardest: two LoanTransactionHelpers
(one wired to a 200 response spec, one to a bare spec for the error cases), an
AccountHelper, a JournalEntryHelper, and twelve reads that pulled single fields
out of the loan JSON by name.
Those reads are the bulk of the change. getLoanDetail(spec, spec, id, "summary")
followed by get("totalOutstanding") becomes
getLoanDetails(id).getSummary().getTotalOutstanding(), and the three status
constants the test carried ("active", "overpaid", "closedObligationsMet") were
only ever keys into that map, so they give way to LoanStatus and the base's
verifyLoanStatus.
Setting the product's penalty income account was a hand-built PUT through
Utils.performServerPut; PutLoanProductsProductIdRequest already models
incomeFromPenaltyAccountId, so it goes through updateLoanProduct instead.
loanChargePaidByList is a Set on the generated model and the server declares no
order for it, so the two-entry assertion now matches on sign rather than on
position. The original indexed get(0)/get(1) but then branched on the sign of
each, so it was already order-tolerant; this makes that explicit.
The three error cases used the second helper and asserted on a parsed error list.
They now assert the globalisation code off CallFailedRuntimeException.
installmentNumber was passed to every loanChargeRefund call and was null every
time, so the typed request omitting it sends the same body.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A loan disbursement can be backed by post dated checks, but the disburse
request model had no field for them, so a test could only send them as
hand-written JSON. The template that lists the installments those checks
attach to, and the tranche amount the undo-last-disbursal response reports,
were missing for the same reason.
GET /loans/{loanId}/postdatedchecks/{installmentId} returns a single check
while declaring an array of them, so no generated client could deserialise
the reply. Correct the declared response and give the loan helper a typed
read of one check.
LoanReschedulingWithinCenterTest took its repayment strategy constant from
LoanApplicationTestBuilder. The identical constant on LoanProductTestBuilder
lets the migrated test drop its last import of the JSON builder.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ary tests to feign
Covers the charge-off accounting scenarios, the paid-off loan charge refund,
the last repayment details, the transaction summary totals and the blocked
transactions on closed or overpaid loans.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers the disbursal date validation, the undo of the last tranche of a
multi-disbursement loan, the fixed principal percentage amortization
schedules, the waive interest and write-off flow, and the repayment with
post dated checks.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…o feign
Covers skipping a repayment falling on the first day of a month, the minimum
days between disbursal and the first repayment, and the JLG loan schedule
following a group meeting change.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
A group savings account can back a guarantee, but PostSavingsAccountsRequest
only carried clientId even though the savings validator accepts groupId, so
add it. Recovering guarantees was only reachable by loan external id; add the
loan id form the tests need.
The approval failures here return several validation codes at once, which the
single-code helper on the base class cannot express, so the test reads every
globalisation code out of the errors array.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Several loan endpoints could not be driven from the generated client, so the
tests that exercise them had to hand-build JSON.
PostLoansLoanIdScheduleRequest was an empty class, so the variable installment
commands (calculateLoanSchedule, addVariations) had no body to send, and the
response carried no periods to read back. Add the exceptions the command
accepts and the schedule it answers with.
PUT /loans/{loanId}/disbursements/{disbursementId} declared no request body at
all, so the generated client took a bare String. Add the request the tranche
update accepts.
PostLoanProductsRequest was missing six fields the product create endpoint
accepts and validates: minimumGap and maximumGap, which a variable installment
product needs; syncExpectedWithDisbursementDate; and the three guarantee
percentages a hold-funds product needs. The loan product builder has a JSON
path and a typed path, and only the JSON one could send them, so a product
built the typed way came back without its configuration.
GetLoansLoanIdResponse.disbursementDetails was a Set even though the endpoint
answers an ordered array whose order carries meaning - tranches come back in
expected disbursement date order and callers read them by position. List is
what the neighbouring transactions and charges fields already use.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The legacy VariableIntallmentsTransactionHelper is REST-assured and builds its
term variations out of raw maps. Its Feign counterpart drives the same three
commands through the loan rescheduling API, and the request builders make the
distinction the two product types need explicit: a declining balance product
drives the schedule by installmentAmount, a flat one by principal.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…feign
LoanDisbursementDetailsIntegrationTest (16 tests) and
VariableInstallmentsIntegrationTest (13 tests) move onto the typed client.
The disbursement detail test read its tranches out of a raw JSON array and
formatted the dates back with SimpleDateFormat; it now reads the typed
GetLoansLoanIdDisbursementDetails. Its three error paths assert the exact
status and globalisation code instead of a ResponseSpecification.
The variable installment test built every term variation as a nested HashMap
across two near-identical helper classes. Both are now expressed through
VariableInstallmentsRequestBuilders, which keeps the flat and declining
balance variants apart.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
GroupSummaryReportsIntegrationTest was the last subclass of the RestAssured
BaseLoanIntegrationTest, and it used nothing from it: the test builds its own
request specification and only creates a group and runs three reports.
What the inheritance did carry was two extensions, and neither has anything to
do for this test. LoanTestLifecycleExtension closes every open loan in the
database before and after each test, and the test creates no loans.
ExternalEventsExtension snapshots the external event configuration and restores
whatever the test changed, and the test changes none of it. Extending
FeignIntegrationTest instead drops both.
The reports are read with genericResultSet=false, the shape the test asserted
on, so FeignReportHelper gains the row-array read next to the generic
resultset one it already had.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
BaseLoanIntegrationTest has no subclasses left. It carried its own RestAssured
request and response specifications, a LoanTransactionHelper and the loan
domain constants, all of which the Feign loan tests now get from
FeignLoanTestBase.
Deleting it orphans two helpers that nothing else called, ExternalEventHelper
and InlineLoanCOBHelper; both already have Feign counterparts in
FeignExternalEventHelper and FeignCobHelper, so they go with it.
Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44

Copy link
Copy Markdown
ContributorAuthor

Correcting this schema shape triggers swagger-brake report R015. This is the intended and correct fix, as no generated client could read the old declared shape. (also mentioned in the jira ticket)

Sign up for freeto 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.

1 participant

@DeathGun44