From a341d959ab798890746200efd0d5983c66f67db7 Mon Sep 17 00:00:00 2001
From: Moritz Bunkus <m.bunkus@linet-services.de>
Date: Fri, 8 Feb 2013 13:16:33 +0100
Subject: [PATCH] ActsAsList: mit remove_from_list() entfernte Items auf
 position = -1 setzen

position ist in allen Tabellen NOT NULL deklariert. Nur in der einen,
die ich zum Testen genutzt hatte, nicht. Das habe ich nun
nachgeholt. Jetzt wird -1 als magischer "not in list"-Wert benutzt.
---
 SL/DB/Helper/ActsAsList.pm | 28 ++++++++++++++++------------
 t/db_helper/acts_as_list.t |  8 ++++----
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/SL/DB/Helper/ActsAsList.pm b/SL/DB/Helper/ActsAsList.pm
index 86ee2ec95..f5bc8c293 100644
--- a/SL/DB/Helper/ActsAsList.pm
+++ b/SL/DB/Helper/ActsAsList.pm
@@ -41,7 +41,7 @@ sub remove_from_list {
   my $worker = sub {
     remove_position($self);
 
-    # Set to NULL manually because $self->update_attributes() would
+    # Set to -1 manually because $self->update_attributes() would
     # trigger the before_save() hook from this very plugin assigning a
     # number at the end of the list again.
     my $table           = $self->meta->table;
@@ -49,7 +49,7 @@ sub remove_from_list {
     my $primary_key_col = ($self->meta->primary_key)[0];
     my $sql             = <<SQL;
       UPDATE ${table}
-      SET ${column} = NULL
+      SET ${column} = -1
       WHERE ${primary_key_col} = ?
 SQL
     $self->db->dbh->do($sql, undef, $self->$primary_key_col);
@@ -165,16 +165,18 @@ sub get_group_by_where {
 sub set_position {
   my ($self) = @_;
   my $column = column_name($self);
+  my $value  = $self->$column;
 
-  return 1 if defined $self->$column;
+  return 1 if defined($value) && ($value != -1);
 
   my $table               = $self->meta->table;
   my ($group_by, @values) = get_group_by_where($self);
-  my $where               = $group_by ? " WHERE ${group_by}" : '';
+  $group_by               = " AND ${group_by}" if $group_by;
   my $sql                 = <<SQL;
     SELECT COALESCE(MAX(${column}), 0)
     FROM ${table}
-    ${where}
+    WHERE (${column} <> -1)
+      ${group_by}
 SQL
 
   my $max_position = $self->db->dbh->selectrow_arrayref($sql, undef, @values)->[0];
@@ -188,10 +190,10 @@ sub remove_position {
   my $column = column_name($self);
 
   $self->load;
-  return 1 unless defined $self->$column;
+  my $value = $self->$column;
+  return 1 unless defined($value) && ($value != -1);
 
   my $table               = $self->meta->table;
-  my $value               = $self->$column;
   my ($group_by, @values) = get_group_by_where($self);
   $group_by               = ' AND ' . $group_by if $group_by;
   my $sql                 = <<SQL;
@@ -208,20 +210,22 @@ SQL
 
 sub do_move {
   my ($self, $direction) = @_;
-  my $column             = column_name($self);
 
   croak "Object has not been saved yet" unless $self->id;
-  croak "No position set yet"           unless defined $self->$column;
+
+  my $column       = column_name($self);
+  my $old_position = $self->$column;
+  croak "No position set yet" unless defined($old_position) && ($old_position != -1);
 
   my $table                                        = $self->meta->table;
-  my $old_position                                 = $self->$column;
   my ($comp_sel, $comp_upd, $min_max, $plus_minus) = $direction eq 'up' ? ('<', '>=', 'MAX', '+') : ('>', '<=', 'MIN', '-');
   my ($group_by, @values)                          = get_group_by_where($self);
   $group_by                                        = ' AND ' . $group_by if $group_by;
   my $sql                                          = <<SQL;
     SELECT ${min_max}(${column})
     FROM ${table}
-    WHERE (${column} ${comp_sel} ?)
+    WHERE (${column} <>          -1)
+      AND (${column} ${comp_sel} ?)
       ${group_by}
 SQL
 
@@ -365,7 +369,7 @@ saved to the database.
 
 =item C<remove_from_list>
 
-Sets this items positional column to C<undef>, saves it and moves all
+Sets this items positional column to C<-1>, saves it and moves all
 following items up by 1.
 
 =item C<reorder_list @ids>
diff --git a/t/db_helper/acts_as_list.t b/t/db_helper/acts_as_list.t
index b91b18c02..1262353c6 100644
--- a/t/db_helper/acts_as_list.t
+++ b/t/db_helper/acts_as_list.t
@@ -139,19 +139,19 @@ test_positions "reoder_list called as class method", [ 1, undef, 1 ], [ 2, undef
 # Aus Liste entfernen
 reset_state();
 get_item(3)->remove_from_list;
-test_positions "remove_from_list on top", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, undef ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
+test_positions "remove_from_list on top", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, -1 ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
 
 reset_state();
 get_item(4)->remove_from_list;
-test_positions "remove_from_list on middle", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, undef ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
+test_positions "remove_from_list on middle", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, -1 ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
 
 reset_state();
 get_item(5)->remove_from_list;
-test_positions "remove_from_list on bottom", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, undef ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
+test_positions "remove_from_list on bottom", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, -1 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ];
 
 reset_state();
 get_item(8)->remove_from_list;
-test_positions "remove_from_list on only item in list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, undef ];
+test_positions "remove_from_list on only item in list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, -1 ];
 
 reset_state();
 $item = get_item(3); $item->remove_from_list; $item->delete;
-- 
2.20.1