Skip to content

Sharing dialog: make autocomplete sorting case insensitive#3889

Merged
MorrisJobke merged 2 commits into
masterfrom
downstream-26950
Mar 24, 2017
Merged

Sharing dialog: make autocomplete sorting case insensitive#3889
MorrisJobke merged 2 commits into
masterfrom
downstream-26950

Conversation

@MorrisJobke

Copy link
Copy Markdown
Member

cc @nextcloud/javascript @nextcloud/sharing for review

@mention-bot

Copy link
Copy Markdown

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @blizzz, @rullzer and @tomneedham to be potential reviewers.

@MorrisJobke MorrisJobke requested a review from schiessle March 17, 2017 00:38
@MorrisJobke

Copy link
Copy Markdown
Member Author

I tested this and it works 👍

Sharing dialog: Names sorting is case sensitive: adding tests #25971

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Comment thread core/js/sharedialogview.js Outdated

if (suggestions.length > 0) {
suggestions.sort(function (a, b) {
return OC.Util.naturalSortCompare(a.label, b.label);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is wrong, the list is sorted on php, when that behaves incorrect we need to fix it there. Otherwise each page is sorted correctly, while the results should be on different pages.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My suggestion would be:

diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php
index a281572ad5..dfbd995fb2 100644
--- a/lib/private/User/Database.php
+++ b/lib/private/User/Database.php
@@ -193,7 +193,7 @@ class Database extends Backend implements IUserBackend {
 
                $displayNames = array();
                $query = \OC_DB::prepare('SELECT `uid`, `displayname` FROM `*PREFIX*users`'
-                       . $searchLike .' ORDER BY `uid` ASC', $limit, $offset);
+                       . $searchLike .' ORDER BY LOWER(`displayname`), LOWER(`uid`) ASC', $limit, $offset);
                $result = $query->execute($parameters);
                while ($row = $result->fetchRow()) {
                        $displayNames[$row['uid']] = $row['displayname'];
@@ -279,7 +279,7 @@ class Database extends Backend implements IUserBackend {
                        $searchLike = ' WHERE LOWER(`uid`) LIKE LOWER(?)';
                }
 
-               $query = \OC_DB::prepare('SELECT `uid` FROM `*PREFIX*users`' . $searchLike . ' ORDER BY `uid` ASC', $limit, $offset);
+               $query = \OC_DB::prepare('SELECT `uid` FROM `*PREFIX*users`' . $searchLike . ' ORDER BY LOWER(`uid`) ASC', $limit, $offset);
                $result = $query->execute($parameters);
                $users = array();
                while ($row = $result->fetchRow()) {
diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
index 3d016700ee..77741efcc7 100644
--- a/lib/private/User/Manager.php
+++ b/lib/private/User/Manager.php
@@ -254,7 +254,7 @@ class Manager extends PublicEmitter implements IUserManager {
                         * @var \OC\User\User $a
                         * @var \OC\User\User $b
                         */
-                       return strcmp($a->getDisplayName(), $b->getDisplayName());
+                       return strcmp(strtolower($a->getDisplayName()), strtolower($b->getDisplayName()));
                });
                return $users;
        }

@MorrisJobke

Copy link
Copy Markdown
Member Author

@nickvergessen I fixed it according to your recommendation. Only drawback with that: Groups are not sorted within the users list, but they are just concatenated at the end of the list.

@nickvergessen

Copy link
Copy Markdown
Member

Yeah, but thats intentional I think

"shareWith": "Petra"
}
}
])).toEqual(true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Test fails with

PhantomJS 2.1.1 (Linux 0.0.0) OC.Share.ShareDialogView autocompletion of users is sorted naturally FAILED
	Expected false to equal true.
core/js/tests/specs/sharedialogviewSpec.js:565:15

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed 🙈

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 23, 2017
@MorrisJobke MorrisJobke merged commit d197f60 into master Mar 24, 2017
@MorrisJobke MorrisJobke deleted the downstream-26950 branch March 24, 2017 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish feature: sharing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants