From: Bill Erickson <berick@esilibrary.com>
Date: Thu, 9 Jan 2014 16:24:08 +0000 (-0500)
Subject: LP#1164720 Prevent empty names in TPAC lists
X-Git-Tag: sprint4-merge-nov22~2386
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=237b7e71654eb90d33d98e6bbe3e89b4913de19e;p=working%2FEvergreen.git

LP#1164720 Prevent empty names in TPAC lists

Form submission for list creation with an empty name value now prevents
list creation and redirects the user to a new error page explaining why
list creation failed.

Signed-off-by: Bill Erickson <berick@esilibrary.com>
Signed-off-by: Ben Shum <bshum@biblio.org>
---

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
index 61b1028a84..ddd8806f6d 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
@@ -1918,9 +1918,15 @@ sub load_myopac_bookbags {
         $self->ctx->{add_rec} = $add_rec;
         # But not in the staff client, 'cause that breaks things.
         unless ($self->ctx->{is_staff}) {
-            $self->ctx->{where_from} = $self->ctx->{referer};
-            if ( my $anchor = $self->cgi->param('anchor') ) {
-                $self->ctx->{where_from} =~ s/#.*|$/#$anchor/;
+            # allow caller to provide the where_from in cases where
+            # the referer is an intermediate error page
+            if ($self->cgi->param('where_from')) {
+                $self->ctx->{where_from} = $self->cgi->param('where_from');
+            } else {
+                $self->ctx->{where_from} = $self->ctx->{referer};
+                if ( my $anchor = $self->cgi->param('anchor') ) {
+                    $self->ctx->{where_from} =~ s/#.*|$/#$anchor/;
+                }
             }
         }
     }
@@ -1968,26 +1974,33 @@ sub load_myopac_bookbag_update {
     }
 
     if ($action eq 'create') {
-        $list = Fieldmapper::container::biblio_record_entry_bucket->new;
-        $list->name($name);
-        $list->description($description);
-        $list->owner($e->requestor->id);
-        $list->btype('bookbag');
-        $list->pub($shared ? 't' : 'f');
-        $success = $U->simplereq('open-ils.actor',
-            'open-ils.actor.container.create', $e->authtoken, 'biblio', $list);
-        if (ref($success) ne 'HASH' && scalar @add_rec) {
-            $list_id = (ref($success)) ? $success->id : $success;
-            foreach my $add_rec (@add_rec) {
-                my $item = Fieldmapper::container::biblio_record_entry_bucket_item->new;
-                $item->bucket($list_id);
-                $item->target_biblio_record_entry($add_rec);
-                $success = $U->simplereq('open-ils.actor',
-                                         'open-ils.actor.container.item.create', $e->authtoken, 'biblio', $item);
-                last unless $success;
+
+        if ($name) {
+            $list = Fieldmapper::container::biblio_record_entry_bucket->new;
+            $list->name($name);
+            $list->description($description);
+            $list->owner($e->requestor->id);
+            $list->btype('bookbag');
+            $list->pub($shared ? 't' : 'f');
+            $success = $U->simplereq('open-ils.actor',
+                'open-ils.actor.container.create', $e->authtoken, 'biblio', $list);
+            if (ref($success) ne 'HASH' && scalar @add_rec) {
+                $list_id = (ref($success)) ? $success->id : $success;
+                foreach my $add_rec (@add_rec) {
+                    my $item = Fieldmapper::container::biblio_record_entry_bucket_item->new;
+                    $item->bucket($list_id);
+                    $item->target_biblio_record_entry($add_rec);
+                    $success = $U->simplereq('open-ils.actor',
+                                             'open-ils.actor.container.item.create', $e->authtoken, 'biblio', $item);
+                    last unless $success;
+                }
             }
             $url = $cgi->param('where_from') if ($success && $cgi->param('where_from'));
+
+        } else { # no name
+            $self->ctx->{bucket_failure_noname} = 1;
         }
+
     } elsif($action eq 'place_hold') {
 
         # @hold_recs comes from anon lists redirect; selected_itesm comes from existing buckets
@@ -2105,9 +2118,7 @@ sub load_myopac_bookbag_update {
 
     return $self->generic_redirect($url) if $success;
 
-    # XXX FIXME Bucket failure doesn't have a page to show the user anything
-    # right now. User just sees a 404 currently.
-
+    $self->ctx->{where_from} = $cgi->param('where_from');
     $self->ctx->{bucket_action} = $action;
     $self->ctx->{bucket_action_failed} = 1;
     return Apache2::Const::OK;
diff --git a/Open-ILS/src/templates/opac/myopac/list/update.tt2 b/Open-ILS/src/templates/opac/myopac/list/update.tt2
new file mode 100644
index 0000000000..2275df6592
--- /dev/null
+++ b/Open-ILS/src/templates/opac/myopac/list/update.tt2
@@ -0,0 +1,44 @@
+[%  PROCESS "opac/parts/header.tt2";
+    PROCESS "opac/parts/misc_util.tt2";
+    WRAPPER "opac/parts/myopac/base.tt2";
+    myopac_page = "lists/update"  
+%]
+
+<!-- we should never see this page on success -->
+
+[% IF ctx.bucket_action_failed %]
+<div id='bookbag_udpate_failures'>
+
+  <div>
+    <strong>[% l("Problem with list management:") %]</strong>
+  </div>
+
+  <div>
+    <ul>
+    [% IF ctx.bucket_action == 'create' %]
+      [% IF ctx.bucket_failure_noname %]         
+        <li>[% l('A list name is required') %]</li>
+      [% END %]   
+    [% END %]   
+    </ul>
+  </div>
+
+  <div>
+    [% url = ctx.referer;
+      # The return link should return the user to the page where the edit
+      # failure occurred.
+      # mkurl() does not support 'page' params w/ existing CGI params.
+      # build the URL manually.
+      IF ctx.where_from;
+        from = ctx.where_from | uri;
+        IF url.match('\?');
+          url = url _ ';where_from=' _ from;
+        ELSE;
+          url = url _ '?where_from=' _ from;
+        END;
+      END; %]
+    <a href="[% url %]">[% l('Return') %]</a>
+  </div>
+</div>
+[% END %]
+[% END %]