Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Filter by Role to user search #1278

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Added Filter by Role to user search #1278

wants to merge 10 commits into from

Conversation

SebiWrn
Copy link
Contributor

@SebiWrn SebiWrn commented Dec 21, 2023

Motivation and Context

You can now filter users by their role, improves administration and overview over admins.

Steps for Testing

Prerequisites:

  • 1 Admin
  1. Log in
  2. Navigate to User management page
  3. View the new Dropdown

Screenshots

image

Copy link

Your Testserver will be ready at https://1278.test.live.mm.rbg.tum.de in a few minutes.

Logins
Kurs1 Kurs2 Kurs3 Kurs4
public public loggedin enrolled
prof1 prof1 prof2 prof1
prof2
student1
student2
student3
student1
student2
student2
student3
student1
student2

@SebiWrn SebiWrn self-assigned this Dec 28, 2023
@SebiWrn SebiWrn enabled auto-merge (squash) January 9, 2024 19:54
@YiranDuan721
Copy link
Contributor

There's a small problem: after filtering, the user's email doesn't seem to be displayed

@karjo24 karjo24 self-assigned this Nov 21, 2024
@karjo24
Copy link
Contributor

karjo24 commented Nov 21, 2024

Filtering locally results in no show of potential results. Simplest case of this is filtering for students without query string. Change the API instead.

@karjo24
Copy link
Contributor

karjo24 commented Nov 21, 2024

There's a small problem: after filtering, the user's email doesn't seem to be displayed

I cannot reproduce this problem in Mozilla Firefox for Ubuntu 132.0.2 (64-Bit) or Google Chrome Version 130.0.6723.58 (Official Build) (64-bit).

Copy link
Member

@joschahenningsen joschahenningsen left a comment

Choose a reason for hiding this comment

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

Looks great, just added two remarks

api/users.go Outdated Show resolved Hide resolved
api/users.go Show resolved Hide resolved
users, err = r.UsersDao.SearchUser(q)
role, err := strconv.ParseUint(rQ, 10, 64)
if err != nil && rQ != "" && rQ != "-1" {
tools.RenderErrorPage(c, http.StatusBadRequest, "invalid role")
Copy link
Member

Choose a reason for hiding this comment

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

API endpoints should not render HTML pages on error but rather use

_ = c.AbortWithError(http.StatusForbidden, tools.RequestError{

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants