Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interop KYC and Interop Loan Disbursement #1330

Merged
merged 3 commits into from
Sep 29, 2020

Conversation

SubhamPramanik
Copy link
Contributor

Description

Describe the changes made and why they were made.

  • InteropAPI to get client KYC using account ID
  • Interop API to disburse loan

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

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per https://github.com/apache/fineract/#pull-requests

  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

  • Create/update unit or integration tests for verifying the changes made.

  • Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes

  • Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

@avikganguly01
Copy link
Contributor

@SubhamPramanik Can you fix the checkstyle violations?

@ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = InteropKycResponseData.class))) })
public String getClientKyc(@PathParam("accountId") @Parameter(description = "accountId") String accountId, @Context UriInfo uriInfo) {
InteropKycResponseData result = interopService.getKyc(accountId);
ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters());
Copy link
Contributor

Choose a reason for hiding this comment

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

Check permission for this action and entity (READ - CLIENTIDENTIFIER) ...context.authenticatedUser().validateHasReadPermission(ENTITY_NAME);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to do it. Can you point to a refernce implementation?

@@ -437,6 +550,19 @@ private SavingsAccount validateAndGetSavingAccount(@NotNull InteropRequestData r
return savingsAccount;
}

// private Loan validateAndGetLoan(@NotNull InteropRequestData request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this if you're not planning to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@thesmallstar
Copy link
Member

@SubhamPramanik thank you for the PR, I see a checkstyle error in your code, that means address_line_1 that you made does not match the required regex it should.

I also see a AvoidHidingCauseException, that happens when you catch a exception and throw another exception essentially hiding what actually caused the exception!

Will you fix this?

cc @vorburger The checkstyles are helping!!

@vorburger
Copy link
Member

@avikganguly01 looks like the review feedback from you here has been addressed? Will you re-review and merge when OK for you? Please use Squash and Merge (notice the drop-down on the green button above), as per https://github.com/apache/fineract#merge-strategy.

@avikganguly01 avikganguly01 merged commit e5f3e87 into apache:develop Sep 29, 2020
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.

4 participants