[mb-commits] branch, data-nes, updated. Add support for creating relationships between works

MusicBrainz Git Server <[email protected]> Mon, 04 Feb 2013 08:43:50 +0000
Newsgroups gmane.comp.audio.musicbrainz.cvs
Message-ID <E1U2HeY-0005NM-TF@wiley>
The branch, data-nes has been updated
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=b97e1cb4720aa655181429a84396a68fe5116bdc (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=480a5e521a24f1bcc108f4d2c64503ad6203fed0 (commit)
      from  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=77e3ac59a922f99be71899f5cd62c55b31801578 (commit)

Summary of changes:
 .../Server/Controller/Edit/Relationship.pm         |   37 +++++----
 lib/MusicBrainz/Server/Controller/Role/Merge.pm    |   86 +++++++++-----------
 .../Server/Controller/Role/Relationship.pm         |    7 +-
 lib/MusicBrainz/Server/Controller/Root.pm          |    6 +-
 lib/MusicBrainz/Server/Controller/Work.pm          |   20 +++--
 lib/MusicBrainz/Server/Data/NES/CoreEntity.pm      |   10 +++
 lib/MusicBrainz/Server/Data/NES/Work.pm            |   37 ++++++++-
 lib/MusicBrainz/Server/Form/Merge.pm               |    4 +-
 lib/MusicBrainz/Server/NES.pm                      |    8 ++-
 root/components/common-macros.tt                   |    4 +-
 root/work/layout.tt                                |    3 +-
 root/work/merge.tt                                 |    4 +-
 12 files changed, 134 insertions(+), 92 deletions(-)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit b97e1cb4720aa655181429a84396a68fe5116bdc
Author: Oliver Charles <[email protected]>
Date:   Mon Feb 4 11:38:59 2013 +0000

    Add support for creating relationships between works

diff --git a/lib/MusicBrainz/Server/Controller/Edit/Relationship.pm b/lib/MusicBrainz/Server/Controller/Edit/Relationship.pm
index 2a3ba7c..eab41f9 100644
--- a/lib/MusicBrainz/Server/Controller/Edit/Relationship.pm
+++ b/lib/MusicBrainz/Server/Controller/Edit/Relationship.pm
@@ -203,10 +203,11 @@ sub create : Local RequireAuth Edit
         $c->detach('/error_500');
     }
 
-    my $source = $source_model->get_by_gid($source_gid);
-    my $dest   = $dest_model->get_by_gid($dest_gid);
+    my ($source, $dest) = $c->model('MB')->with_nes_transaction(sub {
+        return ($source_model->get_by_gid($source_gid), $dest_model->get_by_gid($dest_gid));
+    });
 
-    if ($type0 eq $type1 && $source->id == $dest->id) {
+    if ($type0 eq $type1 && $source->gid == $dest->gid) {
         $c->stash( message => l('A relationship requires 2 different entities') );
         $c->detach('/error_500');
     }
@@ -253,19 +254,23 @@ sub create : Local RequireAuth Edit
             ($entity0, $entity1) = ($entity1, $entity0);
         }
 
-        $c->model('MB')->with_transaction(sub {
-            $self->try_and_insert(
-                $c, $form,
-                $type0, $type1,
-                begin_date   => $form->field('period.begin_date')->value,
-                end_date     => $form->field('period.end_date')->value,,
-                attributes   => \@attributes,
-                link_type_id => $form->field('link_type_id')->value,
-                entity0      => $entity0,
-                entity1      => $entity1,
-                ended        => $form->field('period.ended')->value
-            ) or
-                $self->detach_existing($c);
+        $c->model('MB')->with_nes_transaction(sub {
+            my $edit = $c->model('NES::Edit')->open;
+
+            $c->model('NES::Work')->update(
+                $edit, $c->user, $entity0,
+                MusicBrainz::Server::Entity::Tree::Work->new(
+                    relationships => [
+                        MusicBrainz::Server::Entity::NES::Relationship->new(
+                            link => MusicBrainz::Server::Entity::Link->new(
+                                type_id => $form->field('link_type_id')->value
+                            ),
+                            target => $entity1,
+                            target_type => 'work'
+                        )
+                    ]
+                )
+            );
         });
 
         delete $c->session->{relationship};
diff --git a/lib/MusicBrainz/Server/Controller/Role/Relationship.pm b/lib/MusicBrainz/Server/Controller/Role/Relationship.pm
index 8a4b59f..98c394c 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Relationship.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Relationship.pm
@@ -15,10 +15,9 @@ sub relationships : Chained('load') PathPart('relationships')
 sub relate : Chained('load')
 {
     my ($self, $c) = @_;
+    my $entity = $c->stash->{entity};
 
-    my $type   = model_to_type( $self->{model} );
-    my $entity = $c->stash->{ $self->{entity_name} };
-
+    my $type = model_to_type( $self->{model} );
     if ($c->session->{relationship}) {
         $c->response->redirect($c->uri_for('/edit/relationship/create', {
             type0 => $c->session->{relationship}->{type0},
@@ -33,7 +32,7 @@ sub relate : Chained('load')
             type0   => $type,
             entity0 => $entity->gid,
             name    => $entity->name,
-            id      => $entity->id
+            gid     => $entity->gid
         };
 
         $c->response->redirect(
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 3550332..24edac9 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -67,7 +67,7 @@ sub tree_to_json {
             partition_by { $_->{target_type} }
                 map +{
                     target => $_->target->gid,
-                    type => $_->link_type_id,
+                    type => $_->link->type_id,
                     target_type => $_->target_type
                 }, @{ $tree->relationships }
         }
@@ -128,6 +128,10 @@ sub get_annotation {
     )->{annotation};
 }
 
+my %rel_type_to_model = (
+    work => 'NES::Work'
+);
+
 sub get_relationships {
     my ($self, $revision) = @_;
     my @rels =
@@ -138,6 +142,12 @@ sub get_relationships {
                 when (/url/) {
                     $target = $self->c->model('NES::URL')->get_by_gid($rel->{target});
                 }
+
+                default {
+                    $target = $self->c->model(
+                        $rel_type_to_model{$_} // die 'Unknown relationship type'
+                    )->get_by_gid($rel->{target});
+                }
             }
 
             MusicBrainz::Server::Entity::NES::Relationship->new(
diff --git a/root/components/common-macros.tt b/root/components/common-macros.tt
index a26cf6f..7744bff 100644
--- a/root/components/common-macros.tt
+++ b/root/components/common-macros.tt
@@ -528,12 +528,12 @@ END -%]
 [%- END -%]
 
 [%- MACRO use_in_relationship(entity) BLOCK;
-    UNLESS c.session.relationship.id == entity.id;
+    UNLESS c.session.relationship.gid == entity.gid;
       text = c.session.relationship ? l('Create relationship with {other}', { other => c.session.relationship.name })
                                     : l('Use in a relationship');
       '<li>' _ link_entity(entity, 'relate', text) _ '</li>';
     END;
-    IF c.session.relationship.id;
+    IF c.session.relationship.gid;
       text = l('Cancel creating relationship with {name}', { name => c.session.relationship.name });
       '<li>' _ link_entity(entity, 'cancel_relate', text) _ '</li>';
     END;
diff --git a/root/work/layout.tt b/root/work/layout.tt
index 59c185c..1f9e031 100644
--- a/root/work/layout.tt
+++ b/root/work/layout.tt
@@ -43,7 +43,6 @@
 
                [%# Adds <li> itself %]
                [% use_in_relationship(work) %]
-
                <li>[% relate_to_ellipsis(work) %]</li>
                <li>[% relate_to_url(work) %]</li>
 

commit 480a5e521a24f1bcc108f4d2c64503ad6203fed0
Author: Oliver Charles <[email protected]>
Date:   Mon Feb 4 11:07:40 2013 +0000

    Support merging two works together

diff --git a/lib/MusicBrainz/Server/Controller/Role/Merge.pm b/lib/MusicBrainz/Server/Controller/Role/Merge.pm
index 48bb98c..38a5c84 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Merge.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Merge.pm
@@ -1,7 +1,10 @@
 package MusicBrainz::Server::Controller::Role::Merge;
 use MooseX::Role::Parameterized -metaclass => 'MusicBrainz::Server::Controller::Role::Meta::Parameterizable';
 
+use List::MoreUtils qw( part );
+use MusicBrainz::Server::Data::Utils qw( model_to_type );
 use MusicBrainz::Server::Log qw( log_assertion );
+use MusicBrainz::Server::MergeQueue;
 use MusicBrainz::Server::Translation qw ( l ln );
 
 parameter 'edit_type' => (
@@ -25,10 +28,6 @@ role {
         }
     );
 
-    use List::MoreUtils qw( part );
-    use MusicBrainz::Server::Data::Utils qw( model_to_type );
-    use MusicBrainz::Server::MergeQueue;
-
     method 'merge_queue' => sub {
         my ($self, $c) = @_;
         my $model = $c->model( $self->{model} );
@@ -37,7 +36,9 @@ role {
         my @add = ref($add) ? @$add : ($add);
 
         if (@add) {
-            my @loaded = values %{ $model->get_by_ids(@add) };
+            my @loaded = $c->model('MB')->with_nes_transaction(sub {
+                values %{ $model->get_by_gids(@add) };
+            });
 
             if (!$c->session->{merger} ||
                  $c->session->{merger}->type ne $self->{model}) {
@@ -47,7 +48,7 @@ role {
             }
 
             my $merger = $c->session->{merger};
-            $merger->add_entities(map { $_->id } @loaded);
+            $merger->add_entities(map { $_->gid } @loaded);
 
             if ($merger->ready_to_merge) {
                 $c->response->redirect(
@@ -116,20 +117,22 @@ role {
         my $merger = $c->session->{merger}
             or $c->res->redirect('/'), $c->detach;
 
-        my @entities = values %{
-            $c->model($merger->type)->get_by_ids($merger->all_entities)
-        };
-
         $c->detach
             unless $merger->ready_to_merge;
 
-        my $form = $c->form(
-            form => $params->merge_form,
-            $self->_merge_form_arguments($c, @entities)
-        );
-        if ($self->_validate_merge($c, $form, $merger)) {
-            $self->_merge_submit($c, $form, \@entities);
-        }
+        $c->model('MB')->with_nes_transaction(sub {
+            my @entities = values %{
+                $c->model($merger->type)->get_by_gids($merger->all_entities)
+            };
+
+            my $form = $c->form(
+                form => $params->merge_form,
+                $self->_merge_form_arguments($c, @entities)
+            );
+            if ($self->_validate_merge($c, $form, $merger)) {
+                $self->_merge_submit($c, $form, \@entities);
+            }
+        });
     };
 
     method _validate_merge => sub {
@@ -140,41 +143,25 @@ role {
     method _merge_submit => sub {
         my ($self, $c, $form, $entities) = @_;
 
-        my %entity_id = map { $_->id => $_ } @$entities;
-
-        my $new_id = $form->field('target')->value or die 'Coludnt figure out new_id';
-        my $new = $entity_id{$new_id};
-        my @old_ids = grep { $_ != $new_id } @{ $form->field('merging')->value };
-
-        log_assertion { @old_ids >= 1 } 'Got at least 1 entity to merge';
-
-        $c->model('MB')->with_transaction(sub {
-            $self->_insert_edit(
-                $c, $form,
-                edit_type => $params->edit_type,
-                new_entity => {
-                    id => $new->id,
-                    name => $new->name,
-                },
-                old_entities => [ map +{
-                    id => $entity_id{$_}->id,
-                    name => $entity_id{$_}->name
-                }, @old_ids ],
-                (map { $_->name => $_->value } $form->edit_fields),
-                $self->_merge_parameters($c, $form, $entities)
-            );
-        });
+        my %entity_gid = map { $_->gid => $_ } @$entities;
 
-        $c->session->{merger} = undef;
+        my $new_gid = $form->field('target')->value or die 'Couldnt figure out new_gid';
+        my $new = $entity_gid{$new_gid};
+        my @old_gids = grep { $_ ne $new_gid } @{ $form->field('merging')->value };
 
-        $c->response->redirect(
-            $c->uri_for_action($self->action_for('show'), [ $new->gid ])
+        log_assertion { @old_gids >= 1 } 'Got at least 1 entity to merge';
+
+        my $edit = $c->model('NES::Edit')->open;
+        $c->model($self->{model})->merge(
+            $edit, $c->user,
+            source => [ map { $entity_gid{$_} } @old_gids ],
+            target => $new
         );
-    };
 
-    method _merge_parameters => sub {
-        return ()
-    }
+        $c->session->{merger} = undef;
+        $c->response->redirect(
+            $c->uri_for_action($self->action_for('show'), [ $new->gid ]));
+    };
 };
 
 sub _merge_search {
@@ -184,5 +171,6 @@ sub _merge_search {
                                     $query, shift, shift)
     });
 }
+
 1;
-      
+
diff --git a/lib/MusicBrainz/Server/Controller/Root.pm b/lib/MusicBrainz/Server/Controller/Root.pm
index 5bb4005..a6ef587 100644
--- a/lib/MusicBrainz/Server/Controller/Root.pm
+++ b/lib/MusicBrainz/Server/Controller/Root.pm
@@ -236,9 +236,9 @@ sub begin : Private
     # Merging
     if (my $merger = $c->session->{merger}) {
         my $model = $c->model($merger->type);
-        my @merge = values %{
-            $model->get_by_ids($merger->all_entities)
-        };
+        my @merge = $c->model('MB')->with_nes_transaction(sub {
+            values %{ $model->get_by_gids($merger->all_entities) };
+        });
         $c->model('ArtistCredit')->load(@merge);
 
         $c->stash(
diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index 8281964..b4968b1 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -108,15 +108,17 @@ before 'edit' => sub
 after 'merge' => sub
 {
     my ($self, $c) = @_;
-    $c->model('Work')->load_meta(@{ $c->stash->{to_merge} });
-    $c->model('WorkType')->load(@{ $c->stash->{to_merge} });
-    if ($c->user_exists) {
-        $c->model('Work')->rating->load_user_ratings($c->user->id, @{ $c->stash->{to_merge} });
-    }
-    $c->model('Work')->load_writers(@{ $c->stash->{to_merge} });
-    $c->model('Work')->load_recording_artists(@{ $c->stash->{to_merge} });
-    $c->model('Language')->load(@{ $c->stash->{to_merge} });
-    $c->model('ISWC')->load_for_works(@{ $c->stash->{to_merge} });
+    $c->model('MB')->with_nes_transaction(sub {
+        if ($c->user_exists) {
+            $c->model('Work')->rating->load_user_ratings($c->user->id, @{ $c->stash->{to_merge} });
+        }
+        # $c->model('Work')->load_meta(@{ $c->stash->{to_merge} });
+        # $c->model('Work')->load_writers(@{ $c->stash->{to_merge} });
+        # $c->model('Work')->load_recording_artists(@{ $c->stash->{to_merge} });
+        $c->model('Language')->load(@{ $c->stash->{to_merge} });
+        $c->model('NES::Work')->load_iswcs(@{ $c->stash->{to_merge} });
+        $c->model('WorkType')->load(@{ $c->stash->{to_merge} });
+    });
 };
 
 sub create : Local Edit {
diff --git a/lib/MusicBrainz/Server/Data/NES/CoreEntity.pm b/lib/MusicBrainz/Server/Data/NES/CoreEntity.pm
index 590ece5..1c08b4d 100644
--- a/lib/MusicBrainz/Server/Data/NES/CoreEntity.pm
+++ b/lib/MusicBrainz/Server/Data/NES/CoreEntity.pm
@@ -54,6 +54,16 @@ role {
             $self->request($params->root . '/find-latest', { mbid => $gid }))
     };
 
+    method get_by_gids => sub {
+        my ($self, @gids) = @_;
+        return {
+            map {
+                my $e = $self->get_by_gid($_);
+                $e->gid => $e
+            } @gids
+        };
+    };
+
     method _new_from_core_entity => sub {
         my ($self, $response) = @_;
         return keys %$response == 0
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 4cd0d5a..3550332 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -183,5 +183,30 @@ sub load_relationships {
     }
 }
 
+sub load_iswcs {
+    my ($self, @revisions) = @_;
+    for my $revision (@revisions) {
+        $revision->iswcs($self->get_iswcs($revision));
+    }
+}
+
+sub merge {
+    my ($self, $edit, $editor, %opts) = @_;
+    my @source = @{ $opts{source} };
+    my $target = $opts{target};
+
+    die 'NES: I cannot merge more than one entity at a time!' if @source > 1;
+
+    $self->request(
+        '/work/merge',
+        {
+            edit => $edit->id,
+            editor => $editor->id,
+            source => $source[0]->revision_id,
+            target => $target->gid
+        }
+    );
+}
+
 __PACKAGE__->meta->make_immutable;
 1;
diff --git a/lib/MusicBrainz/Server/Form/Merge.pm b/lib/MusicBrainz/Server/Form/Merge.pm
index 944ae11..a4449b3 100644
--- a/lib/MusicBrainz/Server/Form/Merge.pm
+++ b/lib/MusicBrainz/Server/Form/Merge.pm
@@ -11,7 +11,7 @@ sub edit_field_names { qw() }
 has '+name' => ( default => 'merge' );
 
 has_field 'target' => (
-    type => '+MusicBrainz::Server::Form::Field::Integer',
+    type => '+MusicBrainz::Server::Form::Field::Text',
     required => 1,
     required_message => l('Please pick the entity you want the others merged into.')
 );
@@ -22,7 +22,7 @@ has_field 'merging' => (
 );
 
 has_field 'merging.contains' => (
-    type => '+MusicBrainz::Server::Form::Field::Integer'
+    type => '+MusicBrainz::Server::Form::Field::Text'
 );
 
 1;
diff --git a/lib/MusicBrainz/Server/NES.pm b/lib/MusicBrainz/Server/NES.pm
index 9118ce9..a2e4d45 100644
--- a/lib/MusicBrainz/Server/NES.pm
+++ b/lib/MusicBrainz/Server/NES.pm
@@ -60,12 +60,16 @@ sub with_transaction {
     $self->clear_session_token;
     $self->session_token($self->request('/open-session', {})->{token});
 
+    my $w = wantarray;
     return try {
-        my $ret = $code->();
+        my @r = $code->() if $w;
+        my $r = $code->() if defined $w and not $w;
+        $code->() if not defined $w;
+
         $self->request('/close-session', {});
         $self->clear_session_token;
 
-        return $ret;
+        return $w ? @r : $r;
     }
     catch {
         try { $self->request('/close-session', {}) };
diff --git a/root/work/layout.tt b/root/work/layout.tt
index 1767f8f..59c185c 100644
--- a/root/work/layout.tt
+++ b/root/work/layout.tt
@@ -34,7 +34,7 @@
                [% annotation_links(work) %]
 
                <li>
-                 <a href="[% c.uri_for_action('/work/merge_queue', { 'add-to-merge' => work.id }) %]">
+                 <a href="[% c.uri_for_action('/work/merge_queue', { 'add-to-merge' => work.gid }) %]">
                    [% l('Merge work') %]
                  </a>
                </li>
diff --git a/root/work/merge.tt b/root/work/merge.tt
index b4739b0..c752836 100644
--- a/root/work/merge.tt
+++ b/root/work/merge.tt
@@ -21,8 +21,8 @@
             [% FOR entity=to_merge %]
                  <tr [% ' class="ev"' IF loop.count % 2 == 0 %]>
                     <td>
-                        <input type="hidden" name="merge.merging.[% loop.index %]" value="[% entity.id %]" />
-                        <input type="radio" name="merge.target" value="[% entity.id %]" />
+                        <input type="hidden" name="merge.merging.[% loop.index %]" value="[% entity.gid %]" />
+                        <input type="radio" name="merge.target" value="[% entity.gid %]" />
                     </td>
                     <td>
                         [% descriptive_link(entity) %]

-----------------------------------------------------------------------


hooks/post-receive
-- 
mb_server