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 in:users as a search filter to limit searches to users #40413

Merged
merged 7 commits into from
Oct 12, 2023

Conversation

sorbaugh
Copy link
Contributor

@sorbaugh sorbaugh commented Sep 14, 2023

Summary

Add in:users filter to search bar via SearchProvider to have a mechanism to limit the search load in settings/users.

TODO

  • Add new UserSearch Provider
  • Trim search query before executing actual search

Checklist

apps/settings/lib/Search/UserSearch.php Outdated Show resolved Hide resolved
apps/settings/lib/Search/UserSearch.php Outdated Show resolved Hide resolved
@AndyScherzinger AndyScherzinger changed the title Feature/sorbaugh/in user search Add in:users as a search filter to limit searches to users Sep 14, 2023
@AndyScherzinger AndyScherzinger added this to the Nextcloud 28 milestone Sep 14, 2023
@AndyScherzinger
Copy link
Member

/backport to stable27

@AndyScherzinger
Copy link
Member

/backport to stable26

@AndyScherzinger
Copy link
Member

/backport to stable25

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Tested and the code looks good.
Still need to address the copyright comment.

Enjoy the rebase !

@AndyScherzinger
Copy link
Member

/compile amend /

Comment on lines 66 to 70
$this->settingsManager = $settingsManager;
$this->groupManager = $groupManager;
$this->urlGenerator = $urlGenerator;
$this->userManager = $userManager;
$this->accountManager = $accountManager;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I am overseeing something, but they are all not being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned this one up, thanks for pointing it out 👍


return SearchResult::complete(
$this->l->t('Users'),
[]
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand how it works correctly, that this search plugin intentionally returns an empty result set? For users page it make sense as results are shown on the content and not in the pull down. Maybe I am misinterpreting how that search works though.

The naming would block potential valid future searches for users, although there is the contacts menu for this. It's acceptable, but want to point it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree this is somewhat strange. ATM the main goal is for a customer to more efficiently search for contacts without triggering everything. You correctly pointed out that the goal is to not have a list of users shown in the pulldown. See commit where how this used to be solved (and how we want to revisit in the future)

54a21d4

Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

Just the copyright info ;)

@sorbaugh sorbaugh force-pushed the feature/sorbaugh/in-user-search branch 2 times, most recently from 3194f6b to f712098 Compare October 11, 2023 14:02
@blizzz
Copy link
Member

blizzz commented Oct 11, 2023

/compile /

sorbaugh and others added 6 commits October 12, 2023 08:56
Add regex to prevent filter collision in user-list page

Usage:
1. Type a string in the search bar
2. Add in:users to display only users in the search result
…chlist dialog. Will revisit in future search result list in future issue.

Usage:
1. Type a string in the search bar
2. Add in:users filter to avoid unnecessary searches in other apps
Usage:
1. Type a string in the search bar
2. Add in:users filter to avoid unnecessary searches in other apps
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Signed-off-by: sorbaugh <stephan.orbaugh@nextcloud.com>
@AndyScherzinger
Copy link
Member

/compile /

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nickvergessen nickvergessen merged commit 62eafdb into master Oct 12, 2023
40 checks passed
@nickvergessen nickvergessen deleted the feature/sorbaugh/in-user-search branch October 12, 2023 08:29
@welcome
Copy link

welcome bot commented Oct 12, 2023

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@nickvergessen
Copy link
Member

/backport ab81cd1,65c70d7cc6425af0bf202d70c67eabd579d913a6,c38bafba9e13e4e735ed2d6698b6c4f5d9efc7d5,a3a599855e20b60627ac4e65fed8fdfa6b2c04f8,f66e4ee07246b285f79173c300de943b3a857bad,bdf0fe6a03beccc58a119f05d9cdb7ab643ebcfd to stable27

@blizzz
Copy link
Member

blizzz commented Oct 12, 2023

Backportbot seems to have day off :-/ fastest to do it manually it seems

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

1 similar comment
@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

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

Successfully merging this pull request may close these issues.

Add in:users as a search filter to limit searches to users
7 participants