[ 
https://issues.apache.org/jira/browse/FINERACT-2165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111540#comment-18111540
 ] 

Dhananjay Bhagat commented on FINERACT-2165:
--------------------------------------------

[~adamsaghy] I checked both the server-side handling and the integration tests 
and confirmed that both forms are part of the current implementation and are 
exercised by the tests as well. So both use cases are actively present.

In {{{}ShareAccountDataSerializer.java{}}}, 
{{validateAndApplyAddtionalShares()}} and {{validateAndRedeemShares()}} parse 
{{requestedShares}} using {{{}extractLongNamed(){}}}, treating it as the number 
of shares. On the other hand, {{validateAndApproveAddtionalShares()}} and 
{{validateAndRejectAddtionalShares()}} parse {{requestedShares}} using 
{{{}extractJsonArrayNamed(){}}}, with each array element containing an {{id}} 
representing a share purchase transaction.

So the same {{POST /v1/accounts/\{type}/\{accountId}?command=...}} endpoint 
currently handles 9 different commands, with {{requestedShares}} being a number 
for {{applyadditionalshares}} and {{{}redeemshares{}}}, an array of transaction 
IDs for {{approveadditionalshares}} and {{{}rejectadditionalshares{}}}, and 
unused by the other 5 commands.

When you say “split the endpoint into two,” do you mean splitting the current 
endpoint based on the two different {{requestedShares}} formats: one for 
commands where it is a number and another for commands where it is an array, so 
that each can have its own Swagger/OpenAPI request schema? 
Or did you have a different API structure 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)

Reply via email to