It's implemented and everything _but_ the purge method has been tested
and a patch will be ready in a couple of hours.
However, from a "get Habari out the door" viewpoint, looking at the
implications of this patch I see that that _most_ queries in the Posts
class will need to include a silent "deleted_on is not null"
conditional to continue working as expected.
An elegant method would be to rewrite the conditional array passed
into the query method and silently add this clause; but I think the
special syntax " is not null " doesn't lend itself to this technique.
I'm still prepared to rewrite all the queries in the Posts class to
accommodate delete/undo; since I personally like it and see a user for
it... But since the proposed changes would make the effects reach
beyond the scope of a few methods, I think a vote is appropriate.
Do we:
a) want a stack based undelete now? [ie: a stack defined as 15
elements would retain only the 15 most recently deleted posts]
I haven't figured out how to make the purge method overrideable by
plugins yet, but I intend doing this before we ship. That way, plugins
can twiddle with the size of the stack and/or even completely revamp
the purge method to become date based or something even more
sophisticated if they so desire.
Or
b) would we prefer that delete means blow away from database, do not
pass go, do not collect £200 and if you forgot to make a backup before
delete, too bad. And we can defer the undelete stuff for when the rest
of the API has stabilized.
Constructive criticism and your votes for one of two options
appreciated. For me, changing Posts::delete to go directly in and
DB::o()->delete is a trivial change right at this moment; so it is the
best time to ask.
cheers
Option C) deleted items stay in the deleted state until they are purged.
Purging would, by default, be a manual operation. Plugins could
provided automated time-based removal of the oldest deleted items from
the queue, or whatever else a plugin author dreams up.
> Constructive criticism and your votes for one of two options
> appreciated. For me, changing Posts::delete to go directly in and
> DB::o()->delete is a trivial change right at this moment; so it is the
> best time to ask.
I'm really trying hard to think about what scenarios for undo we want to
facilitate. Personally, I have never deleted a post I later wanted to
retrieve. I'm sure some folks have, so it might be nice to hear from
(or about) them.
I lean toward option C, as long as the undo and restore mechanisms are
fully pluggable.
From a documentation standpoint, I think option C is the easier to
explain: "deleted" items live in your trash bin until you purge the
trash. This is exactly like the wastebasket in one's home or office:
you can easily fetch something out of it; but once the trash is
collected for final deposit at the city dump or trash burning facility
it's gone for good.
--
GPG 9CFA4B35 | ski...@skippy.net | http://skippy.net/
> Do we:
> a) want a stack based undelete now? [ie: a stack defined as 15
> elements would retain only the 15 most recently deleted posts]
I'm still for time-based with a short limit. +1 to that. +0 to
stack-based. If we use the deleted_on column as per my suggestion, we
can have both without much hassle.
> I haven't figured out how to make the purge method overrideable by
> plugins yet, but I intend doing this before we ship. That way, plugins
> can twiddle with the size of the stack and/or even completely revamp
> the purge method to become date based or something even more
> sophisticated if they so desire.
Pass the bin a Posts object or somesuch?
> b) would we prefer that delete means blow away from database, do not
> pass go, do not collect £200 and if you forgot to make a backup before
> delete, too bad. And we can defer the undelete stuff for when the rest
> of the API has stabilized.
-1. I want my money. :P
I also think we should really try and ship with Undo, even if it's
rather basic.
-Matt
Actually, I just implement the methods ::delete, ::recover and ::purge
in the Posts class.
I've seen mockups which have undo for a specific post deletion action
but no other UI for mass recovery or even purge functionality. I'm not
sure if it's even necessary.
How these methods would be invoked via the admin UI is completely up
for discussion.
> > Constructive criticism and your votes for one of two options
> > appreciated. For me, changing Posts::delete to go directly in and
> > DB::o()->delete is a trivial change right at this moment; so it is the
> > best time to ask.
>
> I'm really trying hard to think about what scenarios for undo we want to
> facilitate. Personally, I have never deleted a post I later wanted to
> retrieve. I'm sure some folks have, so it might be nice to hear from
> (or about) them.
>
> I lean toward option C, as long as the undo and restore mechanisms are
> fully pluggable.
Which means purge may never get called. That works fine for me except
that we may want to consider just having a stub for the purge method
for plugins to override instead of "bloating" (if you can call 10 or
so lines of code "bloat") the core with a specific purge
implementation.
I'd be up for any one of those... as I said, I haven't written purge
yet so it's not like I'm going to be throwing code away in either
case.
> From a documentation standpoint, I think option C is the easier to
> explain: "deleted" items live in your trash bin until you purge the
> trash.
Agreed.
I am also +1 on shipping a plugin with the core, that will enable timed
purging. I would rather start small and build up, than try and have
all the bells and whistles right away.
> Option C) deleted items stay in the deleted state until they are purged.
> Purging would, by default, be a manual operation. Plugins could
> provided automated time-based removal of the oldest deleted items from
> the queue, or whatever else a plugin author dreams up.
+1 if we ship with such a plugin, +0 otherwise :)
> I'm really trying hard to think about what scenarios for undo we want to
> facilitate. Personally, I have never deleted a post I later wanted to
> retrieve. I'm sure some folks have, so it might be nice to hear from
> (or about) them.
You must remember that Delete would be a no-questions-asked action if we
have Undo. There will be no "Are you really, really sure?" query. I'd
like for all operations to be as cheap as possible, which includes
getting rid of as many prompts as possible, and requires an easy option
for undoing mistakes.
>>From a documentation standpoint, I think option C is the easier to
> explain: "deleted" items live in your trash bin until you purge the
> trash. This is exactly like the wastebasket in one's home or office:
> you can easily fetch something out of it; but once the trash is
> collected for final deposit at the city dump or trash burning facility
> it's gone for good.
I thought we didn't want to tempt people to use the Trash as 'storage'.
I've seen it happen with Windows' Recyle Bin and Thunderbird's Trash
folder...
-Matt
+1 this is why i'm interested in the software in the first place.
C.
--
hail eris
http://rubberduck.com/
Okay. I like that proposition. BUT, if we have no-questions-asked
deletes, we need to work extra hard to prevent people from being tricked
(XSS or otherwise) into deleting a post.
>> >From a documentation standpoint, I think option C is the easier to
>> explain: "deleted" items live in your trash bin until you purge the
>> trash. This is exactly like the wastebasket in one's home or office:
>> you can easily fetch something out of it; but once the trash is
>> collected for final deposit at the city dump or trash burning facility
>> it's gone for good.
>
> I thought we didn't want to tempt people to use the Trash as 'storage'.
> I've seen it happen with Windows' Recyle Bin and Thunderbird's Trash
> folder...
Yeah, I know. I've changed my mind a bit... Unless we make some fancy
undelete queue / auto-purge solution, there's little we can do about how
people use the trash.
Why isn't there a "deleted" status and two accompanying postinfo
records with 1) the prior post status and 2) the deleted date?
It would make omission of deleted posts much easier, since the current
queries only look for the specific post status they look for. You'd
be able to cull old posts as you saw fit by querying directly on the
postinfo table for the post ids where the name of the data is
"deleted_on" and the value is less than some value.
I'm sure there was a reason to add the deleted_on field, but must have
missed it somewhere.
Owen
Well, something akin to that idea was my original plan mooted here :
http://groups.google.com/group/habari-dev/browse_thread/thread/ad4a14af508b632f/3c31dcb5512f3e50?lnk=gst
(apologies if that link gets munged, the thread is titled "proposal:
Implement undo for post deletion"). I looked at that option and a few
others, it must have gotten lost in the noise :)
> It would make omission of deleted posts much easier, since the current
> queries only look for the specific post status they look for. You'd
> be able to cull old posts as you saw fit by querying directly on the
> postinfo table for the post ids where the name of the data is
> "deleted_on" and the value is less than some value.
Yeah, that would work too.. although I didn't think of storing the
"old" status in postinfo.
> I'm sure there was a reason to add the deleted_on field, but must have
> missed it somewhere.
Two reasons, both of which are situational from my pov.
1. the -info patch(es) haven't been committed yet, I was waiting for
the fuss from the database rearrangements (if any) from last week to
die down before I went back there.
2. With a single deleted_on status, we get both deleted status AND the
time in one field for the price of a single field. I rather liked it
because toggling a single "invisible" field would essentially revert
everything back to the way it was. With your scheme, it would take two
queries (first retrieve prior status from postinfo, then update).
And no, (2) isn't a serious objection... anything that would touch
fewer existing queries sounds good to me.
Sheesh, there's a lot of traffic on this list. ;)
> Two reasons, both of which are situational from my pov.
> 1. the -info patch(es) haven't been committed yet, I was waiting for
> the fuss from the database rearrangements (if any) from last week to
> die down before I went back there.
Yeah, that's a bother, but we need to have the info tables anyway.
Perhaps these should be a priority over deletion undo.
> 2. With a single deleted_on status, we get both deleted status AND the
> time in one field for the price of a single field. I rather liked it
> because toggling a single "invisible" field would essentially revert
> everything back to the way it was. With your scheme, it would take two
> queries (first retrieve prior status from postinfo, then update).
Ah, but you'd be able to do the whole thing through the API:
$post = Posts::get(...);
$post->info->deleted_on = time();
$post->info->deleted_status = $post->status;
$post->status = 'deleted';
$post->update();
Then revert:
$post->status = $post->info->deleted_status;
$post->info->deleted_on = null;
$post->info->deleted_status = null;
$post->update();
Though actually purging is a bit trickier:
$posts = Posts::get('status=deleted');
foreach($posts as $post) {
if($post->deleted_on < $cutoff_time) {
$post->delete();
}
}
Doing it this way might also allow for flexibility in the deletion
criteria, since you could store something other than a timestamp for
$post->info->deleted_on. Maybe a batch index, or something like that.
You could even feed this data to a plugin and let plugins define
their own deletion schemes, which you couldn't necessarily do with a
pre-typed post-level field.
Just a thought.
Owen
What, you haven't been keeping up with the dozens of new mail on this
list each day? I'm beginning to question your dedication, I really am
;)
[snipped lots of the earlier discussion]
> Doing it this way might also allow for flexibility in the deletion
> criteria, since you could store something other than a timestamp for
> $post->info->deleted_on. Maybe a batch index, or something like that.
> You could even feed this data to a plugin and let plugins define
> their own deletion schemes, which you couldn't necessarily do with a
> pre-typed post-level field.
>
> Just a thought.
Actually, this appears an elegant way to do things via the API - which
I quite like. Ok, I'll get the info stuff in soonish and we'll do it
this way.
Yes, I am easily swayed :)