Skip to content

FINERACT-2454: Migrate LoanWithdrawnByApplicantIntegrationTest from RestAssured to feign client#5668

Open
Dpk376 wants to merge 3 commits intoapache:developfrom
Dpk376:feature/FINERACT-2454-migrate-loan-withdrawn-test
Open

FINERACT-2454: Migrate LoanWithdrawnByApplicantIntegrationTest from RestAssured to feign client#5668
Dpk376 wants to merge 3 commits intoapache:developfrom
Dpk376:feature/FINERACT-2454-migrate-loan-withdrawn-test

Conversation

@Dpk376
Copy link

@Dpk376 Dpk376 commented Mar 21, 2026

Description

Closes/relates to FINERACT-2454

Migrates LoanWithdrawnByApplicantIntegrationTest from RestAssured to the type-safe feign client, as part of the ongoing test migration effort.

Changes

  • Replaced RequestSpecification/ResponseSpecification boilerplate with extends BaseLoanIntegrationTest
  • Removed @BeforeEach setup — lifecycle managed by BaseLoanIntegrationTest
  • Replaced ClientHelper.createClient(requestSpec, responseSpec, ...)ClientHelper.createClient(PostClientsRequest)
  • Replaced LoanProductTestBuildercreateOnePeriod30DaysLongNoInterestPeriodicAccrualProduct()
  • Replaced LoanApplicationTestBuilder + CollateralManagementHelper (no feign equivalent) → applyLoan(applyLoanRequest(...))
  • Replaced LoanStatusChecker HashMap-based assertions → verifyLoanStatus(loanId, LoanStatus.*)
  • Replaced withdrawLoanApplicationByClient()withdrawnByApplicantLoan(externalId, PostLoansLoanIdRequest)

Result

102 lines → 60 lines. Zero RestAssured imports remaining.

No behavioral changes — the test still verifies that a pending loan application can be withdrawn by the applicant and transitions to WITHDRAWN_BY_CLIENT status.

Comment on lines +144 to +149
// public static List<GetObligeeData> getObligeeData(final String externalId) {
// GetClientObligeeDetailsResponse response = Calls
// .ok(FineractClientHelper.getFineractClient().clients.retrieveClientObligeeDetailsByExternalId(externalId));
// return response != null && response.getObligees() != null ? new ArrayList<>(response.getObligees()) : new ArrayList<>();
// }

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Dpk376 , good catch on identifying the OpenAPI schema mismatch.

However, committing commented-out/dead code blocks generally goes against clean code guidelines. More importantly, falling back to RestAssured here circumvents the primary objective of this migration epic.

Instead of introducing a workaround and leaving technical debt in the test layer, the standard approach would be to fix the root cause: correcting the OpenAPI response schema (likely by updating the @ApiResponses annotation in the respective API Controller). Once the controller correctly defines the return type as an array, the Feign client will regenerate the correct List mapping, allowing us to completely remove the RestAssured dependency without workarounds.

Should we address the Controller annotation in this PR to keep the migration clean?

Copy link
Author

Choose a reason for hiding this comment

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


Thank you for the direction! You're right — the root cause was an incorrect @ApiResponse annotation in ClientsApiResource.

Root Cause

Both obligee endpoints (clientId and externalId) were annotated with @Schema(implementation = GetClientObligeeDetailsResponse.class), describing the response as a JSON object — but the API actually returns a JSON array directly.

Changes Made

ClientsApiResource.java

  • Updated both obligee endpoint annotations to use @ArraySchema(schema = @Schema(implementation = GetObligeeData.class))

ClientsApiResourceSwagger.java

  • Promoted GetObligeeData to a top-level schema class
  • Removed the GetClientObligeeDetailsResponse wrapper class

ClientHelper.java

  • Removed all commented-out code blocks
  • Replaced the RestAssured workaround with a clean feign client call:
return Calls.ok(FineractClientHelper.getFineractClient().clients.retrieveClientObligeeDetailsByExternalId(externalId));

Happy to address any further feedback!

Comment on lines +151 to +152
// Note: The Feign-generated client incorrectly maps this endpoint to GetClientObligeeDetailsResponse (object),
// but the actual API returns a JSON array directly. Using RestAssured as a workaround until the OpenAPI spec is corrected.
Copy link
Contributor

Choose a reason for hiding this comment

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

please..

Copy link
Contributor

@Saifulhuq01 Saifulhuq01 left a comment

Choose a reason for hiding this comment

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

please check a point cmds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants