FINERACT-1272: Creating Currencies (org.apache.fineract.organisation.monetary.api) #4971

kapil-panchal · 2025-08-19T08:36:52Z

Description

Describe the changes made and why they were made.

Implemented code review comments as per FINERACT-1272: Add the possibility to edit currencies available - Creating Currencies (org.apache.fineract.organisation.monetary.api) #4902
Changed the validation messages.

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/legacy-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.

github-actions · 2025-09-19T00:21:31Z

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

adamsaghy · 2025-09-19T07:28:27Z

...core/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateRequest.java

+    @Size(min = 5, max = 50, message = "{org.apache.fineract.organisation.monetary.currencies" + ".name" + ".size}")
+    private String name;
+
+    @NotNull(message = "{org.apache.fineract.organisation.monetary.currencies.decimalPlaces" + ".notNull}")


no need for string concatenation

Sure, I will remove the String concat.

adamsaghy · 2025-09-19T07:28:49Z

...core/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateRequest.java

+    private String name;
+
+    @NotNull(message = "{org.apache.fineract.organisation.monetary.currencies.decimalPlaces" + ".notNull}")
+    @Min(value = 0, message = "{org.apache.fineract.organisation.monetary.currencies" + ".decimalPlaces" + ".min}")


no need for string concatenation

Sure I will remove the String concat.

adamsaghy · 2025-09-19T07:29:02Z

...core/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateRequest.java

+
+    @NotNull(message = "{org.apache.fineract.organisation.monetary.currencies.decimalPlaces" + ".notNull}")
+    @Min(value = 0, message = "{org.apache.fineract.organisation.monetary.currencies" + ".decimalPlaces" + ".min}")
+    @Max(value = 5, message = "{org.apache.fineract.organisation.monetary.currencies" + ".decimalPlaces" + ".max}")


no need for string concatenation

Sure I will remove the String concat.

adamsaghy · 2025-09-19T07:29:06Z

...core/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateRequest.java

+    @Max(value = 5, message = "{org.apache.fineract.organisation.monetary.currencies" + ".decimalPlaces" + ".max}")
+    private Integer decimalPlaces;
+
+    @PositiveOrZero(message = "{org.apache.fineract.organisation.monetary.currencies" + ".inMultiplesOf" + ".positiveOrZero}")


no need for string concatenation

Sure I will remove the String concat.

adamsaghy · 2025-09-19T07:30:19Z

...ore/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateResponse.java

+@Data
+@NoArgsConstructor
+@AllArgsConstructor
+@SuperBuilder


Non need for SuperBuilder annotation

Sure will remove the SuperBuilder.

adamsaghy · 2025-09-19T07:30:39Z

...ore/src/main/java/org/apache/fineract/organisation/monetary/data/CurrencyCreateResponse.java

+import lombok.experimental.SuperBuilder;
+
+@Data
+@NoArgsConstructor


Do we need no arg constructor?

Sure I will remove the NoArgsContructor.

adamsaghy · 2025-09-19T07:32:00Z

...core/src/main/java/org/apache/fineract/organisation/monetary/domain/ApplicationCurrency.java

 import org.apache.fineract.infrastructure.core.domain.AbstractPersistableCustom;
 import org.apache.fineract.organisation.monetary.data.CurrencyData;

+@Builder


Please dont use Builder for JPA entities.

Sure I will remove the Builder.

adamsaghy · 2025-09-19T07:38:45Z

...he/fineract/organisation/monetary/service/CurrencyWritePlatformServiceJpaRepositoryImpl.java

+import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;

+@Service


We dont need this. We are using Configuration classes to instantiate this bean.

Sure I will remove the Service annotation.

adamsaghy · 2025-09-19T07:39:24Z

...he/fineract/organisation/monetary/service/CurrencyWritePlatformServiceJpaRepositoryImpl.java

+        final ApplicationCurrency currency = currencyMapper.mapToEntity(request);
+        currency.setNameCode(getNameCode(currency.getCode()));
+
+        validateIfExistingCurrency(currency.getCode());


Please do the validation first before any mapping.

I will do the validation before mapping.

adamsaghy

Kindly see my review!

IOhacker · 2025-10-14T15:13:49Z

@kapil-panchal are you working on it?

kapil-panchal · 2025-10-15T13:45:49Z

Hi Victor, yes I am awaiting comments from Aleksandar Vidakovic.

IOhacker · 2025-10-15T14:40:25Z

@kapil-panchal also there are pending items in the code regarding the comments done by @adamsaghy

kapilpanchal123 · 2025-10-27T04:28:53Z

@vidakovic awaiting your inputs on this ticket.

adamsaghy · 2025-10-27T10:03:21Z

@kapil-panchal You can move forward with the comments from others till @vidakovic review's also...

kapilpanchal123 · 2025-10-27T10:06:24Z

@adamsaghy this ticket has the NewCommandInfrastructure implementation that I needed to get verified by @vidakovic?

adamsaghy · 2025-11-25T09:50:51Z

Moving this into draft as it was learned new command handler does not support:

audit
retry
idempotency handling

FINERACT-1272: Feature

ce746e5

github-actions bot added the stale label Sep 19, 2025

adamsaghy requested a review from vidakovic September 19, 2025 07:27

adamsaghy reviewed Sep 19, 2025

View reviewed changes

adamsaghy requested changes Sep 19, 2025

View reviewed changes

github-actions bot removed the stale label Sep 20, 2025

adamsaghy marked this pull request as draft November 25, 2025 09:50

FINERACT-1272: Creating Currencies (org.apache.fineract.organisation.monetary.api) #4971

Are you sure you want to change the base?

FINERACT-1272: Creating Currencies (org.apache.fineract.organisation.monetary.api) #4971

Uh oh!

Conversation

kapil-panchal commented Aug 19, 2025

Description

Checklist

Uh oh!

github-actions bot commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsaghy left a comment

Choose a reason for hiding this comment

Uh oh!

IOhacker commented Oct 14, 2025

Uh oh!

kapil-panchal commented Oct 15, 2025

Uh oh!

IOhacker commented Oct 15, 2025

Uh oh!

kapilpanchal123 commented Oct 27, 2025

Uh oh!

adamsaghy commented Oct 27, 2025

Uh oh!

kapilpanchal123 commented Oct 27, 2025

Uh oh!

adamsaghy commented Nov 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants