Fix our album summary info when some tracks have classical tags but o…

…thers don't

CHANGELOG.rst

@@ -5,6 +5,9 @@
   from the web UI and correct association across possibly-
   inconsistent tags.  Specifically: İ, ğ, and ş.  Also fixed
   normalizing filenames (for zipfile downloads) for capital Ç.
+- Fixed album summary information when some tracks have classical
+  music tags (ensemble, composer, conductor) but other tracks
+  don't.  (Explicitly say that not all tracks have the tags.)
 
 1.0.3 (2016-11-22)
 ------------------

exordium/models.py

@@ -253,20 +253,38 @@ def get_secondary_artists_tuple(self):
         Since these are inherent to Songs, not Albums, the best we
         can really do is just loop through 'em.
 
-        The tuple order is: (groups, conductors, composers)
+        Included in the tuple are a set of booleans detailing if
+        there are tracks without group/conductor/composer tags.
+
+        The tuple order is: (groups, have_empty_group,
+            conductors, have_empty_conductor,
+            composers, have_empty_composer)
         """
         groups = {}
         conductors = {}
         composers = {}
+        have_empty_group = False
+        have_empty_conductor = False
+        have_empty_composer = False
         for song in self.song_set.all():
-            if song.group and song.group not in groups:
-                groups[song.group] = True
-            if song.conductor and song.conductor not in conductors:
-                conductors[song.conductor] = True
-            if song.composer and song.composer not in composers:
-                composers[song.composer] = True
-        return (sorted(groups.keys()), sorted(conductors.keys()),
-                sorted(composers.keys()))
+            if song.group:
+                if song.group not in groups:
+                    groups[song.group] = True
+            else:
+                have_empty_group = True
+            if song.conductor:
+                if song.conductor not in conductors:
+                    conductors[song.conductor] = True
+            else:
+                have_empty_conductor = True
+            if song.composer:
+                if song.composer not in composers:
+                    composers[song.composer] = True
+            else:
+                have_empty_composer = True
+        return (sorted(groups.keys()), have_empty_group,
+                sorted(conductors.keys()), have_empty_conductor,
+                sorted(composers.keys()), have_empty_composer)
 
     def get_album_image(self):
         """

exordium/templates/exordium/album.html

@@ -21,19 +21,19 @@
 {% for artist in groups %}
 {% if forloop.first %}<p>Ensemble{{ groups|pluralize }}: {% endif %}
 <strong><a href="{% url 'exordium:artist' artist.normname %}">{{ artist }}</a></strong>{% if not forloop.last %},{% endif %}
-{% if forloop.last %}</p>{% endif %}
+{% if forloop.last %}{% if have_empty_group %} <em>(Some tracks have no ensemble)</em>{% endif %}</p>{% endif %}
 {% endfor %}
 
 {% for artist in conductors %}
 {% if forloop.first %}<p>Conductor{{ conductors|pluralize }}: {% endif %}
 <strong><a href="{% url 'exordium:artist' artist.normname %}">{{ artist }}</a></strong>{% if not forloop.last %},{% endif %}
-{% if forloop.last %}</p>{% endif %}
+{% if forloop.last %}{% if have_empty_conductor %} <em>(Some tracks have no conductor)</em>{% endif %}</p>{% endif %}
 {% endfor %}
 
 {% for artist in composers %}
 {% if forloop.first %}<p>Composer{{ composers|pluralize }}: {% endif %}
 <strong><a href="{% url 'exordium:artist' artist.normname %}">{{ artist }}</a></strong>{% if not forloop.last %},{% endif %}
-{% if forloop.last %}</p>{% endif %}
+{% if forloop.last %}{% if have_empty_composer %} <em>(Some tracks have no composer)</em>{% endif %}</p>{% endif %}
 {% endfor %}
 
 <p>Album: <strong>{{ album }}</strong></p>

exordium/templates/exordium/song_artist_column.html

@@ -1,5 +1,5 @@
 {# vim: set syntax=htmldjango: #}
-<a href="{% url 'exordium:artist' record.artist.normname %}">{{ record.artist }}</a>{% if record.group and record.num_groups != 1%},
+<a href="{% url 'exordium:artist' record.artist.normname %}">{{ record.artist }}</a>{% if record.group and record.num_groups != 1 %},
 Ensemble: <a href="{% url 'exordium:artist' record.group.normname %}">{{ record.group }}</a>{% endif %}{% if record.conductor and record.num_conductors != 1%},
 Conductor: <a href="{% url 'exordium:artist' record.conductor.normname %}">{{ record.conductor }}</a>{% endif %}{% if record.composer and record.num_composers != 1 %},
 Composer: <a href="{% url 'exordium:artist' record.composer.normname %}">{{ record.composer }}</a>{% endif %}

exordium/tests.py

@@ -8287,6 +8287,9 @@ def test_fully_tagged_album(self):
         self.assertQuerysetEqual(response.context['groups'], [repr(group)])
         self.assertQuerysetEqual(response.context['composers'], [repr(composer)])
         self.assertQuerysetEqual(response.context['conductors'], [repr(conductor)])
+        self.assertEqual(response.context['have_empty_group'], False)
+        self.assertEqual(response.context['have_empty_composer'], False)
+        self.assertEqual(response.context['have_empty_conductor'], False)
         self.assertContains(response, 'Ensemble:')
         self.assertContains(response, 'Conductor:')
         self.assertContains(response, 'Composer:')
@@ -8349,6 +8352,9 @@ def test_fully_tagged_album_two_tracks(self):
         self.assertQuerysetEqual(response.context['groups'], [repr(group) for group in groups])
         self.assertQuerysetEqual(response.context['composers'], [repr(composer) for composer in composers])
         self.assertQuerysetEqual(response.context['conductors'], [repr(conductor) for conductor in conductors])
+        self.assertEqual(response.context['have_empty_group'], False)
+        self.assertEqual(response.context['have_empty_composer'], False)
+        self.assertEqual(response.context['have_empty_conductor'], False)
         self.assertContains(response, 'Ensembles:')
         self.assertContains(response, 'Conductors:')
         self.assertContains(response, 'Composers:')
@@ -8415,6 +8421,9 @@ def test_fully_tagged_album_two_tracks_various(self):
         self.assertQuerysetEqual(response.context['groups'], [repr(group) for group in groups])
         self.assertQuerysetEqual(response.context['composers'], [repr(composer) for composer in composers])
         self.assertQuerysetEqual(response.context['conductors'], [repr(conductor) for conductor in conductors])
+        self.assertEqual(response.context['have_empty_group'], False)
+        self.assertEqual(response.context['have_empty_composer'], False)
+        self.assertEqual(response.context['have_empty_conductor'], False)
         self.assertContains(response, 'Ensembles:')
         self.assertContains(response, 'Conductors:')
         self.assertContains(response, 'Composers:')
@@ -8431,6 +8440,65 @@ def test_fully_tagged_album_two_tracks_various(self):
         self.assertContains(response, '"?sort=tracknum"')
         self.assertContains(response, '2 items')
 
+    def test_album_some_tracks_with_classical_tags_others_without(self):
+        """
+        Tests display of an album in which one track has classical tags defined
+        but the other does not.  The classical tags should be displayed up in
+        the top header, but also with a note that some tracks don't have the
+        tags.  Technically we should have a check for making sure the tracks
+        themselves are showing the information, though we can't really do that
+        without hooking into selenium or whatever.
+        """
+        self.add_mp3(artist='Artist', title='Title 1', tracknum=1,
+            year=2016, group='Group', conductor='Conductor',
+            composer='Composer',
+            album='Album', filename='song1.mp3')
+        self.add_mp3(artist='Artist', title='Title 2', tracknum=2,
+            year=2016, album='Album', filename='song2.mp3')
+        self.run_add()
+
+        self.assertEqual(Album.objects.count(), 1)
+        album = Album.objects.get()
+
+        self.assertEqual(Song.objects.count(), 2)
+        songs = [
+            Song.objects.get(filename='song1.mp3'),
+            Song.objects.get(filename='song2.mp3'),
+        ]
+
+        artist = Artist.objects.get(name='Artist')
+        group = Artist.objects.get(name='Group')
+        conductor = Artist.objects.get(name='Conductor')
+        composer = Artist.objects.get(name='Composer')
+
+        response = self.client.get(reverse('exordium:album', args=(album.pk,)))
+        self.assertEqual(response.status_code, 200)
+        self.assertQuerysetEqual(response.context['songs'].data, [repr(song) for song in songs])
+        self.assertQuerysetEqual(response.context['groups'], [repr(group)])
+        self.assertQuerysetEqual(response.context['composers'], [repr(composer)])
+        self.assertQuerysetEqual(response.context['conductors'], [repr(conductor)])
+        self.assertEqual(response.context['have_empty_group'], True)
+        self.assertEqual(response.context['have_empty_composer'], True)
+        self.assertEqual(response.context['have_empty_conductor'], True)
+        self.assertContains(response, 'Ensemble:')
+        self.assertContains(response, 'Conductor:')
+        self.assertContains(response, 'Composer:')
+        self.assertContains(response, 'Some tracks have no ensemble')
+        self.assertContains(response, 'Some tracks have no conductor')
+        self.assertContains(response, 'Some tracks have no composer')
+        for a in [artist, group, conductor, composer]:
+            self.assertContains(response, reverse('exordium:artist', args=(a.normname,)))
+            self.assertContains(response, str(a))
+        self.assertContains(response, str(album))
+        self.assertContains(response, str(album.artist))
+        self.assertContains(response, 'Year: <strong>2016</strong>')
+        self.assertContains(response, 'Tracks: <strong>2</strong>')
+        self.assertContains(response, 'Length: <strong>0:04</strong>')
+        for song in songs:
+            self.assertContains(response, song.title)
+        self.assertContains(response, '"?sort=tracknum"')
+        self.assertContains(response, '2 items')
+
     def test_miscellaneous_album(self):
         """
         Test a miscellaneous (non-album-tracks) album.  The only real difference

exordium/views.py

@@ -284,11 +284,22 @@ class AlbumView(TitleDetailView):
     def get_context_data(self, **kwargs):
         context = super(AlbumView, self).get_context_data(**kwargs)
         songs = Song.objects.filter(album=self.object).order_by('tracknum')
-        (groups, conductors, composers) = self.object.get_secondary_artists_tuple()
+        (groups, have_empty_group, conductors, have_empty_conductor,
+            composers, have_empty_composer) = self.object.get_secondary_artists_tuple()
         data = []
+        len_groups = len(groups)
+        len_conductors = len(conductors)
+        len_composers = len(composers)
+        if have_empty_group:
+            len_groups += 1
+        if have_empty_conductor:
+            len_conductors += 1
+        if have_empty_composer:
+            len_composers += 1
         for song in songs:
-            song.set_album_secondary_artist_counts(num_groups=len(groups),
-                num_conductors=len(conductors), num_composers=len(composers))
+            song.set_album_secondary_artist_counts(num_groups=len_groups,
+                num_conductors=len_conductors,
+                num_composers=len_composers)
             data.append(song)
         if self.object.miscellaneous:
             table = SongTableNoAlbumNoTracknum(data)
@@ -302,6 +313,9 @@ def get_context_data(self, **kwargs):
         context['groups'] = groups
         context['conductors'] = conductors
         context['composers'] = composers
+        context['have_empty_group'] = have_empty_group
+        context['have_empty_conductor'] = have_empty_conductor
+        context['have_empty_composer'] = have_empty_composer
         return context
 
 class AlbumDownloadView(TitleDetailView):
@@ -311,7 +325,8 @@ class AlbumDownloadView(TitleDetailView):
     def get_context_data(self, **kwargs):
         context = super(AlbumDownloadView, self).get_context_data(**kwargs)
         context['show_download_button'] = False
-        (groups, conductors, composers) = self.object.get_secondary_artists_tuple()
+        (groups, have_empty_group, conductors, have_empty_conductor,
+            composers, have_empty_composer) = self.object.get_secondary_artists_tuple()
         if App.support_zipfile():
             try:
                 (filenames, zipfile) = self.object.create_zip()
@@ -338,6 +353,9 @@ def get_context_data(self, **kwargs):
         context['groups'] = groups
         context['conductors'] = conductors
         context['composers'] = composers
+        context['have_empty_group'] = have_empty_group
+        context['have_empty_conductor'] = have_empty_conductor
+        context['have_empty_composer'] = have_empty_composer
         return context
 
 class BrowseArtistView(TitleListView):