[mb-commits] branch, mbs-5265, updated. MBS-5265, unset cover art on release group when the associated release is del...
MusicBrainz Git Server <[email protected]>
| Newsgroups | gmane.comp.audio.musicbrainz.cvs |
|---|---|
| Message-ID | <E1TTUEo-0001w1-Iq@wiley> |
The branch, mbs-5265 has been updated
via http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=ade18f54a5fca12010837b8652a9a2e2ef758e39 (commit)
via http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=4f36845715e03ac703834801e921482824c1f3e5 (commit)
via http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=700cc933433afa813664c4309c7ab4d9c0f86e19 (commit)
via http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=6242ff3f09a0d21cbd5521663563acce513e5b60 (commit)
from http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=9f2f6ab205b3b0ee82a693e77f81b46dec52055b (commit)
Summary of changes:
lib/MusicBrainz/Server/Controller/ReleaseGroup.pm | 12 ++--
lib/MusicBrainz/Server/Data/Release.pm | 5 ++
lib/MusicBrainz/Server/Data/ReleaseGroup.pm | 57 +++++++++++++++++
lib/MusicBrainz/Server/Data/Utils.pm | 3 +-
lib/MusicBrainz/Server/Entity/ReleaseGroup.pm | 3 +
root/release_group/layout.tt | 2 +
root/release_group/set_cover_art.tt | 15 ++++-
t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm | 67 +++++++++++++++++++++
8 files changed, 155 insertions(+), 9 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 ade18f54a5fca12010837b8652a9a2e2ef758e39
Author: warp <[email protected]>
Date: Wed Oct 31 10:04:56 2012 +0100
MBS-5265, unset cover art on release group when the associated release is deleted.
diff --git a/lib/MusicBrainz/Server/Data/Release.pm b/lib/MusicBrainz/Server/Data/Release.pm
index 1a21243..3a3ceb0 100644
--- a/lib/MusicBrainz/Server/Data/Release.pm
+++ b/lib/MusicBrainz/Server/Data/Release.pm
@@ -572,6 +572,10 @@ sub delete
$self->sql->do('DELETE FROM release_label WHERE release IN (' . placeholders(@release_ids) . ')',
@release_ids);
+ $self->sql->do('DELETE FROM cover_art_archive.release_group_cover_art ' .
+ 'WHERE release IN (' . placeholders(@release_ids) . ')',
+ @release_ids);
+
my @mediums = @{
$self->sql->select_single_column_array(
'SELECT id FROM medium WHERE release IN (' . placeholders(@release_ids) . ')',
diff --git a/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm b/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
index fbe5e86..f67128f 100644
--- a/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
+++ b/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
@@ -185,4 +185,24 @@ test 'Merge releases in the same release group where the release group has cover
is_deeply ($results, $expected, "release group cover art updated after merge");
};
+test 'Delete release which is set as cover art for a release group' => sub {
+
+ my $test = shift;
+ MusicBrainz::Server::Test->prepare_test_database($test->c, '+releasegroup');
+
+ $test->c->sql->do("INSERT INTO cover_art_archive.release_group_cover_art " .
+ "(release_group, release) VALUES (4, 4), (5, 5);");
+
+ $test->c->model('Release')->delete (4);
+
+ my $results = $test->c->sql->select_list_of_hashes (
+ "SELECT release_group, release
+ FROM cover_art_archive.release_group_cover_art
+ ORDER BY release_group, release");
+
+ my $expected = [ { release_group => 5, release => 5 } ];
+
+ is_deeply ($results, $expected, "release group cover art unset after release has been deleted");
+};
+
1;
commit 4f36845715e03ac703834801e921482824c1f3e5
Author: warp <[email protected]>
Date: Mon Oct 29 16:14:31 2012 +0100
MBS-5265, Update release group cover art when merging releases.
diff --git a/lib/MusicBrainz/Server/Data/Release.pm b/lib/MusicBrainz/Server/Data/Release.pm
index 523eaa0..1a21243 100644
--- a/lib/MusicBrainz/Server/Data/Release.pm
+++ b/lib/MusicBrainz/Server/Data/Release.pm
@@ -724,6 +724,7 @@ sub merge
$self->annotation->merge($new_id, @old_ids);
$self->c->model('Collection')->merge_releases($new_id, @old_ids);
$self->c->model('ReleaseLabel')->merge_releases($new_id, @old_ids);
+ $self->c->model('ReleaseGroup')->merge_releases($new_id, @old_ids);
$self->c->model('Edit')->merge_entities('release', $new_id, @old_ids);
$self->c->model('Relationship')->merge_entities('release', $new_id, @old_ids);
$self->c->model('CoverArtArchive')->merge_releases($new_id, @old_ids);
diff --git a/lib/MusicBrainz/Server/Data/ReleaseGroup.pm b/lib/MusicBrainz/Server/Data/ReleaseGroup.pm
index 632ef2a..0095e47 100644
--- a/lib/MusicBrainz/Server/Data/ReleaseGroup.pm
+++ b/lib/MusicBrainz/Server/Data/ReleaseGroup.pm
@@ -618,6 +618,63 @@ sub set_cover_art {
$release_id, $rg_id, $rg_id, $release_id, $rg_id);
}
+sub unset_cover_art {
+ my ($self, $rg_id) = @_;
+
+ $self->sql->do("DELETE FROM cover_art_archive.release_group_cover_art
+ WHERE release_group = ?", $rg_id);
+}
+
+sub merge_releases {
+ my ($self, $new_id, @old_ids) = @_;
+
+ my $rg_ids = $self->c->sql->select_list_of_hashes (
+ "SELECT release_group, id FROM release WHERE id IN ("
+ . placeholders ($new_id, @old_ids) . ")", $new_id, @old_ids);
+
+ my %release_rg;
+ my %release_group_ids;
+ for my $row (@$rg_ids) {
+ $release_rg{$row->{id}} = $row->{release_group};
+ $release_group_ids{$row->{release_group}} = 1;
+ };
+
+ my @release_group_ids = keys %release_group_ids;
+ my $rg_cover_art = $self->c->sql->select_list_of_hashes (
+ "SELECT release_group, release
+ FROM cover_art_archive.release_group_cover_art
+ WHERE release_group IN (" . placeholders (@release_group_ids) . ")",
+ @release_group_ids);
+
+ my %has_cover_art = map { $_->{release_group} => $_->{release} } @$rg_cover_art;
+
+ my $new_rg = $release_rg{$new_id};
+ for my $old_id (@old_ids)
+ {
+ my $old_rg = $release_rg{$old_id};
+
+ if ($new_rg == $old_rg)
+ {
+ # The new release group is the same as the old release group
+ # - if the release group cover art is set to one of the old ids,
+ # move it to the new id.
+ $self->set_cover_art ($new_rg, $new_id)
+ if $has_cover_art{$new_rg} == $old_id;
+ }
+ else
+ {
+ # The new release group is different from the old release group
+ # - if the old release group cover art is set to the id being moved,
+ # unset the old cover art
+ $self->unset_cover_art ($old_rg)
+ if $has_cover_art{$old_rg} == $old_id;
+
+ # Do not change the new release group cover art, regardless of
+ # whether it is set or not.
+ }
+ }
+}
+
__PACKAGE__->meta->make_immutable;
no Moose;
1;
diff --git a/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm b/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
index 5a44a58..fbe5e86 100644
--- a/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
+++ b/t/lib/t/MusicBrainz/Server/Data/ReleaseGroup.pm
@@ -138,4 +138,51 @@ EOSQL
ok(!defined $test->c->model('ReleaseGroup')->get_by_id(1));
};
+test 'Merge releases in seperate release groups where release groups have cover art set' => sub {
+
+ my $test = shift;
+ MusicBrainz::Server::Test->prepare_test_database($test->c, '+releasegroup');
+
+ $test->c->sql->do("INSERT INTO cover_art_archive.release_group_cover_art " .
+ "(release_group, release) VALUES (4, 4), (5, 5);");
+
+ ok( $test->c->model('Release')->merge (
+ new_id => 4, old_ids => [ 5 ],
+ merge_strategy => $MusicBrainz::Server::Data::Release::MERGE_MERGE
+ ), "Merge releases with cover art");
+
+ my $results = $test->c->sql->select_list_of_hashes (
+ "SELECT release_group, release
+ FROM cover_art_archive.release_group_cover_art
+ ORDER BY release_group, release");
+
+ my $expected = [ { release_group => 4, release => 4 } ];
+
+ is_deeply ($results, $expected, "release group cover art unset for rg id 5");
+};
+
+test 'Merge releases in the same release group where the release group has cover art set' => sub {
+
+ my $test = shift;
+ MusicBrainz::Server::Test->prepare_test_database($test->c, '+releasegroup');
+
+ $test->c->sql->do("UPDATE release SET release_group = 4 WHERE id = 5");
+ $test->c->sql->do("INSERT INTO cover_art_archive.release_group_cover_art " .
+ "(release_group, release) VALUES (4, 5)");
+
+ ok( $test->c->model('Release')->merge (
+ new_id => 4, old_ids => [ 5 ],
+ merge_strategy => $MusicBrainz::Server::Data::Release::MERGE_MERGE
+ ), "Merge releases with cover art");
+
+ my $results = $test->c->sql->select_list_of_hashes (
+ "SELECT release_group, release
+ FROM cover_art_archive.release_group_cover_art
+ ORDER BY release_group, release");
+
+ my $expected = [ { release_group => 4, release => 4 } ];
+
+ is_deeply ($results, $expected, "release group cover art updated after merge");
+};
+
1;
commit 700cc933433afa813664c4309c7ab4d9c0f86e19
Author: warp <[email protected]>
Date: Mon Oct 29 13:18:19 2012 +0100
MBS-5265, Don't show "Set Cover Art" form when no cover art can be set for a release group.
diff --git a/lib/MusicBrainz/Server/Controller/ReleaseGroup.pm b/lib/MusicBrainz/Server/Controller/ReleaseGroup.pm
index 2b4c8ff..7604126 100644
--- a/lib/MusicBrainz/Server/Controller/ReleaseGroup.pm
+++ b/lib/MusicBrainz/Server/Controller/ReleaseGroup.pm
@@ -43,6 +43,7 @@ after 'load' => sub
}
$c->model('ReleaseGroupType')->load($rg);
$c->model('ArtistCredit')->load($rg);
+ $c->model('Artwork')->load_for_release_groups ($rg);
$c->stash( can_delete => $c->model('ReleaseGroup')->can_delete($rg->id) );
};
@@ -62,7 +63,6 @@ sub show : Chained('load') PathPart('')
$c->model('ReleaseLabel')->load(@$releases);
$c->model('Label')->load(map { $_->all_labels } @$releases);
$c->model('ReleaseStatus')->load(@$releases);
- $c->model('Artwork')->load_for_release_groups ($rg);
$c->model('Relationship')->load($rg);
$c->stash(
@@ -133,21 +133,23 @@ sub set_cover_art : Chained('load') PathPart('set-cover-art') Args(0) Edit Requi
my ($self, $c, $id) = @_;
my $entity = $c->stash->{entity};
+ return unless $entity->can_set_cover_art;
+
my ($releases, $hits) = $c->model ('Release')->find_by_release_group (
$entity->id);
my $artwork = $c->model ('Artwork')->find_front_cover_by_release (@$releases);
$c->model ('CoverArtType')->load_for (@$artwork);
- $c->model('Artwork')->load_for_release_groups ($entity);
- my $form = $c->form(form => 'ReleaseGroup::SetCoverArt',
- init_object => { release => $entity->cover_art->release->gid });
+ my $cover_art_release = $entity->cover_art ? $entity->cover_art->release : undef;
+ my $form = $c->form(form => 'ReleaseGroup::SetCoverArt', init_object => {
+ release => $cover_art_release ? $cover_art_release->gid : undef });
my $form_valid = $c->form_posted && $form->submitted_and_valid($c->req->params);
my $release = $form_valid
? $c->model ('Release')->get_by_gid ($form->field('release')->value)
- : $entity->cover_art->release;
+ : $cover_art_release;
$c->stash({ form => $form, artwork => $artwork, release => $release });
diff --git a/lib/MusicBrainz/Server/Entity/ReleaseGroup.pm b/lib/MusicBrainz/Server/Entity/ReleaseGroup.pm
index b0352a1..8358f44 100644
--- a/lib/MusicBrainz/Server/Entity/ReleaseGroup.pm
+++ b/lib/MusicBrainz/Server/Entity/ReleaseGroup.pm
@@ -86,6 +86,9 @@ has 'cover_art' => (
predicate => 'has_cover_art',
);
+# Cannot set cover art if none of the associated releases has cover art.
+sub can_set_cover_art { return shift->has_cover_art; }
+
__PACKAGE__->meta->make_immutable;
no Moose;
1;
diff --git a/root/release_group/layout.tt b/root/release_group/layout.tt
index 6b03a66..1ff2064 100644
--- a/root/release_group/layout.tt
+++ b/root/release_group/layout.tt
@@ -36,11 +36,13 @@
[% l('Add release') %]
</a>
</li>
+ [%- IF rg.can_set_cover_art -%]
<li>
<a href="[% c.uri_for_action('/release_group/set_cover_art', [ rg.gid ]) %]">
[% l('Set cover art') %]
</a>
</li>
+ [%- END -%]
<hr/>
diff --git a/root/release_group/set_cover_art.tt b/root/release_group/set_cover_art.tt
index ace5e05..00ad554 100644
--- a/root/release_group/set_cover_art.tt
+++ b/root/release_group/set_cover_art.tt
@@ -1,8 +1,10 @@
[%- BLOCK layout_head -%]
- [% script_manifest('edit.js.manifest') %]
+ [%- script_manifest('edit.js.manifest') -%]
[%- END -%]
-[% WRAPPER 'release_group/layout.tt' title=l('Set Release Group Cover Art') full_width=1 page='edit' %]
+[%- WRAPPER 'release_group/layout.tt' title=l('Set Release Group Cover Art') full_width=1 page='edit' -%]
+
+[%- IF entity.can_set_cover_art -%]
<form id="set-cover-art" class="set-cover-art" action="[% c.req.uri %]" method="post">
[%- USE r = FormRenderer(form) -%]
@@ -46,4 +48,11 @@
</form>
[%# INCLUDE 'release_group/edit_form.tt' %]
-[% END %]
+
+[%- ELSE -%]
+ <p>
+ [%- l('No releases have cover art, cannot set cover art.') -%]
+ </p>
+[%- END -%]
+
+[%- END -%]
commit 6242ff3f09a0d21cbd5521663563acce513e5b60
Author: warp <[email protected]>
Date: Mon Oct 29 12:11:03 2012 +0100
MBS-5265, fix "Use of uninitialized value $offset" warning.
diff --git a/lib/MusicBrainz/Server/Data/Utils.pm b/lib/MusicBrainz/Server/Data/Utils.pm
index 4d57340..e20570a 100644
--- a/lib/MusicBrainz/Server/Data/Utils.pm
+++ b/lib/MusicBrainz/Server/Data/Utils.pm
@@ -211,7 +211,8 @@ sub query_to_list_limited
my $obj = $builder->($row);
push @result, $obj;
}
- my $hits = $sql->row_count + $offset;
+
+ my $hits = $sql->row_count + ($offset || 0);
$sql->finish;
return (\@result, $hits);
}
-----------------------------------------------------------------------
hooks/post-receive
--
mb_server