shareAutocompletion feature updated for not listing the user in autocomplete list in webui#35133
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #35133 +/- ##
============================================
- Coverage 65.47% 65.47% -0.01%
Complexity 18635 18635
============================================
Files 1217 1217
Lines 70543 70543
Branches 1296 1296
============================================
- Hits 46191 46190 -1
- Misses 23975 23976 +1
Partials 377 377
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #35133 +/- ##
============================================
+ Coverage 65.48% 65.49% +<.01%
Complexity 18636 18636
============================================
Files 1217 1217
Lines 70536 70536
Branches 1296 1296
============================================
+ Hits 46191 46195 +4
+ Misses 23968 23964 -4
Partials 377 377
Continue to review full report at Codecov.
|
individual-it
requested changes
May 2, 2019
| And the user "Four" should not be listed in the autocomplete list on the webUI | ||
|
|
||
| @skipOnLDAP | ||
| Scenario: autocompletion of already existing user |
| And the user "Four" should not be listed in the autocomplete list on the webUI | ||
|
|
||
| @skipOnLDAP | ||
| Scenario: autocompletion of already existing user |
Member
There was a problem hiding this comment.
need more explanation, what is the difference to the last scenario
paurakhsharma
left a comment
paurakhsharma
left a comment
Contributor
There was a problem hiding this comment.
Looks good.
Some small changes needed.
skshetry
reviewed
May 2, 2019
| public function theUserShouldNotBeListedInTheAutocompleteListOnTheWebui($username) { | ||
| $names = $this->sharingDialog->getAutocompleteItemsList(); | ||
| if (\in_array($username, $names)) { | ||
| throw new Exception("$username should not be present in the autocomplete items list"); |
Contributor
There was a problem hiding this comment.
Error message should tell what the current state is, not what needs to be.
Below is just a basic suggestion, needs to be improved.
Suggested change
| throw new Exception("$username should not be present in the autocomplete items list"); | |
| throw new Exception("$username was found in the autocomplete items list, but was not expected."); |
haribhandari07
changed the title
Contributor
Author
|
@skshetry @paurakhsharma @phil-davis can you please provide review? |
skshetry
reviewed
May 3, 2019
individual-it
requested changes
May 3, 2019
| And user "Four" should not be listed in the autocomplete list on the webUI | ||
|
|
||
| @skipOnLDAP | ||
| Scenario: autocompletion of a pattern where the name of existing users contain the pattern somewhere in the last |
Member
There was a problem hiding this comment.
s/somewhere in the last/at the end
…autocomplete list in webUI
individual-it
approved these changes
May 6, 2019
Contributor
Author
|
backport on #35152 |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
Description
ShareAutocompletion feature updated so that the users whose name doesn't contain the text typed in "share-with-field" is not listed in autocomplete list in webUI.
Related Issue
Motivation and Context
testing of medial search on group and user #34659
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks: