-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
shareAutocompletion feature updated for not listing the user in autocomplete list in webui #35133
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.
|
And the user "Four" should not be listed in the autocomplete list on the webUI | ||
|
||
@skipOnLDAP | ||
Scenario: autocompletion of already existing user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more explanation please
And the user "Four" should not be listed in the autocomplete list on the webUI | ||
|
||
@skipOnLDAP | ||
Scenario: autocompletion of already existing user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need more explanation, what is the difference to the last scenario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Some small changes needed.
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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message should tell what the current state is, not what needs to be.
Below is just a basic suggestion, needs to be improved.
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."); |
@skshetry @paurakhsharma @phil-davis can you please provide review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/somewhere in the last/at the end
…autocomplete list in webUI
backport on #35152 |
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: