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.

Sign up for GitHub

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

Jump to bottom

feat: Add back searching in disabled user list #45370

Merged
merged 4 commits into from
May 23, 2024
Merged

feat: Add back searching in disabled user list #45370

merged 4 commits into from
May 23, 2024

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented May 16, 2024 •
edited

Summary

When disabled users where moved to their own endpoint we overlooked search, so adding it back.
The search is done case-insensitive in uid and display name.

TODO

  • Decide if we should also search in email field, and implement it
  • Decide if the BC break is ok or should be avoided with a new interface

Checklist

@come-nc come-nc added the 2. developing Work in progress label May 16, 2024
@come-nc come-nc added this to the Nextcloud 30 milestone May 16, 2024
@come-nc come-nc requested review from artonge and Pytal May 16, 2024 14:18
@come-nc come-nc self-assigned this May 16, 2024
@come-nc
Copy link
Contributor Author

come-nc commented May 16, 2024 •
edited

TODO

  • Decide if we should also search in email field, and implement it

Implemented.

  • Decide if the BC break is ok or should be avoided with a new interface

I think this is fine, because a class implementing the interface for 30 also implements the one for 28.0.0, as it’s only a new parameter.
So, as the breaking is minimal and it’s easy for concerned apps (and that’s only apps implementing a user backend with special disabled user treatment, so only user_ldap to my knowlegde) to support all Nextcloud versions.

@come-nc come-nc added 3. to review Waiting for reviews feature: users and groups and removed 2. developing Work in progress labels May 16, 2024
@come-nc
Copy link
Contributor Author

come-nc commented May 16, 2024

/compile /

provokateurin
provokateurin reviewed May 16, 2024
View reviewed changes
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

I would prefer separate methods for searching quite a lot over changing the behavior of the existing methods. Would that be possible or is it too much work or impractical?

@provokateurin
Copy link
Member

Ok in light of the discussion in #44936 it makes sense to extend the existing endpoint instead of creating a new one

artonge
artonge approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment •
edited

Choose a reason for hiding this comment

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

So BC is not a real BC?

Edit: seen your second comment

apps/provisioning_api/lib/Controller/UsersController.php Show resolved Hide resolved
@come-nc
feat: Add back searching in disabled user list
6d56f35 
When disabled users where moved to their own endpoint we overlooked
 search, so adding it back.
The search is done case-insensitive in uid and display name.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
fix: Also search in email field for disabled users
8044edf 
To match what is done for Database backend for enabled users

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the feat/add-back-search-in-disabled-users branch from bfde1f7 to 0505a1f Compare May 23, 2024 07:10
@come-nc
Copy link
Contributor Author

come-nc commented May 23, 2024

/compile /

come-nc and others added 2 commits May 23, 2024 10:42
@come-nc
fix: Update openapi spec for provisioning_api app
96b6662 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@nextcloud-command @come-nc
chore(assets): Recompile assets
9a523e9 
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@come-nc come-nc force-pushed the feat/add-back-search-in-disabled-users branch from e5c0dbb to 9a523e9 Compare May 23, 2024 08:42
@come-nc come-nc merged commit 2fa099a into master May 23, 2024
155 checks passed
@come-nc come-nc deleted the feat/add-back-search-in-disabled-users branch May 23, 2024 12:51
@come-nc
Copy link
Contributor Author

come-nc commented May 23, 2024

/backport to stable29

@come-nc
Copy link
Contributor Author

come-nc commented May 23, 2024

/backport to stable28

@backportbot backportbot bot mentioned this pull request May 23, 2024
Merged
2 tasks
@backportbot backportbot bot mentioned this pull request May 23, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@provokateurin provokateurin provokateurin left review comments

@Altahrim Altahrim Altahrim approved these changes

@artonge artonge artonge approved these changes

@Pytal Pytal Pytal approved these changes

Assignees

@come-nc come-nc

Labels
3. to review Waiting for reviews feature: users and groups
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.

[Bug]: Search in the"disabled users" section does not work
6 participants
@come-nc @provokateurin @Altahrim @artonge @Pytal @nextcloud-command