FINERACT-2253: refactor account module to new command processing#5811
FINERACT-2253: refactor account module to new command processing#5811SamaSVM wants to merge 1 commit into
Conversation
SamaSVM
force-pushed
the
FINERACT-2253/new-command-processing-account
branch
4 times, most recently
from
665094d to
e4328f0
Compare
SamaSVM
force-pushed
the
FINERACT-2253/new-command-processing-account
branch
3 times, most recently
from
7add5f8 to
76768cb
Compare
|
@vidakovic Could you please review this PR when you have a moment? I’d appreciate any feedback you can provide. |
SamaSVM
force-pushed
the
FINERACT-2253/new-command-processing-account
branch
2 times, most recently
from
7f1e564 to
17bfe98
Compare
SamaSVM
force-pushed
the
FINERACT-2253/new-command-processing-account
branch
from
17bfe98 to
7ce2ede
Compare
| private final DefaultToApiJsonSerializer<AccountTransferData> toApiJsonSerializer; | ||
| private final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService; | ||
| private final AccountTransfersReadPlatformService accountTransfersReadPlatformService; | ||
| private final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService; |
There was a problem hiding this comment.
Nope, we want to remove these legacy command processing components. Only read services and the new CommandDispatcher can be in the JAX-RS resource classes.
| @@ -70,7 +75,6 @@ public class AccountTransfersApiResource { | |||
| + "accounttransfers/template?fromClientId=1&fromAccountType=2&fromAccountId=1") | |||
| @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = AccountTransfersApiResourceSwagger.GetAccountTransfersTemplateResponse.class))) | |||
There was a problem hiding this comment.
When we are already type-safe then we don't need these @ApiResponse and especially not @Schema annotations. Response code 200 is anyway trivial, because that's the default anyway. In short: the goal is to get rid of AccountTransfersApiResourceSwagger (and similar "Swagger" suffixed) classes in the first place.
| @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = AccountTransfersApiResourceSwagger.GetAccountTransfersTemplateResponse.class))) | ||
| public AccountTransferData template(@BeanParam AccountTransSearchParam accountTransSearchParam) { | ||
|
|
||
| context.authenticatedUser().validateHasReadPermission(AccountTransfersApiConstants.ACCOUNT_TRANSFER_RESOURCE_NAME); |
There was a problem hiding this comment.
Please see other already migrated packages. Authorization should be configured where it belongs... and that is SecurityConfig.java. Using the platform security context service is bad practice anyway.
| @@ -86,11 +90,10 @@ public AccountTransferData template(@BeanParam AccountTransSearchParam accountTr | |||
| @Operation(summary = "Create new Transfer", operationId = "createAccountTransfer", description = "Ability to create new transfer of monetary funds from one account to another.") | |||
| @RequestBody(required = true, content = @Content(schema = @Schema(implementation = AccountTransferRequest.class))) | |||
There was a problem hiding this comment.
Again, these are the annotations we want to get rid of. If we have proper types then these are not necessary: @RequestBody and @ApiResponse. Please fix this also for all following occurrences, won't comment on each.
There was a problem hiding this comment.
@SamaSVM another thing: there are currently 2 GSoC projects going on that - I think - have contain this package that you worked on. They are already assigned to students. Please contact @avivijay19 and @Aman-Mittal to coordinate... but probably you have to abandon this unfortunately. I will block now all sub-tasks on FINERACT-2169, because they are pretty much all now GSoC projects. FYI
There was a problem hiding this comment.
@vidakovic Understood, thank you for your feedback.
|
@SamaSVM sorry for that... there will be more to do 😉 |
2 participants
Description
Refactored the account module (AccountTransfers and StandingInstruction) to align with the new command processing pattern already established in other modules.
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.