fix an old bug: disallow duplicate linkAlias or product_code
authorTony Cook <tony@develop-help.com>
Wed, 3 Apr 2013 23:51:59 +0000 (10:51 +1100)
committerTony Cook <tony@develop-help.com>
Mon, 8 Apr 2013 00:54:35 +0000 (10:54 +1000)
site/cgi-bin/modules/BSE/Importer/Target/Article.pm
site/cgi-bin/modules/BSE/Importer/Target/Product.pm
t/130-importer/020-article.t
t/130-importer/030-product.t

index dff61ee..300d439 100644 (file)
@@ -6,7 +6,7 @@ use Articles;
 use Products;
 use OtherParents;
 
-our $VERSION = "1.005";
+our $VERSION = "1.006";
 
 =head1 NAME
 
@@ -232,6 +232,7 @@ sub row {
     }
   }
   elsif (!$importer->update_only) {
+    $self->validate_make_leaf($importer, $entry);
     $leaf = $self->make_leaf
       (
        $importer, 
@@ -400,6 +401,10 @@ sub xform_entry {
     $entry->{body}
       or $entry->{body} = $entry->{title};
   }
+
+  if (defined $entry->{linkAlias}) {
+    $entry->{linkAlias} =~ tr/A-Za-z0-9//cd;
+  }
 }
 
 =item children_of()
@@ -585,6 +590,22 @@ sub key_fields {
   return qw(id linkAlias);
 }
 
+=item validate_make_leaf
+
+Perform validation only needed on creation
+
+=cut
+
+sub validate_make_leaf {
+  my ($self, $importer, $entry) = @_;
+
+  if (defined $entry->{linkAlias} && $entry->{linkAlias} ne '') {
+    my $other = Articles->getBy(linkAlias => $entry->{linkAlias});
+    $other
+      and die "Duplicate linkAlias value with article ", $other->id, "\n";
+  }
+}
+
 1;
 
 =back
index 231351b..fef5e5d 100644 (file)
@@ -8,7 +8,7 @@ use BSE::TB::ProductOptions;
 use BSE::TB::ProductOptionValues;
 use BSE::TB::PriceTiers;
 
-our $VERSION = "1.003";
+our $VERSION = "1.004";
 
 =head1 NAME
 
@@ -151,6 +151,11 @@ sub xform_entry {
 
   $self->SUPER::xform_entry($importer, $entry);
 
+  if (defined $entry->{product_code}) {
+    $entry->{product_code} =~ s/\A\s+//;
+    $entry->{product_code} =~ s/\s+\z//;
+  }
+
   if ($self->{use_codes}) {
     $entry->{$self->{code_field}} =~ /\S/
       or die "$self->{code_field} blank with use_codes\n";
@@ -202,8 +207,14 @@ Find an existing product matching the code.
 sub find_leaf {
   my ($self, $leaf_id, $importer) = @_;
 
-  my ($leaf) = Products->getBy($self->{code_field}, $leaf_id)
-    or return;
+  my $leaf;
+  if ($self->{code_field} eq "id") {
+    $leaf = Products->getByPkey($leaf_id);
+  }
+  else {
+    ($leaf) = Products->getBy($self->{code_field}, $leaf_id)
+      or return;
+  }
 
   $importer->event(find_leaf => { id => $leaf_id, leaf => $leaf });
 
@@ -313,6 +324,20 @@ sub key_fields {
   return ( $class->SUPER::key_fields(), "product_code" );
 }
 
+=item validate_make_leaf
+
+=cut
+
+sub validate_make_leaf {
+  my ($self, $importer, $entry) = @_;
+
+  if (defined $entry->{product_code} && $entry->{product_code} ne '') {
+    my $other = Products->getBy(product_code => $entry->{product_code});
+    $other
+      and die "Duplicate product_code with product ", $other->id, "\n";
+  }
+}
+
 1;
 
 =back
index 5370512..5840c35 100644 (file)
@@ -4,7 +4,7 @@ use BSE::Test qw(base_url);
 use File::Spec;
 use File::Temp;
 
-use Test::More tests => 41;
+use Test::More tests => 42;
 
 BEGIN {
   unshift @INC, File::Spec->catdir(BSE::Test::base_dir(), "cgi-bin", "modules");
@@ -78,6 +78,13 @@ update_only=1
 source=CSV
 target=Article
 
+[import profile newdup$when]
+map_linkAlias=1
+map_title=2
+skiplines=0
+source=CSV
+target=Article
+
 CFG
 
 {
@@ -206,6 +213,19 @@ EOS
     is($file->hide_from_list, 1, "hide_from_list");
   }
 
+  { # fail to duplicate a link alias
+    my $fh = File::Temp->new;
+    my $filename = $fh->filename;
+    my $id = $testa->id;
+    print $fh <<EOS;
+"alias$when",test,t101.jpg
+EOS
+    close $fh;
+    my $imp = BSE::Importer->new(cfg => $cfg, profile => "newdup$when", callback => sub { note @_ });
+    $imp->process($filename);
+    is_deeply([ $imp->leaves ], [], "should be no updated articles");
+  }
+
   END {
     $testa->remove($cfg) if $testa;
   }
index af61ccc..e477b5e 100644 (file)
@@ -4,7 +4,7 @@ use BSE::Test qw(base_url);
 use File::Spec;
 use File::Temp;
 
-use Test::More tests => 7;
+use Test::More tests => 8;
 
 BEGIN {
   unshift @INC, File::Spec->catdir(BSE::Test::base_dir(), "cgi-bin", "modules");
@@ -34,6 +34,15 @@ target=Product
 update_only=1
 sep_char=\\t
 code_field=linkAlias
+
+[import profile newdup$when]
+map_product_code=1
+map_title=2
+map_retailPrice=3
+skiplines=0
+source=CSV
+target=Product
+
 CFG
 
 {
@@ -56,23 +65,44 @@ CFG
 }
 
 {
-  my $testa = bse_make_product(cfg => $cfg, title => "test updates",
-                              linkAlias => "P$when", retailPrice => 500);
+  my $testa = bse_make_product
+    (
+     cfg => $cfg,
+     title => "test updates",
+     linkAlias => "P$when",
+     retailPrice => 500,
+     product_code => "C$when",
+    );
 
-  my $fh = File::Temp->new;
-  my $filename = $fh->filename;
-  print $fh <<EOS;
+  {
+    my $fh = File::Temp->new;
+    my $filename = $fh->filename;
+    print $fh <<EOS;
 linkAlias\tbody
 "P$when"\t"This is the body text with multiple lines
 
 Yes, multiple lines with CSV!"
 EOS
-  close $fh;
-  my $imp = BSE::Importer->new(cfg => $cfg, profile => "simpleupdate$when",
-                              callback => sub { note @_ });
-  $imp->process($filename);
-  my $testb = Articles->getByPkey($testa->id);
-  like($testb->body, qr/This is the body/, "check the body is updated");
+    close $fh;
+    my $imp = BSE::Importer->new(cfg => $cfg, profile => "simpleupdate$when",
+                                callback => sub { note @_ });
+    $imp->process($filename);
+    my $testb = Articles->getByPkey($testa->id);
+    like($testb->body, qr/This is the body/, "check the body is updated");
+  }
+
+  { # fail to duplicate a product code
+    my $fh = File::Temp->new;
+    my $filename = $fh->filename;
+    my $id = $testa->id;
+    print $fh <<EOS;
+"C$when",A new title,100
+EOS
+    close $fh;
+    my $imp = BSE::Importer->new(cfg => $cfg, profile => "newdup$when", callback => sub { note @_ });
+    $imp->process($filename);
+    is_deeply([ $imp->leaves ], [], "should be no updated articles");
+  }
 
   END {
     $testa->remove($cfg) if $testa;