fix an old bug: disallow duplicate linkAlias or product_code

diff --git a/site/cgi-bin/modules/BSE/Importer/Target/Article.pm b/site/cgi-bin/modules/BSE/Importer/Target/Article.pm
index dff61eebefc389b2cce7acf110810d9e4f236d8f..300d439eb60a19d9cba5a462f45bf6bc236af2b0 100644 (file)
--- a/site/cgi-bin/modules/BSE/Importer/Target/Article.pm
+++ b/site/cgi-bin/modules/BSE/Importer/Target/Article.pm
@@ -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

diff --git a/site/cgi-bin/modules/BSE/Importer/Target/Product.pm b/site/cgi-bin/modules/BSE/Importer/Target/Product.pm
index 231351bfc678b59d92dd82c9bd64b0e6ff541176..fef5e5d6936b8fea9a191fd85e0c2cd1f918e9cb 100644 (file)
--- a/site/cgi-bin/modules/BSE/Importer/Target/Product.pm
+++ b/site/cgi-bin/modules/BSE/Importer/Target/Product.pm
@@ -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

diff --git a/t/130-importer/020-article.t b/t/130-importer/020-article.t
index 53705122b4b3af36eb68e28db547826b40008813..5840c35b56c8ba49e6e2a6f6cdd654f457adfff5 100644 (file)
--- a/t/130-importer/020-article.t
+++ b/t/130-importer/020-article.t
@@ -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;
   }

diff --git a/t/130-importer/030-product.t b/t/130-importer/030-product.t
index af61ccc19c83caf8353c5bd0e60dea34cc2e2b21..e477b5e55bd74e4c019ad8b4470a24f65ad6a0fa 100644 (file)
--- a/t/130-importer/030-product.t
+++ b/t/130-importer/030-product.t
@@ -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;