Aman-Mittal commented on code in PR #155: URL: https://github.com/apache/fineract-consumer-facing/pull/155#discussion_r4431028872
########## consumer/src/main/java/org/apache/fineract/consumer/savings/command/service/SavingsCommandServiceImpl.java: ########## @@ -0,0 +1,188 @@ +/* + * 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.time.LocalDate; +import java.time.ZoneOffset; +import java.util.Set; +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.model.EnumOptionData; +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.generated.model.SavingsAccountData; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.FineractCaller; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.SavingsAccountApiClient; +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.beans.factory.annotation.Value; +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 static final Long SAVINGS_DEPOSIT_TYPE_ID = 100L; + + @Value("${consumer.savings.command.allowed-product-ids:}") + private Set<Long> allowedProductIds; + + private final UserQueryService userQueryService; + private final AccessPolicyEvaluator accessPolicyEvaluator; + private final OwnedAccountsCache ownedAccountsCache; + private final IdempotencyKeyHolder idempotencyKeyHolder; + private final SavingsAccountApiClient savingsAccountApiClient; + + @Override + public SavingsApplicationCommandData submitApplication(Jwt jwt, SubmitSavingsApplicationCommand command) { + accessPolicyEvaluator.authorize(jwt, ConsumerAction.SAVINGS_APPLICATION_SUBMIT); + validateProductAllowList(command.getProductId()); + Long clientId = resolveClientId(jwt); + LocalDate submittedOnDate = LocalDate.now(ZoneOffset.UTC); + PostSavingsAccountsRequest request = buildSubmitRequest(command, clientId, submittedOnDate); + PostSavingsAccountsResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.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(), + SavingsCommandAccessDeniedException::new); + validateProductAllowList(command.getProductId()); + validateSavingsDepositType(command.getSavingsId()); + Long clientId = resolveClientId(jwt); + LocalDate submittedOnDate = LocalDate.now(ZoneOffset.UTC); + PutSavingsAccountsAccountIdRequest request = buildModifyRequest(command, submittedOnDate); + PutSavingsAccountsAccountIdResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.updateSavingsAccount(command.getSavingsId(), request, null)); + return SavingsApplicationCommandData.builder() + .savingsId(command.getSavingsId()) + .resourceId(response.getResourceId()) + .clientId(clientId) + .build(); + } + + @Override + public SavingsApplicationCommandData withdrawApplication(Jwt jwt, WithdrawSavingsApplicationCommand command) { + accessPolicyEvaluator.authorize(jwt, ConsumerAction.SAVINGS_APPLICATION_WITHDRAW, command.getSavingsId(), + SavingsCommandAccessDeniedException::new); + validateSavingsDepositType(command.getSavingsId()); + Long clientId = resolveClientId(jwt); + LocalDate withdrawnOnDate = LocalDate.now(ZoneOffset.UTC); + PostSavingsAccountsAccountIdRequest request = new PostSavingsAccountsAccountIdRequest() + .withdrawnOnDate(withdrawnOnDate.toString()) + .note(command.getNote()) + .dateFormat(DATE_FORMAT) + .locale(LOCALE); + PostSavingsAccountsAccountIdResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.handleCommandsSavingsAccount(command.getSavingsId(), request, WITHDRAW_COMMAND)); + return SavingsApplicationCommandData.builder() + .savingsId(command.getSavingsId()) + .resourceId(response.getResourceId()) + .clientId(clientId) + .build(); + } + + private void validateSavingsDepositType(Long savingsId) { Review Comment: Some smaller points on this pre-check: - **Null deposit type:** the guard lets the request through when `depositType` (or its id) is null. Rejecting in that case would be safer. - **State:** the account is already fetched here, so you could check `status.submittedAndPendingApproval` and return a clear 400/409. Otherwise a Fineract domain-rule 403 is mapped to a 502 "upstream unavailable". - **Magic number:** `100L` (line 64) would read better as a named constant tied to Fineract's `DepositAccountType.SAVINGS_DEPOSIT`. ########## consumer/src/main/java/org/apache/fineract/consumer/savings/command/service/SavingsCommandServiceImpl.java: ########## @@ -0,0 +1,188 @@ +/* + * 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.time.LocalDate; +import java.time.ZoneOffset; +import java.util.Set; +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.model.EnumOptionData; +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.generated.model.SavingsAccountData; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.FineractCaller; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.SavingsAccountApiClient; +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.beans.factory.annotation.Value; +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 static final Long SAVINGS_DEPOSIT_TYPE_ID = 100L; + + @Value("${consumer.savings.command.allowed-product-ids:}") + private Set<Long> allowedProductIds; + + private final UserQueryService userQueryService; + private final AccessPolicyEvaluator accessPolicyEvaluator; + private final OwnedAccountsCache ownedAccountsCache; + private final IdempotencyKeyHolder idempotencyKeyHolder; + private final SavingsAccountApiClient savingsAccountApiClient; + + @Override + public SavingsApplicationCommandData submitApplication(Jwt jwt, SubmitSavingsApplicationCommand command) { + accessPolicyEvaluator.authorize(jwt, ConsumerAction.SAVINGS_APPLICATION_SUBMIT); + validateProductAllowList(command.getProductId()); + Long clientId = resolveClientId(jwt); + LocalDate submittedOnDate = LocalDate.now(ZoneOffset.UTC); Review Comment: `LocalDate.now(ZoneOffset.UTC)` doesn't match the docs ("today in the tenant timezone"), and Fineract validates against the tenant's business date: - **Tenant west of UTC, late evening:** the date sent is tomorrow, and the submit is rejected as a future date. - **Tenant east of UTC, early morning:** the date sent is yesterday, which can fall before the client's activation date. The same applies at lines 99 and 116. Could the date come from the tenant's zone (configured, or Fineract's business date) rather than UTC? ########## consumer/src/main/java/org/apache/fineract/consumer/infrastructure/fineractclient/service/SavingsAccountApiClient.java: ########## @@ -0,0 +1,39 @@ +/* + * 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.infrastructure.fineractclient.service; + +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.generated.model.SavingsAccountData; + +public interface SavingsAccountApiClient { Review Comment: Since this PR was opened, the #188 refactor has settled on a `*FineractClient` / `*FineractClientImpl` naming convention (annotated `@Service`), split into command and query clients the way `LoansFineractClient` and `LoansQueryFineractClient` are. #231 adds `SavingsFineractClient`, which also wraps `SavingsAccountApi.retrieveSavingsAccount`. Could this become `SavingsCommandFineractClient`, reuse #231's adapter for the read, and get a `*FineractClientImplTest` like the other adapters? ########## docs/consumer/features/savings.adoc: ########## Review Comment: Docs: - The existing "Pagination is page-number based…" paragraph has been deleted. It documents query behaviour this PR doesn't change; could it be restored under "Query endpoints"? - The POST row says "tenant timezone", but the code uses UTC. - The upstream-error section lists only the query-side exceptions. A short table for the `SavingsCommand*` exceptions would help. - The "No consumer UI" limitation cites #144, which this PR closes. Either keep #144 open or point to a follow-up issue for the UI. ########## consumer/src/test/java/org/apache/fineract/consumer/cucumber/steps/SavingsSteps.java: ########## Review Comment: The "Modifying a pending savings application" scenario modifies to the same seeded product, and `savingsApplicationReflectsModifiedValues` only checks the id. The scenario would still pass if modify did nothing. Could it seed a second product, switch to it, and assert the product id/name (and interest rate)? The unit tests also don't yet cover: - the 404 → `SavingsCommandNotFoundException` and 400 → `SavingsApplicationInvalidException` mappings; - upstream failure and 425 on modify/withdraw; - a null `depositType`. ########## consumer/src/main/java/org/apache/fineract/consumer/savings/command/service/SavingsCommandServiceImpl.java: ########## @@ -0,0 +1,188 @@ +/* + * 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.time.LocalDate; +import java.time.ZoneOffset; +import java.util.Set; +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.model.EnumOptionData; +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.generated.model.SavingsAccountData; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.FineractCaller; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.SavingsAccountApiClient; +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.beans.factory.annotation.Value; +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 static final Long SAVINGS_DEPOSIT_TYPE_ID = 100L; + + @Value("${consumer.savings.command.allowed-product-ids:}") + private Set<Long> allowedProductIds; + + private final UserQueryService userQueryService; + private final AccessPolicyEvaluator accessPolicyEvaluator; + private final OwnedAccountsCache ownedAccountsCache; + private final IdempotencyKeyHolder idempotencyKeyHolder; + private final SavingsAccountApiClient savingsAccountApiClient; + + @Override + public SavingsApplicationCommandData submitApplication(Jwt jwt, SubmitSavingsApplicationCommand command) { + accessPolicyEvaluator.authorize(jwt, ConsumerAction.SAVINGS_APPLICATION_SUBMIT); + validateProductAllowList(command.getProductId()); + Long clientId = resolveClientId(jwt); + LocalDate submittedOnDate = LocalDate.now(ZoneOffset.UTC); + PostSavingsAccountsRequest request = buildSubmitRequest(command, clientId, submittedOnDate); + PostSavingsAccountsResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.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(), + SavingsCommandAccessDeniedException::new); + validateProductAllowList(command.getProductId()); + validateSavingsDepositType(command.getSavingsId()); + Long clientId = resolveClientId(jwt); + LocalDate submittedOnDate = LocalDate.now(ZoneOffset.UTC); + PutSavingsAccountsAccountIdRequest request = buildModifyRequest(command, submittedOnDate); + PutSavingsAccountsAccountIdResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.updateSavingsAccount(command.getSavingsId(), request, null)); + return SavingsApplicationCommandData.builder() + .savingsId(command.getSavingsId()) + .resourceId(response.getResourceId()) + .clientId(clientId) + .build(); + } + + @Override + public SavingsApplicationCommandData withdrawApplication(Jwt jwt, WithdrawSavingsApplicationCommand command) { + accessPolicyEvaluator.authorize(jwt, ConsumerAction.SAVINGS_APPLICATION_WITHDRAW, command.getSavingsId(), + SavingsCommandAccessDeniedException::new); + validateSavingsDepositType(command.getSavingsId()); + Long clientId = resolveClientId(jwt); + LocalDate withdrawnOnDate = LocalDate.now(ZoneOffset.UTC); + PostSavingsAccountsAccountIdRequest request = new PostSavingsAccountsAccountIdRequest() + .withdrawnOnDate(withdrawnOnDate.toString()) + .note(command.getNote()) + .dateFormat(DATE_FORMAT) + .locale(LOCALE); + PostSavingsAccountsAccountIdResponse response = callWithIdempotency(jwt, command.getIdempotencyKey(), + () -> savingsAccountApiClient.handleCommandsSavingsAccount(command.getSavingsId(), request, WITHDRAW_COMMAND)); + return SavingsApplicationCommandData.builder() + .savingsId(command.getSavingsId()) + .resourceId(response.getResourceId()) + .clientId(clientId) + .build(); + } + + private void validateSavingsDepositType(Long savingsId) { + SavingsAccountData account = call(() -> savingsAccountApiClient.retrieveSavingsAccount(savingsId)); + EnumOptionData depositType = account.getDepositType(); + if (depositType != null && depositType.getId() != null + && !SAVINGS_DEPOSIT_TYPE_ID.equals(depositType.getId())) { + throw new SavingsApplicationInvalidException(); + } + } + + private Long resolveClientId(Jwt jwt) { + UUID publicId = UUID.fromString(jwt.getSubject()); + UserQueryData user = userQueryService.findByPublicId(publicId); + return user.getFineractClientId(); + } + + private <T> T callWithIdempotency(Jwt jwt, String rawKey, Supplier<T> upstream) { + UUID publicId = UUID.fromString(jwt.getSubject()); + idempotencyKeyHolder.set(IdempotencyKeyDeriver.derive(publicId, rawKey)); + try { + return call(upstream); + } finally { + idempotencyKeyHolder.clear(); + } + } + + private <T> T call(Supplier<T> upstream) { + return FineractCaller.call(upstream, + e -> new SavingsCommandNotFoundException(), + SavingsApplicationInvalidException::new, + SavingsCommandInProgressException::new, + SavingsCommandUpstreamUnavailableException::new); + } + + private void validateProductAllowList(Long productId) { + if (allowedProductIds != null && !allowedProductIds.isEmpty() && !allowedProductIds.contains(productId)) { + throw new SavingsApplicationInvalidException(); + } + } + + private PostSavingsAccountsRequest buildSubmitRequest(SubmitSavingsApplicationCommand c, Long clientId, + LocalDate submittedOnDate) { + return new PostSavingsAccountsRequest() + .clientId(clientId) + .productId(c.getProductId()) + .submittedOnDate(submittedOnDate.toString()) + .dateFormat(DATE_FORMAT) + .locale(LOCALE); + } + + private PutSavingsAccountsAccountIdRequest buildModifyRequest(ModifySavingsApplicationCommand c, + LocalDate submittedOnDate) { + return new PutSavingsAccountsAccountIdRequest() + .productId(c.getProductId()) + .submittedOnDate(submittedOnDate.toString()) Review Comment: Two questions about modify: 1. **It resets `submittedOnDate` to today.** That rewrites the original application date and runs into the same timezone problem. Can it be left out of the PUT? 2. **Only `productId` is sent.** My understanding is that Fineract's modify updates only the fields present, so switching product could keep the old product's interest rate, compounding settings and charges. That would contradict the doc's "interest rate is taken from the product". Could you check this against a real Fineract? If it does keep them, send the new product's defaults from the template. ########## consumer/src/main/java/org/apache/fineract/consumer/savings/command/service/SavingsCommandServiceImpl.java: ########## @@ -0,0 +1,188 @@ +/* + * 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.time.LocalDate; +import java.time.ZoneOffset; +import java.util.Set; +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.model.EnumOptionData; +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.generated.model.SavingsAccountData; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.FineractCaller; +import org.apache.fineract.consumer.infrastructure.fineractclient.service.SavingsAccountApiClient; +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.beans.factory.annotation.Value; +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 static final Long SAVINGS_DEPOSIT_TYPE_ID = 100L; + + @Value("${consumer.savings.command.allowed-product-ids:}") Review Comment: Elsewhere in the module, settings come in through a `@ConfigurationProperties` class injected by the constructor (`RateLimitProperties`, `AuditRetentionProperties`). `@Value` field injection on a `@RequiredArgsConstructor` class is why the tests need `ReflectionTestUtils.setField`. Could this use a small `SavingsCommandProperties` instead? Also, an empty allow-list means every product is accepted, and `SavingsQueryServiceImpl.getApplicationTemplate` doesn't apply the list. Once someone configures it, the UI would offer products that submit then rejects. -- 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]
