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 profile url search #8007

Merged
merged 4 commits into from Dec 27, 2019

Conversation

@MrPetovan
Copy link
Collaborator

MrPetovan commented Dec 24, 2019

Closes #7984

This PR adds the profile URL search like we do with post URLs: it redirects to the contact page if it found one at the URL.

I separated the anonymous search from the logged in search to avoid contact probes by anonymous users. @annando can you please check to make sure I used the right Model\Contact method for this purpose?

Corollary question: Should I separate the post URL search between anonymous and logged in users?

Or third option, should those searches be logged-users only?

@MrPetovan MrPetovan added this to the 2020.03 milestone Dec 24, 2019
@MrPetovan MrPetovan requested a review from nupplaphil Dec 24, 2019
@annando

This comment has been minimized.

Copy link
Collaborator

annando commented Dec 25, 2019

From an UX standpoint I suggest that both searches - this new one and the existing one with @ should behave identically.

@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 25, 2019

Unfortunately it can't. The @ and ! local search partially matches the url, name, location, addr, about and keywords fields in the gcontact table and as a result is not unique. The global search hits the global directory endpoint /search which can return multiple results as well depending on the input.

@annando

This comment has been minimized.

Copy link
Collaborator

annando commented Dec 25, 2019

I see the complicity of the situation. But I do see the user experience as well. Because of this I would like to have the same user experience for a single contact for both ways of searching.

@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 25, 2019

I disagree, Entering a post URL or a profile URL means you want the related uniquely identified object to be available on your local node, while a free text search may yield multiple results and needs a different behavior with a result list.

@annando

This comment has been minimized.

Copy link
Collaborator

annando commented Dec 25, 2019

Have a look at this code: https://github.com/friendica/friendica/blob/develop/src/Module/BaseSearchModule.php#L56-L58

The function Search::getContactsFromProbe is called only in the special case when the search pattern looks like a complete contact.

So I would vote for either call Search::getContactsFromProbe in any case (even the above one). Or in the case of a search pattern like @user@host.tld or @http://host.tld/.../user we should call the new functionality.

@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 25, 2019

I'd rather go for the latter. The performContactSearch is done when we know we have a contact-like query (starting with @ or !) but a given URL may not be a profile URL.

Additionally, can you please answer my question about anonymous users having access to the profile and post URL search?

@annando

This comment has been minimized.

Copy link
Collaborator

annando commented Dec 25, 2019

I have to have a look at this anonymous search. But this will be done this evening.

@MrPetovan MrPetovan force-pushed the MrPetovan:task/7984-add-profile-url-search branch from 3d8f510 to ca31684 Dec 25, 2019
@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 25, 2019

I moved both profile and post URL searches to their own methods for clarity. The profile URL search now includes webfinger addresses as well. I maintained for both methods a logged-in version with probing and an anonymous version without.

private static function tryRedirectToProfile(BaseURL $baseURL, string $search)
{
if (local_user() && preg_match('/^@?([a-z0-9.-_]+@[a-z0-9.-_:]+)$/i', trim($search), $matches)) {
$contact = Contact::getDetailsByAddr($matches[1]);

This comment has been minimized.

Copy link
@annando

annando Dec 25, 2019

Collaborator

You can use Contact::getIdForURL for addresses as well.

@MrPetovan MrPetovan force-pushed the MrPetovan:task/7984-add-profile-url-search branch from ca31684 to 20d48d6 Dec 27, 2019
@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 27, 2019

Reworked again the method, now it also looks up webfinger addresses for anonymous users.

*/
private static function tryRedirectToPost(BaseURL $baseURL, string $search)
{
if (parse_url($search, PHP_URL_SCHEME) != '') {

This comment has been minimized.

Copy link
@annando

annando Dec 27, 2019

Collaborator

I would prefer exiting with return at the top of the function when the check isn't successful. I dislike large if constructs that reach through the whole function.

This comment has been minimized.

Copy link
@MrPetovan

MrPetovan Dec 27, 2019

Author Collaborator

Fair enough.

- Move post URL search to private method in Module\Search\Index
@MrPetovan MrPetovan force-pushed the MrPetovan:task/7984-add-profile-url-search branch from 20d48d6 to a11b47f Dec 27, 2019
@MrPetovan

This comment has been minimized.

Copy link
Collaborator Author

MrPetovan commented Dec 27, 2019

One less indentation level!

@annando annando merged commit c7e4157 into friendica:develop Dec 27, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/drone/pr Build is pending
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@MrPetovan MrPetovan deleted the MrPetovan:task/7984-add-profile-url-search branch Dec 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.