Only in case of $currentAccess the array uses the user id as index #7327
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Joas Schilling <coding@schilljs.com>
Otherwise its a normal string[] with the user ids, in that case the array_merge did it's job just fine, apart from it not being deduplicated. The array+array is only needed when the user id is the key, so integer only user ids are kept as they are instead of being reindexed. Regression from 3820d68 Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
commented
Nov 28, 2017
| @@ -3021,7 +3132,7 @@ public function testGetAccessList() { | |||
|
|
|||
| $this->assertSame($expected['public'], $result['public']); | |||
| $this->assertSame($expected['remote'], $result['remote']); | |||
| $this->assertSame(array_values($expected['users']), array_values($result['users'])); | |||
There was a problem hiding this comment.
This hide the integer user id problem before, because we didn't even look at the user id (key) anymore
schiessle
approved these changes
Nov 28, 2017
blizzz
approved these changes
Nov 28, 2017
blizzz
deleted the
bugfix/7325/access-list-regression-for-not-current-accesss
branch
Codecov Report
@@ Coverage Diff @@
## master #7327 +/- ##
============================================
+ Coverage 50.86% 50.86% +<.01%
- Complexity 24538 24539 +1
============================================
Files 1584 1584
Lines 93794 93798 +4
Branches 1358 1358
============================================
+ Hits 47705 47709 +4
Misses 46089 46089
|
|
@nickvergessen could you create the backport PR? |
|
One should be enough? |
Was not shown on mobile view :/ Sorry |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Regression from 3820d68
Before 3820d68 the method worked for
!$currentAccess, afterwards for$currentAccess, now it works for both and I added the relevant unit tests before.Fix #7325