[mb-commits] branch, data-nes, updated. Allow creating, viewing, and deleting aliases.

MusicBrainz Git Server <[email protected]> Fri, 25 Jan 2013 11:05:20 +0000
Newsgroups gmane.comp.audio.musicbrainz.cvs
Message-ID <E1Tyh60-0005cC-3f@wiley>
The branch, data-nes has been updated
  discards  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=541cd4d96573361b80e834b3b972a023d317f2d4 (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=2bdc4e841f65b5608d88952dc8df4a3ba64a7405 (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=7f5b98ff3aeef14a0639d989546aba5c4ff7b77e (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=b8db95e9622f604b1b39c4aadc93b2867c676c46 (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=70fcfef2332e7d927801625fa14b6dc56796d81c (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 (541cd4d96573361b80e834b3b972a023d317f2d4)
            \
             N -- N -- N (2bdc4e841f65b5608d88952dc8df4a3ba64a7405)

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   |  181 ++++++++++++---------
 lib/MusicBrainz/Server/Controller/Role/Cleanup.pm |    2 +-
 lib/MusicBrainz/Server/Controller/Work.pm         |   14 +-
 lib/MusicBrainz/Server/Data/ISWC.pm               |   51 ------
 lib/MusicBrainz/Server/Data/NES/ISWC.pm           |   34 ++++
 lib/MusicBrainz/Server/Data/NES/Work.pm           |   54 ++++++-
 lib/MusicBrainz/Server/Data/Utils.pm              |   20 ++-
 lib/MusicBrainz/Server/Entity/Alias.pm            |    4 +-
 lib/MusicBrainz/Server/Entity/Tree/Work.pm        |    5 +
 lib/MusicBrainz/Server/Entity/Work.pm             |    2 +-
 lib/MusicBrainz/Server/Form/Alias.pm              |   30 ++--
 lib/MusicBrainz/Server/Form/Confirm.pm            |    5 +
 lib/MusicBrainz/Server/NES/Controller/Utils.pm    |    2 +
 root/alias/delete.tt                              |    2 +
 root/alias/edit_form.tt                           |    1 +
 root/artist/index.tt                              |    2 +-
 root/components/aliases.tt                        |    5 +-
 root/work/index.tt                                |    2 +-
 18 files changed, 250 insertions(+), 166 deletions(-)
 create mode 100644 lib/MusicBrainz/Server/Data/NES/ISWC.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 2bdc4e841f65b5608d88952dc8df4a3ba64a7405
Author: Oliver Charles <[email protected]>
Date:   Fri Jan 18 12:54:37 2013 +0000

    Allow creating, viewing, and deleting aliases.

diff --git a/lib/MusicBrainz/Server/Controller/Role/Alias.pm b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
index 2df415f..25258e9 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Alias.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Alias.pm
@@ -3,120 +3,130 @@ use Moose::Role -traits => 'MooseX::MethodAttributes::Role::Meta::Role';
 
 requires 'load';
 
-use MusicBrainz::Server::Constants qw(
-    $EDIT_ARTIST_ADD_ALIAS $EDIT_ARTIST_DELETE_ALIAS $EDIT_ARTIST_EDIT_ALIAS
-    $EDIT_LABEL_ADD_ALIAS $EDIT_LABEL_DELETE_ALIAS $EDIT_LABEL_EDIT_ALIAS
-    $EDIT_WORK_ADD_ALIAS $EDIT_WORK_DELETE_ALIAS $EDIT_WORK_EDIT_ALIAS
-);
-
-my %model_to_edit_type = (
-    add => {
-        Artist => $EDIT_ARTIST_ADD_ALIAS,
-        Label  => $EDIT_LABEL_ADD_ALIAS,
-        Work   => $EDIT_WORK_ADD_ALIAS,
-    },
-    delete => {
-        Artist => $EDIT_ARTIST_DELETE_ALIAS,
-        Label  => $EDIT_LABEL_DELETE_ALIAS,
-        Work   => $EDIT_WORK_DELETE_ALIAS,
-    },
-    edit => {
-        Artist => $EDIT_ARTIST_EDIT_ALIAS,
-        Label  => $EDIT_LABEL_EDIT_ALIAS,
-        Work   => $EDIT_WORK_EDIT_ALIAS,
-    }
-);
+use MusicBrainz::Server::Entity::Alias;
+use MusicBrainz::Server::NES::Controller::Utils qw( create_update );
 
 my %model_to_search_hint_type_id = (
     Artist => 3,
     Label => 2,
-    Work => 2
+    'NES::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 $entity = $c->stash->{entity};
+    my $m = $self->{model};
+
+    my $aliases = $c->model($m)->get_aliases($entity);
+    alias_type_model($c, $m)->load(@$aliases);
+
     $c->stash(
         aliases => $aliases,
     );
 }
 
-sub alias : Chained('load') PathPart('alias') CaptureArgs(1)
+sub alias : Chained('load') PathPart('alias') CaptureArgs(0)
 {
-    my ($self, $c, $alias_id) = @_;
-    my $alias = $c->model($self->{model})->alias->get_by_id($alias_id)
-        or $c->detach('/error_404');
-    $c->stash( alias => $alias );
+    my ($self, $c) = @_;
+
+    my $qp = $c->req->query_params;
+
+    my $all_aliases = $c->model( $self->{model} )->get_aliases($c->stash->{entity});
+    my ($alias) = grep {
+        $_->name eq $qp->{name}
+    } @$all_aliases or $c->detach('/error_404');
+
+    $c->stash(
+        alias => $alias,
+        all_aliases => $all_aliases
+    );
 }
 
 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
-        },
-        on_creation => sub { $self->_redir_to_aliases($c) }
+    my $entity = $c->stash->{entity};
+
+    create_update(
+        $self, $c,
+        form => $self->_build_alias_form($c),
+        build_tree => sub {
+            my ($values, $revision) = @_;
+
+            my $aliases = $c->model($self->{model})->get_aliases($revision);
+            $self->{tree_entity}->new(
+                aliases => [
+                    @$aliases,
+                    MusicBrainz::Server::Entity::Alias->new($values)
+                ]
+            );
+        }
     );
 }
 
 sub delete_alias : Chained('alias') PathPart('delete') RequireAuth Edit
 {
     my ($self, $c) = @_;
+
+    my $entity = $c->stash->{entity};
     my $alias = $c->stash->{alias};
-    $self->edit_action($c,
-        form => 'Confirm',
-        type => $model_to_edit_type{delete}->{ $self->{model} },
-        edit_args => {
-            alias  => $alias,
-            entity => $c->stash->{ $self->{entity_name} }
-        },
-        on_creation => sub { $self->_redir_to_aliases($c) }
+
+    create_update(
+        $self, $c,
+        form => $c->form(
+            form => 'Confirm',
+            init_object => { revision_id => $entity->revision_id }
+        ),
+        build_tree => sub {
+            my ($values, $revision) = @_;
+
+            return $self->{tree_entity}->new(
+                aliases => [ _aliases_without($c->stash->{all_aliases}, $alias) ]
+            );
+        }
     );
+
+        # on_creation => sub { $self->_redir_to_aliases($c) }
 }
 
 sub edit_alias : Chained('alias') PathPart('edit') RequireAuth Edit
 {
     my ($self, $c) = @_;
+
     my $alias = $c->stash->{alias};
-    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,
-            id => $alias->id,
-            search_hint_type_id => $model_to_search_hint_type_id{ $self->{model} }
-        },
-        item => $alias,
-        type => $model_to_edit_type{edit}->{ $self->{model} },
-        edit_args => {
-            alias  => $alias,
-            entity => $c->stash->{ $self->{entity_name} }
-        },
-        on_creation => sub { $self->_redir_to_aliases($c) }
+    my $entity = $c->stash->{entity};
+
+    create_update(
+        $self, $c,
+        form => $self->_build_alias_form($c, $alias),
+        build_tree => sub {
+            my ($values, $revision) = @_;
+
+            return $self->{tree_entity}->new(
+                aliases => [
+                    _aliases_without($c->stash->{all_aliases}, $alias),
+                    MusicBrainz::Server::Entity::Alias->new($values),
+                ]
+            );
+        }
     );
+
+        # on_creation => sub { $self->_redir_to_aliases($c) }
+}
+
+sub _aliases_without {
+    my ($aliases, $alias) = @_;
+    return grep { $_ != $alias } @$aliases;
 }
 
 sub _redir_to_aliases
@@ -127,5 +137,20 @@ sub _redir_to_aliases
     $c->response->redirect($c->uri_for($action, [ $entity->gid ]));
 }
 
+sub _build_alias_form {
+    my ($self, $c, $alias) = @_;
+    my $model_name = $self->{model};
+
+    $c->form(
+        form => 'Alias',
+        search_hint_type_id => $model_to_search_hint_type_id{ $model_name },
+        type_model => alias_type_model($c, $model_name),
+        init_object => {
+            %{ $alias // {} },
+            revision_id => $c->stash->{entity}->revision_id
+        }
+    )
+}
+
 no Moose::Role;
 1;
diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index 7ec1155..5769cca 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -21,13 +21,13 @@ with 'MusicBrainz::Server::Controller::Role::Load' => {
     entity_name => 'work',
 };
 
+with 'MusicBrainz::Server::Controller::Role::Alias';
 with 'MusicBrainz::Server::Controller::Role::Annotation';
 with 'MusicBrainz::Server::Controller::Role::Cleanup';
 with 'MusicBrainz::Server::Controller::Role::Details';
 with 'MusicBrainz::Server::Controller::Role::EditListing';
 with 'MusicBrainz::Server::Controller::Role::Rating';
 with 'MusicBrainz::Server::Controller::Role::Tag';
-# with 'MusicBrainz::Server::Controller::Role::Alias';
 # with 'MusicBrainz::Server::Controller::Role::Relationship';
 # with 'MusicBrainz::Server::Controller::Role::WikipediaExtract';
 
@@ -63,7 +63,7 @@ sub show : PathPart('') Chained('load')
 
 # NES - originally:
 # for my $action (qw( relationships aliases tags details )) {
-for my $action (qw( tags details )) {
+for my $action (qw( aliases tags details )) {
     after $action => sub {
         my ($self, $c) = @_;
         my $work = $c->stash->{work};
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 8ec39d7..7205f63 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -11,6 +11,7 @@ sub create {
     my ($self, $edit, $editor, $tree) = @_;
 
     $tree->annotation('') unless $tree->annotation_set;
+    $tree->aliases([]) unless $tree->aliases_set;
 
     my $response = $self->request('/work/create', {
         edit => $edit->id,
@@ -27,7 +28,7 @@ sub update {
     die 'Need a base revision' unless $base_revision;
 
     my $final_tree = do {
-        if( $tree->work_set && $tree->iswcs_set ) {
+        if( $tree->work_set && $tree->aliases_set && $tree->iswcs_set ) {
             $tree
         }
         else {
@@ -36,6 +37,9 @@ sub update {
             $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);
 
@@ -62,7 +66,8 @@ sub view_tree {
     return MusicBrainz::Server::Entity::Tree::Work->new(
         work => $revision,
         iswcs => $self->get_iswcs($revision),
-        annotation => $self->get_annotation($revision)
+        annotation => $self->get_annotation($revision),
+        aliases => $self->get_aliases($revision)
     );
 }
 
@@ -82,7 +87,19 @@ sub _work_tree {
         iswcs => [
             map +{ iswc => $_->iswc }, @{ $tree->iswcs }
         ],
-        annotation => $tree->annotation
+        annotation => $tree->annotation,
+        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 }
+        ]
     );
 }
 
@@ -119,6 +136,27 @@ 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},
+                primary_for_locale => $_->{'primary-for-locale'}
+            )
+        } @$response
+    ]
+}
+
 sub get_iswcs {
     my ($self, $revision) = @_;
     warn "Unimplemented";
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
index 2b18199..8d4c4f7 100644
--- a/lib/MusicBrainz/Server/Entity/Tree/Work.pm
+++ b/lib/MusicBrainz/Server/Entity/Tree/Work.pm
@@ -6,6 +6,11 @@ has work => (
     predicate => 'work_set',
 );
 
+has aliases => (
+    is => 'rw',
+    predicate => 'aliases_set',
+);
+
 has iswcs => (
     is => 'rw',
     predicate => 'iswcs_set',
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/Form/Confirm.pm b/lib/MusicBrainz/Server/Form/Confirm.pm
index 19bd985..e7e6b90 100644
--- a/lib/MusicBrainz/Server/Form/Confirm.pm
+++ b/lib/MusicBrainz/Server/Form/Confirm.pm
@@ -5,6 +5,11 @@ extends 'MusicBrainz::Server::Form';
 with 'MusicBrainz::Server::Form::Role::Edit';
 has '+name' => ( default => 'confirm' );
 
+has_field 'revision_id' => (
+    type => 'Integer',
+    required => 1
+);
+
 sub edit_field_names { () }
 
 1;
diff --git a/root/alias/delete.tt b/root/alias/delete.tt
index 1fd05ae..61bfe11 100644
--- a/root/alias/delete.tt
+++ b/root/alias/delete.tt
@@ -12,5 +12,7 @@
 
 <form action="[% c.req.uri %]" method="post">
   [% INCLUDE 'forms/edit-note.tt' %]
+  [% USE r = FormRenderer(form) %]
+  [% r.hidden(form.field('revision_id')) %]
   [% enter_edit() %]
 </form>
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>
diff --git a/root/components/aliases.tt b/root/components/aliases.tt
index 7f5c6e6..f5faf12 100644
--- a/root/components/aliases.tt
+++ b/root/components/aliases.tt
@@ -45,10 +45,11 @@
                     </td>
                     [% IF c.user_exists %]
                         <td>
-                            <a href="[% c.uri_for_action(edit_path, [ entity.gid, alias.id ]) %]">
+                            [% key = { name => alias.name } %]
+                            <a href="[% c.uri_for_action(edit_path, [ entity.gid ], key) %]">
                               [%- l('Edit') -%]
                             </a> |
-                            <a href="[% c.uri_for_action(delete_path, [ entity.gid, alias.id ]) %]">
+                            <a href="[% c.uri_for_action(delete_path, [ entity.gid ], key) %]">
                               [%- l('Remove') -%]
                             </a>
                         </td>

commit 7f5b98ff3aeef14a0639d989546aba5c4ff7b77e
Author: Oliver Charles <[email protected]>
Date:   Thu Jan 24 23:50:05 2013 +0000

    Add ISWC support for works

diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index 5d30722..7ec1155 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -6,6 +6,7 @@ BEGIN { extends 'MusicBrainz::Server::Controller'; }
 use MusicBrainz::Server::Constants qw(
     $EDIT_WORK_MERGE
 );
+use MusicBrainz::Server::Entity::ISWC;
 use MusicBrainz::Server::Entity::Work;
 use MusicBrainz::Server::Entity::Tree::Work;
 use MusicBrainz::Server::Translation qw( l );
@@ -40,7 +41,7 @@ after 'load' => sub
 
     my $work = $c->stash->{work};
     # $c->model('Work')->load_meta($work);
-    # $c->model('ISWC')->load_for_works($work);
+    $c->model('NES::ISWC')->load_for_works($work);
     if ($c->user_exists) {
         $c->model('Work')->rating->load_user_ratings($c->user->id, $work);
     }
@@ -86,7 +87,10 @@ sub work_tree {
     my $values = shift;
     return MusicBrainz::Server::Entity::Tree::Work->new(
         work => MusicBrainz::Server::Entity::Work->new($values),
-        iswcs => $values->{iswcs} // []
+        iswcs => [
+            map { MusicBrainz::Server::Entity::ISWC->new(iswc => $_) }
+                @{ $values->{iswcs} // [] }
+        ]
     );
 }
 
diff --git a/lib/MusicBrainz/Server/Data/ISWC.pm b/lib/MusicBrainz/Server/Data/ISWC.pm
index 8d9f6c5..dc20a3a 100644
--- a/lib/MusicBrainz/Server/Data/ISWC.pm
+++ b/lib/MusicBrainz/Server/Data/ISWC.pm
@@ -4,7 +4,6 @@ use namespace::autoclean;
 
 use List::MoreUtils qw( uniq );
 use MusicBrainz::Server::Data::Utils qw(
-    object_to_ids
     placeholders
     query_to_list
 );
@@ -38,56 +37,6 @@ sub _entity_class
     return 'MusicBrainz::Server::Entity::ISWC';
 }
 
-=method find_by_work
-
-    find_by_work(@work_ids : Array[Integer])
-
-Find L<MusicBrainz::Server::Entity::ISWC> objects that are linked to specific
-works. The works are searched as a disjunction, so you will get all ISWCS linked
-to any of the inputs.
-
-Returns an array of ISWC objects.
-
-=cut
-
-sub find_by_works
-{
-    my ($self, @work_ids) = @_;
-
-    my $query = "SELECT ".$self->_columns."
-                   FROM ".$self->_table."
-                  WHERE work = any(?)
-                  ORDER BY iswc";
-
-    return query_to_list($self->c->sql, sub { $self->_new_from_row($_[0]) },
-                         $query, \@work_ids);
-}
-
-=method load_for_works
-
-    load_for_works(@works : Array[Work])
-
-Load ISWCs for an array of works, and nest the ISWC objects inside each
-respective work.
-
-=cut
-
-sub load_for_works
-{
-    my ($self, @works) = @_;
-    my %id_to_works = object_to_ids (uniq grep defined, @works);
-    my @ids = keys %id_to_works;
-    return unless @ids; # nothing to do
-    my @iswcs = $self->find_by_works(@ids);
-
-    foreach my $iswc (@iswcs) {
-        foreach my $work (@{ $id_to_works{$iswc->work_id} }) {
-            $work->add_iswc($iswc);
-            $iswc->work($work);
-        }
-    }
-}
-
 =method find_by_iswc
 
     find_by_iswc($iswc : Text)
diff --git a/lib/MusicBrainz/Server/Data/NES/ISWC.pm b/lib/MusicBrainz/Server/Data/NES/ISWC.pm
new file mode 100644
index 0000000..ec01293
--- /dev/null
+++ b/lib/MusicBrainz/Server/Data/NES/ISWC.pm
@@ -0,0 +1,34 @@
+package MusicBrainz::Server::Data::NES::ISWC;
+use Moose;
+use namespace::autoclean;
+
+use MusicBrainz::Server::Data::Utils qw( object_to_revision_ids );
+
+with 'MusicBrainz::Server::Data::Role::NES';
+
+sub load_for_works {
+    my ($self, @works) = @_;
+    my %works_by_revision_id = object_to_revision_ids(@works);
+    my %iswc_map = %{
+        $self->request('/iswc/find-by-works', {
+            revisions => [
+                map +{ revision => $_->revision_id }, @works
+            ]
+        })
+    };
+
+    for my $key (keys %iswc_map) {
+        for my $work (@{ $works_by_revision_id{$key} }) {
+            $work->iswcs([
+                map {
+                    MusicBrainz::Server::Entity::ISWC->new( iswc => $_ )
+                  } @{ $iswc_map{$key} }
+            ]);
+        }
+    }
+
+    return;
+}
+
+__PACKAGE__->meta->make_immutable;
+1;
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index babb287..8ec39d7 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -80,7 +80,7 @@ sub _work_tree {
             }
         },
         iswcs => [
-            map +{ iswc => $_ }, @{ $tree->iswcs }
+            map +{ iswc => $_->iswc }, @{ $tree->iswcs }
         ],
         annotation => $tree->annotation
     );
diff --git a/lib/MusicBrainz/Server/Data/Utils.pm b/lib/MusicBrainz/Server/Data/Utils.pm
index f7a4442..f02b703 100644
--- a/lib/MusicBrainz/Server/Data/Utils.pm
+++ b/lib/MusicBrainz/Server/Data/Utils.pm
@@ -39,6 +39,7 @@ our @EXPORT_OK = qw(
     merge_partial_date
     model_to_type
     object_to_ids
+    object_to_revision_ids
     order_by
     partial_date_to_hash
     placeholders
@@ -341,19 +342,30 @@ sub model_to_type
     return $map{$_[0]} || undef;
 }
 
-sub object_to_ids
+sub _object_to
 {
-    my @objects = @_;
+    my ($getter, @objects) = @_;
     my %ret;
     foreach my $object (@objects)
     {
-        $ret{$object->id} = [] unless $ret{$object->id};
-        push @{ $ret{$object->id} }, $object;
+        my $key = $getter->($object);
+        $ret{$key} = [] unless $ret{$key};
+        push @{ $ret{$key} }, $object;
     }
 
     return %ret;
 }
 
+sub object_to_ids
+{
+    _object_to(sub { shift->id }, @_);
+}
+
+sub object_to_revision_ids
+{
+    _object_to(sub { shift->revision_id }, @_);
+}
+
 sub order_by
 {
     my ($order, $default, $map) = @_;
diff --git a/lib/MusicBrainz/Server/Entity/Work.pm b/lib/MusicBrainz/Server/Entity/Work.pm
index fcafc9c..0e8bec3 100644
--- a/lib/MusicBrainz/Server/Entity/Work.pm
+++ b/lib/MusicBrainz/Server/Entity/Work.pm
@@ -77,7 +77,7 @@ has 'writers' => (
 );
 
 has 'iswcs' => (
-    is => 'ro',
+    is => 'rw',
     isa => 'ArrayRef',
     traits => [ 'Array' ],
     default => sub { [] },

commit b8db95e9622f604b1b39c4aadc93b2867c676c46
Author: Oliver Charles <[email protected]>
Date:   Mon Jan 21 18:23:42 2013 +0000

    Re-add Role::Cleanup to display the 'eligible for cleanup' banner on works

diff --git a/lib/MusicBrainz/Server/Controller/Role/Cleanup.pm b/lib/MusicBrainz/Server/Controller/Role/Cleanup.pm
index c86d1e8..924c886 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Cleanup.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Cleanup.pm
@@ -6,7 +6,7 @@ after show => sub {
     my ($self, $c) = @_;
     my $entity = $c->stash->{entity};
     $c->stash(
-        eligble_for_cleanup => $c->model( $self->config->{model} )->is_empty($entity->id)
+        eligible_for_cleanup => $c->model( $self->config->{model} )->is_empty($entity)
     )
 };
 
diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index 886cd27..5d30722 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -21,12 +21,12 @@ with 'MusicBrainz::Server::Controller::Role::Load' => {
 };
 
 with 'MusicBrainz::Server::Controller::Role::Annotation';
+with 'MusicBrainz::Server::Controller::Role::Cleanup';
 with 'MusicBrainz::Server::Controller::Role::Details';
 with 'MusicBrainz::Server::Controller::Role::EditListing';
 with 'MusicBrainz::Server::Controller::Role::Rating';
 with 'MusicBrainz::Server::Controller::Role::Tag';
 # with 'MusicBrainz::Server::Controller::Role::Alias';
-# with 'MusicBrainz::Server::Controller::Role::Cleanup';
 # with 'MusicBrainz::Server::Controller::Role::Relationship';
 # with 'MusicBrainz::Server::Controller::Role::WikipediaExtract';
 
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 18f6b76..babb287 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -140,5 +140,13 @@ sub load_annotation {
             text => $self->get_annotation($work)));
 }
 
+sub is_empty {
+    my ($self, $work) = @_;
+    return $self->request(
+        '/work/eligible-for-cleanup',
+        { revision => $work->revision_id }
+    )->{eligible};
+}
+
 __PACKAGE__->meta->make_immutable;
 1;
diff --git a/root/artist/index.tt b/root/artist/index.tt
index 6a682b9..5235cc3 100644
--- a/root/artist/index.tt
+++ b/root/artist/index.tt
@@ -26,7 +26,7 @@
 [%- WRAPPER "artist/layout.tt" page='index' -%]
     [%- INCLUDE 'artist/annotation.tt' summary=1 -%]
 
-    [% IF eligble_for_cleanup %]
+    [% IF eligible_for_cleanup %]
       <p class="cleanup">
           [% l('This artist has no relationships, recordings, releases or
                 release groups, and will be removed automatically in the next
diff --git a/root/work/index.tt b/root/work/index.tt
index 5a6f846..8958951 100644
--- a/root/work/index.tt
+++ b/root/work/index.tt
@@ -1,5 +1,5 @@
 [%- WRAPPER "work/layout.tt" page='index' -%]
-    [% IF eligble_for_cleanup %]
+    [% IF eligible_for_cleanup %]
       <p class="cleanup">
           [% l('This work has no relationships and will be removed
                 automatically in the next few days. If this is not intended, please add

commit 70fcfef2332e7d927801625fa14b6dc56796d81c
Author: Oliver Charles <[email protected]>
Date:   Fri Jan 18 12:55:43 2013 +0000

    Allow viewing and editing annotations

diff --git a/lib/MusicBrainz/Server/Controller/Role/Annotation.pm b/lib/MusicBrainz/Server/Controller/Role/Annotation.pm
index 1fa3178..6f05b2b 100644
--- a/lib/MusicBrainz/Server/Controller/Role/Annotation.pm
+++ b/lib/MusicBrainz/Server/Controller/Role/Annotation.pm
@@ -6,6 +6,8 @@ use MusicBrainz::Server::Data::Utils qw( model_to_type );
 use MusicBrainz::Server::Translation qw( l ln );
 use MusicBrainz::Server::Validation qw( is_positive_integer );
 
+use MusicBrainz::Server::NES::Controller::Utils qw( run_update_form );
+
 requires 'load', 'show';
 
 my %model_to_edit_type = (
@@ -23,7 +25,7 @@ after 'load' => sub
     my $entity = $c->stash->{entity};
     my $model = $self->{model};
 
-    $c->model($model)->annotation->load_latest($entity);
+    $c->model($model)->load_annotation($entity);
 };
 
 sub latest_annotation : Chained('load') PathPart('annotation')
@@ -87,31 +89,31 @@ after 'show' => sub
 after 'load' => sub {
     my ($self, $c) = @_;
 
-    my (undef, $no) = $c->model($self->{model})->annotation
-        ->get_history($c->stash->{entity}->id, 50, 0);
+    # my (undef, $no) = $c->model($self->{model})->annotation
+    #     ->get_history($c->stash->{entity}->id, 50, 0);
 
-    $c->stash(
-        number_of_revisions => $no,
-    );
+    # $c->stash(
+    #     number_of_revisions => $no,
+    # );
 };
 
 sub edit_annotation : Chained('load') PathPart RequireAuth Edit
 {
     my ($self, $c) = @_;
+
     my $model = $self->{model};
     my $entity = $c->stash->{entity};
-    my $annotation_model = $c->model($model)->annotation;
-    $annotation_model->load_latest($entity);
+    $c->model($model)->load_annotation($entity);
 
     my $form = $c->form(
-        form             => 'Annotation',
-        init_object      => $entity->latest_annotation,
-        annotation_model => $annotation_model,
-        entity_id        => $entity->id
+        form        => 'Annotation',
+        init_object => {
+            text        => $entity->latest_annotation->text,
+            revision_id => $entity->revision_id
+        },
     );
 
-    if ($c->form_posted && $form->submitted_and_valid($c->req->params))
-    {
+    if ($c->form_posted && $form->submitted_and_valid($c->req->params)) {
         if ($form->field('preview')->input) {
             $c->stash(
                 show_preview => 1,
@@ -120,18 +122,18 @@ sub edit_annotation : Chained('load') PathPart RequireAuth Edit
         }
         else
         {
-            $c->model('MB')->with_transaction(sub {
-                $self->_insert_edit(
-                    $c,
-                    $form,
-                    edit_type => $model_to_edit_type{$model},
-                    (map { $_->name => $_->value } $form->edit_fields),
-                    entity => $entity
-                );
-            });
-
-            my $show = $self->action_for('show');
-            $c->response->redirect($c->uri_for_action($show, [ $entity->gid ]));
+            run_update_form(
+                $self, $c, $form,
+                build_tree => sub {
+                    my $values = shift;
+
+                    return $self->{tree_entity}->new(
+                        annotation => $values->{text}
+                    );
+                }
+            );
+
+            $c->response->redirect($c->uri_for_action($self->action_for('show'), [ $entity->gid ]));
             $c->detach;
         }
     }
diff --git a/lib/MusicBrainz/Server/Controller/Work.pm b/lib/MusicBrainz/Server/Controller/Work.pm
index e50d49e..886cd27 100644
--- a/lib/MusicBrainz/Server/Controller/Work.pm
+++ b/lib/MusicBrainz/Server/Controller/Work.pm
@@ -20,14 +20,14 @@ with 'MusicBrainz::Server::Controller::Role::Load' => {
     entity_name => 'work',
 };
 
+with 'MusicBrainz::Server::Controller::Role::Annotation';
 with 'MusicBrainz::Server::Controller::Role::Details';
 with 'MusicBrainz::Server::Controller::Role::EditListing';
 with 'MusicBrainz::Server::Controller::Role::Rating';
 with 'MusicBrainz::Server::Controller::Role::Tag';
 # with 'MusicBrainz::Server::Controller::Role::Alias';
-# with 'MusicBrainz::Server::Controller::Role::Annotation';
-# with 'MusicBrainz::Server::Controller::Role::Relationship';
 # with 'MusicBrainz::Server::Controller::Role::Cleanup';
+# with 'MusicBrainz::Server::Controller::Role::Relationship';
 # with 'MusicBrainz::Server::Controller::Role::WikipediaExtract';
 
 use aliased 'MusicBrainz::Server::Entity::ArtistCredit';
diff --git a/lib/MusicBrainz/Server/Data/NES/Work.pm b/lib/MusicBrainz/Server/Data/NES/Work.pm
index 3165b1b..18f6b76 100644
--- a/lib/MusicBrainz/Server/Data/NES/Work.pm
+++ b/lib/MusicBrainz/Server/Data/NES/Work.pm
@@ -10,6 +10,8 @@ with 'MusicBrainz::Server::Data::Role::NES';
 sub create {
     my ($self, $edit, $editor, $tree) = @_;
 
+    $tree->annotation('') unless $tree->annotation_set;
+
     my $response = $self->request('/work/create', {
         edit => $edit->id,
         editor => $editor->id,
@@ -37,6 +39,9 @@ sub update {
             $original_tree->iswcs($tree->iswcs)
                 if ($tree->iswcs_set);
 
+            $original_tree->annotation($tree->annotation)
+                if ($tree->annotation_set);
+
             $original_tree;
         }
     };
@@ -57,6 +62,7 @@ sub view_tree {
     return MusicBrainz::Server::Entity::Tree::Work->new(
         work => $revision,
         iswcs => $self->get_iswcs($revision),
+        annotation => $self->get_annotation($revision)
     );
 }
 
@@ -75,7 +81,8 @@ sub _work_tree {
         },
         iswcs => [
             map +{ iswc => $_ }, @{ $tree->iswcs }
-        ]
+        ],
+        annotation => $tree->annotation
     );
 }
 
@@ -118,5 +125,20 @@ sub get_iswcs {
     return [];
 }
 
+sub get_annotation {
+    my ($self, $revision) = @_;
+    return $self->request(
+        '/work/view-annotation',
+        { revision => $revision->revision_id }
+    )->{annotation};
+}
+
+sub load_annotation {
+    my ($self, $work) = @_;
+    $work->latest_annotation(
+        MusicBrainz::Server::Entity::Annotation->new(
+            text => $self->get_annotation($work)));
+}
+
 __PACKAGE__->meta->make_immutable;
 1;
diff --git a/lib/MusicBrainz/Server/Data/Role/NES.pm b/lib/MusicBrainz/Server/Data/Role/NES.pm
index 1d19ec9..17e9e8d 100644
--- a/lib/MusicBrainz/Server/Data/Role/NES.pm
+++ b/lib/MusicBrainz/Server/Data/Role/NES.pm
@@ -23,9 +23,15 @@ sub request {
     my $response = $self->c->lwp->post($uri, Content => encode('utf8', $content));
     my $t = tv_interval($t0);
 
+    printf STDERR "Response in ${t}s\n";
+    printf STDERR $response->content;
+
+    if (!$response->is_success) {
+        printf STDERR "FAILURE!\n";
+        die 'Failed request: ' . $response->content;;
+    }
+
     return try {
-        printf STDERR "Response in ${t}s\n";
-        printf STDERR $response->content;
         printf STDERR "\n\n";
 
         return decode_json($response->content);
diff --git a/lib/MusicBrainz/Server/Entity/Tree/Work.pm b/lib/MusicBrainz/Server/Entity/Tree/Work.pm
index 2e637b4..2b18199 100644
--- a/lib/MusicBrainz/Server/Entity/Tree/Work.pm
+++ b/lib/MusicBrainz/Server/Entity/Tree/Work.pm
@@ -11,4 +11,9 @@ has iswcs => (
     predicate => 'iswcs_set',
 );
 
+has annotation => (
+    is => 'rw',
+    predicate => 'annotation_set'
+);
+
 1;
diff --git a/lib/MusicBrainz/Server/Form/Annotation.pm b/lib/MusicBrainz/Server/Form/Annotation.pm
index e215f6a..c33b60c 100644
--- a/lib/MusicBrainz/Server/Form/Annotation.pm
+++ b/lib/MusicBrainz/Server/Form/Annotation.pm
@@ -9,20 +9,19 @@ has_field 'text' => (
     type     => 'Text',
 );
 
-has_field 'changelog' => (
-    type      => '+MusicBrainz::Server::Form::Field::Text',
-    maxlength => 255,
-    default_over_obj => ''
-);
-
-has 'annotation_model' => (
-    is       => 'ro',
-    required => 1
-);
-
-has 'entity_id' => (
-    is       => 'ro',
-    required => 1
+# has 'annotation_model' => (
+#     is       => 'ro',
+#     required => 1
+# );
+
+# has 'entity_id' => (
+#     is       => 'ro',
+#     required => 1
+# );
+
+has_field 'revision_id' => (
+    type => 'Integer',
+    value => 'required'
 );
 
 has_field 'preview' => (
@@ -32,15 +31,15 @@ has_field 'preview' => (
 
 sub edit_field_names { qw( text changelog ) }
 
-sub validate
-{
-    my ($self) = @_;
+# sub validate
+# {
+#     my ($self) = @_;
 
-    # The "text" field is required only if the previous annotation was blank
-    my $previous_annotanion = $self->annotation_model->get_latest($self->entity_id);
-    $self->field('text')->required($previous_annotanion && $previous_annotanion->text ? 0 : 1);
-    $self->field('text')->validate_field;
-}
+#     # The "text" field is required only if the previous annotation was blank
+#     my $previous_annotanion = $self->annotation_model->get_latest($self->entity_id);
+#     $self->field('text')->required($previous_annotanion && $previous_annotanion->text ? 0 : 1);
+#     $self->field('text')->validate_field;
+# }
 
 __PACKAGE__->meta->make_immutable;
 no Moose;
diff --git a/lib/MusicBrainz/Server/NES/Controller/Utils.pm b/lib/MusicBrainz/Server/NES/Controller/Utils.pm
index ff5ff27..708c098 100644
--- a/lib/MusicBrainz/Server/NES/Controller/Utils.pm
+++ b/lib/MusicBrainz/Server/NES/Controller/Utils.pm
@@ -2,54 +2,77 @@ package MusicBrainz::Server::NES::Controller::Utils;
 use strict;
 use warnings;
 
+use Scalar::Util qw( blessed );
 use Sub::Exporter -setup => {
-    exports => [qw( create_edit create_update )]
+    exports => [qw( create_edit create_update run_edit_form run_update_form )]
 };
 
-sub create_edit {
-    my ($controller, $c, %opts) = @_;
+sub run_edit_form {
+    my ($c, $form, %opts) = @_;
 
-    my $form = do {
-        if (my $build_form = $opts{build_form}) {
-            $build_form->()
-        }
-        else {
-            my $form = do {
-                my %args = (
-                    ctx => $c,
-                );
+    my $values = $form->values;
+    my $edit = $c->model('NES::Edit')->open;
 
-                $args{init_object} = $opts{subject}
-                    if defined $opts{subject};
+    my $work = $opts{on_post}->($values, $edit);
 
-                $c->form(form => $opts{form}, %args);
+    if ($values->{edit_note}) {
+        $c->model('EditNote')->add_note(
+            $edit->id,
+            {
+                editor_id => $c->user->id,
+                text => $values->{edit_note}
             }
-        }
-    };
+        );
+    }
 
-    if ($c->form_posted && $form->submitted_and_valid($c->req->body_params)) {
-        my $values = $form->values;
-        my $edit = $c->model('NES::Edit')->open;
-
-        my $work = $opts{on_post}->($values, $edit);
-
-        if ($values->{edit_note}) {
-            $c->model('EditNote')->add_note(
-                $edit->id,
-                {
-                    editor_id => $c->user->id,
-                    text => $values->{edit_note}
-                }
+    # NES:
+    # my $privs = $c->user->privileges;
+    # if ($c->user->is_auto_editor &&
+    #     $form->field('as_auto_editor') &&
+    #     !$form->field('as_auto_editor')->value) {
+    # }
+
+    return $work;
+}
+
+sub run_update_form {
+    my ($controller, $c, $form, %opts) = @_;
+
+    run_edit_form(
+        $c, $form,
+        on_post => sub {
+            my ($values, $edit) = @_;
+
+            my $revision = $c->model( $controller->{model} )->get_revision(
+                $values->{revision_id});
+
+            $c->model( $controller->{model} )->update(
+                $edit, $c->user, $revision,
+                $opts{build_tree}->($values, $revision)
             );
+
+            return $revision
         }
+    );
+}
 
+sub _run_form {
+    my ($controller, $c, %opts) = @_;
 
-        # NES:
-        # my $privs = $c->user->privileges;
-        # if ($c->user->is_auto_editor &&
-        #     $form->field('as_auto_editor') &&
-        #     !$form->field('as_auto_editor')->value) {
-        # }
+    my $form = $opts{form};
+    $form = do {
+        my %args = (
+            ctx => $c,
+        );
+
+        $args{init_object} = $opts{subject}
+            if defined $opts{subject};
+
+        $c->form(form => $form, %args);
+    } unless blessed($form);
+
+    if ($c->form_posted && $form->submitted_and_valid($c->req->body_params)) {
+        my $work = $opts{callback}->($form);
 
         $c->response->redirect(
             $c->uri_for_action($controller->action_for('show'), [ $work->gid ]));
@@ -60,22 +83,26 @@ sub create_edit {
     }
 }
 
-sub create_update {
+sub create_edit {
     my ($controller, $c, %opts) = @_;
-    create_edit(
+    _run_form(
         $controller, $c,
         %opts,
-        on_post => sub {
-            my ($values, $edit) = @_;
-            my $revision = $c->model( $controller->{model} )->get_revision(
-                $values->{revision_id});
-
-            $c->model( $controller->{model} )->update(
-                $edit, $c->user, $revision,
-                $opts{build_tree}->($values, $revision)
-            );
+        callback => sub {
+            my $form = shift;
+            run_edit_form($c, $form, %opts);
+        }
+    );
+}
 
-            return $revision;
+sub create_update {
+    my ($controller, $c, %opts) = @_;
+    _run_form(
+        $controller, $c,
+        %opts,
+        callback => sub {
+            my $form = shift;
+            run_update_form($controller, $c, $form, %opts);
         }
     );
 }
diff --git a/root/annotation/edit.tt b/root/annotation/edit.tt
index ae26e56..bd9e507 100644
--- a/root/annotation/edit.tt
+++ b/root/annotation/edit.tt
@@ -14,6 +14,7 @@
     [% WRAPPER form_row %]
       [% r.label('text', l('Annotation:')) %]
       [% r.textarea('text', { cols => 80, rows => 10 }) %]
+      [% r.hidden(form.field('revision_id')) %]
       [% field_errors(r.form, 'text') %]
     [% END %]
 

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


hooks/post-receive
-- 
mb_server