• 
      

    Allow datagrid Column subclasses to set attributes on the class level.

    Review Request #14824 — Created Feb. 17, 2026 and submitted

    Information

    Djblets
    release-5.x

    Reviewers

    Historically, Column subclasses had to override __init__ and set
    attributes there. This was a bit messy and error-prone, as it was common
    to mix up positional and keyword arguments when subclassing, and made
    for harder customization for more complex class hierarchies.

    Now, subclasses can set these arguments directly on the class as
    attributes. The base Column class has defaults for all these fields,
    which can be overridden by subclasses and further overridden when
    calling __init__(). Built-in subclasses also provide their own fitting
    this pattern.

    To make this work, the arguments in __init__() are all set to UNSET
    by default. If UNSET, class attributes (or more complex default
    calculations) are used, but if any value is explicitly provided, it will
    be used instead.

    To help avoid the mess of positional and keyword arguments in these
    classes, all arguments other than label are now keyword-only, with
    positional arguments deprecated and scheduled for removal in Djblets 7.

    This is now the recommended approach for column subclasses. It has the
    benefit of storing less state per-instance, as well, as we only set the
    values on the instance if they're explicitly provided during
    construction.

    As an added bonus, we've deprecated cell_clickable which is confusing
    and has never been largely ignored by the Djblets code. It's used by
    the template to inject an onclick="javascript:window.location = ...",
    but there's no way to set it without overriding it on a column instance.

    Unit tests pass.

    Tested updating existing columns to use this, and verified the new
    settings were correct.

    Summary ID
    Allow datagrid Column subclasses to set attributes on the class level.
    Historically, `Column` subclasses had to override `__init__` and set attributes there. This was a bit messy and error-prone, as it was common to mix up positional and keyword arguments when subclassing, and made for harder customization for more complex class hierarchies. Now, subclasses can set these arguments directly on the class as attributes. The base `Column` class has defaults for all these fields, which can be overridden by subclasses and further overridden when calling `__init__()`. Built-in subclasses also provide their own fitting this pattern. To make this work, the arguments in `__init__()` are all set to `UNSET` by default. If `UNSET`, class attributes (or more complex default calculations) are used, but if any value is explicitly provided, it will be used instead. To help avoid the mess of positional and keyword arguments in these classes, all arguments other than `label` are now keyword-only, with positional arguments deprecated and scheduled for removal in Djblets 7. This is now the recommended approach for column subclasses. It has the benefit of storing less state per-instance, as well, as we only set the values on the instance if they're explicitly provided during construction. As an added bonus, we've deprecated `cell_clickable` which is confusing and has never been largely ignored by the Djblets code. It's used by the template to inject an `onclick="javascript:window.location = ..."`, but there's no way to set it without overriding it on a column instance.
    c7feb1be9cedf8c56814fb0e9e0cd613fc328214
    Description From Last Updated

    Should we do format_html(self.detailed_label) here to be safe?

    maubinmaubin

    Can you add a Raises section for this in the docstring.

    maubinmaubin

    How come we're setting this to False instead of using the cell_clickable arg?

    maubinmaubin

    This is a no-op. We should just have: if detailed_label is UNSET: detailed_label = self.detailed_label

    daviddavid

    In DateTimeSinceColumn you included label in the kwarg-only section. Should we do that here too?

    daviddavid

    This needs to come after the *args docstring.

    maubinmaubin

    I think we still want to pass label positionally to the parent __init__(), and then pass sortable=sortable after *args.

    maubinmaubin

    Can we emit a DeprecationWarning if this is passed in?

    daviddavid

    This class doesn't have a test case to test the init method, like the others do.

    daviddavid

    Copy-pasteo: DateTimeColumn -> DateTimeSinceColumn

    daviddavid

    Can we assert on label in here as well?

    daviddavid

    Can we add 'label': 'My Column' to this? (this would catch the label issue that Michelle flagged)

    daviddavid

    We might want to make this if not (self.label or label):, because otherwise if label is passed in, we do …

    daviddavid
    maubin
    1. 
        
    2. djblets/datagrid/grids.py (Diff revision 1)
       
       
      Show all issues

      Should we do format_html(self.detailed_label) here to be safe?

    3. djblets/datagrid/grids.py (Diff revision 1)
       
       
       
       
       
      Show all issues

      Can you add a Raises section for this in the docstring.

    4. djblets/datagrid/grids.py (Diff revision 1)
       
       
      Show all issues

      How come we're setting this to False instead of using the cell_clickable arg?

      1. Had to look into this, because it's what we were doing before.

        This attribute controls whether we do a onclick="javascript:window.location = ...", which I remember from ancient times, and oh boy is it ancient. 2007. And it's been hard-coded to False since it was introduced.

        This basically has just never been used, and we should just deprecate the field entirely. But I want to hold off on that in this change.

    5. 
        
    david
    1. 
        
    2. djblets/datagrid/grids.py (Diff revision 1)
       
       
      Show all issues

      This is a no-op. We should just have:

      if detailed_label is UNSET:
          detailed_label = self.detailed_label
      
      1. Ah thanks. I had iterated on that code a bit.

    3. djblets/datagrid/grids.py (Diff revision 1)
       
       
      Show all issues

      In DateTimeSinceColumn you included label in the kwarg-only section. Should we do that here too?

      1. That was a mistake. label is positional. It should just be in *args, but that'll have to wait for the deprecation to be complete.

    4. 
        
    chipx86
    maubin
    1. 
        
    2. djblets/datagrid/grids.py (Diff revisions 1 - 2)
       
       
       
       
       
      Show all issues

      This needs to come after the *args docstring.

    3. djblets/datagrid/grids.py (Diff revisions 1 - 2)
       
       
       
      Show all issues

      I think we still want to pass label positionally to the parent __init__(), and then pass sortable=sortable after *args.

    4. 
        
    david
    1. 
        
    2. djblets/datagrid/grids.py (Diff revision 2)
       
       
      Show all issues

      Can we emit a DeprecationWarning if this is passed in?

    3. djblets/datagrid/tests.py (Diff revision 2)
       
       
      Show all issues

      This class doesn't have a test case to test the init method, like the others do.

    4. djblets/datagrid/tests.py (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      Can we add 'label': 'My Column' to this? (this would catch the label issue that Michelle flagged)

    5. 
        
    chipx86
    maubin
    1. Ship It!
    2. 
        
    david
    1. 
        
    2. djblets/datagrid/tests.py (Diff revisions 2 - 3)
       
       
      Show all issues

      Copy-pasteo: DateTimeColumn -> DateTimeSinceColumn

    3. djblets/datagrid/tests.py (Diff revisions 2 - 3)
       
       
      Show all issues

      Can we assert on label in here as well?

    4. djblets/datagrid/grids.py (Diff revision 3)
       
       
      Show all issues

      We might want to make this if not (self.label or label):, because otherwise if label is passed in, we do this format_html and then immediately overwrite it in super().__init__()

    5. 
        
    chipx86
    maubin
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.x (bc257f8)