From ba11b612dbf23056113723c6d986d6db48688091 Mon Sep 17 00:00:00 2001
From: Oliver Charles <oliver.g.charles@googlemail.com>
Date: Tue, 12 Jun 2012 15:32:08 +0100
Subject: [PATCH] MBS-3803: Store the old barcode in edit barcode edits

---
 .../Server/Controller/WS/2/Release.pm         |  5 +-
 .../Server/Edit/Release/EditBarcodes.pm       | 22 ++++-
 root/edit/details/edit_barcodes.tt            | 15 ++-
 .../Server/Edit/Release/EditBarcodes.pm       | 93 ++++++++++---------
 4 files changed, 80 insertions(+), 55 deletions(-)

diff --git a/lib/MusicBrainz/Server/Controller/WS/2/Release.pm b/lib/MusicBrainz/Server/Controller/WS/2/Release.pm
index f04fa1bdc47..b98c18da942 100644
--- a/lib/MusicBrainz/Server/Controller/WS/2/Release.pm
+++ b/lib/MusicBrainz/Server/Controller/WS/2/Release.pm
@@ -294,10 +294,7 @@ sub release_submit : Private
                     privileges => $c->user->privileges,
                     edit_type => $EDIT_RELEASE_EDIT_BARCODES,
                     submissions => [ map +{
-                        release => {
-                            id => $gid_map{ $_->{release} }->id,
-                            name => $gid_map{ $_->{release} }->name
-                        },
+                        release => $gid_map{ $_->{release} },
                         barcode => $_->{barcode}
                     }, @submit ]
                 );
diff --git a/lib/MusicBrainz/Server/Edit/Release/EditBarcodes.pm b/lib/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
index b79c4f633dc..6d916bdce2d 100644
--- a/lib/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
+++ b/lib/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
@@ -4,6 +4,7 @@ use namespace::autoclean;
 
 use MusicBrainz::Server::Constants qw( :expire_action :quality );
 use MusicBrainz::Server::Constants qw( $EDIT_RELEASE_EDIT_BARCODES );
+use MusicBrainz::Server::Edit::Types qw( Nullable );
 use MusicBrainz::Server::Translation qw( l ln );
 use MooseX::Types::Moose qw( ArrayRef Int Str );
 use MooseX::Types::Structured qw( Dict );
@@ -12,6 +13,7 @@ extends 'MusicBrainz::Server::Edit';
 with 'MusicBrainz::Server::Edit::Release';
 with 'MusicBrainz::Server::Edit::Release::RelatedEntities';
 
+use aliased 'MusicBrainz::Server::Entity::Barcode';
 use aliased 'MusicBrainz::Server::Entity::Release';
 
 sub edit_name { l('Edit barcodes') }
@@ -25,6 +27,7 @@ has '+data' => (
                 name => Str
             ],
             barcode => Str,
+            old_barcode => Nullable[Str]
         ]]
     ]
 );
@@ -79,7 +82,9 @@ sub build_display_data
             map +{
                 release => $loaded->{Release}->{ $_->{release}{id} }
                     || Release->new( name => $_->{release}{name} ),
-                barcode => $_->{barcode},
+                new_barcode => Barcode->new($_->{barcode}),
+                exists $_->{old_barcode} ?
+                    (old_barcode => Barcode->new($_->{old_barcode})) : ()
             }, @{ $self->data->{submissions} }
         ]
     }
@@ -95,4 +100,19 @@ sub accept {
     }
 }
 
+sub initialize {
+    my ($self, %opts) = @_;
+    $opts{submissions} = [
+        map +{
+            release => {
+                id => $_->{release}->id,
+                name => $_->{release}->name,
+            },
+            barcode => $_->{barcode},
+            old_barcode => $_->{release}->barcode->code
+        }, @{ $opts{submissions} }
+    ];
+    $self->data(\%opts);
+}
+
 1;
diff --git a/root/edit/details/edit_barcodes.tt b/root/edit/details/edit_barcodes.tt
index aeeca147237..b36eb921a5e 100644
--- a/root/edit/details/edit_barcodes.tt
+++ b/root/edit/details/edit_barcodes.tt
@@ -1,12 +1,17 @@
+[% PROCESS 'edit/details/macros.tt' %]
 <table class="details edit-barcodes">
     [% FOR submission=edit.display_data.submissions %]
     <tr>
         <th>[% l('Release:') %]</th>
-        <td>[% descriptive_link(submission.release) %]</td>
-    </tr>
-    <tr>
-        <th>[% l('Barcode:') %]</th>
-        <td>[% submission.barcode | html %]</td>
+        <td colspan="2">[% descriptive_link(submission.release) %]</td>
     </tr>
+    [% IF !submission.exists('old_barcode') %]
+      <tr>
+          <th>[% l('Barcode:') %]</th>
+          <td colspan="2">[% submission.barcode | html %]</td>
+      </tr>
+    [% ELSE %]
+       [% display_diff(l('Barcode:'), submission.old_barcode.format, submission.new_barcode.format) %]
+    [% END %]
     [% END %]
 </table>
diff --git a/t/lib/t/MusicBrainz/Server/Edit/Release/EditBarcodes.pm b/t/lib/t/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
index df898de5b0b..eaeea463787 100644
--- a/t/lib/t/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
+++ b/t/lib/t/MusicBrainz/Server/Edit/Release/EditBarcodes.pm
@@ -11,50 +11,59 @@ use MusicBrainz::Server::Constants qw( $EDIT_RELEASE_EDIT_BARCODES );
 use MusicBrainz::Server::Test qw( accept_edit reject_edit );
 
 test all => sub {
+    my $test = shift;
+    my $c = $test->c;
+
+    MusicBrainz::Server::Test->prepare_test_database($c, '+release');
+
+    my $edit = _create_edit($c);
+    isa_ok($edit, 'MusicBrainz::Server::Edit::Release::EditBarcodes');
+
+    my ($edits) = $c->model('Edit')->find({ release => [1, 2] }, 10, 0);
+    is(@$edits, 1);
+    is($edits->[0]->id, $edit->id);
+
+    cmp_set(
+        [ map +{
+            barcode => $_->{barcode},
+            old_barcode => $_->{old_barcode}
+        }, @{ $edit->data->{submissions} } ],
+        [
+            { barcode => '5099703257021', old_barcode => '731453398122' },
+            { barcode => '5199703257021', old_barcode => undef }
+        ]
+    );
 
-my $test = shift;
-my $c = $test->c;
-
-MusicBrainz::Server::Test->prepare_test_database($c, '+release');
-
-my $edit = _create_edit($c);
-isa_ok($edit, 'MusicBrainz::Server::Edit::Release::EditBarcodes');
-
-my ($edits) = $c->model('Edit')->find({ release => [1, 2] }, 10, 0);
-is(@$edits, 1);
-is($edits->[0]->id, $edit->id);
-
-cmp_set($edit->related_entities->{artist},
-        [ 1 ],
-        'is related to the release artists');
-
-cmp_set($edit->related_entities->{release},
-        [ 1, 2 ],
-        'is related to the releases');
+    cmp_set($edit->related_entities->{artist},
+            [ 1 ],
+            'is related to the release artists');
 
-cmp_set($edit->related_entities->{release_group},
-        [ 1 ],
-        'is related to the release groups');
+    cmp_set($edit->related_entities->{release},
+            [ 1, 2 ],
+            'is related to the releases');
 
-my $r1 = $c->model('Release')->get_by_id(1);
-my $r2 = $c->model('Release')->get_by_id(2);
-is($r1->edits_pending, 3);
-is($r1->barcode->format, '731453398122');
-is($r2->edits_pending, 1);
-is($r2->barcode->format, '');
+    cmp_set($edit->related_entities->{release_group},
+            [ 1 ],
+            'is related to the release groups');
 
-reject_edit($c, $edit);
+    my $r1 = $c->model('Release')->get_by_id(1);
+    my $r2 = $c->model('Release')->get_by_id(2);
+    is($r1->edits_pending, 3);
+    is($r1->barcode->format, '731453398122');
+    is($r2->edits_pending, 1);
+    is($r2->barcode->format, '');
 
-$edit = _create_edit($c);
-accept_edit($c, $edit);
+    reject_edit($c, $edit);
 
-$r1 = $c->model('Release')->get_by_id(1);
-$r2 = $c->model('Release')->get_by_id(2);
-is($r1->edits_pending, 2);
-is($r1->barcode->format, '5099703257021');
-is($r2->edits_pending, 0);
-is($r2->barcode->format, '5199703257021');
+    $edit = _create_edit($c);
+    accept_edit($c, $edit);
 
+    $r1 = $c->model('Release')->get_by_id(1);
+    $r2 = $c->model('Release')->get_by_id(2);
+    is($r1->edits_pending, 2);
+    is($r1->barcode->format, '5099703257021');
+    is($r2->edits_pending, 0);
+    is($r2->barcode->format, '5199703257021');
 };
 
 sub _create_edit {
@@ -66,17 +75,11 @@ sub _create_edit {
         editor_id => 1,
         submissions => [
             {
-                release => {
-                    id => $old_rel->id,
-                    name => $old_rel->name,
-                },
+                release => $old_rel,
                 barcode => '5099703257021',
             },
             {
-                release => {
-                    id => $new_rel->id,
-                    name => $new_rel->name
-                },
+                release => $new_rel,
                 barcode => '5199703257021'
             }
         ]