improve validation and error reporting for article tags
authorTony Cook <tony@develop-help.com>
Fri, 22 Nov 2013 00:59:29 +0000 (11:59 +1100)
committerTony Cook <tony@develop-help.com>
Fri, 22 Nov 2013 00:59:29 +0000 (11:59 +1100)
site/cgi-bin/modules/BSE/Edit/Article.pm
site/data/db/bse_msg_base.data
site/data/db/bse_msg_defaults.data
site/htdocs/js/admin_edit.js
site/templates/admin/include/edit_common.tmpl
site/templates/preload.tmpl

index b47353c..0d44e29 100644 (file)
@@ -16,7 +16,7 @@ use List::Util qw(first);
 use constant MAX_FILE_DISPLAYNAME_LENGTH => 255;
 use constant ARTICLE_CUSTOM_FIELDS_CFG => "article custom fields";
 
-our $VERSION = "1.043";
+our $VERSION = "1.044";
 
 =head1 NAME
 
@@ -1758,6 +1758,27 @@ sub save_columns {
   return @columns;
 }
 
+sub _validate_tags {
+  my ($self, $tags, $errors) = @_;
+
+  my $fail = 0;
+  my @errors;
+  for my $tag (@$tags) {
+    my $error;
+    if ($tag =~ /\S/
+       && !BSE::TB::Tags->valid_name($tag, \$error)) {
+      push @errors, "msg:bse/admin/edit/tags/invalid_$error";
+      $errors->{tags} = \@errors;
+      ++$fail;
+    }
+    else {
+      push @errors, undef;
+    }
+  }
+
+  return $fail;
+}
+
 sub save_new {
   my ($self, $req, $article, $articles) = @_;
 
@@ -1788,15 +1809,8 @@ sub save_new {
   my $save_tags = $cgi->param("_save_tags");
   my @tags;
   if ($save_tags) {
-    @tags = grep /\S/, $cgi->param("tags");
-    my $error;
-    for my $tag (@tags) {
-      BSE::TB::Tags->valid_name($tag, \$error)
-         or last;
-    }
-    if ($error) {
-      $errors{tags} = "msg:bse/admin/edit/badtag/$error";
-    }
+    @tags = $cgi->param("tags");
+    $self->_validate_tags(\@tags, \%errors);
   }
 
   if (keys %errors) {
@@ -1949,7 +1963,7 @@ sub save_new {
 
   if ($save_tags) {
     my $error;
-    $article->set_tags(\@tags, \$error);
+    $article->set_tags([ grep /\S/, @tags ], \$error);
   }
 
   generate_article($articles, $article) if $Constants::AUTO_GENERATE;
@@ -2123,15 +2137,8 @@ sub save {
   my $save_tags = $cgi->param("_save_tags");
   my @tags;
   if ($save_tags) {
-    @tags = grep /\S/, $cgi->param("tags");
-    my $error;
-    for my $tag (@tags) {
-      BSE::TB::Tags->valid_name($tag, \$error)
-         or last;
-    }
-    if ($error) {
-      $errors{tags} = "msg:bse/admin/edit/badtag/$error";
-    }
+    @tags = $cgi->param("tags");
+    $self->_validate_tags(\@tags, \%errors);
   }
   $self->validate_old($article, \%data, $articles, \%errors, scalar $req->is_ajax)
     or return $self->_service_error($req, $article, $articles, undef, \%errors, "FIELD");
@@ -2233,7 +2240,7 @@ sub save {
 
   if ($save_tags) {
     my $error;
-    $article->set_tags(\@tags, \$error);
+    $article->set_tags([ grep /\S/, @tags ], \$error);
   }
 
   # fix the kids too
index 0b46ea5..23260f1 100644 (file)
@@ -1,5 +1,5 @@
 --
-# VERSION=1.006
+# VERSION=1.007
 id: bse/
 description: BSE messages
 
@@ -149,6 +149,9 @@ description: field error if the tag name is empty
 id: bse/admin/edit/tags/invalid_badchars
 description: field error if the tag name contains invalid characters
 
+id: bse/admin/edit/tags/invalid_emptyvalue
+description: field error if the tag value component is empty
+
 id: bse/admin/edit/tags/bad_id
 description: tag_id field error if the tag id isn't present or isn't numeric
 
index 31e1a37..c424f4e 100644 (file)
@@ -100,6 +100,9 @@ message: Tags must have a non-empty name
 id: bse/admin/edit/tags/invalid_badchars
 message: Tags cannot contain control characters, backslash (\) or forward slash (/)
 
+id: bse/admin/edit/tags/invalid_emptyvalue
+message: Tags must have text after the colon (:)
+
 id: bse/admin/edit/tags/bad_id
 message: Invalid tag id
 
index 6f37c73..bf122dc 100644 (file)
@@ -18,7 +18,7 @@ Event.observe(document, "dom:loaded", function () {
     $("tags").insertBefore(new_div, add_div);
     ev.stop();
   }.bind(this, add_div));
-    if ($("#tags")) {
+    if ($("tags")) {
        $("tags").appendChild(add_div);
        $$('#tags div.tag').each(function(div) {
            var del = new Element("a", { href: "#" });
index 7deb7a1..d9ee8fa 100644 (file)
            <td>
              <input type="hidden" name="_save_tags" value="1" />
              <div id="tags">
-             <:iterator begin tags:>
-             <div class="tag"><input type="text" name="tags" value="<:tag name:>" /></div>
-             <:iterator end tags:>
+             <:.set tags = cgi.param("_save_tags") ? [ cgi.param("tags") ] : [ article.tags ] -:>
+             <:.for tag in tags -:>
+             <div class="tag"><input type="text" name="tags" value="<:= tag :>" /><:.call "error_img_n", "field":"tags", "index":loop.index :></div>
+             <:.end for :>
+             <:.if !cgi.param("_save_tags") -:>
              <div class="tag"><input type="text" name="tags" value="" /></div>
+             <:.end if-:>
              </div>
            </td>
-           <td class="help"><:help edit tags:><:error_img tags:></td>
+           <td class="help"><:help edit tags:></td>
          </tr>
           <tr> 
             <th>Always Dynamic:</th>
index b5f5b9a..a054420 100644 (file)
@@ -127,10 +127,12 @@ Page <:= pages.page :> of <:= pages.pagecount :>
   <:.if errors.$field -:>
     <:.set msg = errors.$field -:>
     <:.set msg = msg.is_list ? msg[index] : msg -:>
-    <:.set image = cfg.entry("error_img", "image", dist_image_uri _ "/admin/error.gif") -:>
-    <:.set width = cfg.entry("error_img", "width", 16) -:>
-    <:.set height = cfg.entry("error_img", "height", 16) -:>
+    <:.if msg.defined -:>
+      <:.set image = cfg.entry("error_img", "image", dist_image_uri _ "/admin/error.gif") -:>
+      <:.set width = cfg.entry("error_img", "width", 16) -:>
+      <:.set height = cfg.entry("error_img", "height", 16) -:>
     <img src="<:= image -:>" alt="<:= msg :>" title="<:= msg :>" width="<:= width :>" height="<:= height :>" class="error_img">
+    <:.end if -:>
   <:.end if -:>
 <:.end define -:>