Merge pull request #680 from tcely/issue-362-sponsorblock_categories

Rework `sponsorblock_categories` and `CommaSepChoiceField`

tubesync/sync/fields.py

@@ -1,6 +1,8 @@
-from django.forms import MultipleChoiceField,  CheckboxSelectMultiple, Field, TypedMultipleChoiceField
+from collections import namedtuple
-from django.db import models
+from functools import lru_cache
-from typing import Any, Optional, Dict
+from typing import Any, Dict
+from django import forms
+from django.db import connection, models
 from django.utils.translation import gettext_lazy as _
 
 
@@ -22,103 +24,174 @@ class SponsorBlock_Category(models.TextChoices):
     MUSIC_OFFTOPIC = 'music_offtopic', _( 'Non-Music Section' )
 
 
+CommaSepChoice = namedtuple(
+    'CommaSepChoice', [
+        'allow_all',
+        'all_choice',
+        'all_label',
+        'possible_choices',
+        'selected_choices',
+        'separator',
+    ],
+    defaults = (
+        False,
+        None,
+        'All',
+        list(),
+        list(),
+        ',',
+    ),
+)
+
 # this is a form field!
-class CustomCheckboxSelectMultiple(CheckboxSelectMultiple):
+class CustomCheckboxSelectMultiple(forms.CheckboxSelectMultiple):
     template_name = 'widgets/checkbox_select.html'
     option_template_name = 'widgets/checkbox_option.html'
+    # perhaps set the 'selected' attribute too?
+    # checked_attribute = {'checked': True, 'selected': True}
 
     def get_context(self, name: str, value: Any, attrs) -> Dict[str, Any]:
-        ctx = super().get_context(name, value, attrs)['widget']
+        data = value
-        ctx["multipleChoiceProperties"] = []
+        select_all = False
-        for _group, options, _index in ctx["optgroups"]:
+        if isinstance(data, CommaSepChoice):
-            for option in options:
+            select_all = (data.allow_all and data.all_choice in data.selected_choices)
-                if not isinstance(value,str) and not isinstance(value,list) and ( option["value"] in value.selected_choices or ( value.allow_all and value.all_choice in value.selected_choices ) ):
+            value = list(data.selected_choices)
-                    checked = True
+        context = super().get_context(name, value, attrs)
-                else:
+        widget = context['widget']
-                    checked = False
+        options = widget['optgroups']
+        # This is a new key in widget
+        widget['multipleChoiceProperties'] = list()
+        for _group, single_option_list, _index in options:
+            for option in single_option_list:
+                option['selected'] |= select_all
+                widget['multipleChoiceProperties'].append(option)
 
-                ctx["multipleChoiceProperties"].append({
+        return { 'widget': widget }
-                    "template_name": option["template_name"],
-                    "type": option["type"],
-                    "value": option["value"],
-                    "label": option["label"],
-                    "name": option["name"],
-                    "checked": checked})
 
-        return { 'widget': ctx }
 
 # this is a database field!
-class CommaSepChoiceField(models.Field):
+class CommaSepChoiceField(models.CharField):
-    "Implements comma-separated storage of lists"
+    '''
+    Implements comma-separated storage of lists
+    '''
 
-    # If 'text' isn't correct add the vendor override here.
+    form_class = forms.MultipleChoiceField
-    _DB_TYPES = {}
+    widget = CustomCheckboxSelectMultiple
+    from common.logger import log
 
     def __init__(self, *args, separator=",", possible_choices=(("","")), all_choice="", all_label="All", allow_all=False, **kwargs):
-        super().__init__(*args, **kwargs)
+        kwargs.setdefault('max_length', 128)
         self.separator = str(separator)
-        self.possible_choices = possible_choices
+        self.possible_choices = possible_choices or choices
-        self.selected_choices = []
+        self.selected_choices = list()
         self.allow_all = allow_all
         self.all_label = all_label
         self.all_choice = all_choice
+        self.choices = self.get_all_choices()
+        super().__init__(*args, **kwargs)
+        self.validators.clear()
 
+
+    # Override these functions to prevent unwanted behaviors
+    def to_python(self, value):
+        return value
+
+    def get_internal_type(self):
+        return super().get_internal_type()
+
+
+    # standard functions for this class
     def deconstruct(self):
+        # set it back to the default for models.Field
+        # this way it is never in the returned values
+        self.choices = None
         name, path, args, kwargs = super().deconstruct()
+        self.choices = self.get_all_choices()
         if ',' != self.separator:
             kwargs['separator'] = self.separator
         kwargs['possible_choices'] = self.possible_choices
-        return name, path, args, kwargs
-
-    def db_type(self, connection):
-        value = self._DB_TYPES.get(connection.vendor, None)
-        return value if value is not None else 'text'
-
-    def get_my_choices(self):
-        choiceArray = []
-        if self.possible_choices is None:
-            return choiceArray
         if self.allow_all:
-            choiceArray.append((self.all_choice, _(self.all_label)))
+            kwargs['allow_all'] = self.allow_all
-
+            if self.all_choice:
-        for t in self.possible_choices:
+                kwargs['all_choice'] = self.all_choice
-            choiceArray.append(t)
+            if 'All' != self.all_label:
-
+                kwargs['all_label'] = self.all_label
-        return choiceArray
+        return name, path, args, kwargs
 
     def formfield(self, **kwargs):
         # This is a fairly standard way to set up some defaults
         # while letting the caller override them.
-        defaults = {'form_class': MultipleChoiceField, 
+        defaults = {
-                    'choices': self.get_my_choices,
+            'form_class': self.form_class,
-                    'widget': CustomCheckboxSelectMultiple,
+            # 'choices_form_class': self.form_class,
-                    'label': '',
+            'widget': self.widget,
-                    'required': False}
+            # use a callable for choices
+            'choices': self.get_all_choices,
+            'label': '',
+            'required': False,
+        }
+        # Keep the part from CharField we want,
+        # then call Field to skip the 'max_length' entry.
+        db_empty_string_as_null = connection.features.interprets_empty_strings_as_nulls
+        if self.null and not db_empty_string_as_null:
+            defaults['empty_value'] = None
         defaults.update(kwargs)
-        return super().formfield(**defaults)
+        return models.Field.formfield(self, **defaults)
+        # This is a more compact way to do the same thing
+        # return super().formfield(**{
+        #     'form_class': self.form_class,
+        #     **kwargs,
+        # })
 
-    def from_db_value(self, value, expr, conn):
+    @lru_cache(maxsize=10)
-        if 0 == len(value) or value is None:
+    def from_db_value(self, value, expression, connection):
-            self.selected_choices = []
+        '''
-        else:
+        Create a data structure to be used in Python code.
-            self.selected_choices = value.split(self.separator)
 
-        return self
+        This is called quite often with the same input,
+        because the database value doesn't change often.
+        So, it's being cached to prevent excessive logging.
+        '''
+        self.log.debug(f'fdbv:1: {type(value)} {repr(value)}')
+        if isinstance(value, str) and len(value) > 0:
+            value = value.split(self.separator)
+        if not isinstance(value, list):
+            value = list()
+        self.selected_choices = value
+        args_dict = {key: self.__dict__[key] for key in CommaSepChoice._fields}
+        return CommaSepChoice(**args_dict)
 
     def get_prep_value(self, value):
-        if value is None:
+        '''
-            return ""
+        Create a value to be stored in the database.
-        if not isinstance(value,list):
+        '''
-            return ""
+        data = value
+        if not isinstance(data, CommaSepChoice):
+            # The data was lost; we can regenerate it.
+            args_dict = {key: self.__dict__[key] for key in CommaSepChoice._fields}
+            args_dict['selected_choices'] = list(value)
+            data = CommaSepChoice(**args_dict)
+        value = data.selected_choices
+        s_value = super().get_prep_value(value)
+        if set(s_value) != set(value):
+            self.log.warn(f'CommaSepChoiceField:get_prep_value: values did not match. '
+                          f'CommaSepChoiceField({value}) versus CharField({s_value})')
+        if not isinstance(value, list):
+            return ''
+        if data.all_choice in value:
+            return data.all_choice
+        return data.separator.join(value)
 
-        if self.all_choice not in value:
+    # extra functions not used by any parent classes
-            return self.separator.join(value)
+    def get_all_choices(self):
-        else:
+        choice_list = list()
-            return self.all_choice
+        if self.possible_choices is None:
+            return choice_list
+        if self.allow_all:
+            choice_list.append((self.all_choice, _(self.all_label)))
+
+        for choice in self.possible_choices:
+            choice_list.append(choice)
+
+        return choice_list
 
-    def get_text_for_value(self, val):
-        fval = [i for i in self.possible_choices if i[0] == val]
-        if len(fval) <= 0:
-            return []
-        else:
-            return fval[0][1]

tubesync/sync/migrations/0027_alter_source_sponsorblock_categories.py

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='source',
             name='sponsorblock_categories',
-            field=sync.fields.CommaSepChoiceField(default='all', help_text='Select the sponsorblocks you want to enforce', possible_choices=[('sponsor', 'Sponsor'), ('intro', 'Intermission/Intro Animation'), ('outro', 'Endcards/Credits'), ('selfpromo', 'Unpaid/Self Promotion'), ('preview', 'Preview/Recap'), ('filler', 'Filler Tangent'), ('interaction', 'Interaction Reminder'), ('music_offtopic', 'Non-Music Section')], verbose_name=''),
+            field=sync.fields.CommaSepChoiceField(all_choice='all', all_label='(All Categories)', allow_all=True, default='all', help_text='Select the SponsorBlock categories that you wish to be removed from downloaded videos.', max_length=128, possible_choices=[('sponsor', 'Sponsor'), ('intro', 'Intermission/Intro Animation'), ('outro', 'Endcards/Credits'), ('selfpromo', 'Unpaid/Self Promotion'), ('preview', 'Preview/Recap'), ('filler', 'Filler Tangent'), ('interaction', 'Interaction Reminder'), ('music_offtopic', 'Non-Music Section')], verbose_name=''),
         ),
     ]

tubesync/sync/models.py

@@ -118,12 +118,13 @@ class Source(models.Model):
 
     sponsorblock_categories = CommaSepChoiceField(
         _(''),
+        max_length=128,
         possible_choices=SponsorBlock_Category.choices,
         all_choice='all',
         allow_all=True,
-        all_label='(all options)',
+        all_label='(All Categories)',
         default='all',
-        help_text=_('Select the sponsorblocks you want to enforce')
+        help_text=_('Select the SponsorBlock categories that you wish to be removed from downloaded videos.')
     )
     embed_metadata = models.BooleanField(
         _('embed metadata'),

tubesync/sync/templates/widgets/checkbox_option.html

@@ -1,7 +1,7 @@
 <!--<input type="{{ option.type }}" name="{{ option.name }}" value="{{ option.value }}" id="{{ option.value }}"><BR>
     <label for="{{ option.value }}">{{option.label}}</label>-->
 
-<label>
+<div>
-    <input type="{{ option.type }}" name="{{ option.name }}" value="{{ option.value }}" id="{{ option.value }}" {% if option.checked %}checked{% endif %}>
+    <input type="{{ option.type }}" name="{{ option.name }}" value="{{ option.value }}" id="{{ option.value }}" {% if option.selected %}checked{% endif %}>
     <span>{{option.label}}</span>
-</label>
+</div>

tubesync/sync/tests.py

@@ -172,6 +172,8 @@ class FrontEndTestCase(TestCase):
         response = c.get('/source-add')
         self.assertEqual(response.status_code, 200)
         # Create a new source
+        data_categories = ('sponsor', 'preview',)
+        exected_categories = ['sponsor', 'preview']
         data = {
             'source_type': 'c',
             'key': 'testkey',
@@ -190,6 +192,7 @@ class FrontEndTestCase(TestCase):
             'prefer_60fps': False,
             'prefer_hdr': False,
             'fallback': 'f',
+            'sponsorblock_categories': data_categories,
             'sub_langs': 'en',
         }
         response = c.post('/source-add', data)
@@ -203,6 +206,9 @@ class FrontEndTestCase(TestCase):
         source_uuid = path_parts[1]
         source = Source.objects.get(pk=source_uuid)
         self.assertEqual(str(source.pk), source_uuid)
+        # Check that the SponsorBlock categories were saved
+        self.assertEqual(source.sponsorblock_categories.selected_choices,
+                         exected_categories)
         # Check a task was created to index the media for the new source
         source_uuid = str(source.pk)
         task = Task.objects.get_task('sync.tasks.index_source_task',
@@ -215,6 +221,13 @@ class FrontEndTestCase(TestCase):
         # Check the source detail page loads
         response = c.get(f'/source/{source_uuid}')
         self.assertEqual(response.status_code, 200)
+        # save and refresh the Source
+        source.refresh_from_db()
+        source.save()
+        source.refresh_from_db()
+        # Check that the SponsorBlock categories remain saved
+        self.assertEqual(source.sponsorblock_categories.selected_choices,
+                         exected_categories)
         # Update the source key
         data = {
             'source_type': Source.SOURCE_TYPE_YOUTUBE_CHANNEL,
@@ -234,6 +247,7 @@ class FrontEndTestCase(TestCase):
             'prefer_60fps': False,
             'prefer_hdr': False,
             'fallback': Source.FALLBACK_FAIL,
+            'sponsorblock_categories': data_categories,
             'sub_langs': 'en',
         }
         response = c.post(f'/source-update/{source_uuid}', data)
@@ -247,6 +261,10 @@ class FrontEndTestCase(TestCase):
         source_uuid = path_parts[1]
         source = Source.objects.get(pk=source_uuid)
         self.assertEqual(source.key, 'updatedkey')
+        # Check that the SponsorBlock categories remain saved
+        source.refresh_from_db()
+        self.assertEqual(source.sponsorblock_categories.selected_choices,
+                         exected_categories)
         # Update the source index schedule which should recreate the scheduled task
         data = {
             'source_type': Source.SOURCE_TYPE_YOUTUBE_CHANNEL,
@@ -266,6 +284,7 @@ class FrontEndTestCase(TestCase):
             'prefer_60fps': False,
             'prefer_hdr': False,
             'fallback': Source.FALLBACK_FAIL,
+            'sponsorblock_categories': data_categories,
             'sub_langs': 'en',
         }
         response = c.post(f'/source-update/{source_uuid}', data)
@@ -278,6 +297,9 @@ class FrontEndTestCase(TestCase):
         self.assertEqual(path_parts[0], 'source')
         source_uuid = path_parts[1]
         source = Source.objects.get(pk=source_uuid)
+        # Check that the SponsorBlock categories remain saved
+        self.assertEqual(source.sponsorblock_categories.selected_choices,
+                         exected_categories)
         # Check a new task has been created by seeing if the pk has changed
         new_task = Task.objects.get_task('sync.tasks.index_source_task',
                                          args=(source_uuid,))[0]