NotificationsYou must be signed in to change notification settings
Fork905
Star10k

feat: support filtering users table by login type#17238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

ethanndickson merged 15 commits intocoder:mainfromutsavll0:main

Apr 9, 2025

Merged

feat: support filtering users table by login type#17238

ethanndickson merged 15 commits intocoder:mainfromutsavll0:main

Apr 9, 2025

+226 −24

Conversation

Copy link

Contributor

utsavll0 commentedApr 3, 2025

#15896 Mentions ability to add support for filtering by login type

The issue mentions that backend API support exists but the backend did not seem to have the support for this filter. So I have added the ability to filter it.

I also added a corresponding update to readme file to make sure the docs will correctly showcase this feature

utsavll0 added2 commits

April 3, 2025 00:53

ability to filter users based on login type

d0e840a

update documentation also

3016e40

cdr-botbot added the communityPull Requests and issues created by the community. label

Apr 3, 2025

Copy link

github-actionsbot commentedApr 3, 2025•
edited
Loading

All contributors have signed the CLA ✍️ ✅
_{Posted by theCLA Assistant Lite bot.}

github-actionsbot assignedutsavll0

Apr 3, 2025

utsavll0 changed the title~~PR: Fixes #15896 Support filtering Users table by login type~~feat(backend) support filtering Users table by login type

Apr 3, 2025

utsavll0 changed the title~~feat(backend) support filtering Users table by login type~~feat: support filtering Users table by login type

Apr 3, 2025

Copy link

ContributorAuthor

utsavll0 commentedApr 3, 2025

I have read the CLA Document and I hereby sign the CLA

cdrci2 added a commit to coder/cla that referenced this pull request

Apr 3, 2025

@utsavll0has signed the CLA incoder/coder#17238

767bf9f

utsavll0 changed the title~~feat: support filtering Users table by login type~~feat: support filtering users table by login type

Apr 3, 2025

Copy link

Member

ethanndickson commentedApr 3, 2025•
edited
Loading

Thanks for the PR!

Looking good so far, but we'll need some tests. I believe the right place for those iscoderd/users_test.go

Also, please runmake fmt/go,make lint/go, andmake gen after each change!

utsavll0and others added2 commits

April 3, 2025 13:00

add tests

de02f49

Merge branch 'main' into main

0bab070

Copy link

ContributorAuthor

utsavll0 commentedApr 3, 2025•
edited
Loading

@ethanndickson I added a few tests for the PR and I also ran the commands which you mentioned

Edit: Can you please help me in running the users_test.go file? I am not able to figure out why the test is failing. Making a call from the frontend works fine but the unit test keeps on failing. I am not sure if there is some extra config required to make it run

change test case

241c898

ethanndickson reviewed

Apr 7, 2025

View reviewed changes

Copy link

Member

ethanndickson left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Your newly written tests are failing as, by default, DB queries are ran against a fake DB indbmem.go. You'll need to update theGetUsers Go function with the same logic as the Postgres query.

To rungo test against postgres, just setDB=ci:
DB=ci go test -run "TestGetUsersFilters" github.com/coder/coder/v2/coderd
(This is essentially whatmake test-postgres does)

coderd/users_test.go OutdatedShow resolvedHide resolved

utsavll0 added3 commits

April 7, 2025 01:06

filter by multiple login types

5ddc42a

add filter by logintype to mock db

f73e1f2

update tests

b649181

ethanndickson reviewed

Apr 7, 2025

View reviewed changes

Copy link

Member

ethanndickson left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Nice work on these tests! Just a few comments, mostly on style.

CI is all passing except forweekly-docs / check-docs, which you can safely ignore (it should go away if you merge / rebase your branch off main)

coderd/users_test.go OutdatedShow resolvedHide resolved

utsavll0and others added2 commits

April 7, 2025 10:18

cleanup tests

15407e0

Merge branch 'main' into main

6ae52ce

Copy link

ContributorAuthor

utsavll0 commentedApr 8, 2025

Thank you for your feed@ethanndickson. I am to blame for these silly errors because of my over-reliance on copilot. I have fixed the style and removed unnecessary comments from the code. Please have a look again.