[
https://issues.apache.org/jira/browse/FINERACT-2165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112088#comment-18112088
]
Dhananjay Bhagat commented on FINERACT-2165:
--------------------------------------------
Thanks [~adamsaghy] . So based on the current implementation, the ambiguity is
specifically in {*}requestedShares{*}:
{{applyadditionalshares}} and {{redeemshares}} expect a numeric value, while
{{approveadditionalshares}} and {{rejectadditionalshares}} expect an array of
share-purchase transaction IDs.
I will leave the existing {{/v1/accounts/\{type}/\{accountId}?command=...}}
endpoint unchanged for backward compatibility, and on your suggestion introduce
v2 endpoints with concrete request models, while reusing the existing
command-processing flow underneath.
My proposed design is to introduce {*}4 explicit v2 operations{*}: one each for
{{{}applyadditionalshares{}}}, {{{}redeemshares{}}},
{{{}approveadditionalshares{}}}, and {{{}rejectadditionalshares{}}}. Although
there are only 2 different *requestedShares* representations, these correspond
to 4 distinct operations, so having one endpoint per operation would allow both
the URL and request schema to unambiguously describe what the API is doing. I
don’t think combining them into only 2 endpoints based solely on the
*requestedShares* type would be as explicit, since each endpoint would still
need some way to distinguish between the different operations.
Does this approach look correct to you or did you have a different design in
mind?
> Use "Fineract-client-feign" in integration tests
> ------------------------------------------------
>
> Key: FINERACT-2165
> URL: https://issues.apache.org/jira/browse/FINERACT-2165
> Project: Apache Fineract
> Issue Type: Improvement
> Affects Versions: 1.11.0
> Reporter: Adam Saghy
> Assignee: Adam Saghy
> Priority: Major
> Labels: beginner, beginner-friendly, code-debt, improvement,
> newjoiner-safe
>
> `Fineract-client` was introduced as part of
> https://issues.apache.org/jira/browse/FINERACT-1189 and some integration
> tests were using it (https://issues.apache.org/jira/browse/FINERACT-1209) but
> unfortunately many were not used and new integration tests got implemented
> but still not using the `fineract-client`!
> Since then a new variant was introduced:
> *[FINERACT-2380|https://issues.apache.org/jira/browse/FINERACT-2380]: create
> feign client*
>
> *Why Fineract-client-feign is better?*
> * Type safety comes out of box!
> ** Type safe requests, and responses!
> * No need to "fabricate" the endpoints!
> * We can move away from the RequestSpecification and ResponseSpecification
> configurations!
> * Fineract-client is automatically generated based on the swagger
> configuration / request / response object!
> *Why would it be beneficial to rewrite our integration tests?*
> * No more struggling with Map<String, Object>-s to fabricate request objects
> and trying to fetch out fields from response Object / Map...
> * No more GSON conversion back and forth
> * No more "is this json string contains XYZ?"
> ** Mostly was done to test exception messages...
> * No more N+1 method with slightly different parameter list
> * *Fineract-client got tested more thoroughly!*
> * *Our swagger configurations got tested more thoroughly!*
> * *Better maintainability!*
> * *Better readability!*
> * *Less error-prone!*
> I would like to give it a try as part of this story!
> # step: Marking all the methods as *deprecated* in the Helper classes which
> were using Rest-Assured for communicating with the backend!
> # step: Inform the Fineract-Dev mail list to not use "RestAssured based"
> solutions for any of the newly created integration tests!
> # step: Ask the community to start using the `fineract-client` based
> communication for all the newly created integration tests!
> ## Check in the helper classes whether for the required action there is a
> "Fineract-client based" implementation
> ## *If yes, please use that!*
> ## *If no, please implement it!*
> ### If it is broken, example: swagger request or response is not complete ->
> there is a missing field or the data type is incorrect -> *Please fix it!*
> # step: Ask the community and the newcomers to start removing the methods
> that were marked as *deprecated* and rewrite them to use *`fineract-client`
> instead!*
> ## *This means the integration tests that were using the deprecated methods
> must also be changed!*
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
