Summary

Adding the reviews a user has made to their profile

Review Request #5484 — Created Feb. 16, 2014 and submitted June 3, 2014, 12:07 a.m.

Information

Owner

tamijo

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

3e0b97a...

Reviewers

Groups

reviewboard, students

People

Description

I have added the reviews a user has made to their profile page, which can be accessed at /users/<username>/reviews/
The columns shown include:
* summary (of review request)
* submitter of review request
* date reviewed

Testing Done

Created unit tests to test that the reviews page works, and reviews on private review requests are not shown.

Files

Issues

Description	From	Last Updated
Since this is just a mixin now, it should be named "UserPageDataGridMixin".	chipx86	March 1, 2014, 10:03 a.m.
The suffix for this class is "Grid," but should be "DataGrid" to match the others.	chipx86	March 1, 2014, 10:03 a.m.
Trailing whitespace.	chipx86	March 1, 2014, 10:03 a.m.
Too many blank lines.	chipx86	March 1, 2014, 10:02 a.m.
Some older views are inconsistent in this way, but docstrings must follow the following format: """One-line summary.""" or: """One-line summary. …	chipx86	March 1, 2014, 10:04 a.m.
Please wrap this to 80 columns.	david	March 27, 2014, 3:23 p.m.
This should use _() for localization.	david	March 27, 2014, 3:24 p.m.
Please wrap at 80 columns.	david	March 27, 2014, 3:24 p.m.
This should use _() for localization.	david	March 27, 2014, 3:24 p.m.
Docstrings should start with a single line for a summary, then (optionally) a blank line and paragraphs of description. Please …	david	March 27, 2014, 3:24 p.m.
Your code here uses 2-space indentation instead of 4.	david	March 27, 2014, 3:24 p.m.
I don't know how permanent this is, but it should probably be formatted more like this: datagrid.tabs = [ (UserPageReviewRequestDataGrid.tab_title(username), …	david	March 27, 2014, 3:36 p.m.
Please wrap to 80 columns.	david	March 27, 2014, 3:24 p.m.
Please put a space ofter the :. Here and throughout this file.	david	March 27, 2014, 3:34 p.m.
Remove this line.	david	March 27, 2014, 3:34 p.m.
Remove this line.	david	March 27, 2014, 3:34 p.m.
Indent 2 spaces in LESS files.	david	March 27, 2014, 3:34 p.m.
Indent 1 space in HTML. Also note that we add spaces between {% and the tag names for block tags …	david	March 27, 2014, 3:24 p.m.
If this isn't necessary, can you remove it?	david	April 16, 2014, 10:22 a.m.
If this isn't necessary, can you remove it?	david	April 16, 2014, 10:22 a.m.
There's an extra space between 'review_summary' and ,	david	April 16, 2014, 10:22 a.m.
Please capitalize the "Do"	david	April 16, 2014, 10:22 a.m.
Maybe mention that this is an abstract class?	david	April 17, 2014, 1 p.m.
Wrap to 80 columns.	david	April 16, 2014, 10:23 a.m.
This doesn't match between here and UserPageReviewRequestDataGrid (one has "user page", one has "user's page"). Can you also include a …	david	April 17, 2014, 1 p.m.
Trailing whitespace. Can you also rewrap this to put one item per line?	david	April 16, 2014, 10:25 a.m.
Indent 1 more space.	david	April 16, 2014, 10:30 a.m.
Indent 1 more space.	david	April 16, 2014, 10:30 a.m.
Indent 1 more space.	david	April 16, 2014, 10:30 a.m.
Indent 1 more space.	david	April 16, 2014, 10:29 a.m.
Indent 1 more space.	david	April 16, 2014, 10:29 a.m.
Remove these two lines.	david	April 16, 2014, 10:30 a.m.
Can you switch the order of these two URLs?	david	April 16, 2014, 10:40 a.m.
Comments should have a space between # and the comment text, and start with a capital letter.	david	April 16, 2014, 10:30 a.m.
Maybe put these all on the same line?	david	April 17, 2014, 8:44 a.m.
Maybe put these all on the same line? Also, watch out for trailing whitespace.	david	April 17, 2014, 8:44 a.m.
Comments should have a space between # and the comment text, and start with a capital letter.	david	April 16, 2014, 10:32 a.m.
Indent 1 more space.	david	April 16, 2014, 10:33 a.m.
Indent 1 more space.	david	April 16, 2014, 10:33 a.m.
Indent 1 more space.	david	April 16, 2014, 10:34 a.m.
Hmm. It looks like it's been quite some time since you pulled from the origin repository. We've made some changes …	david	April 21, 2014, 2:29 p.m.
Indent 2 more spaces.	david	April 23, 2014, 12:48 p.m.
Can you add a blank line here?	david	April 23, 2014, 12:49 p.m.
"Group" hasn't been imported here. You should import it at the top of this method (like in ReviewRequestManager._query)	david	April 23, 2014, 12:50 p.m.
These are pretty generic class names, and the !important makes me scared. Can you maybe make these a bit more …	david	April 24, 2014, 8:24 p.m.
Missing trailing period.	chipx86	May 13, 2014, 3:24 p.m.
This can be one line, like the one above.	chipx86	May 13, 2014, 3:24 p.m.
This is all a copy/paste of the load_extra_data in ReviewRequestDataGrid. We should pull that out into a ShowClosedReviewRequestsMixin.	chipx86	May 15, 2014, 12:18 p.m.
Should remove the blank line.	chipx86	May 13, 2014, 3:24 p.m.
Wrong class.	chipx86	May 13, 2014, 3:50 p.m.
Space before %	chipx86	May 13, 2014, 3:26 p.m.
Should remove the blank line.	chipx86	May 13, 2014, 3:26 p.m.
Space before %	chipx86	May 13, 2014, 3:27 p.m.
The latter part of that regex is too verbose. We know it'll just be [a-z-]	chipx86	May 13, 2014, 3:27 p.m.
Should be on one line.	chipx86	May 13, 2014, 5:17 p.m.
No blank line.	chipx86	May 13, 2014, 3:31 p.m.
No need for this comment.	chipx86	May 13, 2014, 3:31 p.m.
Let's do review-requests instead. Also, is None	chipx86	May 13, 2014, 3:31 p.m.
This should be: datagrid_cls = UserPageReviewRequestDataGrid Then below, you can just do: datagrid = datagrid_cls(request, .....)	chipx86	May 14, 2014, 11:13 p.m.
The parameters are the same for both classes. Let's compute the class here, and initialize it after the conditionals.	chipx86	May 13, 2014, 5:51 p.m.
No need for this comment.	chipx86	May 13, 2014, 3:31 p.m.
Blank line between these.	chipx86	May 13, 2014, 3:31 p.m.
Missing period.	chipx86	May 13, 2014, 3:33 p.m.
There's a bunch of !important rules in here. We should remove all of them, and instead nest the rules better …	chipx86	May 13, 2014, 4:56 p.m.
Should just list the class, not the element. Same with all the ones below.	chipx86	May 13, 2014, 3:35 p.m.
The &. ones go above the child element/class rules.	chipx86	May 13, 2014, 3:36 p.m.
THis should go above the .datagrid-title rule above.	chipx86	May 13, 2014, 3:37 p.m.
Too many spaces before endif.	chipx86	May 13, 2014, 4:56 p.m.
Shouldn't have  . If we're trying to add some spacing, that should be done with CSS.	chipx86	May 13, 2014, 4:56 p.m.
It's not valid to have a <ul> inside another <ul>. It needs to be in <li>.	chipx86	May 13, 2014, 5:15 p.m.
Indentation got a bit wacky here.	david	May 16, 2014, 8:47 a.m.
Col: 17 E126 continuation line over-indented for hanging indent	reviewbot	May 19, 2014, 6:21 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	May 19, 2014, 6:22 p.m.
local variable 'review1' is assigned to but never used	reviewbot	May 19, 2014, 6:17 p.m.
local variable 'review2' is assigned to but never used	reviewbot	May 19, 2014, 6:17 p.m.
'SignalHook' imported but unused	reviewbot	May 16, 2014, 11:16 a.m.
'TemplateHook' imported but unused	reviewbot	May 16, 2014, 11:16 a.m.
'URLHook' imported but unused	reviewbot	May 16, 2014, 11:16 a.m.
Col: 21 E128 continuation line under-indented for visual indent	reviewbot	June 2, 2014, 10:30 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	May 29, 2014, 11:30 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	May 29, 2014, 11:30 p.m.
'SignalHook' imported but unused	reviewbot	May 29, 2014, 11:28 p.m.
'TemplateHook' imported but unused	reviewbot	May 29, 2014, 11:28 p.m.
'URLHook' imported but unused	reviewbot	May 29, 2014, 11:28 p.m.
Col: 21 E128 continuation line under-indented for visual indent	reviewbot	June 2, 2014, 10:46 p.m.
Needs a trailing period.	chipx86	June 2, 2014, 10:42 p.m.
For each of these, you should have a trailing comma after the value in the dictionary, and the }) should …	chipx86	June 2, 2014, 10:42 p.m.
These can probably just be constants on the class, right?	chipx86	June 2, 2014, 10:46 p.m.
I don't think this is needed.	chipx86	June 2, 2014, 10:44 p.m.
This code is pretty similar to what's in ReviewRequestManager. I don't expect you to worry about refactoring all this right …	chipx86	June 2, 2014, 10:48 p.m.

reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Since this is just a mixin now, it should be named "UserPageDataGridMixin".
```

reviewboard/datagrids/grids.py (Diff revision 1)

The issue has been resolved. Show all issues

The suffix for this class is "Grid," but should be "DataGrid" to match the others.

reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Too many blank lines.
```

reviewboard/datagrids/views.py (Diff revision 1)

The issue has been resolved. Show all issues

Some older views are inconsistent in this way, but docstrings must follow the following format:

"""One-line summary."""

or:

"""One-line summary.

Multi-line description.
"""

Change Summary:

fixed Django errors. All of the columns have not beed added, and the submitter columns show the review request summary instead of the actual submitter. The frontend also still needs to show the link to the reviews page on the profile.

Description:

~		I am working on adding the reviews a user has made to their profile page. It doesn't work at the moment - Django is throwing some errors, though I am working on sorting them out.
	~	I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
	+	The columns shown will include:
	+	* summary (of review request)
	+	* the star column
	+	* submitter of review request
	+	* date reviewed
	+	* time stamp

Commit:

f3fa1ecf4dacd32e489297d8e2a2464c8282cd56

Diff:

Revision 2 (+138 -10)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py

Change Summary:

fixed summary and submitter column

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
		* the star column
		* submitter of review request
		* date reviewed
		* time stamp
	+
	+	The proposed design of the reviews page is discussed here

Commit:

f3fa1ecf4dacd32e489297d8e2a2464c8282cd56

1f330d1a02d5b3a3565c2413f80a6de20e05b034

Diff:

Revision 3 (+161 -10)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py

Reading through this code, it looks like most of the functionality is here. Can you specify in the description what is left to do, and if it's pretty close, remove the WIP tag?

Change Summary:: updated description to show what remains in progress.
Description:: I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
The columns shown will include:
* summary (of review request)
* the star column
* submitter of review request
~ * date reviewed
~ * date reviewed
- * time stamp

~
The proposed design of the reviews page is discussed here
~
The proposed design of the reviews page is discussed here
+ Things that still need to be finished:
+ * the star column
+ * adding the link to reviews page as a tab (see design above)

Change Summary:

modified submitter view to take 'grid' parameter, and added tab bar to the profile page that shows links to the reviews and review requests page

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
		* the star column
		* submitter of review request
		* date reviewed

		The proposed design of the reviews page is discussed here
		Things that still need to be finished:
~		* the star column
	~	* adding the show-closed option to the tab bar as a dropdown
-		* adding the link to reviews page as a tab (see design above)

Commit:

1f330d1a02d5b3a3565c2413f80a6de20e05b034

34fa25efdf5a08f4384b0144cbadbebb9f087e13

Diff:

Revision 4 (+214 -18)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

Change Summary:: added images
Added Files:

Change Summary:

added show-closed dropdown for active datagrid

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
		* the star column
		* submitter of review request
		* date reviewed

		The proposed design of the reviews page is discussed here
		Things that still need to be finished:
~		* adding the show-closed option to the tab bar as a dropdown
	~	* cleanup css
	+	* testing

Commit:

34fa25efdf5a08f4384b0144cbadbebb9f087e13

e0f130f5a6c1bdb2d358e533d3dcac5210467e7a

Diff:

Revision 5 (+276 -15)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

reviewboard/datagrids/grids.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Please wrap this to 80 columns.
```
reviewboard/datagrids/grids.py (Diff revision 5)
The issue has been resolved. Show all issues
```
This should use _() for localization.
```
reviewboard/datagrids/grids.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Please wrap at 80 columns.
```
reviewboard/datagrids/grids.py (Diff revision 5)
The issue has been resolved. Show all issues
```
This should use _() for localization.
```

reviewboard/datagrids/views.py (Diff revision 5)

The issue has been resolved. Show all issues

Docstrings should start with a single line for a summary, then (optionally) a blank line and paragraphs of description. Please also wrap to 80 columns.

reviewboard/datagrids/views.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Your code here uses 2-space indentation instead of 4.
```

reviewboard/datagrids/views.py (Diff revision 5)

The issue has been resolved. Show all issues

I don't know how permanent this is, but it should probably be formatted more like this:

datagrid.tabs = [
    (UserPageReviewRequestDataGrid.tab_title(username),
     reverse('user', args=[username])),
    (UserPageReviewsDataGrid.tab_title(username),
     reverse('user', args=[username, 'reviews'])),
]

tamijo March 27, 2014, 3:37 p.m.

should this be taken out into a helper function, or done in another way?

david March 27, 2014, 3:48 p.m.
```
I think it's fine here.
```

reviewboard/extensions/hooks.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Please wrap to 80 columns.
```
reviewboard/static/rb/css/common.less (Diff revision 5)
The issue has been resolved. Show all issues
```
Please put a space ofter the :. Here and throughout this file.
```
reviewboard/static/rb/css/common.less (Diff revision 5)
The issue has been resolved. Show all issues
```
Remove this line.
```
reviewboard/static/rb/css/common.less (Diff revision 5)
The issue has been resolved. Show all issues
```
Remove this line.
```
reviewboard/static/rb/css/common.less (Diff revision 5)
The issue has been resolved. Show all issues
```
Indent 2 spaces in LESS files.
```

reviewboard/templates/datagrids/review_request_listview.html (Diff revision 5)

The issue has been resolved. Show all issues

Indent 1 space in HTML. Also note that we add spaces between {% and the tag names for block tags (separate from the HTML indentation). This file hasn't been updated to indent the tags correctly -- would you mind doing so while you're changing this? See reviewboard/templates/reviews/review_request_box.html for a good example.

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
-		* the star column
		* submitter of review request
		* date reviewed

		The proposed design of the reviews page is discussed here
		Things that still need to be finished:
		* cleanup css
		* testing

Commit:

e0f130f5a6c1bdb2d358e533d3dcac5210467e7a

c99ee36937a90d4ee5e63507987667d0bd726a7b

Diff:

Revision 6 (+293 -22)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

Added Files:

Change Summary:

fixed formatting issues

Commit:

c99ee36937a90d4ee5e63507987667d0bd726a7b

857077bd5a2e449ce9d6ce475f9202e6757cfd0b

Diff:

Revision 7 (+290 -22)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

Change Summary:

moved show-closed tag to the topbar (out of dropdown)

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
		* submitter of review request
		* date reviewed

		The proposed design of the reviews page is discussed here
		Things that still need to be finished:
-		* cleanup css
		* testing

Commit:

857077bd5a2e449ce9d6ce475f9202e6757cfd0b

5a4b56afcdf16424b215e761a3e39e466bf9d96b

Diff:

Revision 8 (+249 -19)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

Removed Files:

Added Files:

Change Summary:

added tests

Description:

		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
		The columns shown will include:
		* summary (of review request)
		* submitter of review request
		* date reviewed

~		The proposed design of the reviews page is discussed here
~		Things that still need to be finished:
	~	What remains:
	~	* making sure private reviews are not shown.
-		* testing

Testing Done:

Created unit tests to test that the reviews page works, and reviews on private review requests are not shown.

Commit:

5a4b56afcdf16424b215e761a3e39e466bf9d96b

a4183fb397e7c68b96378a4c12a66fd81826a410

Diff:

Revision 9 (+288 -20)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/datagrids/review_request_listview.html

This is looking pretty good! I have a bunch of style comments.

reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
If this isn't necessary, can you remove it?
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
If this isn't necessary, can you remove it?
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
There's an extra space between 'review_summary' and ,
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Please capitalize the "Do"
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Maybe mention that this is an abstract class?
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Wrap to 80 columns.
```

reviewboard/datagrids/grids.py (Diff revision 9)

The issue has been resolved. Show all issues

This doesn't match between here and UserPageReviewRequestDataGrid (one has "user page", one has "user's page"). Can you also include a mention of reviews vs. review requests in the docstring summary?

reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Trailing whitespace. Can you also rewrap this to put one item per line?
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Remove these two lines.
```
reviewboard/datagrids/urls.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Can you switch the order of these two URLs?
```

reviewboard/datagrids/views.py (Diff revision 9)

The issue has been resolved. Show all issues

Comments should have a space between # and the comment text, and start with a capital letter.

reviewboard/datagrids/views.py (Diff revision 9)

The issue has been dropped. Show all issues

Maybe put these all on the same line?

tamijo April 16, 2014, 10:48 a.m.

Putting them on one line makes them exceed 80 columns - should I just move the first variable (request) to the same line as the function call, and keep the other variables as is?

david April 16, 2014, 2:09 p.m.

In that case, I'd just leave it as-is and drop this issue.

reviewboard/datagrids/views.py (Diff revision 9)

The issue has been dropped. Show all issues

Maybe put these all on the same line? Also, watch out for trailing whitespace.

reviewboard/datagrids/views.py (Diff revision 9)

The issue has been resolved. Show all issues

Comments should have a space between # and the comment text, and start with a capital letter.

reviewboard/datagrids/views.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/datagrids/views.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```
reviewboard/extensions/hooks.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Indent 1 more space.
```

Change Summary:

Moved the filtering of reviews into a manager, and fixed filtering for private requests.

Summary:

[WIP] Adding the reviews a user has made to their profile

Adding the reviews a user has made to their profile

Description:

~		I am working on adding the reviews a user has made to their profile page, which will be shown at /users/<username>/reviews/
~		The columns shown will include:
	~	I have added the reviews a user has made to their profile page, which can be accessed at /users/<username>/reviews/
	~	The columns shown include:
		* summary (of review request)
		* submitter of review request
		* date reviewed
-
-		What remains:
-		* making sure private reviews are not shown.

Commit:

a4183fb397e7c68b96378a4c12a66fd81826a410

aff945f0c511469140e0ec705a88c442e8d39d8b

Diff:

Revision 10 (+344 -20)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Change Summary:

fixed a bug in displaying user reviews (an issue with authentication)

Commit:

aff945f0c511469140e0ec705a88c442e8d39d8b

f48ab4c561938448589e1a16361b3be91cf70f9f

Diff:

Revision 11 (+345 -20)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

reviewboard/reviews/managers.py (Diff revision 11)

The issue has been resolved. Show all issues

Hmm. It looks like it's been quite some time since you pulled from the origin repository. We've made some changes to the ReviewRequest version of this query to improve performance. Can you rebase on top of the latest master code, and add the accessible_repo_ids and accessible_group_ids mechanism that ReviewRequestManager._query uses?

Change Summary:

updated code to incorporate changes from master repository

Commit:

f48ab4c561938448589e1a16361b3be91cf70f9f

331fb33531840e9947133362c45e13cf4c4a6465

Diff:

Revision 12 (+350 -20)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

```
Almost there!
```
reviewboard/datagrids/views.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Indent 2 more spaces.
```
reviewboard/reviews/managers.py (Diff revision 12)
The issue has been resolved. Show all issues
```
Can you add a blank line here?
```

reviewboard/reviews/managers.py (Diff revision 12)

The issue has been resolved. Show all issues

"Group" hasn't been imported here. You should import it at the top of this method (like in ReviewRequestManager._query)

reviewboard/static/rb/css/common.less (Diff revision 12)

The issue has been resolved. Show all issues

These are pretty generic class names, and the !important makes me scared. Can you maybe make these a bit more specific, so that these rules won't bleed out into other parts of the UI?

tamijo April 24, 2014, 9:36 a.m.

I modified the rules to be more specific and encapsulated, though I still use !important in some places to override the datagrids css rules. is this ok?

david April 24, 2014, 1:50 p.m.
```
This looks great.
```

Change Summary:

modified css rules to be more descriptive, and added missing import

Commit:

331fb33531840e9947133362c45e13cf4c4a6465

bd1be98495762eae9c6d8651715734a99b83e1b9

Diff:

Revision 13 (+350 -20)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Ship it!

This looks good to me. Because it's a big change, Christian is also going to take a look before we push it. Nice work!

reviewboard/datagrids/columns.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Missing trailing period.
```
reviewboard/datagrids/columns.py (Diff revision 13)
The issue has been resolved. Show all issues
```
This can be one line, like the one above.
```

reviewboard/datagrids/grids.py (Diff revision 13)

The issue has been resolved. Show all issues

This is all a copy/paste of the load_extra_data in ReviewRequestDataGrid. We should pull that out into a ShowClosedReviewRequestsMixin.

tamijo May 13, 2014, 5:28 p.m.

is there a way to programmatically decide what is being filtered in the queryset (lines 213, 216)? ReviewRequestDataGrid filters on just status, instead of the review_request

chipx86 May 14, 2014, 7:32 p.m.

Ah, good point.
What I'd do then is to have the mixin require an attribute on the class that defines what to filter by. Something like status_query_field, which can be set to status or review_request__status. You can then use this with:

self.queryset = self.queryset.filter(**{
    self.status_query_field: 'P',
})

reviewboard/datagrids/grids.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Should remove the blank line.
```
reviewboard/datagrids/grids.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Wrong class.
```
reviewboard/datagrids/grids.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Space before %
```
reviewboard/datagrids/grids.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Should remove the blank line.
```
reviewboard/datagrids/grids.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Space before %
```
reviewboard/datagrids/urls.py (Diff revision 13)
The issue has been resolved. Show all issues
```
The latter part of that regex is too verbose. We know it'll just be [a-z-]
```

reviewboard/datagrids/views.py (Diff revision 13)

The issue has been resolved. Show all issues

Should be on one line.

tamijo May 13, 2014, 3:29 p.m.

this comment has more content after this line about the grid parameter, should i remove that content?

chipx86 May 13, 2014, 3:31 p.m.

You can probably simplify it to: "A user's profile page, showing their review requests and reviews."

david May 13, 2014, 3:31 p.m.

Christian just means put these two together and leave the rest as-is:

"""A user's profile page...

That way the docstring doesn't start with a blank line.

reviewboard/datagrids/views.py (Diff revision 13)
The issue has been resolved. Show all issues
```
No blank line.
```
reviewboard/datagrids/views.py (Diff revision 13)
The issue has been resolved. Show all issues
```
No need for this comment.
```
reviewboard/datagrids/views.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Let's do review-requests instead.
Also, is None
```

reviewboard/datagrids/views.py (Diff revision 13)

The issue has been resolved. Show all issues

The parameters are the same for both classes. Let's compute the class here, and initialize it after the conditionals.

reviewboard/datagrids/views.py (Diff revision 13)
The issue has been resolved. Show all issues
```
No need for this comment.
```
reviewboard/datagrids/views.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/reviews/managers.py (Diff revision 13)
The issue has been resolved. Show all issues
```
Missing period.
```

reviewboard/static/rb/css/common.less (Diff revision 13)

The issue has been resolved. Show all issues

There's a bunch of !important rules in here. We should remove all of them, and instead nest the rules better so that the browser will naturally choose them.

reviewboard/static/rb/css/common.less (Diff revision 13)
The issue has been resolved. Show all issues
```
Should just list the class, not the element.
Same with all the ones below.
```
reviewboard/static/rb/css/common.less (Diff revision 13)
The issue has been resolved. Show all issues
```
The &. ones go above the child element/class rules.
```
reviewboard/static/rb/css/common.less (Diff revision 13)
The issue has been resolved. Show all issues
```
THis should go above the .datagrid-title rule above.
```
reviewboard/templates/datagrids/review_request_listview.html (Diff revision 13)
The issue has been resolved. Show all issues
```
Too many spaces before endif.
```
reviewboard/templates/datagrids/review_request_listview.html (Diff revision 13)
The issue has been resolved. Show all issues
```
Shouldn't have &nbsp;. If we're trying to add some spacing, that should be done with CSS.
```
reviewboard/templates/datagrids/review_request_listview.html (Diff revision 13)
The issue has been resolved. Show all issues
```
It's not valid to have a <ul> inside another <ul>. It needs to be in <li>.
```

Change Summary:

resolved issues with css and formatting

Commit:

bd1be98495762eae9c6d8651715734a99b83e1b9

52e95a6d4b8a454bb40768a3537d5fd758086632

Diff:

Revision 14 (+357 -28)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

```
Looking good!
```

reviewboard/datagrids/views.py (Diff revisions 13 - 14)

The issue has been resolved. Show all issues

This should be:
datagrid_cls = UserPageReviewRequestDataGrid

Then below, you can just do:
datagrid = datagrid_cls(request, .....)

Change Summary:

fixed initiation of datagrid class; moved show_closed logic for datagrids into mixin. I'm not sure if this is done correctly..using super as other functions have done doesn't seem to work

Commit:

52e95a6d4b8a454bb40768a3537d5fd758086632

fea04241d76278ff05cefeb3e93d9a995c3a5667

Diff:

Revision 15 (+375 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Change Summary:

changes to ShowClosedReviewRequestMixin to fix inheritance issue

Commit:

fea04241d76278ff05cefeb3e93d9a995c3a5667

69de9e4f1fa9d857c68aa1f97b5c028de72b745d

Diff:

Revision 16 (+375 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

reviewboard/datagrids/grids.py (Diff revision 16)

The issue has been resolved. Show all issues

Indentation got a bit wacky here.

Change Summary:

fix indentation issue

Commit:

69de9e4f1fa9d857c68aa1f97b5c028de72b745d

be7853e3ccd2308b54099a307ff43c86e30b102e

Diff:

Revision 17 (+373 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/datagrids/views.py
    reviewboard/datagrids/columns.py
    reviewboard/datagrids/tests.py
    reviewboard/datagrids/urls.py
    reviewboard/extensions/hooks.py
    reviewboard/datagrids/grids.py
    reviewboard/reviews/managers.py
  Ignored Files:
    reviewboard/static/rb/css/common.less
    reviewboard/templates/datagrids/review_request_listview.html

reviewboard/datagrids/grids.py (Diff revision 17)
The issue has been resolved. Show all issues
```
Col: 17
 E126 continuation line over-indented for hanging indent
```
reviewboard/datagrids/grids.py (Diff revision 17)
The issue has been resolved. Show all issues
```
Col: 5
 E303 too many blank lines (2)
```

reviewboard/reviews/managers.py (Diff revision 17)

The issue has been dropped. Show all issues

Col: 21
 E128 continuation line under-indented for visual indent

tamijo May 19, 2014, 6:24 p.m.

hm i think adding an indent here causes the line to go over 80 columns - should i indent it anyway?
Also the indent here is the same indent used in other lines in this function

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
    reviewboard/datagrids/views.py
    reviewboard/datagrids/columns.py
    reviewboard/datagrids/tests.py
    reviewboard/datagrids/urls.py
    reviewboard/extensions/hooks.py
    reviewboard/datagrids/grids.py
    reviewboard/reviews/managers.py
  Ignored Files:
    reviewboard/static/rb/css/common.less
    reviewboard/templates/datagrids/review_request_listview.html

reviewboard/datagrids/tests.py (Diff revision 17)
The issue has been resolved. Show all issues
```
 local variable 'review1' is assigned to but never used
```
reviewboard/datagrids/tests.py (Diff revision 17)
The issue has been resolved. Show all issues
```
 local variable 'review2' is assigned to but never used
```
reviewboard/extensions/hooks.py (Diff revision 17)
The issue has been dropped. Show all issues
```
 'SignalHook' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 17)
The issue has been dropped. Show all issues
```
 'TemplateHook' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 17)
The issue has been dropped. Show all issues
```
 'URLHook' imported but unused
```

Change Summary:

adressed review bot issues with formatting

Commit:

be7853e3ccd2308b54099a307ff43c86e30b102e

900f673fd2e0ad0dea511f2448bf110bbe5e27be

Diff:

Revision 18 (+372 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/datagrids/views.py
    reviewboard/datagrids/columns.py
    reviewboard/datagrids/tests.py
    reviewboard/datagrids/urls.py
    reviewboard/extensions/hooks.py
    reviewboard/datagrids/grids.py
    reviewboard/reviews/managers.py
  Ignored Files:
    reviewboard/static/rb/css/common.less
    reviewboard/templates/datagrids/review_request_listview.html

reviewboard/datagrids/tests.py (Diff revision 18)
The issue has been resolved. Show all issues
```
Col: 38
 E127 continuation line over-indented for visual indent
```
reviewboard/datagrids/tests.py (Diff revision 18)
The issue has been resolved. Show all issues
```
Col: 38
 E127 continuation line over-indented for visual indent
```

reviewboard/reviews/managers.py (Diff revision 18)

The issue has been resolved. Show all issues

Col: 21
 E128 continuation line under-indented for visual indent

tamijo May 29, 2014, 11:45 p.m.

added a backslash instead - is this fine?

chipx86 June 2, 2014, 10:21 p.m.

I know we haven't really been using this form elsewhere, but you can just use the |= operator:

group_query |= Q(review_request__target_groups__in=...)

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
    reviewboard/datagrids/views.py
    reviewboard/datagrids/columns.py
    reviewboard/datagrids/tests.py
    reviewboard/datagrids/urls.py
    reviewboard/extensions/hooks.py
    reviewboard/datagrids/grids.py
    reviewboard/reviews/managers.py
  Ignored Files:
    reviewboard/static/rb/css/common.less
    reviewboard/templates/datagrids/review_request_listview.html

reviewboard/extensions/hooks.py (Diff revision 18)
The issue has been dropped. Show all issues
```
 'SignalHook' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 18)
The issue has been dropped. Show all issues
```
 'TemplateHook' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 18)
The issue has been dropped. Show all issues
```
 'URLHook' imported but unused
```

Change Summary:

addresssed more review bot formatting issues

Commit:

900f673fd2e0ad0dea511f2448bf110bbe5e27be

1e26f7715e0fd9382aaaec1c5d52f90e3c82bfa9

Diff:

Revision 19 (+372 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

A few small things. This is pretty great! Just a couple trivial style changes, and we'll be able to put this in for 2.1 :)

reviewboard/datagrids/grids.py (Diff revision 19)
The issue has been resolved. Show all issues
```
Needs a trailing period.
```

reviewboard/datagrids/grids.py (Diff revision 19)

The issue has been resolved. Show all issues

For each of these, you should have a trailing comma after the value in the dictionary, and the }) should be aligned with the opening statement on its own line, like:

self.queryset = self.queryset.filter(**{
    ...
});

reviewboard/datagrids/grids.py (Diff revision 19)
The issue has been resolved. Show all issues
```
These can probably just be constants on the class, right?
```
reviewboard/datagrids/grids.py (Diff revision 19)
The issue has been resolved. Show all issues
```
I don't think this is needed.
```

reviewboard/reviews/managers.py (Diff revision 19)

The issue has been resolved. Show all issues

This code is pretty similar to what's in ReviewRequestManager. I don't expect you to worry about refactoring all this right now, but we should probably do this at some point.

Can you put a TODO comment above all this code saying it should be consolidated with the queries in ReviewRequestManager?

Commit:

1e26f7715e0fd9382aaaec1c5d52f90e3c82bfa9

2f16cf27c5c7c49a655d481c472b8bd751e117f3

Diff:

Revision 20 (+371 -51)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Change Summary:

rebased from master

Commit:

2f16cf27c5c7c49a655d481c472b8bd751e117f3

0b39a1ed46defadd6824bf4b632ccdaec4d03c84

Diff:

Revision 21 (+411 -50)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Change Summary:

minor fix: moved class variable from init

Commit:

0b39a1ed46defadd6824bf4b632ccdaec4d03c84

3e0b97ab947529a28577a146e785b8c9f3bc8153

Diff:

Revision 22 (+411 -50)

Show changes

	reviewboard/datagrids/columns.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/tests.py
	reviewboard/datagrids/urls.py
	reviewboard/datagrids/views.py
	reviewboard/extensions/hooks.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	1 more

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (f33e964)