From f12b3596807e5a46c6ed3e86c43e01492d2dc6c1 Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Fri, 8 Feb 2013 11:43:58 +0100 Subject: [PATCH 1/1] ActsAsList: neu: remove_from_list(), add_to_list(), Unit-Tests --- SL/DB/Helper/ActsAsList.pm | 151 +++++++++++++++++++++---- t/db_helper/acts_as_list.t | 223 +++++++++++++++++++++++++++++++++++++ 2 files changed, 351 insertions(+), 23 deletions(-) create mode 100644 t/db_helper/acts_as_list.t diff --git a/SL/DB/Helper/ActsAsList.pm b/SL/DB/Helper/ActsAsList.pm index 5734c2e6d..86ee2ec95 100644 --- a/SL/DB/Helper/ActsAsList.pm +++ b/SL/DB/Helper/ActsAsList.pm @@ -3,7 +3,7 @@ package SL::DB::Helper::ActsAsList; use strict; use parent qw(Exporter); -our @EXPORT = qw(move_position_up move_position_down reorder_list configure_acts_as_list); +our @EXPORT = qw(move_position_up move_position_down add_to_list remove_from_list reorder_list configure_acts_as_list); use Carp; @@ -35,6 +35,83 @@ sub move_position_down { do_move($self, 'down'); } +sub remove_from_list { + my ($self) = @_; + + my $worker = sub { + remove_position($self); + + # Set to NULL 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; + my $column = column_name($self); + my $primary_key_col = ($self->meta->primary_key)[0]; + my $sql = <db->dbh->do($sql, undef, $self->$primary_key_col); + $self->$column(undef); + }; + + return $self->db->in_transaction ? $worker->() : $self->db->do_transaction($worker); +} + +sub add_to_list { + my ($self, %params) = @_; + + croak "Invalid parameter 'position'" unless ($params{position} || '') =~ m/^ (?: before | after | first | last ) $/x; + + if ($params{position} eq 'last') { + set_position($self); + $self->save; + return; + } + + my $table = $self->meta->table; + my $primary_key_col = ($self->meta->primary_key)[0]; + my $column = column_name($self); + my ($group_by, @values) = get_group_by_where($self); + $group_by = " AND ${group_by}" if $group_by; + my $new_position; + + if ($params{position} eq 'first') { + $new_position = 1; + + } else { + # Can only be 'before' or 'after' -- 'last' has been checked above + # already. + + my $reference = $params{reference}; + croak "Missing parameter 'reference'" if !$reference; + + my $reference_pos; + if (ref $reference) { + $reference_pos = $reference->$column; + } else { + ($reference_pos) = $self->db->dbh->selectrow_array(qq|SELECT ${column} FROM ${table} WHERE ${primary_key_col} = ?|, undef, $reference); + } + + $new_position = $params{position} eq 'before' ? $reference_pos : $reference_pos + 1; + } + + my $query = < ?) + ${group_by} +SQL + + my $worker = sub { + $self->db->dbh->do($query, undef, $new_position - 1, @values); + $self->update_attributes($column => $new_position); + }; + + return $self->db->in_transaction ? $worker->() : $self->db->do_transaction($worker); +} + sub reorder_list { my ($class_or_self, @ids) = @_; @@ -75,9 +152,14 @@ sub get_group_by_where { my $group_by = get_spec(ref $self, 'group_by') || []; $group_by = [ $group_by ] if $group_by && !ref $group_by; - my @where = map { my $value = $self->$_; defined($value) ? "(${_} = " . $value . ")" : "(${_} IS NULL)" } @{ $group_by }; + my (@where, @values); + foreach my $column (@{ $group_by }) { + my $value = $self->$column; + push @values, $value if defined $value; + push @where, defined($value) ? "(${column} = ?)" : "(${column} IS NULL)"; + } - return join ' AND ', @where; + return (join(' AND ', @where), @values); } sub set_position { @@ -86,16 +168,16 @@ sub set_position { return 1 if defined $self->$column; - my $table = $self->meta->table; - my $where = get_group_by_where($self); - $where = " WHERE ${where}" if $where; - my $sql = <meta->table; + my ($group_by, @values) = get_group_by_where($self); + my $where = $group_by ? " WHERE ${group_by}" : ''; + my $sql = <db->dbh->selectrow_arrayref($sql)->[0]; + my $max_position = $self->db->dbh->selectrow_arrayref($sql, undef, @values)->[0]; $self->$column($max_position + 1); return 1; @@ -108,17 +190,18 @@ sub remove_position { $self->load; return 1 unless defined $self->$column; - my $table = $self->meta->table; - my $value = $self->$column; - my $group_by = get_group_by_where($self); - $group_by = ' AND ' . $group_by if $group_by; - my $sql = <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 = < ${value}) ${group_by} + WHERE (${column} > ?) + ${group_by} SQL - $self->db->dbh->do($sql); + $self->db->dbh->do($sql, undef, $value, @values); return 1; } @@ -132,28 +215,28 @@ sub do_move { 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 = get_group_by_where($self); + 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 = <db->dbh->selectrow_arrayref($sql)->[0]; + my $new_position = $self->db->dbh->selectrow_arrayref($sql, undef, $old_position, @values)->[0]; return undef unless defined $new_position; $sql = <db->dbh->do($sql); + $self->db->dbh->do($sql, undef, $old_position, $new_position, @values); $self->update_attributes($column => $new_position); } @@ -263,6 +346,28 @@ regarding their sort order by exchanging their C values. Swaps the object with the object one step below the current one regarding their sort order by exchanging their C values. +=item C + +Adds this item to the list. The parameter C is required and +can be one of C, C, C and C. With C +the item is inserted as the first item in the list and all other +item's positions are shifted up by one. For C the +item is inserted at the end of the list. + +For C and C an additional parameter C is +required. This is either a Rose model instance or the primary key of +one. The current item will then be inserted either before or after the +referenced item by shifting all the appropriate item positions up by +one. + +After this function C<$self>'s positional column has been set and +saved to the database. + +=item C + +Sets this items positional column to C, saves it and moves all +following items up by 1. + =item C Re-orders the objects given in C<@ids> by their position in C<@ids> by diff --git a/t/db_helper/acts_as_list.t b/t/db_helper/acts_as_list.t new file mode 100644 index 000000000..b91b18c02 --- /dev/null +++ b/t/db_helper/acts_as_list.t @@ -0,0 +1,223 @@ +use Test::More tests => 44; +use Test::Exception; + +use strict; + +use lib 't'; +use utf8; + +use Data::Dumper; +use Support::TestSetup; + +use_ok 'SL::DB::RequirementSpec'; +use_ok 'SL::DB::RequirementSpecItem'; +use_ok 'SL::DB::RequirementSpecTextBlock'; + +sub reset_state { + "SL::DB::Manager::${_}"->delete_all(all => 1) for qw(RequirementSpecTextBlock RequirementSpecItem RequirementSpec); + + SL::DB::RequirementSpec->new(id => 2, type_id => 1, status_id => 1, customer_id => 12395, hourly_rate => 42.24, title => "Azumbo")->save; + + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => undef, id => 1, position => 1, fb_number => "A01", title => "Mühköh", description => "The Kuh.")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => undef, id => 2, position => 2, fb_number => "A02", title => "Geheim", description => "Kofferkombination")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 1, id => 3, position => 1, fb_number => "FB0001", title => "Yäääh", description => "Und so")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 1, id => 4, position => 2, fb_number => "FB0012", title => "Blubb", description => "blabb")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 1, id => 5, position => 3, fb_number => "FB0022", title => "Fingo", description => "fungo")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 4, id => 6, position => 1, fb_number => "UFB002", title => "Suppi", description => "Suppa")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 4, id => 7, position => 2, fb_number => "UFB000", title => "Suppa", description => "Suppi")->save; + SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, parent_id => 2, id => 8, position => 1, fb_number => "FB0018", title => "Neuneins", description => "Eins")->save; + + SL::DB::RequirementSpec->new->db->dbh->do(qq|SELECT pg_catalog.setval('| . $_->[0] . qq|', | . $_->[1] . qq|, true)|) for (['requirement_spec_items_id_seq', 8], [ 'requirement_specs_id_seq', 2 ]); +} + +sub values_eq { + my ($val1, $val2) = @_; + return (defined($val1) == defined($val2)) + && (!defined($val1) || ($val1 == $val2)); +} + +sub test_positions { + my ($message, @positions) = @_; + + my $failures = + join ' ', + map { join ':', map { $_ // 'undef' } @{ $_ } } + grep { !values_eq($_->[1], $_->[3]) || !values_eq($_->[2], $_->[4]) } + map { my $item = SL::DB::RequirementSpecItem->new(id => $_->[0])->load; [ @{ $_ }, $item->parent_id, $item->position ] } + @positions; + + is($failures, '', $message); +} + +sub new_item { + return SL::DB::RequirementSpecItem->new(requirement_spec_id => 2, fb_number => 'dummy', title => 'dummy', @_); +} + +sub get_item { + return SL::DB::RequirementSpecItem->new(id => $_[0])->load; +} + +Support::TestSetup::login(); +my $item; + +# 1 +# `+- 3 +# +- 4 +# | `+- 6 +# | `- 7 +# `- 5 +# 2 +# `- 8 + +reset_state(); +test_positions "reset_state", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +# Einfügen neuer Objekte: "set_position" +new_item(parent_id => 1)->save; +test_positions "set_position via new with parent_id NOT NULL", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ], [ 9, 1, 4 ]; + +reset_state(); +new_item()->save; +test_positions "set_position via new with parent_id IS NULL", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ], [ 9, undef, 3 ]; + +# Löschen von Objekten: "remove_position" +reset_state(); +get_item(3)->delete; +test_positions "remove_position via delete with parent_id NOT NULL", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(1)->delete; +test_positions "remove_position via delete with parent_id IS NULL 1", [ 2, undef, 1 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(2)->delete; +test_positions "remove_position via delete with parent_id IS NULL 2", [ 1, undef, 1 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ]; + +# Hoch schieben +reset_state(); +get_item(3)->move_position_up; +test_positions "move_position_up when at top of sub-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(); +get_item(4)->move_position_up; +test_positions "move_position_up when in middle of sub-list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 3, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(5)->move_position_up; +test_positions "move_position_up when at end of sub-list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 5, 1, 2 ], [ 4, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(8)->move_position_up; +test_positions "move_position_up when only element in sub-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 ]; + +# Herunter schieben +reset_state(); +get_item(3)->move_position_down; +test_positions "move_position_down when at top of sub-list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 3, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(4)->move_position_down; +test_positions "move_position_down when in middle of sub-list", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 5, 1, 2 ], [ 4, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +get_item(5)->move_position_down; +test_positions "move_position_down when at bottom of sub-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(); +get_item(8)->move_position_down; +test_positions "move_position_down when only element in sub-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 ]; + +# Listen neu anordnen +reset_state(); +get_item(8)->reorder_list(4, 5, 3); +test_positions "reoder_list called as instance method", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 3, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +reset_state(); +SL::DB::RequirementSpecItem->reorder_list(4, 5, 3); +test_positions "reoder_list called as class method", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 3, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +# 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 ]; + +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 ]; + +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 ]; + +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 ]; + +reset_state(); +$item = get_item(3); $item->remove_from_list; $item->delete; +test_positions "remove_from_list and delete afterwards", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 4, 1, 1 ], [ 5, 1, 2 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 2, 1 ]; + +# Zu Liste hinzufügen +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'last'); +test_positions "add_to_list position 'last'", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 4 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'first'); +test_positions "add_to_list position 'first'", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 2 ], [ 4, 1, 3 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 1 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'before', reference => 3); +test_positions "add_to_list position 'before' first by ID", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 2 ], [ 4, 1, 3 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 1 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'before', reference => get_item(3)); +test_positions "add_to_list position 'before' first by reference", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 2 ], [ 4, 1, 3 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 1 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'before', reference => 4); +test_positions "add_to_list position 'before' middle by ID", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 3 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 2 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'before', reference => get_item(4)); +test_positions "add_to_list position 'before' middle by reference", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 3 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 2 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'after', reference => 5); +test_positions "add_to_list position 'after' last by ID", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 4 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'after', reference => get_item(5)); +test_positions "add_to_list position 'after' last by reference", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 4 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'after', reference => 4); +test_positions "add_to_list position 'after' middle by ID", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 3 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(1); $item->add_to_list(position => 'after', reference => get_item(4)); +test_positions "add_to_list position 'after' middle by reference", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 4 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 1, 3 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(3); $item->add_to_list(position => 'last'); +test_positions "add_to_list position 'last' in empty", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 3, 1 ]; + +reset_state(); +$item = get_item(8); $item->remove_from_list; $item->parent_id(3); $item->add_to_list(position => 'first'); +test_positions "add_to_list position 'first' in empty", [ 1, undef, 1 ], [ 2, undef, 2 ], [ 3, 1, 1 ], [ 4, 1, 2 ], [ 5, 1, 3 ], [ 6, 4, 1 ], [ 7, 4, 2 ], [ 8, 3, 1 ]; + + + +# Parametervalidierung +throws_ok { new_item()->move_position_up } qr/not.*been.*saved/i, 'move up not saved yet'; +throws_ok { new_item()->move_position_down } qr/not.*been.*saved/i, 'move down not saved yet'; + +throws_ok { $item = get_item(8); $item->position(undef); $item->move_position_up } qr/no.*position.*set.*yet/i, 'move up no position set'; +throws_ok { $item = get_item(8); $item->position(undef); $item->move_position_down } qr/no.*position.*set.*yet/i, 'move down no position set'; + +throws_ok { get_item(8)->add_to_list; } qr/invalid.*parameter.*position/i, 'missing position'; +throws_ok { get_item(8)->add_to_list(position => 'gonzo') } qr/invalid.*parameter.*position/i, 'invalid position'; +throws_ok { get_item(8)->add_to_list(position => 'before') } qr/missing.*parameter.*reference/i, 'missing reference for position "before"'; +throws_ok { get_item(8)->add_to_list(position => 'after') } qr/missing.*parameter.*reference/i, 'missing reference for position "after"'; + +done_testing(); -- 2.20.1