i have received an attachment but i cant access it, i cant open it 
through IMP as directed, what can i do?

I'm sorry, but the code needs more cleanup than I have the bandwidth 
for right now. I got some of the way through it, so I'm uploading what 
I have, but there's more, especially in the template (echo for whole 
HTML strings with gettext wrappers?).



I gave it a shot, but unfortunately there's too much else going on for 
me in Horde and professionally right now. Perhaps someone else will 
pick it up, or else it'll be picked up for IMP 5 with other stalled 
tickets.

YES.   Consider it assigned.

Are you willing to assign the copyright on this patch to the Horde Project?

That's a good question.  To be honest, I didn't write that part of the 
patch and I'd ask the guy who wrote it, but he doesn't work here 
anymore.  Apologies on my side though, because I agree that it should 
be changed.  I do see some other instances of this in the 
handle_folderselect() and handle_trashselect() functions in that same 
file, but I think I'd be inclined to make it more 
readable/trustworthy.  Something like the following would be better, 
IMO.



          // We might need to create the preference...

         $updated = FALSE;

          if (!isset($prefs->_prefs['purge_folders_list'])) {

              $updated = $updated || $prefs->add('purge_folders_list',

  serialize($arr));

          } else {

              $updated = $updated ||

  $prefs->setValue('purge_folders_list', serialize($arr))$

          }



What do you think ?

What is the bitwise or in these lines for?



         // We might need to create the preference...

        if (!isset($prefs->_prefs['purge_folders_list'])) {

             $updated = $updated | $prefs->add('purge_folders_list', 
serialize($arr));

         } else {

             $updated = $updated | 
$prefs->setValue('purge_folders_list', serialize($arr))$

         }

Not sure exactly what happened, but I just tweaked it and it applies 
to CVS head now with one minor fuzzing.   I checked the hunk that it 
complained about and it applied as it should.



Attached is the updated patch, sorry for any lost cycles.  This one 
should apply fine.

The problem isn't the patch failing to apply, it's patch not 
understanding the file (malformed patch != couldn't merge)

Took a sec and glanced at my original checkout and current CVS Head, 
looks like someone added a "define" line at the top of 
imp/lib/prefs.php that wasn't there when i made the patch a month ago. 
  That's probably causing the conflict.  Can you make the adjustment, 
or do I need to update my patch against CVS head again ?

The patch doesn't apply:



patching file config/prefs.php.dist

patching file lib/Maintenance/imp.php

patching file lib/Maintenance/Task/purge_folders.php

patch: **** malformed patch at line 167: diff --new-file -u -r 
imp-prepatch/lib/prefs.php imp/lib/prefs.php

I just need to look at it, one way or another. Been too busy with too 
many things so far, but it's in my inbox.

Poke :)   Let me know if I can assist further...

I just realized I forgot to add back in the display switch I 
mentioned.  Updated patch attached.



Brook

Alrighty, here it is.



Chuck, your comments make a lot more sense to me now that I've 
implemented them.  It was not clear to me previously that locking a 
maintenance preference also enables/disables the user's ability to 
skip the action (maybe it didn't used to be this way, I don't 
remember).  So, I've removed everything that was trying to do so, 
which made the framework patch unnecessary.



I've also removed the hook function from the horde patch (also not 
needed now), as I found a better way to deal with unset folder values. 
  So, what remains is a pretty light-weight IMP patch, which I feel is 
much more in line with the current code base.  In other words, I think 
this is a much better patch.



So, basically, you just have 3 prefs now.



'purge_folders' : checkbox which enables/disables the entire thing.

'purge_folders_max_days' : sets the max number of days before purging 
all folders.



and then there is a special and implicit pref pair, which I count as 
one thing.

'purge_folders_list_folders'

'purge_folders_list'



The main thing to understand as far as behavior is that the routine 
always assumes the max days, unless you override that value for a 
certain folder (which, if set greater than the max days, still uses 
max days).



The one area that might need some work would be the per-folder 
preferences display.  The folder list will not display when the admin 
has set the value of purge_folders to 0 and set locked to 1.  I 
thought this would make more sense than showing the user something 
that doesn't do anything anyway.  I did this because I would assume 
the admin would lock both purge_folders and purge_folders_max_days, 
but couldn't really lock the special and implicit values (I may be 
wrong here).  There is also a somewhat kludgey hack in the form of 
inline css style on the text input for the per-folder options, in 
which I reduced the size of the text in the box because it was making 
gaps in the folder image tree (which we might want to remove).



Anyway.  I hope this gets us closer.  Please let me know if I can make 
any more revisions, or if it's fine, as is.  Thank everyone for all 
your help and suggestions.



Brook

That's the expected behavior.
Yes and yes. The reason is that you might want to set different values 
for different users but still keep them from changing it.
Correct.

Okay, so finally catching back up with this.  I've got nearly 
everything completed Chuck's suggestions, just a quick question before 
I submit a patch (IMP only at this point).



I'm testing out the suggestion of letting the locked preference 
'purge_folders' dictate whether users can enable or disable folder 
purges (removing other, redundant preferences and config parameters 
that did this in the old patch).



I'm thinking of not displaying my per-folder preferences, which are 
stored an implicit preference, if purge_folders is set to 0 and 
locked.  However, when I go to check this case from within my template 
include file (using $prefs->getValue() and $prefs->isLocked()), I'm 
hitting some weird cases.



As a user, I had previously set purge_folders to 0.  Then, as an 
administrator, I set it to 1 and locked it (always run purge folders) 
in the prefs.php file.  For some reason, the prefs function returns my 
user-set value, but also that the preference is locked.



So, my question is:  Is it the responsibility of the administrator to 
clear out these preference values from the preference storage backend, 
then lock, or is there a reason why the user preference value 
overrides the one set and locked by the administrator in the prefs.php 
file ?  Is it the programmer's responsibility to wipe out old values 
after the preference is locked, or is this not advisable ?



Thanks for any clarification.  Patches today or tomorrow.

No worries.  Sounds good to me.

I'll un-stall it when there are new patches, definitely. If it's 
really bugging you that it's stalled now, though, I guess we can 
change it. I promise it won't get lost as long as there are updates, 
though.

Still working on this...  had some issues getting the newer horde 
framework up and running, but making progress now...  please remove 
stalled status.  I expect to have some patches in line with Chuck's 
comments sometime next week.



Thanks,



Brook

My belief still remains that this is a total waste of time trying to 
hack the existing Maintenance code to allow us to do something like 
this.  We should instead scrap the code and rewrite to do this cleanly 
for Horde 4.0.

Great, thanks.

Yes, I'm still available.  I apologize for the delayed comms and I'm 
glad you poked it again, as the last post somehow missed my radar.   
Let me look over these notes (they seem very reasonable/doable to me 
at first glance).  I'll follow up with any questions, especially 
regarding interfacing with the framework.  Good deal.  I'm open to any 
other additional comments or suggestions, so feel free.  I should be 
able to work this into my queue in the coming week.



Brook

Are you available to work on these patches?

If you're willing to update the patches a bit that could be great. I 
agree with you that resolving this is worthwhile, it being one of the 
oldest tickets around, plus the bounty, etc.



So, I just took another look at the patches myself, perhaps feeling a 
bit clearer. I think one of your primary issues can be resolved with a 
bit of info about the prefs system: there are no hooks on special 
preferences because they are supposed to define interface elements and 
they don't have values.



You should define an implicit preference that actually holds the 
value/s you want to store, and leave the special pref to define the UI 
for it. I know that's not completely intuitive and it needs to be 
documented, but let's stick to the topic. :) So after that hooks 
should work fine on the implicit pref.



That means that I think the framework patch can go away entirely. If 
there are issues about confirming we should address those separately 
but that doesn't apply to solving the issue of this ticket so let's 
simplify and leave it out.



The Horde patch is just for hooks.php.dist so that's fine.



In the IMP patch, I'd like to remove the allow_user_confirm_maint bit 
from the scope of this ticket, if it's still needed. 
disable_purge_folders should be enforceable by locking the preference. 
I can see the last config setting (max_days_purge_folders) since 
that's not easily representable with a locked preference. But if you 
don't need it, I'd be inclined to let it go.



I can look more closely at the rest of the IMP patch if you agree with 
the above and can work on revising the patches accordingly.



Michael, if we envision this ticket for now as only providing 
configurability for which folders are purged, was there anything else 
that concerned with just the IMP portion of the patch?

While I agree that the one example you give can be considered a hack 
(that's where I was hoping the Horde gurus could help me out).  I 
would contend that the other portions of the patch adhere to the Horde 
coding standards.  If you could cite other examples where it is the 
case that other Horde standards have been broken, I'd be open to 
adjusting the patch appropriately.
To perhaps clarify, these references are there (as the comments 
suggest) because Horde doesn't/didn't allow "special" types to have 
hooks, or at least I couldn't find a way to make it work (if you know 
this not to be the case, please advise).  I agree with your other 
comments along those lines, in that maintenance tasks were never 
designed to handle these cases in the first place, and I consider this 
a shortcoming in the present Horde framework.  The only reason 
purge_folders references are made in these two cases are:



(a) because we want to ensure they can't disable maintenance if the 
admin says they can't ( I think this may have already been done by 
someone else, so this might not be required anymore).  This was to 
meet our customer requirement for disabling the user's ability to skip 
maintenance and is non-essential for the functionality required in the 
feature request.



(b) we wanted to store an array of values under a single preference 
key per user, which in Horde, to my knowledge, you can't do while 
maintaining hooked functionality, short of defining a whole new 
'array' datatype.



I believe it is the case that the remainder of the code (outside those 
two cases) adheres to Horde/IMP coding standards, but things may have 
changed since we wrote this (nearly a year ago now).  If I remember 
correctly, the fact that you have to store each folder's preferences 
separately with a way of keying them uniquely was the primary issue.   
This was done to adhere to preference storage and user interface 
generation standards set forth by the current maintenance 
implementation.
While I agree that Horde should eventually have its entire maintenance 
interface rewritten, or at least extended to allow for more 
flexibility on specialized datatypes, and thus functionality, I'm 
guessing this would be a rather large undertaking.  A more generalized 
interface on per folder maintenance tasks, as you suggest, would 
require either hacking the framework severely in its current form, or 
rewriting it entirely.



A more reasonable approach in my mind would be possible if you could 
offer advice on how the framework might be minimally modified to more 
elegantly allow this specific functionality to exist today.  I'd be 
glad to work on it, but I'd need a little guidance.



The fact that there has been a bounty on this feature for several 
years should serve as justification for working towards integrating 
possible solutions with the maintenance framework in its current form. 
  Providing features such as this in the short-term would probably 
provide more incentive to undertake the larger tasks, in my opinion.



I've just gotten some more time delegated at work to assist, which I 
hope can be of maximum effectiveness through your team's support, 
suggestions, and guidance.



Cheers,



Brook

I agree. Thanks for looking more closely at the code.

This is *way* too hackish for my taste - e.g. the framework patch 
contains numerous references to imp's purge_folder code, which 
obviously we can't do.



My belief was always that this ticket was envisioning some kind of 
widget accessed via the preference page (or maybe an entirely 
different maintenance pref page) that would allow one to define the 
various parameters needed for each task.  But quite frankly, the 
maintenance code was never designed to handle this kind of thing in 
the first place so trying to hack a solution is probably not as good 
of a solution as reforming the maintenance framework in the first place.



As mentioned in one of my note's for the Horde 4 TODO, i believe the 
ideal solution is to have a single login script for all Horde 
applications (rather than duplicating code in imp, gollem, etc.).  If 
this is done, all maintenance tasks could be done in this login script.

Understandable.  I'd take it on myself, except for that very reason.
Great, all I wanted to hear.  Again, please let me know if I can 
assist when it pops to the top of your queue.



Thanks bunches,



Brook

I'd certainly like to see this committed and closed, but your comment 
that you hadn't tested the code with head but had put the code by hand 
where you thought it should go did not inspire confidence. So I needed 
to leave it for when I had a larger chunk of time to devote to it 
(which I haven't), or for someone else with either time or more exact 
knowledge of that specific piece of code.



It's definitely still on the radar.

Well, it's been about two months now, so I'm assuming this isn't going 
to make it in, or isn't really a priority ?   It's a shame, as I spent 
the better part of a day patching this against HEAD, as asked.  Not to 
mention the fact that it solves all the problems from the original 
request (which was the oldest of all feature requests), with the 
exception of the archiving feature.  I'm sure it could be extended to 
provide this, however.



I think I may have not made myself clear on all the functionality 
before, so I'm hoping explaining this again will make it more 
attractive and perhaps more likely to make it into mainline Horde/IMP.



This patch will allow you to have per folder options for the number of 
days after which messages will be purged.  It creates a nice folder 
tree on the maintenance tasks screen for the user where they may set 
the max number of days emails will be kept for that folder.   
Administrators have additional options as well, being able to set the 
max number of days any user may keep messages in any folder (-1 being 
no limit).  Administrators may also allow/disallow users to skip 
maintenance on login.



I know you guys are busy and hard at work on more central development 
items, so please let me know if I can help further, as I said, I'm not 
familiar with the current development path of Horde, but I might be 
able to spend more time / get more allocated at work, if there is any 
interest.  However, I've already patched against HEAD, so I'm not sure 
what remains to be done at this point.



Brook

Any progress ?

Michael, could you take a look at this?

Here's the patch.  It is untested at present against HEAD, but I put 
the code in by hand where I expect it should go.  It should work.  All 
the files in IMP I modified are basically the same in the immediate 
proximity of my changes, so that should jive fine.  Horde has moved a 
lot of its files into framework, which I'm unfamiliar with, but I put 
the code in the places I believe it should go, so everything will 
likely just work.



I implemented a sort of "don't allow users to override confirmation", 
which I believe may have been done a different way.  Let me know if I 
can help further.



Everything is against CVS HEAD as of this morning, EST.

Yes.  I'll probably be able to get to this later this week / early 
next week.  I will patch what I have against HEAD.  I will need to get 
some hours allocated at work to follow-up on your other suggestions, 
as we've already completed this code for our customer.  It shouldn't 
be a problem, but will take a little longer.  In the mean time, I 
think what is already there is very useful.

Are you going to update the patch?

This patch, as you describe it, doesn't really fulfill the bounty. You 
*only* added a general "purge folders" task (which is great), but not 
the whole set of configurability as required by the request.

Beside that you should replace the existing purge trash, purge spam, 
rename sent-mail folders tasks. And finally, patches have to be 
against CVS HEAD to be accepted into the main source.

Here is an archive containing two patches.  One is against horde-3.1.1 
and the other against imp-h3-4.1.1.   These patches add a maintenance 
task, 'purge_folders' to horde/imp.  Config options allow 
administrators to control the maximum days a user can store mail in 
their folders, and if they can enable and disable folder purges and 
maintenance tasks.  Users can configure folder purges on a per-folder 
basis, or disable them for all or some folders, depending on 
administrative configuration.



This code has been used in production for a little while now and is 
running well.  All patches/fixes/enhancements are received with 
warmth.  Feel free to contact me with any questions or comments.
Of course, we'd be interested in any of this funding, if still available.

Bounty added: http://horde.org/bounties/details.php#imp_maintenance

IMP has login maintenance functions to e.g. delete old messages from 
Trash and sent items. I'd like a little more configuration here...



I have a folder setup to receive mailing list email, but I want to 
maintain history - e.g. the last 2 weeks' worth of messages. It's 
hassle to manually delete all messages older than 2 weeks. I'd like 
IMP to do this for me.



So, maintenance should be configured as a list of operations, with the 
user able to add/delete operations at will. Each operation should have 
a frequency (every login, daily, weekly, monthly...) and a type 
(delete messages older than N days/months in 
all-folders/specific-folder, archive a folder based on date - like 
current sent-items handling, maybe other options too)



That would really help manage mailing list mail with IMP.



Thanks!