From: Tony Cook Date: Wed, 3 Apr 2013 23:51:59 +0000 (+1100) Subject: fix an old bug: disallow duplicate linkAlias or product_code X-Git-Tag: v0.25~92^2~12 X-Git-Url: http://git.imager.perl.org/bse.git/commitdiff_plain/1455f602d7e78b406216db628aad44cfac9a4474 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 dff61eeb..300d439e 100644 --- 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 231351bf..fef5e5d6 100644 --- 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 53705122..5840c35b 100644 --- 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 <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 af61ccc1..e477b5e5 100644 --- 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 <new; + my $filename = $fh->filename; + print $fh <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 <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;