Modernize datagrid columns, fix bugs, and add JSON serialization.

Review Request #14844 — Created Feb. 20, 2026 and updated

Information

Review Board
release-7.1.x

Reviewers

This is a major update to the existing datagrid columns, largely
preparing them for the new Dashboard API resource, and fixing up a
number of issues in their implementation.

All columns now provide a stable JSON representation that can be used in
the API, returning structured data, API-compatible objects (which will
be turned into payloads or links), or raw values.

In some cases, this is done by overriding the new to_json(), but in
most we just now calculate the state for both the JSON and HTML
representations in the column-controlled get_raw_object_value(). This
keeps the data calculation in one place, and simplifies both output
functions (often using default logic).

HTML rendering has been fixed in all places to correctly use
format_html() or escape() for results. This will future-proof these
columns for when the datagrid code starts escaping non-SafeStrings.

Most of the __init__() methods are now gone, instead using the new
class-defined attributes. This is much easier to maintain and
specialize.

Types have been fixed and annotated all throughout, catching issues in
some of the logic.

Many bugs (such as extra spaces at the end of values, wrong data types
being returned where strings are expected) that weren't previously
caught have been fixed, with tests updated to strictly check return
types.

Unit tests pass.

Tested all columns in the dashboard in both the browser and with
a test script using the in-progress dashboard API.

Summary ID
Modernize datagrid columns, fix bugs, and add JSON serialization.
This is a major update to the existing datagrid columns, largely preparing them for the new Dashboard API resource, and fixing up a number of issues in their implementation. All columns now provide a stable JSON representation that can be used in the API, returning structured data, API-compatible objects (which will be turned into payloads or links), or raw values. In some cases, this is done by overriding the new `to_json()`, but in most we just now calculate the state for both the JSON and HTML representations in the column-controlled `get_raw_object_value()`. This keeps the data calculation in one place, and simplifies both output functions (often using default logic). HTML rendering has been fixed in many places to correctly use `format_html()` when we're constructing actual HTML to display. This will future-proof these columns for when the datagrid code starts escaping non-SafeStrings. Most of the `__init__()` methods are now gone, instead using the new class-defined attributes. This is much easier to maintain and specialize. Types have been fixed and annotated all throughout, catching issues in some of the logic. Many bugs (such as extra spaces at the end of values, wrong data types being returned where strings are expected) that weren't previously caught have been fixed, with tests updated to strictly check return types.
72d15e72a7b1924eebb0dc388815883dbbb0e2fd
Description From Last Updated

This should be bool

daviddavid

This should be list of dict and explain what the dict return value contains.

daviddavid

We should use a walrus operator to pull out diffset_history and last_diff_updated instead of accessing them each time.

maubinmaubin

This should be Group

daviddavid

This should just say "The review request being ..."

maubinmaubin

These should probably have a default of None

daviddavid

This is returning an unclosed <div>

daviddavid

This should be Group

daviddavid

This should probably pass a default of 0

daviddavid

This should probably have a default of ''

daviddavid

This should probably have a default of VISIBLE

daviddavid

This description doesn't apply here.

maubinmaubin

This wasn't sortable before, but is now. I don't think that was intentional given the contents of this column.

daviddavid

This looks like it's supposed to be the description for the ToMeColumn, we should move it there.

maubinmaubin

This was copy/pasted from BugsColumn without being updated.

daviddavid

Typos: josn -> json

daviddavid

Typos: josn -> json

daviddavid

'django.utils.html.conditional_escape' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

We're no longer setting link_func. It seems like Column.__init__ will set it to the datagrid's link_to_object, not the column's link_to_object.

daviddavid

This should be called "group" instead of "user"

daviddavid

We're using getattr without defaults, which could raise AttributeError. Seems like maybe we want to default to False?

daviddavid

Same here re: link_func

daviddavid

Another getattr without a default.

daviddavid

Another getattr without a default.

daviddavid

Another getattr without a default.

daviddavid

This was copy/pasted from something else.

daviddavid
david
  1. 
      
  2. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should be bool

  3. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
    Show all issues

    This should be list of dict and explain what the dict return value contains.

  4. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should be Group

  5. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
    Show all issues

    These should probably have a default of None

    1. Here and others below are all safe. If we end up in a situation where this field isn't there, then the entire datagrid queryset machinery is broken, and it's useful to know that.

  6. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This is returning an unclosed <div>

  7. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should be Group

  8. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should probably pass a default of 0

  9. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should probably have a default of ''

  10. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should probably have a default of VISIBLE

  11. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This wasn't sortable before, but is now. I don't think that was intentional given the contents of this column.

  12. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    This was copy/pasted from BugsColumn without being updated.

  13. Show all issues

    Typos: josn -> json

  14. Show all issues

    Typos: josn -> json

  15. 
      
maubin
  1. 
      
  2. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
    Show all issues

    We should use a walrus operator to pull out diffset_history and last_diff_updated instead of accessing them each time.

  3. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
    Show all issues

    This should just say "The review request being ..."

  4. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
     
    Show all issues

    This description doesn't apply here.

  5. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
    Show all issues

    This looks like it's supposed to be the description for the ToMeColumn, we should move it there.

  6. 
      
chipx86
Review request changed
Change Summary:
  • Fixed several doc issues and typos.
  • Fixed bad rendering of the Diff Last Updated column value and improved tests there.
  • Added specific types for some of the raw values.
  • Fixed unclosed HTML tags.
  • Reverted being able to sort the Diff Size column. This was accidental.
  • Made use of a walrus operator to pull data out for the Diff Last Updated value.
Commits:
Summary ID
Modernize datagrid columns, fix bugs, and add JSON serialization.
This is a major update to the existing datagrid columns, largely preparing them for the new Dashboard API resource, and fixing up a number of issues in their implementation. All columns now provide a stable JSON representation that can be used in the API, returning structured data, API-compatible objects (which will be turned into payloads or links), or raw values. In some cases, this is done by overriding the new `to_json()`, but in most we just now calculate the state for both the JSON and HTML representations in the column-controlled `get_raw_object_value()`. This keeps the data calculation in one place, and simplifies both output functions (often using default logic). HTML rendering has been fixed in many places to correctly use `format_html()` when we're constructing actual HTML to display. This will future-proof these columns for when the datagrid code starts escaping non-SafeStrings. Most of the `__init__()` methods are now gone, instead using the new class-defined attributes. This is much easier to maintain and specialize. Types have been fixed and annotated all throughout, catching issues in some of the logic. Many bugs (such as extra spaces at the end of values, wrong data types being returned where strings are expected) that weren't previously caught have been fixed, with tests updated to strictly check return types.
826620197224da1889c20928695d233654abe1f7
Modernize datagrid columns, fix bugs, and add JSON serialization.
This is a major update to the existing datagrid columns, largely preparing them for the new Dashboard API resource, and fixing up a number of issues in their implementation. All columns now provide a stable JSON representation that can be used in the API, returning structured data, API-compatible objects (which will be turned into payloads or links), or raw values. In some cases, this is done by overriding the new `to_json()`, but in most we just now calculate the state for both the JSON and HTML representations in the column-controlled `get_raw_object_value()`. This keeps the data calculation in one place, and simplifies both output functions (often using default logic). HTML rendering has been fixed in many places to correctly use `format_html()` when we're constructing actual HTML to display. This will future-proof these columns for when the datagrid code starts escaping non-SafeStrings. Most of the `__init__()` methods are now gone, instead using the new class-defined attributes. This is much easier to maintain and specialize. Types have been fixed and annotated all throughout, catching issues in some of the logic. Many bugs (such as extra spaces at the end of values, wrong data types being returned where strings are expected) that weren't previously caught have been fixed, with tests updated to strictly check return types.
0bc60fb26398a6b7df0d21751a62516979d1a68a

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. 
      
  2. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    We're no longer setting link_func. It seems like Column.__init__ will set it to the datagrid's link_to_object, not the column's link_to_object.

  3. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
     
    Show all issues

    This should be called "group" instead of "user"

  4. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
     
    Show all issues

    We're using getattr without defaults, which could raise AttributeError. Seems like maybe we want to default to False?

    1. These are correct and discussed in the prior review when you flagged these. These are provided as part of the queryset. If these blow up, something fundamentally broke with the datagrids preparation procedures, and we absolutely want to know that.

  5. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
    Show all issues

    Same here re: link_func

  6. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
    Show all issues

    Another getattr without a default.

  7. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
    Show all issues

    Another getattr without a default.

  8. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
    Show all issues

    Another getattr without a default.

  9. reviewboard/datagrids/columns.py (Diff revision 2)
     
     
    Show all issues

    This was copy/pasted from something else.

  10. 
      
chipx86
Review request changed
Change Summary:
  • Restored the broken link_func values.
  • Fixed some bad docs.
  • Fixed an incorrect variable name.
Commits:
Summary ID
Modernize datagrid columns, fix bugs, and add JSON serialization.
This is a major update to the existing datagrid columns, largely preparing them for the new Dashboard API resource, and fixing up a number of issues in their implementation. All columns now provide a stable JSON representation that can be used in the API, returning structured data, API-compatible objects (which will be turned into payloads or links), or raw values. In some cases, this is done by overriding the new `to_json()`, but in most we just now calculate the state for both the JSON and HTML representations in the column-controlled `get_raw_object_value()`. This keeps the data calculation in one place, and simplifies both output functions (often using default logic). HTML rendering has been fixed in many places to correctly use `format_html()` when we're constructing actual HTML to display. This will future-proof these columns for when the datagrid code starts escaping non-SafeStrings. Most of the `__init__()` methods are now gone, instead using the new class-defined attributes. This is much easier to maintain and specialize. Types have been fixed and annotated all throughout, catching issues in some of the logic. Many bugs (such as extra spaces at the end of values, wrong data types being returned where strings are expected) that weren't previously caught have been fixed, with tests updated to strictly check return types.
0bc60fb26398a6b7df0d21751a62516979d1a68a
Modernize datagrid columns, fix bugs, and add JSON serialization.
This is a major update to the existing datagrid columns, largely preparing them for the new Dashboard API resource, and fixing up a number of issues in their implementation. All columns now provide a stable JSON representation that can be used in the API, returning structured data, API-compatible objects (which will be turned into payloads or links), or raw values. In some cases, this is done by overriding the new `to_json()`, but in most we just now calculate the state for both the JSON and HTML representations in the column-controlled `get_raw_object_value()`. This keeps the data calculation in one place, and simplifies both output functions (often using default logic). HTML rendering has been fixed in many places to correctly use `format_html()` when we're constructing actual HTML to display. This will future-proof these columns for when the datagrid code starts escaping non-SafeStrings. Most of the `__init__()` methods are now gone, instead using the new class-defined attributes. This is much easier to maintain and specialize. Types have been fixed and annotated all throughout, catching issues in some of the logic. Many bugs (such as extra spaces at the end of values, wrong data types being returned where strings are expected) that weren't previously caught have been fixed, with tests updated to strictly check return types.
72d15e72a7b1924eebb0dc388815883dbbb0e2fd

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2.