Tickets :: [#9219] display_tasklists is saved on every page access

6.0.0-beta1

7/5/25

Summary	display_tasklists is saved on every page access
Queue	Nag
Queue Version	Git master
Type	Enhancement
State	Rejected
Priority	1. Low
Owners
Requester	slusarz (at) horde (dot) org
Created	09/03/2010 (5419 days ago)
Due
Updated	09/23/2010 (5399 days ago)
Assigned	09/22/2010 (5400 days ago)
Resolved	09/23/2010 (5399 days ago)
Milestone
Patch	No

09/23/2010 07:07:51 PM	Michael Slusarz	Comment #6 State ⇒ Rejected	Reply to this comment
Seeing this all over the place: 2010-09-22T00:39:08-06:00 DEBUG: HORDE [nag] Horde_Prefs: Storing preference value (display_tasklists) [pid 30516 on line 257 of "/disk2/src/horde/framework/Prefs/lib/Horde/Prefs.php"] Move it further down into the if-clause where the saving is actually triggered. :) Done. As someone who never uses nag, I have no idea why this needs/should be updated every page load. Even if the value isn't changing, we shouldn't be wasting time recreating at all. At a minimum, this really needs to be cached (see, e.g., caching IMP's folder tree using session caching as a fallback if no Cache system is available). It's almost a no-op, so I wouldn't really care about that. And you probably see this because of the Copy/Move to Tasklist feature. OK. I'll close this ticket.

09/23/2010 07:06:31 PM	Git Commit	Comment #5	Reply to this comment
Changes have been made in Git for this ticket: ~~Request #9219~~: Move pref saving logging to a more appropriate location http://git.horde.org/diff.php/framework/Prefs/lib/Horde/Prefs.php?rt=horde-git&r1=77e3dd782f525dadbb88512854d903ac2d7f9cb9&r2=b4a73fd1a465e79c3be80e12e225f7cc6c268dcd

09/22/2010 11:54:37 PM	Jan Schneider	Comment #4	Reply to this comment
Seeing this all over the place: 2010-09-22T00:39:08-06:00 DEBUG: HORDE [nag] Horde_Prefs: Storing preference value (display_tasklists) [pid 30516 on line 257 of "/disk2/src/horde/framework/Prefs/lib/Horde/Prefs.php"] Move it further down into the if-clause where the saving is actually triggered. :) As someone who never uses nag, I have no idea why this needs/should be updated every page load. Even if the value isn't changing, we shouldn't be wasting time recreating at all. At a minimum, this really needs to be cached (see, e.g., caching IMP's folder tree using session caching as a fallback if no Cache system is available). It's almost a no-op, so I wouldn't really care about that. And you probably see this because of the Copy/Move to Tasklist feature.

09/22/2010 06:45:33 AM	Michael Slusarz	Comment #3 State ⇒ Assigned	Reply to this comment
Seeing this all over the place: 2010-09-22T00:39:08-06:00 DEBUG: HORDE [nag] Horde_Prefs: Storing preference value (display_tasklists) [pid 30516 on line 257 of "/disk2/src/horde/framework/Prefs/lib/Horde/Prefs.php"] As someone who never uses nag, I have no idea why this needs/should be updated every page load. Even if the value isn't changing, we shouldn't be wasting time recreating at all. At a minimum, this really needs to be cached (see, e.g., caching IMP's folder tree using session caching as a fallback if no Cache system is available).

09/21/2010 04:16:01 PM	Jan Schneider	Comment #2 State ⇒ Feedback	Reply to this comment
This happens in other applications, for example Kronolith, all the time. But the prefs library is only saving it when the preference value changed. Are you saying this check doesn't work anymore, or is the preference value really changed?

09/03/2010 05:44:45 PM	Michael Slusarz	Comment #1 Priority ⇒ 1. Low Type ⇒ Enhancement Summary ⇒ display_tasklists is saved on every page access Queue ⇒ Nag Milestone ⇒ Patch ⇒ No State ⇒ New	Reply to this comment
The code in Nag#initialize() saves the value of display_tasklists every time it is run. Should be refactored to only save if tasklist changes. Noticed this in my logs when doing a dimp poll refresh - for some reason, display_tasklists was being updated. Not familiar with the code but making the assumption that display_tasklists doesn't change on a regular basis. (If it does, it should not be a preference at all).