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

Improve user share suggestions #23265

Closed
wants to merge 4 commits into from
Closed

Improve user share suggestions #23265

wants to merge 4 commits into from

Conversation

kinimodmeyer
Copy link

@kinimodmeyer kinimodmeyer commented Oct 7, 2020

Fixes #1963

@jancborchardt wrote the best way to start is to pick a issues with label "good first issue".
that´s why i just choosed this issue here from the list. ⛏️

  • with this change you find user "Dominik Meyer" also when you search for "Dom Me" in the share dialog
  • with this change you find user "Dominik Meyer" also when you enter his local share "uid@domain:port"

this is my first contribution so be friendly 😺

PS next to my code contribution is saw this code-comment:

// sqlite doesn't like re-using a single named parameter here

i not understand this comment exactly, what is not possible with sqlite?
my change works in sqlite even tho i reuse the same column (uid), but i guess this comment means something else.

PPS i personally would love 🥰 to find the user if you start with typing the lastname like "Meyer Dominik". but this is out of scope here i guess.

@kinimodmeyer kinimodmeyer changed the title Improve user share suggestions #1963 Improve user share suggestions Oct 7, 2020
Dominik Meyer added 3 commits October 8, 2020 09:16
… as well

keep whitespace for more excat search.

Signed-off-by: Dominik Meyer <kinimodmeyer@gmail.com>
Signed-off-by: Dominik Meyer <kinimodmeyer@gmail.com>
Signed-off-by: Dominik Meyer <kinimodmeyer@gmail.com>
@kinimodmeyer
Copy link
Author

kinimodmeyer commented Oct 11, 2020

"continuous-integration/drone/pr" failed but i don´t see the reason in https://drone.nextcloud.com/nextcloud/server/33829/14/5 ? can someone give me a hint or is it a pipeline issue? how is the process overall here now. @nickvergessen @schiessle

@nickvergessen
Copy link
Member

It had a timeout, it's okay to ignore it.

Comment on lines +281 to +289
$serverAbsolutePath = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
$serverFederationPath = rtrim(\OC\Share\Share::removeProtocolFromUrl($serverAbsolutePath), '/');

/*
* the given search looks like a local federation.
*/
if (mb_strpos($search, '@'.$serverFederationPath) !== false) {
$query->orWhere($query->expr()->eq('uid', $query->createPositionalParameter(explode('@', $search)[0])));
}
Copy link
Member

Choose a reason for hiding this comment

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

This should not be here from my pov.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for your review @nickvergessen
can you give me a little more information what you mean with this?
do you mean it should be in another class/method and somehow gets injected there?

Copy link
Member

Choose a reason for hiding this comment

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

Well "federation a like" results are returned by other search plugins?

lib/private/User/Database.php Outdated Show resolved Hide resolved
Signed-off-by: Dominik Meyer <kinimodmeyer@gmail.com>
@kinimodmeyer kinimodmeyer closed this by deleting the head repository Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve user share suggestions
3 participants