[GitHub] [fineract] ptuomola commented on pull request #1207: FINERACT-1095 Added status parameter in Clients API

[GitBox <gi...@apache.org>]

ptuomola commented on pull request #1207:
URL: https://github.com/apache/fineract/pull/1207#issuecomment-668755830


   > @ptuomola I am collectively answering to your doubts here:
   > 
   > 1. I  am actually adding clients, so there would be some clients in the database for sure. (Check the first for loop).
   > 2. I am adding 15 clients and each time I am assigning him pending or active status randomly, that means
   > 
   > The probability that there are no active/pending clients = 1/pow(2,15) = 0.00003051757, I hope you will agree we can ignore this and consider that there is at least one active/pending client.
   
   No that wasn't my concern - I know you create active and pending clients. My concern was: what if the query selection doesn't work as expected, and instead returns an empty list without matching any clients? I don't see the test checking for the case where there is no clients returned - it just checks that whatever clients are returned are in the correct status. 
   
   So something like assertNotEquals(list.size(), 0) for both cases...
   
   > 1. The client status never actually becomes invalid, it just means that there is no such status. In that case, we as you said we can either give an exception or show all entries.
   >    @ptuomola as you said, should we instead throw an exception? still confused or we should instead show all the clients if it does not map to any of them.
   
   My personal view would be that we should throw an exception if the caller sends a status that is not a valid value for ClientStatus. Otherwise the caller thinks that there are no clients matching this status and never finds out that they actually sent the wrong value. Empty list should mean that there are no matches. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org