Aman-Mittal commented on code in PR #155:
URL:
https://github.com/apache/fineract-consumer-facing/pull/155#discussion_r4292938858
##########
consumer/build.gradle:
##########
@@ -408,6 +408,37 @@ def fixClientImageResponseShape(paths) {
]]
}
+def addMissingSavingsAccountCommandFields(schemas) {
Review Comment:
This patch is a no-op against the vendored spec. In `spec/fineract.json`:
- `PostSavingsAccountsAccountIdRequest` already has `withdrawnOnDate` and
`note`.
- `PutSavingsAccountsAccountIdRequest` already has `productId`,
`submittedOnDate` and `dateFormat`.
So every `containsKey` check is true and nothing is added. The function only
adds two new `GradleException` failure points and a misleading entry in
`fineract-integration.adoc`. The PR description also says `fineract.json` was
patched, but it isn't in the diff.
Please drop this function, its call at L523 and the doc change, unless
there's a spec version where these fields are actually missing.
##########
consumer/src/main/java/org/apache/fineract/consumer/savings/command/data/SubmitSavingsApplicationCommandRequest.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.consumer.savings.command.data;
+
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
+import java.time.LocalDate;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.ToString;
+
+@Getter
+@RequiredArgsConstructor
+@Builder
+@ToString(onlyExplicitlyIncluded = true)
+public final class SubmitSavingsApplicationCommandRequest {
+
+ @NotNull
+ @Positive
+ private final Long productId;
Review Comment:
**Security / functional: product allow-listing.** Any savings `productId` in
the tenant is accepted, including products that were never meant to be
self-service (staff-only, internal or legacy products). The same goes for
`productId` on modify. The application template endpoint lists every product as
well, so the UI won't stop this either.
Could we restrict both submit and modify to a consumer-offered set? For
example, a configurable allow-list, or a product-level flag/datatable that the
template endpoint also filters on. If that is out of scope for this PR, please
open a tracking issue.
##########
consumer/src/main/java/org/apache/fineract/consumer/savings/command/data/SubmitSavingsApplicationCommandRequest.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.consumer.savings.command.data;
+
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
+import java.time.LocalDate;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.ToString;
+
+@Getter
+@RequiredArgsConstructor
+@Builder
+@ToString(onlyExplicitlyIncluded = true)
+public final class SubmitSavingsApplicationCommandRequest {
+
+ @NotNull
+ @Positive
+ private final Long productId;
+
+ @NotNull
+ private final LocalDate submittedOnDate;
Review Comment:
**Security / data integrity: client-controlled dates.** `submittedOnDate`
has `@NotNull` but no bounds. Fineract only rejects future dates and dates
before client activation, so a consumer can backdate a submission (and the date
set on modify) as far as their activation date. Back office can then backdate
approval and activation to match, which affects interest accrual and reporting.
The same applies to `withdrawnOnDate` in
`WithdrawSavingsApplicationCommandRequest`.
For a consumer channel I'd suggest one of these:
- Derive the date server-side (today, in the tenant timezone) and drop it
from the request. This is my preference; the client has no real reason to
choose it.
- At minimum, add `@PastOrPresent` plus a check that the date is today (or
within a small tolerance).
I know the loans command takes the same approach, so this could be done for
both in a follow-up. If you go that route, please call it out explicitly.
##########
docs/consumer/architecture/fineract-integration.adoc:
##########
@@ -22,7 +22,7 @@ The transport is Feign, a declarative HTTP client: you write
a Java interface wi
The Feign interfaces are not written by hand. They are generated on every
build from a vendored copy of Fineract's OpenAPI spec (`spec/fineract.json`) by
two Gradle tasks in `consumer/build.gradle`:
-. `sanitizeFineractSpec` patches known defects in the upstream spec before
generation: it strips null-only enums, retypes integer money fields
(`principal`, `amount`, `depositAmount`) to `number`, adds balance fields
missing from `GetClientsSavingsAccounts` and `GetClientsLoanAccounts`, adds
`mobileNo` and `imagePresent` to `GetClientsClientIdResponse`, and fixes the
response shapes of the obligee-details and client-image endpoints. The
schema-targeted patches (the missing balance and profile fields, the
obligee-details fix, and the client-image fix) fail the build with a
`GradleException` if the schema or path they target disappears, so a spec
upgrade cannot silently drop those fixes. The enum-stripping and money-retyping
passes are pattern-based and simply match nothing if the spec changes. Finally,
the spec is pruned to the operations whose tag is listed in `fineractApiTags`
in `consumer/build.gradle` (plus the untagged operations in
`fineractUntaggedOperationIds`), and to the sc
hemas those operations reach. Only those APIs are generated, which keeps
unused Feign clients and models out of the jar and out of startup. Using a new
generated `*Api` means adding its tag to that list first; the build fails if a
listed tag or operation disappears from the spec.
+. `sanitizeFineractSpec` patches known defects in the upstream spec before
generation: it strips null-only enums, retypes integer money fields
(`principal`, `amount`, `depositAmount`) to `number`, adds balance fields
missing from `GetClientsSavingsAccounts` and `GetClientsLoanAccounts`, adds
`mobileNo` and `imagePresent` to `GetClientsClientIdResponse`, adds missing
application fields to `PostSavingsAccountsAccountIdRequest` and
`PutSavingsAccountsAccountIdRequest`, and fixes the response shapes of the
obligee-details and client-image endpoints. The schema-targeted patches (the
missing balance, profile, and savings application fields, the obligee-details
fix, and the client-image fix) fail the build with a `GradleException` if the
schema or path they target disappears, so a spec upgrade cannot silently drop
those fixes. The enum-stripping and money-retyping passes are pattern-based and
simply match nothing if the spec changes. Finally, the spec is pruned to the
operations whose tag
is listed in `fineractApiTags` in `consumer/build.gradle` (plus the untagged
operations in `fineractUntaggedOperationIds`), and to the schemas those
operations reach. Only those APIs are generated, which keeps unused Feign
clients and models out of the jar and out of startup. Using a new generated
`*Api` means adding its tag to that list first; the build fails if a listed tag
or operation disappears from the spec.
Review Comment:
This says the sanitizer "adds missing application fields to
`PostSavingsAccountsAccountIdRequest` and
`PutSavingsAccountsAccountIdRequest`", but those fields already exist in the
vendored spec (see the comment on `build.gradle`). Please revert this sentence
along with the sanitizer.
Separately: there's no consumer-facing doc for the three new endpoints
(request/response shape, the `withdrawnByApplicant` command, error codes). If
there's a feature doc for the loans command endpoints, please add the savings
equivalent.
##########
consumer/src/test/resources/features/savings.feature:
##########
@@ -44,3 +44,23 @@ Feature: Consumer savings reads
When I get the other client's savings account
Then the savings request is denied as forbidden
+ Scenario: Submitting the same savings application twice with one idempotency
key creates one account
+ When I submit a savings application twice with the same idempotency key
+ Then both savings submissions return the same savings id
+ And I have exactly one more savings account
+
+ Scenario: Submitting a savings application without an idempotency key is
rejected
+ When I submit a savings application without an idempotency key
+ Then the savings submission is rejected for the missing idempotency key
+
+ Scenario: Modifying a pending savings application
+ Given I have submitted a savings application
+ When I modify my pending savings application
+ Then the savings application is updated successfully
+
+ Scenario: Withdrawing a pending savings application
Review Comment:
**E2E gaps.** The happy paths are covered, but the scenarios that matter
most for a write API are missing:
1. **Ownership (IDOR):** modifying and withdrawing *another client's*
savings application should return 403
`error.msg.consumer.savings.command.access.denied`. `foreignSavingsId` is
already seeded for the read-side denial scenario, so this is cheap to add.
Ideally, also check that the foreign account's status is unchanged afterwards.
2. **State transitions:** withdrawing an already-active account (seeded)
should give 400; so should withdrawing the same application twice, and
modifying after withdrawal.
3. **Idempotent replay** for modify and withdraw, not only submit: the same
key should return the same result and not cause a double transition or error.
4. **Scope:** an Open Banking token calling submit should be denied, since
the policy is `CONSUMER_ONLY`.
5. Optional: `command` other than `withdrawnByApplicant` should return 400.
Also, the file's `Feature:` title is still "Consumer savings reads". Either
rename it, or move the command scenarios into a `savings-application.feature`.
There are also two trailing blank lines at the end of the file.
##########
consumer/src/test/java/org/apache/fineract/consumer/savings/command/api/SavingsCommandControllerTest.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.consumer.savings.command.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+import java.time.LocalDate;
+import java.util.UUID;
+import
org.apache.fineract.consumer.infrastructure.idempotency.exception.IdempotencyKeyMalformedException;
+import
org.apache.fineract.consumer.savings.command.data.ModifySavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.ModifySavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.data.SubmitSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.SubmitSavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.data.WithdrawSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.WithdrawSavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsApplicationInvalidException;
+import
org.apache.fineract.consumer.savings.command.service.SavingsCommandService;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.security.oauth2.jwt.Jwt;
+
+@ExtendWith(MockitoExtension.class)
+class SavingsCommandControllerTest {
Review Comment:
This test calls the controller methods directly, so `@Valid` and the
constraints on the request DTOs (`@NotNull`/`@Positive productId`, `@NotNull`
dates, `@Size(max = 1000) note`) are never exercised. Could you add a
`@WebMvcTest` slice, or a plain `Validator` test, covering:
- missing or zero/negative `productId` → 400
- missing `submittedOnDate` / `withdrawnOnDate` → 400
- a `note` longer than 1000 characters → 400
These are the request contract for the new endpoints, so they should be
pinned by tests.
##########
consumer/src/main/java/org/apache/fineract/consumer/savings/command/service/SavingsCommandServiceImpl.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.consumer.savings.command.service;
+
+import java.util.UUID;
+import java.util.function.Supplier;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.consumer.infrastructure.access.data.ConsumerAction;
+import
org.apache.fineract.consumer.infrastructure.access.service.AccessPolicyEvaluator;
+import
org.apache.fineract.consumer.infrastructure.access.service.OwnedAccountsCache;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.api.SavingsAccountApi;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PostSavingsAccountsAccountIdRequest;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PostSavingsAccountsAccountIdResponse;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PostSavingsAccountsRequest;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PostSavingsAccountsResponse;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PutSavingsAccountsAccountIdRequest;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.generated.model.PutSavingsAccountsAccountIdResponse;
+import
org.apache.fineract.consumer.infrastructure.fineractclient.service.FineractCaller;
+import
org.apache.fineract.consumer.infrastructure.idempotency.service.IdempotencyKeyDeriver;
+import
org.apache.fineract.consumer.infrastructure.idempotency.service.IdempotencyKeyHolder;
+import
org.apache.fineract.consumer.savings.command.data.ModifySavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.SavingsApplicationCommandData;
+import
org.apache.fineract.consumer.savings.command.data.SubmitSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.WithdrawSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsApplicationInvalidException;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsCommandAccessDeniedException;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsCommandInProgressException;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsCommandNotFoundException;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsCommandUpstreamUnavailableException;
+import org.apache.fineract.consumer.user.query.data.UserQueryData;
+import org.apache.fineract.consumer.user.query.service.UserQueryService;
+import org.springframework.security.oauth2.jwt.Jwt;
+import org.springframework.stereotype.Service;
+
+@Service
+@RequiredArgsConstructor
+public class SavingsCommandServiceImpl implements SavingsCommandService {
+
+ private static final String LOCALE = "en";
+ private static final String DATE_FORMAT = "yyyy-MM-dd";
+
+ private final UserQueryService userQueryService;
+ private final AccessPolicyEvaluator accessPolicyEvaluator;
+ private final OwnedAccountsCache ownedAccountsCache;
+ private final IdempotencyKeyHolder idempotencyKeyHolder;
+ private final SavingsAccountApi savingsAccountApi;
+
+ @Override
+ public SavingsApplicationCommandData submitApplication(Jwt jwt,
SubmitSavingsApplicationCommand command) {
+ accessPolicyEvaluator.authorize(jwt,
ConsumerAction.SAVINGS_APPLICATION_SUBMIT);
+ Long clientId = resolveClientId(jwt);
+ PostSavingsAccountsRequest request = buildSubmitRequest(command,
clientId);
+ PostSavingsAccountsResponse response = callWithIdempotency(jwt,
command.getIdempotencyKey(),
+ () -> savingsAccountApi.submitSavingsApplication(request));
+ ownedAccountsCache.evict(clientId);
+ return SavingsApplicationCommandData.builder()
+ .savingsId(response.getSavingsId())
+ .resourceId(response.getResourceId())
+ .clientId(clientId)
+ .build();
+ }
+
+ @Override
+ public SavingsApplicationCommandData modifyApplication(Jwt jwt,
ModifySavingsApplicationCommand command) {
+ accessPolicyEvaluator.authorize(jwt,
ConsumerAction.SAVINGS_APPLICATION_MODIFY, command.getSavingsId(),
Review Comment:
**Security: deposit-type confusion on modify/withdraw.** The `owned(...,
ResourceType.SAVINGS, ...)` check passes for any id in
`OwnedAccountsCache.savingsIds`. That set comes from `GET
/clients/{id}/accounts`, which returns **all** `m_savings_account` rows for the
client, including fixed and recurring deposit accounts (the response carries
`depositType`).
On the Fineract side,
`SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.modifyApplication`
and `applicantWithdrawsFromApplication` load the account through
`SavingsAccountAssembler.assembleFrom(savingsId, false)` →
`findSavingsWithNotFoundDetection`, which has no deposit-type filter. So a
consumer with a pending FD/RD application can call `PUT /api/v1/savings/{id}`
or `POST /api/v1/savings/{id}?command=withdrawnByApplicant` on it:
- Withdraw goes through the plain savings path instead of
`DepositApplicationProcessWritePlatformServiceJpaRepositoryImpl.applicantWithdrawsFromApplication`,
so the deposit-specific handling is skipped.
- Modify with a regular savings `productId` can re-point a deposit account
at a non-deposit product.
Suggestion: before calling upstream, confirm the target's `depositType` is
`SAVINGS` (100). Either keep the `depositType` in `OwnedAccounts` and check it
here, or do a cheap `retrieveSavingsAccount` pre-check. Please also add a unit
test for the rejection. The same gap probably exists on the query side, so it
may be worth a separate issue.
##########
consumer/src/test/java/org/apache/fineract/consumer/cucumber/steps/SavingsSteps.java:
##########
@@ -138,12 +164,107 @@ public void savingsDeniedForbidden() {
assertThat(errorStatus).isEqualTo(FORBIDDEN);
}
+ @When("I submit a savings application twice with the same idempotency key")
+ public void submitSavingsApplicationTwiceWithSameKey() {
+ savingsCountBeforeSubmit = savingsApi.listSavingsAccounts().size();
+ String idempotencyKey = UUID.randomUUID().toString();
+ firstSubmission =
savingsCommandApi.submitSavingsApplication(idempotencyKey,
savingsApplicationRequest());
+ secondSubmission =
savingsCommandApi.submitSavingsApplication(idempotencyKey,
savingsApplicationRequest());
+ }
+
+ @Then("both savings submissions return the same savings id")
+ public void bothSavingsSubmissionsReturnSameSavingsId() {
+ assertThat(firstSubmission.getSavingsId()).isNotNull();
+
assertThat(secondSubmission.getSavingsId()).isEqualTo(firstSubmission.getSavingsId());
+ }
+
+ @Then("I have exactly one more savings account")
+ public void exactlyOneMoreSavingsAccount() {
+
assertThat(savingsApi.listSavingsAccounts()).hasSize(savingsCountBeforeSubmit +
1);
+ }
+
+ @When("I submit a savings application without an idempotency key")
+ public void submitSavingsApplicationWithoutKey() {
+ lastError = captureError(() ->
savingsCommandApi.submitSavingsApplication(null, savingsApplicationRequest()));
+ }
+
+ @Then("the savings submission is rejected for the missing idempotency key")
+ public void savingsSubmissionRejectedForMissingKey() {
+ assertThat(lastError.status()).isEqualTo(BAD_REQUEST);
+
assertThat(readCode(lastError.contentUTF8())).isEqualTo(MissingRequestHeaderExceptionHandler.CODE);
+ }
+
+ @Given("I have submitted a savings application")
+ public void submittedSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ SavingsApplicationCommandData submission =
savingsCommandApi.submitSavingsApplication(
+ idempotencyKey, savingsApplicationRequest());
+ pendingSavingsId = submission.getSavingsId();
+ assertThat(pendingSavingsId).isNotNull();
+ }
+
+ @When("I modify my pending savings application")
+ public void modifyPendingSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ commandResult = savingsCommandApi.modifySavingsApplication(
+ idempotencyKey, pendingSavingsId,
modifySavingsApplicationRequest());
+ }
+
+ @Then("the savings application is updated successfully")
+ public void savingsApplicationUpdatedSuccessfully() {
+ assertThat(commandResult).isNotNull();
+ assertThat(commandResult.getSavingsId()).isEqualTo(pendingSavingsId);
+ }
+
+ @When("I withdraw my pending savings application")
+ public void withdrawPendingSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ commandResult = savingsCommandApi.withdrawSavingsApplication(
+ idempotencyKey, pendingSavingsId, WITHDRAW_COMMAND,
withdrawSavingsApplicationRequest());
+ }
+
+ @Then("the savings application is withdrawn successfully")
+ public void savingsApplicationWithdrawnSuccessfully() {
Review Comment:
These assertions can't fail meaningfully.
`SavingsApplicationCommandData.savingsId` for modify and withdraw is set from
`command.getSavingsId()`, which is the path variable, so
`commandResult.getSavingsId() == pendingSavingsId` holds whatever Fineract did.
Please re-read the account with
`savingsApi.getSavingsAccount(pendingSavingsId)` and assert on the state:
- after withdraw: status code `savingsAccountStatusType.withdrawnByApplicant`
- after modify: the changed field actually changed (see the next comment)
Asserting `resourceId` is not null would also help.
##########
consumer/src/main/java/org/apache/fineract/consumer/savings/command/api/SavingsCommandController.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.consumer.savings.command.api;
+
+import io.swagger.v3.oas.annotations.Operation;
+import jakarta.validation.Valid;
+import lombok.RequiredArgsConstructor;
+import
org.apache.fineract.consumer.infrastructure.idempotency.service.IdempotencyKeyPolicyEvaluator;
+import org.apache.fineract.consumer.infrastructure.web.data.ConsumerHeaders;
+import
org.apache.fineract.consumer.savings.command.data.ModifySavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.ModifySavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.data.SavingsApplicationCommandData;
+import
org.apache.fineract.consumer.savings.command.data.SubmitSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.SubmitSavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.data.WithdrawSavingsApplicationCommand;
+import
org.apache.fineract.consumer.savings.command.data.WithdrawSavingsApplicationCommandRequest;
+import
org.apache.fineract.consumer.savings.command.exception.SavingsApplicationInvalidException;
+import
org.apache.fineract.consumer.savings.command.service.SavingsCommandService;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.security.core.annotation.AuthenticationPrincipal;
+import org.springframework.security.oauth2.jwt.Jwt;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestHeader;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+@RequestMapping(value = "/api/v1/savings", produces =
MediaType.APPLICATION_JSON_VALUE)
Review Comment:
**Functional completeness vs #144.** The BFF side is here, but there is no
consumer UI to use it. For loans there is
`frontend/src/app/features/loans/loan-apply.component.ts` plus a
`bff-loan.api.ts` adapter. #144's goal is user-facing parity with the removed
self-service API, so a customer still can't apply for savings after this merges.
Please either add the savings apply/withdraw flow (with a Playwright
journey) here, or open a linked follow-up issue and change the PR description
from "Closes #144" to "Part of #144". #144 also lists reports, surveys, share
accounts and pockets, so closing it from this PR would be premature in any case.
##########
consumer/src/test/java/org/apache/fineract/consumer/cucumber/steps/SavingsSteps.java:
##########
@@ -138,12 +164,107 @@ public void savingsDeniedForbidden() {
assertThat(errorStatus).isEqualTo(FORBIDDEN);
}
+ @When("I submit a savings application twice with the same idempotency key")
+ public void submitSavingsApplicationTwiceWithSameKey() {
+ savingsCountBeforeSubmit = savingsApi.listSavingsAccounts().size();
+ String idempotencyKey = UUID.randomUUID().toString();
+ firstSubmission =
savingsCommandApi.submitSavingsApplication(idempotencyKey,
savingsApplicationRequest());
+ secondSubmission =
savingsCommandApi.submitSavingsApplication(idempotencyKey,
savingsApplicationRequest());
+ }
+
+ @Then("both savings submissions return the same savings id")
+ public void bothSavingsSubmissionsReturnSameSavingsId() {
+ assertThat(firstSubmission.getSavingsId()).isNotNull();
+
assertThat(secondSubmission.getSavingsId()).isEqualTo(firstSubmission.getSavingsId());
+ }
+
+ @Then("I have exactly one more savings account")
+ public void exactlyOneMoreSavingsAccount() {
+
assertThat(savingsApi.listSavingsAccounts()).hasSize(savingsCountBeforeSubmit +
1);
+ }
+
+ @When("I submit a savings application without an idempotency key")
+ public void submitSavingsApplicationWithoutKey() {
+ lastError = captureError(() ->
savingsCommandApi.submitSavingsApplication(null, savingsApplicationRequest()));
+ }
+
+ @Then("the savings submission is rejected for the missing idempotency key")
+ public void savingsSubmissionRejectedForMissingKey() {
+ assertThat(lastError.status()).isEqualTo(BAD_REQUEST);
+
assertThat(readCode(lastError.contentUTF8())).isEqualTo(MissingRequestHeaderExceptionHandler.CODE);
+ }
+
+ @Given("I have submitted a savings application")
+ public void submittedSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ SavingsApplicationCommandData submission =
savingsCommandApi.submitSavingsApplication(
+ idempotencyKey, savingsApplicationRequest());
+ pendingSavingsId = submission.getSavingsId();
+ assertThat(pendingSavingsId).isNotNull();
+ }
+
+ @When("I modify my pending savings application")
+ public void modifyPendingSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ commandResult = savingsCommandApi.modifySavingsApplication(
+ idempotencyKey, pendingSavingsId,
modifySavingsApplicationRequest());
+ }
+
+ @Then("the savings application is updated successfully")
+ public void savingsApplicationUpdatedSuccessfully() {
+ assertThat(commandResult).isNotNull();
+ assertThat(commandResult.getSavingsId()).isEqualTo(pendingSavingsId);
+ }
+
+ @When("I withdraw my pending savings application")
+ public void withdrawPendingSavingsApplication() {
+ String idempotencyKey = UUID.randomUUID().toString();
+ commandResult = savingsCommandApi.withdrawSavingsApplication(
+ idempotencyKey, pendingSavingsId, WITHDRAW_COMMAND,
withdrawSavingsApplicationRequest());
+ }
+
+ @Then("the savings application is withdrawn successfully")
+ public void savingsApplicationWithdrawnSuccessfully() {
+ assertThat(commandResult).isNotNull();
+ assertThat(commandResult.getSavingsId()).isEqualTo(pendingSavingsId);
+ }
+
+ private SubmitSavingsApplicationCommandRequest savingsApplicationRequest()
{
+ return new SubmitSavingsApplicationCommandRequest()
+ .productId(fineractSeeder.savingsProductId())
+ .submittedOnDate(FIXED_LOCAL_DATE);
+ }
+
+ private ModifySavingsApplicationCommandRequest
modifySavingsApplicationRequest() {
Review Comment:
The modify request sends the same `productId` and `submittedOnDate` that the
application was submitted with. Fineract's `modifyApplication` computes an
empty `changes` map and returns 200 without writing anything, so this scenario
doesn't show that modify works end to end.
Please change something observable, such as a second seeded savings product
or a different (valid) `submittedOnDate`, and assert the new value on a re-read.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
