Modify

Opened 13 years ago

Closed 8 years ago

#9946 closed enhancement (fixed)

make the user overview page and the user creation page separate pages

Reported by: Maarten Derickx Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: performance users admin
Cc: Trac Release: 0.12

Description

Dear AccountManagerPlugin maintainers,

Since we have quite a few users on our system it takes some time to load the user admin page (trac.ourdomain.org/admin/accounts/users to be precise). It would be nice if we would not need to wait to load a list of all users (takes between 3 to 5 minutes now) before we can add a new one using this webinterface.

Attachments (1)

t9946-r13241-1.diff (1.4 KB) - added by anonymous 12 years ago.
Patch against r13241 of the trunk.

Download all attachments as: .zip

Change History (25)

comment:1 in reply to:  description Changed 13 years ago by Ryan J Ollos

Trac Release: 0.110.12

Replying to mderickx:

Since we have quite a few users on our system it takes some time to load the user admin page (trac.ourdomain.org/admin/accounts/users to be precise).

Here is the original mailing list post about the issue. This is only an issue since upgrading to Trac 0.12, right? How many users do you have?

comment:2 Changed 13 years ago by Maarten Derickx

Yes indeed that was only since upgrading to trac 0.12 . I put more relevant information at the google groups discussion. We have slightly more then 800 accounts.

comment:3 in reply to:  2 Changed 13 years ago by Steffen Hoffmann

Keywords: performance users admin added

Replying to mderickx:

Yes indeed that was only since upgrading to trac 0.12 . I put more relevant information at the google groups discussion. We have slightly more then 800 accounts.

Do you use account locking at that system or other advanced features? Some have been added to AcctMgr lately, and despite not getting complaints by now, i.e. checking all accounts for lock condition might contribute to the lag that you notice.

I understand that your idea could help for creating new users, but I'd rather not separate the user form from that page, because it's much harder to change account information without seeing current values. We should profile this instead and improve performance as much as possible, since I'm aware of at least one other installation with that many users.

comment:4 Changed 13 years ago by Maarten Derickx

I'm not aware of us using any advanced features, we just use a password file specific for trac. The relevant config can be found at the google groups discussion.

On the profile part of things. I'm reasonably fammiliar with python but not so much with the inner workings of trac and the account manager plugin. If you give me some hints on where to start looking for the functions that are being called when the mentioned page is loaded I can maybe track down the source of the slowness myself.

comment:5 Changed 12 years ago by Ryan J Ollos

I also oppose separating out the account list and create account features into separate pages. Profiling and optimization sound like good goals, and we could also consider paginating the account list.

comment:6 Changed 12 years ago by Maarten Derickx

That would also help a lot. Another possible solution might be modifying the page in such a way that it loads the account creation form first and then the account overview list. I'm b.t.w. still willing to help with the profiling, but still have no clue where to start looking. Could someone give me the name of the function that returns the account list?

comment:7 in reply to:  6 Changed 12 years ago by Ryan J Ollos

Replying to mderickx:

Could someone give me the name of the function that returns the account list?

I'll have to take a closer look at the code, but plan to get back to you soon.

comment:8 in reply to:  6 Changed 12 years ago by Steffen Hoffmann

Replying to mderickx:

That would also help a lot. Another possible solution might be modifying the page in such a way that it loads the account creation form first and then the account overview list.

I don't know, if this would even work in all recent browsers. Better look for another solution.

I'm b.t.w. still willing to help with the profiling, but still have no clue where to start looking. Could someone give me the name of the function that returns the account list?

Great. I do really care for performance. Most of the data harvesting work is done inside acct_mgr.admin.fetch_user_data, but more in AccountManagerAdminPanel._do_users too.

Odd Simon Simonsen wrote about profiling Trac requests in his blog, including support code to help with that task, an excellent jump-start and reference: https://www.coderesort.com/u/simon/blog/2011/09/06/trac_profiling

comment:9 Changed 12 years ago by Steffen Hoffmann

(In [12515]) AccountManagerPlugin: Allow filtering of account list in user admin panel by account status, refs #9946 and #10682.

The filter implementation of Trac's query module was used as a guide. We won't care for banned accounts by default, just active ones and these marked as pending for approval, on registration time and via the email verification process, if enabled.

This could help a bit with rendering performance of the user admin panel too, i.e. if you accumulate many banned accounts over time.

comment:10 Changed 12 years ago by Steffen Hoffmann

(In [12539]) AccountManagerPlugin: Fix user admin panel forms after [12515], refs #9946 and #10682.

The main form has been broken by adding another form, the user list filter, because HTML doesn't allow nesting of forms. Obviously I've been concerned too much about visual style to do full regression testing, sorry.

Another correction: Do no longer hide all bottons, when user deletion is not supported by any password store. This would unwarrantedly disable account approval/banning and access to the account properties clean-up too.

comment:11 Changed 12 years ago by Steffen Hoffmann

(In [12606]) AccountManagerPlugin: Fix filter field names in user admin panel, refs #9946, #10682 and #10684.

This was flawed since the start with [12515] and I still didn't recognize the duplicate 'email' field in both, account property editor and new filter form when fixing their illegal nesting in [12539].

Thanks to Ryan J Ollos for notification to make me go and finally find it.

comment:12 Changed 12 years ago by Steffen Hoffmann

(In [12660]) AccountManagerPlugin: Complete filter fix for user admin panel, refs #9946, #10682 and #10684.

While removing the name clash in [12606], the same changes actually broke saving of filter values completely.

Thanks to slav0nic for confirmation of that unexpected discovery.

comment:13 Changed 12 years ago by Steffen Hoffmann

(In [12738]) AccountManagerPlugin: Move pager to user list in accounts admin panel, refs #809, #9946, #10682, #10745, #10754 and #10873.

This should fix broken display of extra-long user lists, and it makes selections from user list useful for the clean-up page as requested.

Dumped access to anonymous session attributes for the current solution, so these are no longer available for clean-up in the admin web-UI, but I'll re-implement this later on, if it will be missed too much.

comment:14 Changed 12 years ago by Steffen Hoffmann

There have been more complains from other installations, where the user list would not even load at all, caused i.e. by thousands of spam account registrations.

I'm very eager to learn, if the new pager resolves your issue with long load times of accounts users admin panel too.

comment:15 Changed 12 years ago by Steffen Hoffmann

From testing with Trac 0.11 I recognized, that placement of the "max items per page" form is pretty much less than ideal yet.

Floating it over the list is certainly not an option, so I'll try to move it above the list to save some precious space for the user list.

comment:16 Changed 12 years ago by Steffen Hoffmann

(In [12739]) AccountManagerPlugin: Tweak 'max_per_page' preferences dialog to save space for user list, refs #9946.

Hey, someone has shrunken the preferences side-panel! Done to allow full-width user listing, not only in pager mode.

comment:17 Changed 12 years ago by Steffen Hoffmann

(In [12745]) AccountManagerPlugin: CSS clean-up, getting decent presentation for Trac 0.12 too, refs #9946.

While most changes are just to unify formatting, some more are specifically done to prevent badly interfering Trac 0.12 styles from admin.css.

comment:18 Changed 12 years ago by Steffen Hoffmann

I was deliberately shocked, when testing latest trunk in production today.

At first I thought it was another IE bug, but thanks to Firebug I could trace it down to styles inherited from admin.css. I had tested with Trac 0.11 and 1.0 before, but this one apparently was a 0.12-only issue.

Another reason to hurry for next release and subsequent branching to refocus development target. I'll certainly not want to support yet another major Trac version with the same code base.

comment:19 in reply to:  16 Changed 12 years ago by Ryan J Ollos

Replying to hasienda:

(In [12739]) AccountManagerPlugin: Tweak 'max_per_page' preferences dialog to save space for user list, refs #9946.

Hey, someone has shrunken the preferences side-panel! Done to allow full-width user listing, not only in pager mode.

I like the animation. I'll keep it in mind for use elsewhere.

Some minor tweaks:

  • The behavior seems to be a bit nicer if the handlerIn executes quickly, so that the menu expands immediately on hover-over. The handlerOut is nice with the value of 1000. Expand quickly, contract slowly is the idea.
  • The Update button or its shadow is still barely visible on the 3 browsers I tested (Chromium 25, Firefox 21, Epiphany 3.4.1 on Ubuntu 12.04), but it is hidden if the contracted height is changed from 2.6em -> 2.4em.
  • Moving the mouse back-and-forth over the div causes many events to be queued, resulting in an oscillating effect. The stop method provides us with an easy way to avoid that.
  • accountmanagerplugin/trunk/acct_mgr/templates/admin_users.html

    diff --git a/accountmanagerplugin/trunk/acct_mgr/templates/admin_users.html b/accountmanagerplugin/trunk/acct_mgr/templates/admin_users.html
    index ed332eb..78ec43a 100644
    a b  
    6767        $("fieldset legend.foldable").enableFolding(true);
    6868        // Shrink max_per_page pager preferences after 1st touch
    6969        $('.holdingbox').hover(function(){
    70           $('.panel').animate({height: '6em'}, 1000);
     70          $('.panel').stop().animate({height: '6em'}, 250);
    7171        }, function(){
    72           $('.panel').animate({height: '2.6em'}, 1000);
     72          $('.panel').stop().animate({height: '2.4em'}, 1000);
    7373        });
    7474      });
    7575    /*]]>*/</script>

comment:20 Changed 12 years ago by Steffen Hoffmann

(In [13241]) AccountManagerPlugin: Polish slide animation off 'Max accounts per page' preference box in users admin panel, refs #9946 and #10682.

Thanks to Ryan J Ollos for providing the patch to this improvement as well as background information to learn more about the underlying issues.

comment:21 Changed 12 years ago by Steffen Hoffmann

I've been aware especially of the oscillating effect, but not as fluent with JS stuff.

So I'm grateful to getting your proposal, even if you comments to #10682 suggest, that the preference setting should be done below the filter section. The sliding box would vanish in effect. But as you said, this is a nice effect to keep in mind for other occasions anyway.

OTOH I still felt a bit uneasy about its probably too eye-candy look, because Trac core looks less "playful" in general. But this is likely a matter of taste. While I think of Trac's simplicity as a feature, others depreciate it as 'old-fashioned'. Bottom line: I needed it for getting a decent look of the separated filter and preference boxes in all of Trac 0.11/0.12/1.0, but will easily drop it for a cleaner look.

More important than that would have been feedback of admins with large user base, to confirm positive effects of the major changes (filter, pager) done to resolve the performance issue raised here. Seems either 'not good enough' to notice or even take the time for reporting back, or they're just happy and silent now. Nevertheless, explicit feedback is more valuable than mere absence of critics.

comment:22 in reply to:  21 Changed 12 years ago by Ryan J Ollos

Replying to hasienda:

OTOH I still felt a bit uneasy about its probably too eye-candy look, because Trac core looks less "playful" in general. But this is likely a matter of taste. While I think of Trac's simplicity as a feature, others depreciate it as 'old-fashioned'. Bottom line: I needed it for getting a decent look of the separated filter and preference boxes in all of Trac 0.11/0.12/1.0, but will easily drop it for a cleaner look.

I feel similarly. I could imagine the form being gone by version 0.6, but it is nice to be able to experiment with jQuery in the meantime. I tend to lean on the conservative side with regard to effects, but I find the hover effect you've implemented to be subtle and tasteful. I've spent a bit more time working with the feature, and have a few more fixes to provide.

The patch that follows addresses a few more issues:

  • The div would shrink even when either input element had focus. Instead, keep the div expanded when an input element has focus, and fire an event to allow it to shrink when the input element loses focus.
  • The div would not expand when tabbing to the input element. Expand the div when the input element has focus.
  • When tabbing past the input element to the Update button, the form would not expand and the input element would move out of view and the Update button would come into view.

Changed 12 years ago by anonymous

Attachment: t9946-r13241-1.diff added

Patch against r13241 of the trunk.

comment:23 Changed 12 years ago by Ryan J Ollos

There's a jQuery < 1.6 compatibility function in the patch, taken from here. I've tested with Trac 0.11.0 and 1.0.2dev.

comment:24 Changed 8 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.