Summary

Refactoring the alphabetic paginator changes into new AlphanumericDatagrid subclass.

Review Request #6521 — Created Oct. 29, 2014 and submitted Nov. 14, 2014, 11:29 p.m.

Information

Owner

rmdone

Repository

Djblets

Branch

master

Bugs

829

Depends On

~~6522~~, ~~6368~~, ~~6369~~

Blocks

6522

Commit

03f56e7...

Reviewers

Groups

djblets, students

People

Description

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.

Testing Done

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).

Issues

Description	From	Last Updated
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Oct. 29, 2014, 6:22 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 29, 2014, 6:22 a.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Oct. 29, 2014, 6:22 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Oct. 29, 2014, 6:22 a.m.
Alphabetical order.	chipx86	Oct. 29, 2014, 7:23 a.m.
Summary should be on the first line, like: """Renders the paginator ... ... """	chipx86	Oct. 29, 2014, 7:26 a.m.
I don't recall what the original exception handler was set up for, but I think it was because custom columns …	chipx86	Oct. 29, 2014, 7:26 a.m.
Docstrings should be in proper sentences, with trailing periods. We should also have a bit more detail in this particular …	chipx86	Oct. 29, 2014, 7:30 a.m.
These blank lines should be removed.	chipx86	Oct. 29, 2014, 7:30 a.m.
This should take only the arguments needed, and pass the rest as args, kwargs. Like: def __init__(self, request, sortable_column, args, …	chipx86	Oct. 29, 2014, 7:31 a.m.
This (and the one below) would be nicer as: queryset = queryset.filter(**{ sortable_column + '...': ..., })	chipx86	Oct. 29, 2014, 7:33 a.m.
We use parens for multi-line conditions, with "or" on the previous line, like: elif (self.current_letter.isdecimal() or .....):	chipx86	Oct. 29, 2014, 7:34 a.m.
Let's define this regex as a constant on the class, and use ' instead of ".	chipx86	Oct. 29, 2014, 8:04 a.m.
Col: 47 E127 continuation line over-indented for visual indent	reviewbot	Oct. 29, 2014, 8:22 a.m.
Make sure sentences end in periods. There must be a blank line between the summary and body.	chipx86	Nov. 2, 2014, 4:36 a.m.
No blank line here.	chipx86	Nov. 2, 2014, 4:36 a.m.
Must be on one line, or various doc builders will cut it off.	chipx86	Nov. 2, 2014, 4:38 a.m.
Is this still used? If not, it can be removed.	chipx86	Nov. 2, 2014, 4:46 a.m.
'string' imported but unused	reviewbot	Nov. 2, 2014, 4:48 a.m.
Can you put 0 first instead of last?	david	Nov. 11, 2014, 3:43 a.m.
Would this be always true since you are always grabbing the letter A on line 1130?	AD adrw.hong	Nov. 14, 2014, 10:35 p.m.

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

djblets/datagrid/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (99 > 79 characters)
```
djblets/datagrid/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
djblets/datagrid/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (87 > 79 characters)
```
djblets/datagrid/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

Change Summary:

Fixing some line length issues.

Commit:

50e78890d91c8af52e0285d42513dcd904c2635c

be68f99bf3cd1648d3e70debccdffb97ca9c74d1

Diff:

Revision 2 (+159 -101)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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.
```
djblets/datagrid/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Alphabetical order.
```

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

Summary should be on the first line, like:

"""Renders the paginator ...

...
"""

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

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.

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

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.

djblets/datagrid/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
These blank lines should be removed.
```

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

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)

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

This (and the one below) would be nicer as:
queryset = queryset.filter(**{
    sortable_column + '...': ...,
})

djblets/datagrid/grids.py (Diff revision 2)

The issue has been resolved. Show all issues

We use parens for multi-line conditions, with "or" on the previous line, like:
elif (self.current_letter.isdecimal() or
      .....):

djblets/datagrid/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's define this regex as a constant on the class, and use ' instead of ".
```

Change Summary:

Adding deprecation waring to templatetags

Commit:

be68f99bf3cd1648d3e70debccdffb97ca9c74d1

f4f7761c31b3c8aec61fb385d130ee8091026a12

Diff:

Revision 3 (+164 -101)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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:

f4f7761c31b3c8aec61fb385d130ee8091026a12

42fb3f5d1642d4f818ba13709f84cf8aef9403a5

Diff:

Revision 4 (+164 -101)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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

djblets/datagrid/grids.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Col: 47
 E127 continuation line over-indented for visual indent
```

Change Summary:

Fixed line indent

Commit:

42fb3f5d1642d4f818ba13709f84cf8aef9403a5

fdc69193ea598af38ae21422268a26c263b967e2

Diff:

Revision 5 (+164 -101)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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:

fdc69193ea598af38ae21422268a26c263b967e2

744fed7d56d746b513bb14ea41a753045c47d898

Diff:

Revision 6 (+164 -101)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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

djblets/datagrid/grids.py (Diff revision 6)

The issue has been resolved. Show all issues

Make sure sentences end in periods.

There must be a blank line between the summary and body.

djblets/datagrid/grids.py (Diff revision 6)
The issue has been resolved. Show all issues
```
No blank line here.
```
djblets/datagrid/grids.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Must be on one line, or various doc builders will cut it off.
```
djblets/datagrid/templatetags/datagrid.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Is this still used? If not, it can be removed.
```

Change Summary:

Fixed some style issues. Removed unused alphanumeric paginator template tag.

Commit:

744fed7d56d746b513bb14ea41a753045c47d898

407a93c66f25aca7f1a630a9f9d36aabf97854f0

Diff:

Revision 7 (+158 -109)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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

djblets/datagrid/templatetags/datagrid.py (Diff revision 7)
The issue has been resolved. Show all issues
```
 'string' imported but unused
```

Change Summary:

Removed unused string import

Commit:

407a93c66f25aca7f1a630a9f9d36aabf97854f0

05f3d298eb276415fa8fdc8b62dd998a1604b86f

Diff:

Revision 8 (+158 -110)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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

djblets/datagrid/grids.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Can you put 0 first instead of last?
```

Change Summary:

Moved '0' to beginning of button list.

Commit:

05f3d298eb276415fa8fdc8b62dd998a1604b86f

03f56e7a323457ff33308b4d6076093d803a87bf

Diff:

Revision 9 (+158 -110)

Show changes

	djblets/datagrid/grids.py
	djblets/datagrid/templates/datagrid/alphabetic_paginator.html
	djblets/datagrid/templates/datagrid/alphanumeric_paginator.html
	djblets/datagrid/templates/datagrid/listview.html
	djblets/datagrid/templates/datagrid/numeric_paginator.html
	djblets/datagrid/templates/datagrid/paginator.html
	djblets/datagrid/templatetags/datagrid.py

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

djblets/datagrid/grids.py (Diff revision 9)

The issue has been dropped. Show all issues

Would this be always true since you are always grabbing the letter A on line 1130?

rmdone Nov. 14, 2014, 10:35 p.m.

request.GET.get('letter', 'A') checks the URL for a querystring '?letter=ARG' and returns ARG. If no argument is provided, it defaults to 'A'.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (4d49079)