I think this was accidentally added to your patch. Can you please remove this from the commit, and re-post?

Remove this blank line.

And this one.

This comment doesn't explain why we're oing this. Also comments should be full sentences (i.e., end with a period).

And this one.

Please delete this file from your review request. You can do git rm package-lock.json and git commit --amend -C HEAD

Hey Jeff, I re-opened some of Barret's issues because those newlines still look like they're in the latest revision.

Can you update your description to explain what the bug was and how your change fixes it? See https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea for our guide on this.
You should also list the bug number in the "Bugs" field, and not in the description.

I just noticed what file this is in. The DataGrid class should be entirely generic--it's easily possible that someone could use this for a model that does include a column named fullname.
Can we instead make the change in Review Board's reviewboard.datagrids.grids.UsersDataGrid? We can add a precomute_objects override that first makes necessary changes to self.sort_list and then calls the superclass' version.

David touched upon an important point, the location of where the change should be made. This change is too broad, as DataGrids are used for many things.
Another thing that's important is the design of the change. This change is somewhat of a "bandaid." It's addressing a symptom of the problem, but the real problem is that we were trying to allow sorting on the fullname field (which does not have sortable=True, so it shouldn't even be possible), when it's not really a field. It's working around an improper setting. Albeit in an interesting way -- turning it into two sortable fields.
So let's boil down the real issues:

We're allowing values in ?sort= that are not valid sortable columns. This needs to be fixed (and have a unit test proving it).
We want to allow sorting on the Full Name header, but we want something more specialized with the sorting (sorting by first name and then last name).


Issue #1 -- Bad Sort Values
The fix would be to add some validation where we're processing self.request.GET.get('sort', ...) in grids.py. It needs to be updated to make sure that each value in there is something we can sort on. Each must be something that a column could return. So an approach would be to first loop through all columns being shown on the datagrid that have sortable=True and then fetch the sortable database field from each, put those all in a set(), and then ignore any value in ?sort= that aren't in that set. This fixes not only ?sort=fullname, but ?sort=whatever.

Issue #2 -- Smarter Sorting of Full Names
Let's make a proper design for this, rather than working around a limitation in the current design. Columns can specify the database field they'd like to use for sorting through Column.get_sort_field(), which defaults to returning self.db_field. If you search, you'll find where we call get_sort_field(), and how it places the result in the list of fields to sort with. How about we update this to allow returning a list/tuple? Then we can have:

class FullNameColumn(...):
    ...

    def get_sort_field(self):
        return ('first_name', 'last_name')


The code that create the sort list can check if it got multiple items from this call, and add each independently to the sort list.
This would require a change in Djblets to provide this capability (plus a matching unit test), and a second change in Review Board to define this method for the column.

Summary
That's three review requests: 1 for the sorting fix (fix + unit test), 1 for the Djblets multi-field sorting (implementation + unit test), 1 for defining FullNameColumn.get_sort_field in Review Board.
Sound good? I can go over this with you on Slack if it helps.

There's no need for the u prefix. We use Unicode strings by default.