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

Add User Settings Tests #101

Merged
merged 10 commits into from
Dec 5, 2021
Merged

Conversation

1337joe
Copy link
Member

@1337joe 1337joe commented Dec 2, 2021

Changes

  • Minor cleanup and refactoring to improve reusability of code
  • Implemented testing of user/admin filters - displays counts for each and warns if admins aren't users
  • Implemented test query of a user log in

Screenshots
LDAP User Settings
image

Valid filters with matches
image

Empty user filter, admin warning
image

No user found
image

User found
image

Unable to search due to LDAP connection failure
image

Issues
Fixes #40

@crobibero crobibero added the major-feature This PR introduces a major new feature label Dec 3, 2021
LDAP-Auth/Api/LdapController.cs Outdated Show resolved Hide resolved
LDAP-Auth/Api/LdapController.cs Outdated Show resolved Hide resolved
LDAP-Auth/LDAPAuthenticationProviderPlugin.cs Outdated Show resolved Hide resolved
LDAP-Auth/LDAPAuthenticationProviderPlugin.cs Outdated Show resolved Hide resolved
1337joe and others added 3 commits December 3, 2021 13:00
Import specific provider instead of filtering
Capitalize log placeholders

Co-authored-by: Cody Robibero <cody@robibe.ro>
@crobibero
Copy link
Member

Last thing I noticed- if I enter a totally invalid filter, such as (memberOf=cn=group,ou=groups,dc=example,dc=com, the API returns a 500 that the page doesn't handle

Catch filter errors
Display filter-specific error text on testing
Handle unexpected (non-json) error messages
@1337joe
Copy link
Member Author

1337joe commented Dec 4, 2021

Error 500 returns a string, not json, so that took a bit of working around to ensure it would display. On the bright side, the filter error message contains the specific failure reason (not as the Message, but embedded in the ToString):
image

@crobibero crobibero merged commit 00425c4 into jellyfin:master Dec 5, 2021
@crobibero crobibero added feature This PR or Issue requests or introduces a new feature and removed major-feature This PR introduces a major new feature labels Dec 5, 2021
@1337joe 1337joe deleted the add-settings-tests branch January 5, 2022 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This PR or Issue requests or introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Test button on the settings page?
2 participants