Refactoring the alphabetic paginator changes into new AlphanumericDatagrid subclass.
Review Request #6521 — Created Oct. 28, 2014 and submitted
Changing the implementation of the alphabetic paginator to a subclass of the Datagrid: AlphanumericDatagrid.
The paginator(s) are no longer rendered using template tags, but instead a render_paginator function, which uses settings that are overloaded by the alphanumericDataGrid.
Unit tests all pass
All button cases manually tested. (Forward, back, first and last arrows, each letter).
Verified that changes to not affect any other module that inherits from datagrid (such as the groups list, or the all review requests list, the current 'master' of reviewboard with no changes to the userGrid).
Description | From | Last Updated |
---|---|---|
Col: 80 E501 line too long (99 > 79 characters) |
reviewbot | |
Col: 1 E302 expected 2 blank lines, found 1 |
reviewbot | |
Col: 80 E501 line too long (87 > 79 characters) |
reviewbot | |
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot | |
Alphabetical order. |
chipx86 | |
Summary should be on the first line, like: """Renders the paginator ... ... """ |
chipx86 | |
I don't recall what the original exception handler was set up for, but I think it was because custom columns … |
chipx86 | |
Docstrings should be in proper sentences, with trailing periods. We should also have a bit more detail in this particular … |
chipx86 | |
These blank lines should be removed. |
chipx86 | |
This should take only the arguments needed, and pass the rest as *args, **kwargs. Like: def __init__(self, request, sortable_column, *args, … |
chipx86 | |
This (and the one below) would be nicer as: queryset = queryset.filter(**{ sortable_column + '...': ..., }) |
chipx86 | |
We use parens for multi-line conditions, with "or" on the previous line, like: elif (self.current_letter.isdecimal() or .....): |
chipx86 | |
Let's define this regex as a constant on the class, and use ' instead of ". |
chipx86 | |
Col: 47 E127 continuation line over-indented for visual indent |
reviewbot | |
Make sure sentences end in periods. There must be a blank line between the summary and body. |
chipx86 | |
No blank line here. |
chipx86 | |
Must be on one line, or various doc builders will cut it off. |
chipx86 | |
Is this still used? If not, it can be removed. |
chipx86 | |
'string' imported but unused |
reviewbot | |
Can you put 0 first instead of last? |
david | |
Would this be always true since you are always grabbing the letter A on line 1130? |
AD adrw.hong |
- Change Summary:
-
Fixing some line length issues.
- Commit:
-
50e78890d91c8af52e0285d42513dcd904c2635cbe68f99bf3cd1648d3e70debccdffb97ca9c74d1
- Diff:
-
Revision 2 (+159 -101)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
-
Looking good! A few small things.
-
-
-
I don't recall what the original exception handler was set up for, but I think it was because custom columns themselves may end up breaking things, and we wanted some indication.
The paginator shouldn't break, so we should be able to remove the try/except.
-
Docstrings should be in proper sentences, with trailing periods. We should also have a bit more detail in this particular docstring about what advantages it offers.
-
-
This should take only the arguments needed, and pass the rest as
*args, **kwargs
. Like:def __init__(self, request, sortable_column, *args, **kwargs): ... super(AlphanumericDataGrid, self).__init__(request, *args, **kwargs)
-
This (and the one below) would be nicer as:
queryset = queryset.filter(**{ sortable_column + '...': ..., })
-
We use parens for multi-line conditions, with "or" on the previous line, like:
elif (self.current_letter.isdecimal() or .....):
-
- Change Summary:
-
Adding deprecation waring to templatetags
- Commit:
-
be68f99bf3cd1648d3e70debccdffb97ca9c74d1f4f7761c31b3c8aec61fb385d130ee8091026a12
- Diff:
-
Revision 3 (+164 -101)
-
Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
- Change Summary:
-
Fixes from Christian Hammond's review. Moved the responsibility of defining the "everything else" regex to the calling subclass. (Default unmatchable regex).
- Commit:
-
f4f7761c31b3c8aec61fb385d130ee8091026a1242fb3f5d1642d4f818ba13709f84cf8aef9403a5
- Diff:
-
Revision 4 (+164 -101)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
-
- Change Summary:
-
Fixed line indent
- Commit:
-
42fb3f5d1642d4f818ba13709f84cf8aef9403a5fdc69193ea598af38ae21422268a26c263b967e2
- Diff:
-
Revision 5 (+164 -101)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
- Change Summary:
-
Changing default regex for everything else button to [0-9] as opposed to nothing.
- Commit:
-
fdc69193ea598af38ae21422268a26c263b967e2744fed7d56d746b513bb14ea41a753045c47d898
- Diff:
-
Revision 6 (+164 -101)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
- Change Summary:
-
Fixed some style issues. Removed unused alphanumeric paginator template tag.
- Commit:
-
744fed7d56d746b513bb14ea41a753045c47d898407a93c66f25aca7f1a630a9f9d36aabf97854f0
- Diff:
-
Revision 7 (+158 -109)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
-
- Change Summary:
-
Removed unused string import
- Commit:
-
407a93c66f25aca7f1a630a9f9d36aabf97854f005f3d298eb276415fa8fdc8b62dd998a1604b86f
- Diff:
-
Revision 8 (+158 -110)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html
- Change Summary:
-
Moved '0' to beginning of button list.
- Commit:
-
05f3d298eb276415fa8fdc8b62dd998a1604b86f03f56e7a323457ff33308b4d6076093d803a87bf
- Diff:
-
Revision 9 (+158 -110)
-
Tool: Pyflakes Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py djblets/datagrid/templatetags/datagrid.py Ignored Files: djblets/datagrid/templates/datagrid/listview.html djblets/datagrid/templates/datagrid/paginator.html djblets/datagrid/templates/datagrid/alphanumeric_paginator.html djblets/datagrid/templates/datagrid/alphabetic_paginator.html djblets/datagrid/templates/datagrid/numeric_paginator.html