Admin: Users page: function call wrong arguments #26234

Closed
LeonardoRM opened this Issue Sep 27, 2016 · 5 comments

Projects

None yet

3 participants

@LeonardoRM

Steps to reproduce

  1. As admin, go to users page.
  2. May take a long time to process the initialUsersToLoad.

Expected behaviour

Load in a reasonable time.

Actual behaviour

Makes browser to show the 'slow script' warning.

Server configuration

Operating system: Debian jessie

Web server: apache2-2.4.10

Database: mariadb-server-10.0.23

PHP version: 5.6

ownCloud version: 9.0.0 (but code provided is the same in master)

Are you using an external user-backend, if yes which one: IMAP

Client configuration

Browser: Firefox 49

Operating system: kubuntu 14.04

Hello guys!
If you see this file, there's a function call with arguments that I think don't match and are interpreted incorrectly. The file is settings/js/users/users.js

L399:

var $tr = UserList.add(user, user.lastLogin, false, user.backend);

L52:

add: function (user, sort) {

L174:

if (sort) {
    UserList.doSort();
}

So, this is what I think is happening:
L399 calls the function add with 2nd to 4th parameters not corresponding with the ones add expects. 2nd and 4th arguments are already included in user object, so there's no need for those. And 3rd argument (false) seems to be the one expected as sort in definition of function add.

L174, within function add, checks sort which is an integer (corresponding to user.lastLogin) and the if condition goes true, doing the sort every time there's a call to add.

I did a quick workaround changing L174 with this:
if (sort===true) {
to prevent taking an integer as true.

Does it make any sense?
Thanks!

@DeepDiver1975 DeepDiver1975 added this to the 9.2 milestone Sep 29, 2016
@DeepDiver1975
Member

@PVince81 @butonic mind taking a look? THX

@PVince81
Collaborator
PVince81 commented Oct 4, 2016

Hmmmm nice catch. I do remember seeing that the sort was done over and over again I thought it was expected behavior but never noticed this arguments discrepancy.

That's good, so this means there is potential to make this much faster. I'll have a look.

@PVince81
Collaborator
PVince81 commented Oct 5, 2016

Indeed, this is wrong. Let's do some git archeology to find out how it happened in the first place.

@PVince81
Collaborator
PVince81 commented Oct 5, 2016

Looks like it happened a long time ago in #12921 where the function signature was changed, but the call wasn't changed properly.

Will fix.

@PVince81
Collaborator
PVince81 commented Oct 5, 2016

Fix is here #26282

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