[jifty-devel] Jifty-DBI: patch proposal for a delete method for the Collection class

[Jesse Vincent]

On Thu, Oct 19, 2006 at 08:26:21PM +0200, Frederic BLANC wrote:
>  Hello,
> 
>  Here is a patch intended to add a delete method to the Jifty::DBI::Collection
> class.
>  IMO, providing such a method will be more efficient than the answer given
> to the "How do I delete all records from a table using a Model?" question
> (http://jifty.org/view/HowDoI); by the way, the new answer would be:
> 
> my $system_user = CalLabCert::CurrentUser->superuser;
> my $collection = CalLabCert::Model::DatFileCollection->new(
>                                            current_user => $system_user
> );
> $collection->unlimit();
> # but we can use a limit(), as long it does not join tables.
> $collection->delete();

What would you think about making the method name something a little
more explicit? 

$collection->delete_all_records() ?

		->delete_records()?

For something adding new SQL (especially new SQL that alters the
database), tests are essential.

This is also, I think, the first time, users can alter the database's
content from the collection class. Given the way we use JDBI in Jifty,
this would have the side-effect of walking around our access control.

This also doesn't quite have the effect I'd expect. 

If I create a collection, check that it has the right things in it and
then call delete:
	1) I'm not sure I get the right result with regard to paging
	2) If someone else has added a matching row in the meantime,
	   I'm going to be a VERY sad user ;)

Would you be happy with a solution that called the records' delete
methods instead?  That has the added benefit of catching model-specific
delete code too :)


Best,
Jesse