order_by fixes including enable_order_by and coerce_order_by

diff --git a/Changes b/Changes
index b30a4c7..cdfe071 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 Revision history for Reaction
-
+0.003000 - 
+        - Fix Order role to allow for exclusions (for non-indexed columns) and
+          coerceion of the column name to allow proper ORDER BY generation in
+          belongs_to rels and fields that do not map directly to columns.
 0.002000 - 29 Apr 2008
         - Update CheckUniques role to use around instead of overrides
         - Stop using ACCEPT_CONTEXT, use InstancePerContext instead

diff --git a/lib/ComponentUI/Controller/TestModel/Bar.pm b/lib/ComponentUI/Controller/TestModel/Bar.pm
index 0839506..4b592c0 100644 (file)
--- a/lib/ComponentUI/Controller/TestModel/Bar.pm
+++ b/lib/ComponentUI/Controller/TestModel/Bar.pm
@@ -8,7 +8,19 @@ __PACKAGE__->config(
   collection_name => 'Bar',
   action => {
     base => { Chained => '/base', PathPart => 'testmodel/bar' },
+    list => {
+      ViewPort => {
+        enable_order_by => [qw/name foo published_at/],
+        coerce_order_by => { foo => ['foo.last_name', 'foo.first_name'] },
+      }
+    }
   },
 );
 
+sub get_collection {
+  my ($self, $c) = @_;
+  my $collection = $self->next::method($c);
+  return $collection->where({}, { prefetch => 'foo' });
+}
+
 1;

diff --git a/lib/ComponentUI/Controller/TestModel/Baz.pm b/lib/ComponentUI/Controller/TestModel/Baz.pm
index f1d7c8b..2fece5a 100644 (file)
--- a/lib/ComponentUI/Controller/TestModel/Baz.pm
+++ b/lib/ComponentUI/Controller/TestModel/Baz.pm
@@ -6,7 +6,14 @@ use Reaction::Class;
 __PACKAGE__->config(
   model_name => 'TestModel',
   collection_name => 'Baz',
-  action => { base => { Chained => '/base', PathPart => 'testmodel/baz' } },
+  action => {
+    base => { Chained => '/base', PathPart => 'testmodel/baz' },
+    list => {
+      ViewPort => {
+        enable_order_by => [qw/id name/],
+      },
+    },
+  },
 );
 
 1;

diff --git a/lib/ComponentUI/Controller/TestModel/Foo.pm b/lib/ComponentUI/Controller/TestModel/Foo.pm
index 56839a2..917ea17 100644 (file)
--- a/lib/ComponentUI/Controller/TestModel/Foo.pm
+++ b/lib/ComponentUI/Controller/TestModel/Foo.pm
@@ -13,6 +13,7 @@ __PACKAGE__->config(
         action_prototypes => { delete_all => 'Delete all records' },
         excluded_fields => [qw/id/],
         action_order => [qw/delete_all create/],
+        enable_order_by => [qw/last_name/],
         Member => {
           action_order => [qw/view update delete/],
         },

diff --git a/lib/Reaction/UI/ViewPort/Collection/Role/Order.pm b/lib/Reaction/UI/ViewPort/Collection/Role/Order.pm
index bb19f05..648b8cd 100644 (file)
--- a/lib/Reaction/UI/ViewPort/Collection/Role/Order.pm
+++ b/lib/Reaction/UI/ViewPort/Collection/Role/Order.pm
@@ -4,13 +4,49 @@ use Reaction::Role;
 
 use namespace::clean -except => [ qw(meta) ];
 
+has enable_order_by => (is => 'rw', isa => 'ArrayRef');
+has coerce_order_by => (isa => 'HashRef', is => 'rw');
+
+has order_by => (
+  isa => 'Str',
+  is => 'rw',
+  trigger_adopt('order_by'),
+  clearer => 'clear_order_by'
+);
+
+has order_by_desc => (
+  isa => 'Int',
+  is => 'rw',
+  trigger_adopt('order_by'),
+  lazy_build => 1
+);
+
+sub _build_order_by_desc { 0 }
 
-has order_by      => (isa => 'Str', is => 'rw', trigger_adopt('order_by'), clearer => 'clear_order_by');
-has order_by_desc => (isa => 'Int', is => 'rw', trigger_adopt('order_by'), lazy_build => 1);
-sub _build_order_by_desc { 0 };
 sub adopt_order_by {
   shift->clear_current_collection;
-};
+}
+
+sub can_order_by {
+  my ($self,$order_by) = @_;
+  return 1 unless $self->has_enable_order_by;
+  return scalar grep { $order_by eq $_ } @{ $self->enable_order_by };
+}
+
+sub _order_search_attrs {
+  my $self = shift;
+  my %attrs;
+  if ($self->has_order_by) {
+    my $order_by = $self->order_by;
+    if( $self->has_coerce_order_by ){
+      $order_by = $self->coerce_order_by->{$order_by}
+        if exists $self->coerce_order_by->{$order_by};
+    }
+    my $key = $self->order_by_desc ? '-desc' : '-asc';
+    $attrs{order_by} = { $key => $order_by };
+  }
+  return \%attrs;
+}
 
 after clear_order_by => sub {
   my ($self) = @_;
@@ -22,15 +58,7 @@ around _build_current_collection => sub {
   my $orig = shift;
   my ($self) = @_;
   my $collection = $orig->(@_);
-  my %attrs;
-
-  #XXX DBICism that needs to be fixed
-  if ($self->has_order_by) {
-    $attrs{order_by} = $self->order_by;
-    $attrs{order_by} .= ' DESC' if ($self->order_by_desc);
-  }
-
-  return $collection->where(undef, \%attrs);
+  return $collection->where(undef, $self->_order_search_attrs);
 };
 
 around accept_events => sub { ('order_by', 'order_by_desc', shift->(@_)); };

diff --git a/lib/Reaction/UI/Widget/ListView.pm b/lib/Reaction/UI/Widget/ListView.pm
index 1db21ba..2fa13f8 100644 (file)
--- a/lib/Reaction/UI/Widget/ListView.pm
+++ b/lib/Reaction/UI/Widget/ListView.pm
@@ -24,13 +24,16 @@ implements fragment action {
   render 'viewport';
 };
 
-around fragment header_cell {
-  arg order_uri => event_uri {
-    order_by => $_,
-    order_by_desc => ((($_{viewport}->order_by||'') ne $_
-                      || $_{viewport}->order_by_desc) ? 0 : 1)
-  };
-  call_next;
+implements fragment maybe_sortable_header_cell {
+  my $vp = $_{viewport};
+  if( $_{viewport}->can_order_by($_) ){
+    my $current = $vp->order_by;
+    my $desc = ( $vp->order_by_desc || ( $current || '') ne $_) ? 0 : 1;
+    arg order_uri => event_uri { order_by => $_, order_by_desc => $desc };
+    render 'sortable_header_cell';
+  } else {
+    render 'header_cell_contents';
+  }
 };
 
 implements fragment page_list {

diff --git a/share/skin/base/layout/list_view.tt b/share/skin/base/layout/list_view.tt
index 40be738..71a1324 100644 (file)
--- a/share/skin/base/layout/list_view.tt
+++ b/share/skin/base/layout/list_view.tt
@@ -10,9 +10,13 @@
 
 [% actions %]
 
-=for layout header_cell_contents
+=for layout sortable_header_cell
 
-<a href="[% order_uri %]">[% call_next %]</a>
+<a href="[% order_uri %]">[% header_cell_contents %]</a>
+
+=for layout header_cell
+
+[% maybe_sortable_header_cell %]
 
 =for layout actions
 

diff --git a/share/skin/default/layout/list_view.tt b/share/skin/default/layout/list_view.tt
index ead72b9..e8ef41d 100644 (file)
--- a/share/skin/default/layout/list_view.tt
+++ b/share/skin/default/layout/list_view.tt
@@ -10,9 +10,13 @@
 
 [% actions %]
 
-=for layout header_cell_contents
+=for layout sortable_header_cell
 
-<a href="[% order_uri %]">[% call_next %]</a>
+<a href="[% order_uri %]">[% header_cell_contents %]</a>
+
+=for layout header_cell
+
+<th> [% maybe_sortable_header_cell %] </th>
 
 =for layout actions