[mb-commits] branch, data-nes, updated. Added support for creating aliases

MusicBrainz Git Server <[email protected]> Tue, 15 Jan 2013 17:53:33 +0000
Newsgroups gmane.comp.audio.musicbrainz.cvs
Message-ID <E1TvAhZ-0000Ls-3Z@wiley>
The branch, data-nes has been updated
  discards  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=ca7b540ffb01b0c453e73ce411109a226904cd7a (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=c9153d25cf38b65f96107e82da2b2e8b63cc2260 (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=43ab95ad7dee3d6b6a73790b2e3ce378f2e8daa4 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (ca7b540ffb01b0c453e73ce411109a226904cd7a)
            \
             N -- N -- N (c9153d25cf38b65f96107e82da2b2e8b63cc2260)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Summary of changes:
 lib/MusicBrainz/Server/Controller/Role/Alias.pm |   62 ++++++++++++------
 lib/MusicBrainz/Server/Controller/Work.pm       |   76 ++++-----------------
 lib/MusicBrainz/Server/Data/NES/Work.pm         |   79 ++++++++++++++++++++--
 lib/MusicBrainz/Server/Entity/Alias.pm          |    4 +-
 lib/MusicBrainz/Server/Entity/Tree/Work.pm      |   19 ++++++
 lib/MusicBrainz/Server/Form/Alias.pm            |   30 ++++-----
 lib/MusicBrainz/Server/NES/Controller/Utils.pm  |   72 +++++++++++++++++++++
 root/alias/edit_form.tt                         |    1 +
 8 files changed, 234 insertions(+), 109 deletions(-)
 create mode 100644 lib/MusicBrainz/Server/Entity/Tree/Work.pm
 create mode 100644 lib/MusicBrainz/Server/NES/Controller/Utils.pm

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 c9153d25cf38b65f96107e82da2b2e8b63cc2260
Author: Oliver Charles <[email protected]>
Date:   Tue Jan 15 17:53:15 2013 +0000

    Added support for creating aliases

diff --git a/lib/MusicBrainz/Server/Controller/Role/Alias.pm b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
index 5662c53..d144d4a 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Alias.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
@@ -9,6 +9,9 @@ use MusicBrainz::Server::Constants qw(
     $EDIT_WORK_ADD_ALIAS $EDIT_WORK_DELETE_ALIAS $EDIT_WORK_EDIT_ALIAS
 );
 
+use MusicBrainz::Server::Entity::Alias;
+use MusicBrainz::Server::NES::Controller::Utils qw( create_update );
+
 my %model_to_edit_type = (
     add => {
         Artist => $EDIT_ARTIST_ADD_ALIAS,
@@ -30,7 +33,7 @@ my %model_to_edit_type = (
 my %model_to_search_hint_type_id = (
     Artist => 3,
     Label => 2,
-    Work => 2
+    'NES::Work' => 2
 );
 
 sub alias_type_model {
@@ -67,25 +70,30 @@ sub alias : Chained('load') PathPart('alias') CaptureArgs(1)
 sub add_alias : Chained('load') PathPart('add-alias') RequireAuth Edit
 {
     my ($self, $c) = @_;
-    my $type = $self->{entity_name};
-    my $entity = $c->stash->{ $type };
-    my $alias_model = $c->model( $self->{model} )->alias;
-    $self->edit_action($c,
-        form => 'Alias',
-        form_args => {
-            parent_id => $entity->id,
-            alias_model => $alias_model,
-            search_hint_type_id => $model_to_search_hint_type_id{ $self->{model} }
-        },
-        type => $model_to_edit_type{add}->{ $self->{model} },
-        edit_args => {
-            entity => $entity
-        },
-        item => {
-            name => $entity->name,
-            id => $entity->id
+    my $entity = $c->stash->{entity};
+    my $model_name = $self->{model};
+
+    create_update(
+        $self, $c,
+        build_form => sub {
+            $c->form(
+                form => 'Alias',
+                search_hint_type_id => $model_to_search_hint_type_id{ $model_name },
+                type_model => alias_type_model($model_name),
+                init_object => { revision_id => $entity->revision_id }
+            );
         },
-        on_creation => sub { $self->_redir_to_aliases($c) }
+        build_tree => sub {
+            my ($values, $revision) = @_;
+
+            my $aliases = $c->model($model_name)->get_aliases($revision);
+            $self->{tree_entity}->new(
+                aliases => [
+                    @$aliases,
+                    MusicBrainz::Server::Entity::Alias->new($values)
+                ]
+            );
+        }
     );
 }
 
diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index 2e8eef8..86e656e 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -9,7 +9,14 @@ use MusicBrainz::Server::Constants qw(
     $EDIT_WORK_ADD_ISWCS
     $EDIT_WORK_REMOVE_ISWC
 );
+use MusicBrainz::Server::Entity::Work;
+use MusicBrainz::Server::Entity::Tree::Work;
 use MusicBrainz::Server::Translation qw( l );
+use MusicBrainz::Server::NES::Controller::Utils qw( create_edit create_update );
+
+__PACKAGE__->config(
+    tree_entity => 'MusicBrainz::Server::Entity::Tree::Work',
+);
 
 with 'MusicBrainz::Server::Controller::Role::Load' => {
     model       => 'NES::Work',
@@ -67,68 +74,22 @@ for my $action (qw( aliases details )) {
     };
 }
 
-sub create_edit {
-    my ($self, $c, %opts) = @_;
-
-    my $form = do {
-        my %args = (
-            ctx => $c,
-        );
-
-        $args{init_object} = $opts{subject}
-            if defined $opts{subject};
-
-        $c->form(form => $opts{form}, %args);
-    };
-
-    if ($c->form_posted && $form->submitted_and_valid($c->req->body_params)) {
-        my $work = do {
-            my $edit = $c->model('NES::Edit')->open;
-            $opts{on_post}->($form->values, $edit);
-        };
-
-        $c->response->redirect(
-            $c->uri_for_action($self->action_for('show'), [ $work->gid ]));
-    }
-    elsif (!$c->form_posted && %{ $c->req->query_params }) {
-        $form->process( params => $c->req->query_params );
-        $form->clear_errors;
-    }
-}
-
 sub edit : Chained('load') {
     my ($self, $c) = @_;
 
-    $self->create_edit(
-        $c,
+    create_update(
+        $self, $c,
         form => 'Work::Edit',
         subject => $c->stash->{work},
-        on_post => sub {
-            my ($values, $edit) = @_;
-
-            my $original_work = $c->model('NES::Work')->get_revision(
-                $values->{revision_id});
-
-            $c->model('NES::Work')->update(
-                $edit, $c->user, $values->{revision_id},
-                work_tree($values)
-            );
-
-            return $original_work;
-        }
+        build_tree => \&work_tree
     );
 }
 
 sub work_tree {
     my $values = shift;
-    return (
-        {
-            type => $values->{type_id},
-            language => $values->{language_id},
-            name => $values->{name},
-            comment => $values->{comment}
-        },
-        $values->{iswcs} // []
+    return MusicBrainz::Server::Entity::Tree::Work->new(
+        work => MusicBrainz::Server::Entity::Work->new($values),
+        iswcs => $values->{iswcs} // []
     );
 }
 
@@ -181,8 +142,8 @@ after 'merge' => sub
 sub create : Local Edit {
     my ($self, $c) = @_;
 
-    $self->create_edit(
-        $c,
+    create_edit(
+        $self, $c,
         form => 'Work',
         on_post => sub {
             my ($values, $edit) = @_;
@@ -191,13 +152,6 @@ sub create : Local Edit {
                 $edit, $c->user,
                 work_tree($values)
             );
-
-            # NES:
-            # my $privs = $c->user->privileges;
-            # if ($c->user->is_auto_editor &&
-            #     $form->field('as_auto_editor') &&
-            #     !$form->field('as_auto_editor')->value) {
-            # }
         }
     );
 }
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 100fd4a..924a3d1 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -1,41 +1,98 @@
 package MusicBrainz::Server::Data::NES::Work;
 use Moose;
 
+use MusicBrainz::Server::Data::Utils qw( partial_date_to_hash );
 use MusicBrainz::Server::Entity::Work;
+use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw( boolean );
 
 with 'MusicBrainz::Server::Data::Role::NES';
 
 sub create {
-    my ($self, $edit, $editor, $work, $iswcs) = @_;
+    my ($self, $edit, $editor, $tree) = @_;
+
+    $tree->aliases([]) unless $tree->aliases_set;
 
     my $response = $self->request('/work/create', {
         edit => $edit->id,
         editor => $editor->id,
-        _work_tree($work, $iswcs)
+        _work_tree($tree)
     });
 
     return $self->get_revision($response->{ref});
 }
 
 sub update {
-    my ($self, $edit, $editor, $base_revision, $work, $iswcs) = @_;
+    my ($self, $edit, $editor, $base_revision, $tree) = @_;
+
+    die 'Need a base revision' unless $base_revision;
+
+    my $final_tree = do {
+        if( $tree->work_set && $tree->aliases_set && $tree->iswcs_set ) {
+            $tree
+        }
+        else {
+            my $original_tree = $self->view_tree($base_revision);
+
+            $original_tree->work($tree->work)
+                if ($tree->work_set);
+
+            $original_tree->aliases($tree->aliases)
+                if ($tree->aliases_set);
+
+            $original_tree->iswcs($tree->iswcs)
+                if ($tree->iswcs_set);
+
+            $original_tree;
+        }
+    };
 
     my $response = $self->request('/work/update', {
         edit => $edit->id,
         editor => $editor->id,
-        revision => $base_revision,
-        _work_tree($work, $iswcs)
+        revision => $base_revision->revision_id,
+        _work_tree($final_tree)
     });
 
     return undef;
 }
 
+sub view_tree {
+    my ($self, $revision) = @_;
+
+    return MusicBrainz::Server::Entity::Tree::Work->new(
+        work => $revision,
+        iswcs => $self->get_iswcs($revision),
+        aliases => $self->get_aliases($revision)
+    );
+}
+
 sub _work_tree {
-    my ($work, $iswcs) = @_;
+    my $tree = shift;
+
     return (
-        work => $work,
+        work => do {
+            my $work = $tree->work;
+            {
+                 type => $work->type_id,
+                 language => $work->language_id,
+                 name => $work->name,
+                 comment => $work->comment
+            }
+        },
         iswcs => [
-            map +{ iswc => $_ }, @$iswcs
+            map +{ iswc => $_ }, @{ $tree->iswcs }
+        ],
+        aliases => [
+            map +{
+                name => $_->name,
+                'sort-name' => $_->sort_name,
+                'begin-date' => partial_date_to_hash($_->begin_date),
+                'end-date' => partial_date_to_hash($_->end_date),
+                ended => $_->ended,
+                'primary-for-locale' => boolean($_->primary_for_locale),
+                type => $_->type_id,
+                locale => $_->locale
+            }, @{ $tree->aliases }
         ]
     );
 }
@@ -91,5 +148,11 @@ sub get_aliases {
     ]
 }
 
+sub get_iswcs {
+    my ($self, $revision) = @_;
+    warn "Unimplemented";
+    return [];
+}
+
 __PACKAGE__->meta->make_immutable;
 1;
diff --git a/lib/MusicBrainz/Server/Entity/Alias.pm b/lib/MusicBrainz/Server/Entity/Alias.pm
index 36467df..1d29dec 100644
--- a/lib/MusicBrainz/Server/Entity/Alias.pm
+++ b/lib/MusicBrainz/Server/Entity/Alias.pm
@@ -18,12 +18,12 @@ has 'sort_name' => (
 
 has 'locale' => (
     is  => 'rw',
-    isa => 'Str',
+    isa => 'Maybe[Str]',
 );
 
 has 'type_id' => (
     is => 'rw',
-    isa => 'Int',
+    isa => 'Maybe[Int]',
 );
 
 has 'type' => (
diff --git a/lib/MusicBrainz/Server/Entity/Tree/Work.pm b/lib/MusicBrainz/Server/Entity/Tree/Work.pm
new file mode 100644
index 0000000..acf63f0
--- /dev/null
+++ b/lib/MusicBrainz/Server/Entity/Tree/Work.pm
@@ -0,0 +1,19 @@
+package MusicBrainz::Server::Entity::Tree::Work;
+use Moose;
+
+has work => (
+    is => 'rw',
+    predicate => 'work_set',
+);
+
+has aliases => (
+    is => 'rw',
+    predicate => 'aliases_set',
+);
+
+has iswcs => (
+    is => 'rw',
+    predicate => 'iswcs_set',
+);
+
+1;
diff --git a/lib/MusicBrainz/Server/Form/Alias.pm b/lib/MusicBrainz/Server/Form/Alias.pm
index c918b56..16ffbfc 100644
--- a/lib/MusicBrainz/Server/Form/Alias.pm
+++ b/lib/MusicBrainz/Server/Form/Alias.pm
@@ -43,14 +43,8 @@ has 'id' => (
     is  => 'rw',
 );
 
-has 'parent_id' => (
-    isa => 'Int',
-    is  => 'ro',
-    required => 1,
-);
-
-has 'alias_model' => (
-    isa => 'MusicBrainz::Server::Data::Alias',
+has 'type_model' => (
+    isa => 'MusicBrainz::Server::Data::AliasType',
     is  => 'ro',
     required => 1
 );
@@ -61,6 +55,10 @@ has search_hint_type_id => (
     required => 1
 );
 
+has_field revision_id => (
+    type => 'Integer',
+);
+
 sub edit_field_names {
     qw( name locale sort_name period.begin_date period.end_date
         type_id primary_for_locale )
@@ -94,7 +92,7 @@ sub options_locale {
 
 sub options_type_id {
     my $self = shift;
-    $self->_select_all($self->alias_model->parent->alias_type);
+    $self->_select_all($self->type_model);
 }
 
 sub validate_primary_for_locale {
@@ -115,13 +113,13 @@ after validate => sub {
         $sort_name_field->validate_field;
     }
 
-    if ($self->alias_model->exists({ name => $self->field('name')->value,
-                                     locale => $self->field('locale')->value,
-                                     type_id => $self->field('type_id')->value,
-                                     not_id => $self->init_object ? $self->init_object->{id} : undef,
-                                 })) {
-        $self->field('name')->add_error('This alias already exists');
-    }
+    # if ($self->alias_model->exists({ name => $self->field('name')->value,
+    #                                  locale => $self->field('locale')->value,
+    #                                  type_id => $self->field('type_id')->value,
+    #                                  not_id => $self->init_object ? $self->init_object->{id} : undef,
+    #                              })) {
+    #     $self->field('name')->add_error('This alias already exists');
+    # }
 };
 
 1;
diff --git a/lib/MusicBrainz/Server/NES/Controller/Utils.pm b/lib/MusicBrainz/Server/NES/Controller/Utils.pm
new file mode 100644
index 0000000..fe16de7
--- /dev/null
+++ b/lib/MusicBrainz/Server/NES/Controller/Utils.pm
@@ -0,0 +1,72 @@
+package MusicBrainz::Server::NES::Controller::Utils;
+use strict;
+use warnings;
+
+use Sub::Exporter -setup => {
+    exports => [qw( create_edit create_update )]
+};
+
+sub create_edit {
+    my ($controller, $c, %opts) = @_;
+
+    my $form = do {
+        if (my $build_form = $opts{build_form}) {
+            $build_form->()
+        }
+        else {
+            my $form = do {
+                my %args = (
+                    ctx => $c,
+                );
+
+                $args{init_object} = $opts{subject}
+                    if defined $opts{subject};
+
+                $c->form(form => $opts{form}, %args);
+            }
+        }
+    };
+
+    if ($c->form_posted && $form->submitted_and_valid($c->req->body_params)) {
+        my $work = do {
+            my $edit = $c->model('NES::Edit')->open;
+            $opts{on_post}->($form->values, $edit);
+        };
+
+        # NES:
+        # my $privs = $c->user->privileges;
+        # if ($c->user->is_auto_editor &&
+        #     $form->field('as_auto_editor') &&
+        #     !$form->field('as_auto_editor')->value) {
+        # }
+
+        $c->response->redirect(
+            $c->uri_for_action($controller->action_for('show'), [ $work->gid ]));
+    }
+    elsif (!$c->form_posted && %{ $c->req->query_params }) {
+        $form->process( params => $c->req->query_params );
+        $form->clear_errors;
+    }
+}
+
+sub create_update {
+    my ($controller, $c, %opts) = @_;
+    create_edit(
+        $controller, $c,
+        %opts,
+        on_post => sub {
+            my ($values, $edit) = @_;
+            my $revision = $c->model('NES::Work')->get_revision(
+                $values->{revision_id});
+
+            $c->model( $controller->{model} )->update(
+                $edit, $c->user, $revision,
+                $opts{build_tree}->($values, $revision)
+            );
+
+            return $revision;
+        }
+    );
+}
+
+1;
diff --git a/root/alias/edit_form.tt b/root/alias/edit_form.tt
index b95c158..d27091d 100644
--- a/root/alias/edit_form.tt
+++ b/root/alias/edit_form.tt
@@ -20,6 +20,7 @@
         [% form_row_checkbox(r, 'primary_for_locale', l('This is the primary alias for this locale')) %]
       </span>
       [% form_row_select(r, 'type_id', l('Type:')) %]
+      [% r.hidden(form.field('revision_id')) %]
     </fieldset>
 
     <fieldset>

commit 43ab95ad7dee3d6b6a73790b2e3ce378f2e8daa4
Author: Oliver Charles <[email protected]>
Date:   Tue Jan 15 16:26:18 2013 +0000

    Basic support for viewing work aliases

diff --git a/lib/MusicBrainz/Server/Controller/Role/Alias.pm b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
index 2df415f..5662c53 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Alias.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
@@ -33,14 +33,24 @@ my %model_to_search_hint_type_id = (
     Work => 2
 );
 
+sub alias_type_model {
+    my ($c, $parent) = @_;
+    my %type_model = (
+        'NES::Work' => 'Work'
+    );
+    return $c->model($type_model{$parent})->alias_type;
+}
+
 sub aliases : Chained('load') PathPart('aliases')
 {
     my ($self, $c) = @_;
 
     my $entity = $c->stash->{$self->{entity_name}};
-    my $m = $c->model($self->{model});
-    my $aliases = $m->alias->find_by_entity_id($entity->id);
-    $m->alias_type->load(@$aliases);
+    my $m = $self->{model};
+
+    my $aliases = $c->model($m)->get_aliases($entity);
+    alias_type_model($c, $m)->load(@$aliases);
+
     $c->stash(
         aliases => $aliases,
     );
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 4d59a76..100fd4a 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -71,5 +71,25 @@ sub tags {
     $self->c->model('Work')->tags;
 }
 
+sub get_aliases {
+    my ($self, $work) = @_;
+    my $response = $self->request('/work/view-aliases', {
+        revision => $work->revision_id
+    });
+    return [
+        map {
+            MusicBrainz::Server::Entity::Alias->new(
+                name => $_->{name},
+                sort_name => $_->{'sort-name'},
+                locale => $_->{locale},
+                type_id => $_->{type},
+                begin_date => MusicBrainz::Server::Entity::PartialDate->new($_->{begin_date}),
+                end_date => MusicBrainz::Server::Entity::PartialDate->new($_->{end_date}),
+                ended => $_->{ended}
+            )
+        } @$response
+    ]
+}
+
 __PACKAGE__->meta->make_immutable;
 1;
diff --git a/lib/MusicBrainz/Server/Entity/Work.pm b/lib/MusicBrainz/Server/Entity/Work.pm
index 86b1dfd..fcafc9c 100644
--- a/lib/MusicBrainz/Server/Entity/Work.pm
+++ b/lib/MusicBrainz/Server/Entity/Work.pm
@@ -15,8 +15,8 @@ use MooseX::Types::Moose qw( ArrayRef Object Str );
 
 has 'type_id' => (
     is => 'rw',
-    isa => 'Int'
-    );
+    isa => 'Maybe[Int]'
+);
 
 has 'type' => (
     is => 'rw',
@@ -37,8 +37,8 @@ sub l_type_name
 
 has 'language_id' => (
     is => 'rw',
-    isa => 'Int'
-    );
+    isa => 'Maybe[Int]'
+);
 
 has 'language' => (
     is => 'rw',

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


hooks/post-receive
-- 
mb_server