Description

Fixes #760 Add to List Users API the ability to filter users by location

Type of Change:

• Code

Code/Quality Assurance Only

• New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

I have updated location for few users then I tried to search from /users and /users/verified endpoints and It was giving expected results.

Checklist:

• [x] My PR follows the style guidelines of this project
• [x] I have performed a self-review of my own code or materials
• [x] I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
• [x] I have made corresponding changes to the documentation

Code/Quality Assurance Only

• [x] My changes generate no new warnings
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes

Codecov Report

Merging #848 (62d68cc) into develop (1643e7e) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #848   +/-   ##
========================================
  Coverage    95.99%   96.00%           
========================================
  Files           96       96           
  Lines         5293     5306   +13     
========================================
+ Hits          5081     5094   +13     
  Misses         212      212           

Impacted Files	Coverage Δ
app/api/dao/user.py	85.08% <100.00%> (ø)
app/api/resources/user.py	89.04% <100.00%> (+0.10%)	:arrow_up:
tests/test_data.py	100.00% <100.00%> (ø)
tests/users/test_api_list_users.py	99.30% <100.00%> (+0.05%)	:arrow_up:

Hi @paritoshsinghrahar,

Can you please review my PR?

Thanks

Hi @paritoshsinghrahar,

Can you please review my PR?

Thanks

Sure. I'll get back to you.

@Ashokrayal On an initial review looks good. Great Effort, though.

Hi @paritoshsinghrahar @isabelcosta, @vj-codes, Can someone review this PR?

@isabelcosta This looks good,

But before merging any of the filter PRs, I'd be willing to have a discussion on what the best approach is to implement this or refactor this later. Currently, the function and the parameters suffers from all kinds of code smell, so I am open to all kinds of ideas on how to implement filtering in a clean way.

@isabelcosta @SanketDG does this PR need further discussion before being tested and merged?

@SanketDG I agree with you. Will wait for the test and then merge. But as part of this, can you create an issue for creating a cleaner way for filtering data? If not, I can, just let me know :) Thank you so much for bringing this up!

I will surely do it today

Here are some basic ideas to abstract away filtering into a common piece of code (so that it doesn't end up having 20 arguments in the function signature)

• Use **kwargs as a container for all filter operations.
• Loop through kwargs and check which "keys" are fields of UserModel.
• Do the standard filter query using these key value pairs from **kwargs

Putting on hold as the best approach is not decided yet

@vj-codes could you create, in case it does not exist yet, an issue to implement filtering in a generic way.