Fix track number 0 not displaying in chat announcements (#1205)

Author: aw-was-here

nowplaying/kick/chat.py

@@ -239,7 +239,7 @@ async def run_chat(self, oauth_handler: nowplaying.kick.oauth2.KickOAuth2) -> No
     @staticmethod
     def _finalize(variable: Any) -> str:
         ''' helper routine to avoid NoneType exceptions '''
-        if variable:
+        if variable is not None:
             return variable
         return ''
 

nowplaying/metadata.py

@@ -582,7 +582,7 @@ def _got_tag(self, tag: object) -> None:
                 'composer', 'disc', 'disc_total', 'duration', 'genre', 'lang', 'publisher', 'title',
                 'track', 'track_total', 'label'
         ]:
-            if key not in self.metadata and hasattr(tag, key) and getattr(tag, key):
+            if key not in self.metadata and hasattr(tag, key) and getattr(tag, key) is not None:
                 self.metadata[key] = str(getattr(tag, key))
 
         # Handle the 'key' field separately to decode JSON if needed

nowplaying/twitch/chat.py

@@ -324,7 +324,7 @@ async def handle_request(self, command, params, username):  # pylint: disable=un
     @staticmethod
     def _finalize(variable):
         ''' helper routine to avoid NoneType exceptions '''
-        if variable:
+        if variable is not None:
             return variable
         return ''
 

nowplaying/utils.py

@@ -116,7 +116,7 @@ def __init__(self, filename=None):
     @staticmethod
     def _finalize(variable):
         ''' helper routine to avoid NoneType exceptions '''
-        if variable:
+        if variable is not None:
             return variable
         return ''
 

tests/templates/tracktest.txt

@@ -0,0 +1,2 @@
+{% if track %}Track: {{ track }}{% endif %}
+{% if disc %}Disc: {{ disc }}{% endif %}

tests/test_metadata.py

@@ -5,14 +5,15 @@
 import logging
 import multiprocessing
 import sqlite3
+import types
 
 import pytest
 import pytest_asyncio
 
 import nowplaying.bootstrap  # pylint: disable=import-error
 import nowplaying.metadata  # pylint: disable=import-error
 import nowplaying.upgrade  # pylint: disable=import-error
-import nowplaying.imagecache  # pylint: disable=import-error
+import nowplaying.imagecache  # pylint: disable=import-error,no-member
 
 
 @pytest_asyncio.fixture
@@ -24,7 +25,7 @@ async def get_imagecache(bootstrap):
     dbdir.mkdir()
     logpath = config.testdir.joinpath('debug.log')
     stopevent = multiprocessing.Event()
-    imagecache = nowplaying.imagecache.ImageCache(cachedir=dbdir, stopevent=stopevent)
+    imagecache = nowplaying.imagecache.ImageCache(cachedir=dbdir, stopevent=stopevent)  # pylint: disable=no-member
     icprocess = multiprocessing.Process(target=imagecache.queue_process,
                                         name='ICProcess',
                                         args=(
@@ -767,3 +768,27 @@ def test_decode_musical_key(input_value, expected_output):
     """Test the _decode_musical_key method handles various key formats."""
     result = nowplaying.metadata.TinyTagRunner._decode_musical_key(input_value)  # pylint: disable=protected-access
     assert result == expected_output
+
+
+@pytest.mark.parametrize(
+    "test_value,expected_result",
+    [
+        (0, True),  # 0 should be processed (our fix)
+        (1, True),  # 1 should be processed (normal case)
+        (None, False),  # None should be ignored
+        ('0', True),  # String '0' should be processed
+        ('', True),  # Empty string should be processed (only None is filtered)
+    ])
+def test_metadata_handles_zero_values(test_value, expected_result):
+    """Test that metadata processing handles 0 values correctly."""
+    # Create a minimal mock tag object that focuses on testing the specific condition
+    mock_tag = types.SimpleNamespace(track=test_value)
+
+    # Test the specific condition that was problematic
+    has_attr = hasattr(mock_tag, 'track')
+    value_check = getattr(mock_tag, 'track') is not None  # Our fix: was getattr(mock_tag, 'track')
+
+    should_process = has_attr and value_check
+
+    assert should_process == expected_result, \
+        f"Value {test_value} should {'be processed' if expected_result else 'be ignored'}"

tests/test_templatehandler.py

@@ -7,7 +7,7 @@
 import pytest
 
 import nowplaying.utils  # pylint: disable=import-error
-import nowplaying.textoutput  # pylint: disable=import-error
+import nowplaying.textoutput  # pylint: disable=import-error,no-member
 
 
 @pytest.fixture
@@ -33,7 +33,7 @@ def test_writingmeta(gettemplatehandler):  # pylint: disable=redefined-outer-nam
             'title': 'this is the title',
         }
 
-        nowplaying.textoutput.writetxttrack(filename=filename,
+        nowplaying.textoutput.writetxttrack(filename=filename,  # pylint: disable=no-member
                                             templatehandler=gettemplatehandler,
                                             metadata=metadata)
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
@@ -51,7 +51,7 @@ def test_missingmeta(gettemplatehandler):  # pylint: disable=redefined-outer-nam
 
         metadata = {}
 
-        nowplaying.textoutput.writetxttrack(filename=filename,
+        nowplaying.textoutput.writetxttrack(filename=filename,  # pylint: disable=no-member
                                             templatehandler=gettemplatehandler,
                                             metadata=metadata)
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
@@ -71,7 +71,7 @@ def test_missingtemplate(gettemplatehandler):  # pylint: disable=redefined-outer
             'title': 'this is the title',
         }
 
-        nowplaying.textoutput.writetxttrack(filename=filename,
+        nowplaying.textoutput.writetxttrack(filename=filename,  # pylint: disable=no-member
                                             templatehandler=gettemplatehandler,
                                             metadata=metadata)
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
@@ -90,7 +90,7 @@ def test_missingfilename(gettemplatehandler):  # pylint: disable=redefined-outer
             'title': 'this is the title',
         }
 
-        nowplaying.textoutput.writetxttrack(filename=filename,
+        nowplaying.textoutput.writetxttrack(filename=filename,  # pylint: disable=no-member
                                             templatehandler=gettemplatehandler,
                                             metadata=metadata)
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
@@ -103,7 +103,7 @@ def test_cleartemplate():  # pylint: disable=redefined-outer-name
     ''' try writing a text '''
     with tempfile.TemporaryDirectory() as newpath:
         filename = os.path.join(newpath, 'test.txt')
-        nowplaying.textoutput.writetxttrack(filename=filename, clear=True)
+        nowplaying.textoutput.writetxttrack(filename=filename, clear=True)  # pylint: disable=no-member
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
             content = tempfn.readlines()
 
@@ -114,8 +114,47 @@ def test_justafile():  # pylint: disable=redefined-outer-name
     ''' try writing a text '''
     with tempfile.TemporaryDirectory() as newpath:
         filename = os.path.join(newpath, 'test.txt')
-        nowplaying.textoutput.writetxttrack(filename=filename)
+        nowplaying.textoutput.writetxttrack(filename=filename)  # pylint: disable=no-member
         with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
             content = tempfn.readlines()
 
         assert content[0] == '{{ artist }} - {{ title }}'
+
+
+@pytest.mark.templatesettings(template='tracktest.txt')
+@pytest.mark.parametrize(
+    "track_value,disc_value,expected_track,expected_disc",
+    [
+        ('0', '0', True, True),  # Track 0 and disc 0 should show
+        ('1', '1', True, True),  # Track 1 and disc 1 should show
+        ('', '', False, False),  # Empty strings should not show
+        (None, None, False, False),  # None values should not show
+    ])
+def test_track_disc_handling(  # pylint: disable=redefined-outer-name
+                             gettemplatehandler, track_value, disc_value, expected_track,
+                             expected_disc):
+    ''' test track and disc number handling with various values '''
+    with tempfile.TemporaryDirectory() as newpath:
+        filename = os.path.join(newpath, 'test.txt')
+
+        metadata = {}
+        if track_value is not None:
+            metadata['track'] = track_value
+        if disc_value is not None:
+            metadata['disc'] = disc_value
+
+        nowplaying.textoutput.writetxttrack(filename=filename,  # pylint: disable=no-member
+                                            templatehandler=gettemplatehandler,
+                                            metadata=metadata)
+        with open(filename) as tempfn:  # pylint: disable=unspecified-encoding
+            content = tempfn.read()
+
+        if expected_track:
+            assert f'Track: {track_value}' in content
+        else:
+            assert 'Track:' not in content
+
+        if expected_disc:
+            assert f'Disc: {disc_value}' in content
+        else:
+            assert 'Disc:' not in content