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

Also array_slice wide email matches of local users #8596

Merged
merged 2 commits into from
Mar 6, 2018
Merged

Also array_slice wide email matches of local users #8596

merged 2 commits into from
Mar 6, 2018

Conversation

nickvergessen
Copy link
Member

Fix #8454

@nickvergessen
Also array_slice wide email matches of local users
12420ae 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added this to the Nextcloud 14 milestone Mar 1, 2018
juliusknorr
juliusknorr approved these changes Mar 1, 2018
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Works as expected now 👍 Thanks @nickvergessen

nickvergessen
nickvergessen commented Mar 1, 2018
View reviewed changes
lib/private/Collaboration/Collaborators/MailPlugin.php
@@ -175,10 +174,13 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {

if (!$this->shareeEnumeration) {
$result['wide'] = [];
$userResults['wide'] = [];
Copy link
Member Author

Choose a reason for hiding this comment

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

PS: also enumeration was possible before, pretty bad...

rullzer
rullzer approved these changes Mar 1, 2018
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Yes please!

@rullzer
Copy link
Member

rullzer commented Mar 1, 2018

Ah failing tests

@nickvergessen
Add the users result only when non empty so tests stay the same
e05f338 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@codecov
Copy link

codecov bot commented Mar 2, 2018

Codecov Report

Merging #8596 into master will decrease coverage by 16.9%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #8596       +/-   ##
=============================================
- Coverage     51.93%   35.02%   -16.91%     
  Complexity    25418    25418               
=============================================
  Files          1609     1609               
  Lines         95329    95331        +2     
  Branches       1378     1378               
=============================================
- Hits          49506    33389    -16117     
- Misses        45823    61942    +16119
Impacted Files Coverage Δ Complexity Δ
...private/Collaboration/Collaborators/MailPlugin.php 0% <0%> (-73.2%) 27 <0> (ø)
apps/systemtags/lib/Settings/Admin.php 0% <0%> (-100%) 3% <0%> (ø)
...lib/Middleware/Exceptions/NotSubAdminException.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/BackgroundJob/QueuedJob.php 0% <0%> (-100%) 1% <0%> (ø)
...mments/tests/Unit/Controller/NotificationsTest.php 0% <0%> (-100%) 4% <0%> (ø)
...ty/Exceptions/CrossSiteRequestForgeryException.php 0% <0%> (-100%) 1% <0%> (ø)
...te/Authentication/Token/DefaultTokenCleanupJob.php 0% <0%> (-100%) 1% <0%> (ø)
...te/Authentication/LoginCredentials/Credentials.php 0% <0%> (-100%) 4% <0%> (ø)
lib/private/Hooks/LegacyEmitter.php 0% <0%> (-100%) 1% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Filter/Calendar.php 0% <0%> (-100%) 7% <0%> (ø)
... and 529 more

@nickvergessen
Copy link
Member Author

Fixed

@blizzz
Copy link
Member

blizzz commented Mar 2, 2018

test case for it possible?

@MorrisJobke MorrisJobke merged commit 65cc155 into master Mar 6, 2018
@MorrisJobke MorrisJobke deleted the bugfix/8454/sharee-email-matches-not-limited branch March 6, 2018 21:21
@MorrisJobke
Copy link
Member

test case for it possible?

Still pending and should be done together with the Backport.

@nickvergessen nickvergessen mentioned this pull request Mar 7, 2018
Merged
@nickvergessen
Copy link
Member Author

Tests in #8710

@MorrisJobke MorrisJobke mentioned this pull request Mar 9, 2018
Merged
10 tasks
@nickvergessen nickvergessen mentioned this pull request Mar 9, 2018
Merged
@nickvergessen
Copy link
Member Author

Backport in #8749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rullzer rullzer rullzer approved these changes

@juliusknorr juliusknorr juliusknorr approved these changes

@blizzz blizzz Awaiting requested review from blizzz

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.
5 participants
@nickvergessen @rullzer @blizzz @MorrisJobke @juliusknorr