[mb-commits] branch, data-nes, updated. Allow viewing and editing annotations

MusicBrainz Git Server <[email protected]> Sun, 20 Jan 2013 14:56:34 +0000
Newsgroups gmane.comp.audio.musicbrainz.cvs
Message-ID <E1TwwK2-0007hy-4P@wiley>
The branch, data-nes has been updated
  discards  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=ffa4c53140e9c4425f6442216c3e93953326562a (commit)
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=541cd4d96573361b80e834b3b972a023d317f2d4 (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 (ffa4c53140e9c4425f6442216c3e93953326562a)
            \
             N -- N -- N (541cd4d96573361b80e834b3b972a023d317f2d4)

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/Data/NES/Work.pm |    2 +-
 1 files changed, 1 insertions(+), 1 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 541cd4d96573361b80e834b3b972a023d317f2d4
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..bd7b95c 100644
--- a/lib/MusicBrainz/Server/NES/Controller/Utils.pm
+++ b/lib/MusicBrainz/Server/NES/Controller/Utils.pm
@@ -2,54 +2,75 @@ 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) {
+    # }
+}
+
+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) = @_;
+
+    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);
 
-        # NES:
-        # my $privs = $c->user->privileges;
-        # if ($c->user->is_auto_editor &&
-        #     $form->field('as_auto_editor') &&
-        #     !$form->field('as_auto_editor')->value) {
-        # }
+    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 +81,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