[dev] Registry_Application's prefs methods flawed?

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> Quoting Jan Schneider <jan at horde.org>:
>
>> Zitat von Michael M Slusarz <slusarz at horde.org>:
>>
>>> Quoting Jan Schneider <jan at horde.org>:
>>>
>>>> While fixing some issue with populating preference values in  
>>>> Kronolith, I noticed that the prefs methods in the  
>>>> Horde_Registry_Application sub-classes uses pref groups  
>>>> ($ui->group) to determine which preference to manipulate  
>>>> ($ui->override). This seems flawed because the individual prefs  
>>>> can be assigned different groups in the config/prefs.php  
>>>> configuration files.
>>>
>>> This is expected, and it is a compromise.  It is not possible to  
>>> move things around between pref groups easily at this time because  
>>> the conditional logic is hardcoded in the App_Registry_Application  
>>> file, as you discovered.  This was a compromise since the  
>>> alternative would be to have all of this conditional code living  
>>> in prefs.php instead, or (more likely) having each individual  
>>> preference be overloaded with configuration options.  Or, even  
>>> more extreme, have each preference live in a separate  
>>> class/object.  Unfortunately, prefs are one of those things that  
>>> need to be exposed to ALL admins so we have to keep the API  
>>> somewhat workable.
>>>
>>> IMHO, we really shouldn't be having prefs appear on multiple  
>>> screens anyway.  It just adds to the prefs clutter and is  
>>> duplicative and confusing.  If its one of those things where one  
>>> pref might control prefs living in a different group, then we are  
>>> probably Doing It Wrong; these prefs should logically be broken  
>>> into and a new, separate group.  I would rather see usage of  
>>> linkage between groups pages via links rather than a duplicated  
>>> pref entry.  And some prefs simply can't be split up (identity  
>>> prefs) because they rely on other elements in the page  
>>> (javascript; specific handling based on the group name).
>>>
>>> Conversely, if you really do want to have prefs appear in multiple  
>>> groups, then you can simply add the necessary code to deal with  
>>> that individual pref in the App_Registry_Application methods.  The  
>>> valid list of prefs for the current group can always be obtained  
>>> via:
>>> $ui->getChangeablePrefs($ui->group)
>>
>> I wasn't actually referring to having prefs in several groups,  
>> though this obviously experience the same problems I described. I  
>> referred to the admin's capability to assign a pref to a different  
>> group than originally intended.
>>
>> Regarding identities the idea was (and I actually thought this  
>> already happened during one of the pref rewrites) that the identity  
>> stuff is flexible enough to theoretically allow adding and removing  
>> any prefs to/from identities. Of course this would still require  
>> some code that actually uses the identity to receive a pref value,  
>> instead of retrieving it from the prefs object directly. But this  
>> could be a custom application for example.
>
> This is still the case.  What I am saying is that identities prefs  
> MUST live in the identities group, because they are handled  
> differently by the Prefs UI handling code.  e.g. The identities page  
> automatically inserts the identity switching JS code into the page -  
> this would get VERY complicated/messy if you would allow identities  
> prefs to appear in any group.

Yes, this absolutely makes sense.

>> Back to grouping prefs: I think it makes a lot of sense to have  
>> some preferences in multiple groups. It was actually me who did  
>> this in the first place because some users are expecting prefs in a  
>> different group than others. Beside that, it simply makes sense to  
>> access certain prefs from different places. Most prominent and  
>> obvious examples are the special folders. It makes perfectly sense  
>> to have a group where all special folders can be managed, while  
>> still being able to set the trash folder from the group for  
>> deleting messages.
>
> I don't necessarily disagree with you, but the other solution (that  
> makes more sense to me, btw) is to move each mailbox pref into a  
> different group.  Thus, all preference options dealing with the  
> trash folder (trash mailbox location, use virtual trash?, login  
> tasks to purge the trash mailbox) should appear in a single Trash  
> preference group.

That's already the case, isn't it? They are preferences of the message  
deletion group. This is no contradiction to what I said though, or am  
I missing something? Do you want this pref to *only* be in the  
trash/deletion group?

>> From the 3 possible approaches you outlined in your first  
>> paragraph, the second one sounds like the one we want to use, I  
>> don't really understand why you don't deem that one appropriate.  
>> After all this happens in lib/Application.php which is not a place  
>> for admin's to go, and whether we identify special handling for  
>> prefs by pref group or pref name doesn't seem to make much of a  
>> difference.
>
> I think you are referring to "conditional code living in prefs.php  
> instead, or (more likely) having each individual preference be  
> overloaded with configuration options."  And I'm not sure how that  
> would work(?)  We should never be conditionally excluding individual  
> prefs entries with if () blocks.

Not in config/prefs.php, but in Application.php. We actually do this,  
but embedded in a group-switch-statement, instead of a  
pref-switch-statement.

> And for various prefs/groups, we don't know whether to show/hide  
> prefs until AFTER we parse the prefs.php file.  So that's not going  
> to work unless we start doing something like defining functions to  
> run post-parsing.  And all this does is completely clutter up the  
> preferences file.  My intent for prefs.php is to keep it as simple  
> as possible - I am for any solution to allow this over the ability  
> to let an admin switch pref groups (which few admins will ever do).

I think we misunderstood each other. I don't want the old prefs.php  
files back with all this embedded logic. I only want to change the  
prefs methods in Application.php to trigger pref-specific behavior  
based on prefs, not on groups.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/