Skip to content

reenable medial search, default to true #28032

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
Merged
PVince81 merged 2 commits into from
May 31, 2017
Merged

reenable medial search, default to true #28032

PVince81 merged 2 commits into from
May 31, 2017

Conversation

@butonic
Copy link
Member

@butonic butonic commented May 29, 2017 •
edited
Loading

This reverts commit d9fc93c and enables medial search for users by default, as was the case for all backends except ldap. restores pre oc10.0.1 behavior.

@butonic butonic added this to the 10.0.2 milestone May 29, 2017
jvillafanez
jvillafanez reviewed May 30, 2017
View reviewed changes
lib/private/User/AccountMapper.php
->orderBy('display_name');

$allowMedialSearches = $this->config->getSystemValue('accounts.enable_medial_search', true);
if ($allowMedialSearches) {
Copy link
Member

@jvillafanez jvillafanez May 30, 2017

Choose a reason for hiding this comment

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

Move this at the beginning of the function to avoid splitting the query?

@jvillafanez
Copy link
Member

jvillafanez commented May 30, 2017

Except for my comment, the rest is fine 👍

@butonic butonic force-pushed the reenable-medial-search branch from 7c6cb5f to 54d3ef9 Compare May 30, 2017 07:31
@PVince81
Copy link
Contributor

PVince81 commented May 30, 2017

moving to 10.0.3

@PVince81 PVince81 modified the milestones: 10.0.3, 10.0.2 May 30, 2017
@butonic butonic added p1-urgent Critical issue, need to consider hotfix with just that issue green-ticket labels May 30, 2017
@PVince81
Copy link
Contributor

PVince81 commented May 30, 2017

failing tests, please fix

    Test\User\AccountMapperTest.testFindAll
    Test\User\AccountMapperTest.testFindByUserId
    Test\User\AccountMapperTest.testFindByDisplayName
    Test\User\AccountMapperTest.testFindByEmail
    Test\User\AccountMapperTest.testFindBySearchTerm
    Test\User\AccountMapperTest.testFindLimitAndOffset

@PVince81 PVince81 merged commit ab43944 into master May 31, 2017
@PVince81 PVince81 deleted the reenable-medial-search branch May 31, 2017 09:43
@PVince81
Copy link
Contributor

PVince81 commented May 31, 2017

@butonic backport to stable10 ?

@PVince81 PVince81 modified the milestones: 10.1, 10.0.3 May 31, 2017
@butonic
Copy link
Member Author

butonic commented May 31, 2017 •
edited
Loading

has been reported as a regression, so yes.

  • will submit pr after lunch.

@PVince81 PVince81 mentioned this pull request May 31, 2017
Closed
@butonic butonic mentioned this pull request May 31, 2017
Closed
@butonic butonic mentioned this pull request Jun 1, 2017
Merged
@DeepDiver1975 DeepDiver1975 modified the milestones: 10.1, development Oct 10, 2017
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jvillafanez jvillafanez jvillafanez left review comments

@PVince81 PVince81 Awaiting requested review from PVince81

@tomneedham tomneedham Awaiting requested review from tomneedham

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees

No one assigned

Labels

feature:sharing p1-urgent Critical issue, need to consider hotfix with just that issue performance regression settings:users Type:Bug

Projects

None yet

Milestone

10.0.5

Development

Successfully merging this pull request may close these issues.

5 participants

@butonic @jvillafanez @PVince81 @DeepDiver1975