FINERACT-2293: Migrate client address module to new command pipeline#5657
FINERACT-2293: Migrate client address module to new command pipeline#5657avivijay19 wants to merge 2 commits intoapache:developfrom
Conversation
602cd1a to
91c3b39
Compare
371162c to
f2556aa
Compare
| } | ||
|
|
||
| @Override | ||
| public void addNewClientAddress(Client client, List<ClientAddressCreateRequest> requests) { |
There was a problem hiding this comment.
Why do we need this loop inside this service? I bet this is called only in 1 place... let's create the loop there... no added value hiding 3 lines of code in a service and they add basically no new functionality. We are not saving anything here... unless you tell me we are calling this function in 57 places...
| } | ||
| } | ||
|
|
||
| private Address createAddressFromRequest(ClientAddressCreateRequest request) { |
There was a problem hiding this comment.
Can move to createClientAddres function once we have remove that other one with the loop.
| import org.springframework.jdbc.core.JdbcTemplate; | ||
|
|
||
| @Configuration | ||
| public class AddressConfiguration { |
There was a problem hiding this comment.
ClientAddressConfiguration.... consistency...
|
|
||
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import java.time.LocalDate; | ||
| import java.util.HashMap; |
There was a problem hiding this comment.
As soon as we read "XXXSwagger.java" something is wrong aka not finished. Again, these are the dummy classes we intend to remove... mentioned it already a couple of times... maybe go slower with the changes and double check yourself before you submit.
|
|
||
| if (isAddressEnabled) { | ||
| this.addressWritePlatformService.addNewClientAddress(newClient, command); | ||
| final com.google.gson.JsonArray addressArray = command.arrayOfParameterNamed("address"); |
There was a problem hiding this comment.
This is not type safety! This kind of thing is now consistently slipping through the cracks. ANYTHING:
- GSON
- xxxParamterNamed
- JsonArray
- JsonXXX
- JsonCommand
- CommandWrapper
- CommandBuilder
... needs to go, because this is the goal of this exercise, to make things type safe... all of the above is not. Again, maybe go a bit slower... otherwise this feels a bit like a prompt loop. I don't mind reviewing... but let's keep an eye on these details.
|
@avivijay19 plz resolve conflicts |
|
@avivijay19 Is this abandoned? |
|
@adamsaghy work still going on |
|
@avivijay19 Moved into draft. Let us know once it is ready to be reviewed. |
f350682 to
b9c6178
Compare
Delete dead code: AddressCommandFromApiJsonDeserializer, Address.fromJson(), Address.fromJsonObject(). Rename FieldConfigurationReadPlatformService to FieldConfigurationReadService, remove PlatformSecurityContext, add security rule for fieldconfiguration endpoint. Co-Authored-By: Claude Opus 4.6 <[email protected]>
b9c6178 to
2777146
Compare
Description
Describe the changes made and why they were made. (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!
Your assigned reviewer(s) will follow our guidelines for code reviews.