- Download Diff

Summary:

[WIP] Add archived state to review requests

Review Request #6830 — Created Jan. 25, 2015 and discarded April 2, 2015, 1:14 p.m.

Information
Owner:	cristocrat
Repository:	Review Board
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	reviewboard, students
People:

Description

Orig: January 25, 2015
- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
- added show_archived attribute to DashboardDataGrid
- added ReviewRequestVisitManager

January 31, 2015
- scrapped ReviewRequestVisitManager idea
- added a query that filters archived requests to ReviewRequestManager
- wrote and applied a evolution for visibility in the ReviewRequestVisit table

February 11, 2015
- added archive and mute buttons in template with handlers
- added a reviewRequestVisit JavaScript resource that talks to the webAPI
- began review_request_visit API

February 13, 2015
- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
- began archive banner

February 16, 2015
- url() and archive() in archivedReviewRequestModel.js
- added decorators in the archived_review_request webAPI resource

February 17, 2015
- finished implementing query that filters archived review requests
- added more functional hyperlink that toggles review request visibility with url query
- works as it should if there are no other existing url queries, but still need to figure out how to append to existing, non-"show-archived" url queries

February 19, 2015
- fixed archive query
- completed POST request for archiving
- made "Hide" button disappear when review request is already archived

February 28, 2015

Testing Done

Issues

Description	From	Last Updated
'ConcurrencyManager' imported but unused	reviewbot	Jan. 31, 2015, 5:44 p.m.
undefined name 'ReviewRequestVisit'	reviewbot	Jan. 31, 2015, 5:44 p.m.
The first line of this file should be from __future__ import unicode_literals. With that, you can change initial=u'V' to initial='V'.	david	Feb. 7, 2015, 10:10 a.m.
Col: 80 E501 line too long (109 > 79 characters)	reviewbot	Jan. 31, 2015, 6:13 p.m.
Col: 2 W292 no newline at end of file	reviewbot	Jan. 31, 2015, 6:13 p.m.
You should revert the change to this line.	david	Feb. 7, 2015, 10:10 a.m.
'ReviewRequestVisit' imported but unused	reviewbot	Jan. 31, 2015, 6:11 p.m.
You don't need the '' default in the get statement. It would return a None which would be a false …	VT VTL-Developer	Feb. 7, 2015, 10:10 a.m.
I think that we want this to be a filter on the existing queryset (rather than replacing it entirely). Something …	david	Feb. 19, 2015, 2:21 p.m.
Col: 5 E101 indentation contains mixed spaces and tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 5 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 6 E128 continuation line under-indented for visual indent	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Feb. 13, 2015, 1:04 a.m.
undefined name 'ReviewRequestVisit'	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 1 W191 indentation contains tabs	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 61 W292 no newline at end of file	reviewbot	Feb. 13, 2015, 1:04 a.m.
Col: 9 E128 continuation line under-indented for visual indent	reviewbot	Feb. 19, 2015, 2:29 p.m.
Col: 33 W291 trailing whitespace	reviewbot	Feb. 13, 2015, 10:33 p.m.
'User' imported but unused	reviewbot	Feb. 15, 2015, 11:30 p.m.
'augment_method_from' imported but unused	reviewbot	Feb. 15, 2015, 11:30 p.m.
'webapi_check_local_site' imported but unused	reviewbot	Feb. 15, 2015, 11:30 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Feb. 13, 2015, 10:36 p.m.
local variable 'user' is assigned to but never used	reviewbot	Feb. 15, 2015, 11:30 p.m.
local variable 'review_request' is assigned to but never used	reviewbot	Feb. 15, 2015, 11:30 p.m.
undefined name 'DOES_NOT_EXIST'	reviewbot	Feb. 13, 2015, 10:38 p.m.
local variable 'user' is assigned to but never used	reviewbot	Feb. 15, 2015, 11:30 p.m.
local variable 'review_request' is assigned to but never used	reviewbot	Feb. 15, 2015, 11:30 p.m.
undefined name 'DOES_NOT_EXIST'	reviewbot	Feb. 13, 2015, 10:38 p.m.
'augment_method_from' imported but unused	reviewbot	Feb. 15, 2015, 11:30 p.m.
'webapi_check_local_site' imported but unused	reviewbot	Feb. 15, 2015, 11:31 p.m.
'resources' imported but unused	reviewbot	Feb. 15, 2015, 11:31 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Feb. 13, 2015, 10:38 p.m.
Col: 9 E128 continuation line under-indented for visual indent	reviewbot	Feb. 19, 2015, 2:29 p.m.
These sorts of methods don't really help that much. It's nicer to just have the calling code set the field …	david	Feb. 19, 2015, 2:21 p.m.
I'd not sure if this is temporary test code or not, but we should just be passing filter_archived as a …	david	Feb. 17, 2015, 10:10 p.m.
This isn't quite right. Please see my earlier suggestion at https://reviews.reviewboard.org/r/6830/#comment20262 In this context, it would end up looking something …	david	Feb. 19, 2015, 2:21 p.m.
'User' imported but unused	reviewbot	Feb. 17, 2015, 9:57 p.m.
'augment_method_from' imported but unused	reviewbot	Feb. 17, 2015, 9:57 p.m.
Col: 60 E231 missing whitespace after ','	reviewbot	Feb. 17, 2015, 9:58 p.m.
local variable 'user' is assigned to but never used	reviewbot	Feb. 19, 2015, 2:21 p.m.
This should probably be more like: visit, is_new = ReviewRequestVisit.objects.get_or_create( user=user, review_request=review_request) visit.visible = ReviewRequestVisit.ARCHIVED visit.save()	david	Feb. 17, 2015, 10:05 p.m.
undefined name 'archived_review_request'	reviewbot	Feb. 17, 2015, 9:59 p.m.
Col: 26 E225 missing whitespace around operator	reviewbot	Feb. 19, 2015, 2:22 p.m.
Col: 5 W604 backticks are deprecated, use 'repr()'	reviewbot	Feb. 19, 2015, 2:22 p.m.
Col: 7 E226 missing whitespace around arithmetic operator	reviewbot	Feb. 19, 2015, 2:22 p.m.
Col: 1 E112 expected an indented block	reviewbot	Feb. 19, 2015, 2:22 p.m.
Col: 9 E128 continuation line under-indented for visual indent	reviewbot	Feb. 19, 2015, 2:30 p.m.
Col: 9 E128 continuation line under-indented for visual indent	reviewbot	Feb. 19, 2015, 2:29 p.m.
This isn't used anymore.	david	Feb. 28, 2015, 10:10 p.m.
Col: 19 E702 multiple statements on one line (semicolon)	reviewbot	March 5, 2015, 10:57 a.m.
Should have the model name before it: "reviewrequestvisit_visibility" I don't see any profile settings in here?	chipx86	March 3, 2015, 8:35 p.m.
Blank line between these.	chipx86	March 3, 2015, 8:35 p.m.
Should use one of the constants instead of hard-coding V. I'm not sure we want to index this here. If …	chipx86	March 7, 2015, 10:47 p.m.
This is already defined in ReviewRequestDataGrid.	chipx86	March 3, 2015, 8:36 p.m.
I feel like we're doing a bunch of unnecessary work for this 'show_archived' flag. I also think we can't assume …	chipx86	March 8, 2015, 10:20 a.m.
Best named hidden_q or just queryset.	chipx86	March 5, 2015, 10:52 a.m.
Instead of the ~Q(...), use Q(visibility__ne=...)	chipx86	March 8, 2015, 10:01 a.m.
Should use the consatnt instead of "P".	chipx86	March 7, 2015, 11:16 p.m.
I don't think we need this anymore?	chipx86	March 5, 2015, 10:53 a.m.
Should be added alphabetically.	chipx86	March 5, 2015, 10:54 a.m.
Col: 19 E702 multiple statements on one line (semicolon)	reviewbot	March 5, 2015, 10:57 a.m.
'Q' imported but unused	reviewbot	March 17, 2015, 11:34 p.m.
Rather than using a continuation character here, can you put the whole expression in parens? if (review_request.public and review_request.status == …	david	March 17, 2015, 11:35 p.m.
These should probably be defined inside the UserSession object (right after this.watchedGroups = ...). We should also wrap these strings …	david	March 17, 2015, 11:40 p.m.
Same comment as above.	david	March 17, 2015, 11:41 p.m.
Same comment as above.	david	March 17, 2015, 11:44 p.m.
Same comment as above.	david	March 17, 2015, 11:44 p.m.
Same comment as above.	david	March 17, 2015, 11:45 p.m.
Same comment as above.	david	March 17, 2015, 11:45 p.m.
This should say "review request" instead of "change"	david	March 17, 2015, 11:49 p.m.
This should say "review request" instead of "change"	david	March 17, 2015, 11:49 p.m.
No blank line here (django and djblets are both "3rd party" libraries from Review Board's point of view).	david	March 24, 2015, 12:34 p.m.
Can you add docstrings for the class and method?	david	March 24, 2015, 12:34 p.m.
Please add visibility=self.model.ARCHIVED to the filter call (we don't want to update muted review requests).	david	March 24, 2015, 12:34 p.m.
Col: 21 E126 continuation line over-indented for hanging indent	reviewbot	March 17, 2015, 11:58 p.m.
Please add a blank line after the docstring.	david	March 24, 2015, 12:34 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	March 17, 2015, 11:58 p.m.
review_published and reply_published pass a Review instance as their argument rather than a ReviewRequest. You'll need a separate handler for …	david	March 18, 2015, 2:50 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	March 17, 2015, 11:58 p.m.
Since you have parens, put the opening paren on the previous line instead of using \\	david	March 24, 2015, 12:33 p.m.
I think djblets.util.http.get_url_params_except will do what you want.	david	March 24, 2015, 12:33 p.m.
Col: 7 E111 indentation is not a multiple of four	reviewbot	March 17, 2015, 11:58 p.m.
Please don't change the signatures of these, and instead have a separate handler.	david	March 18, 2015, 2:50 p.m.
I don't understand what the visibility attribute is used for. Can you explain?	david	March 18, 2015, 2:50 p.m.
Please use "review request" instead of "change"	david	March 24, 2015, 12:33 p.m.
Please use "review request" instead of "change"	david	March 24, 2015, 12:33 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py

reviewboard/accounts/models.py (Diff revision 1)
Show all issues
```
 'ConcurrencyManager' imported but unused
```
reviewboard/datagrids/grids.py (Diff revision 1)
Show all issues
```
 undefined name 'ReviewRequestVisit'
```

Description:

~		Orig: January 25, 2015
	~	Orig: January 25, 2015
	+	- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
	+	- added show_archived attribute to DashboardDataGrid
	+	- added ReviewRequestVisitManager

~		Added visibility field to ReviewRequestVisit and show_archived attribute to DashboardDataGrid
	~	January 31, 2015
	+	- scrapped ReviewRequestVisitManager idea
	+	- added a query that filters archived requests to ReviewRequestManager
	+	- wrote and applied a evolution for visibility in the ReviewRequestVisit table

Commit:

-	f8c550c5d914b28d0377bdd6377000cac9a846c6
+	acc7bd2e8fcf47ba65e6751ac89be7d76fdc2167

Diff:

Revision 2 (+44 -2)

Show changes

	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/managers.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py

reviewboard/accounts/evolutions/visibility.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (109 > 79 characters)
```
reviewboard/accounts/evolutions/visibility.py (Diff revision 2)
Show all issues
```
Col: 2
 W292 no newline at end of file
```
reviewboard/datagrids/grids.py (Diff revision 2)
Show all issues
```
 'ReviewRequestVisit' imported but unused
```

Groups:

+	students

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

Show all issues

You don't need the '' default in the get statement. It would return a None which would be a false for your if statement.

reviewboard/accounts/evolutions/visibility.py (Diff revision 2)

Show all issues

The first line of this file should be from __future__ import unicode_literals. With that, you can change initial=u'V' to initial='V'.

reviewboard/accounts/models.py (Diff revision 2)
Show all issues
```
You should revert the change to this line.
```

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

Show all issues

I think that we want this to be a filter on the existing queryset (rather than replacing it entirely). Something like this?

if not show_archived:
    hidden = ReviewRequestVisit.objects.filter(
        user=user, visibility__ne=ReviewRequestVisit.VISIBLE)
    hidden = hidden.values_list('review_request_id', flat=True)

    self.queryset = self.queryset.exclude(pk__in=hidden)


In fact, it might be even more appropriate to plumb the show_archived field through and do this at the bottom of ReviewRequestManager._query, which already has some similar code (which filters out review requests that are "private")

cristocrat Feb. 7, 2015, 8:24 p.m.

For your latter suggestion, how would my ReviewRequestManager.archived method be able to catch any other URL parameters? Like if I wanted to see all review requests that are archived AND to-me, how would ReviewRequestManager.archived get ReviewrequestManager.to_user_directly's extra query?

Change Summary:

Create a ReviewRequestVisit resource

Commit:

-	acc7bd2e8fcf47ba65e6751ac89be7d76fdc2167
+	d761bad5a5db274969e67f4c0e85995403c5a49f

Diff:

Revision 3 (+161 -5)

Show changes

	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/resources/models/reviewRequestVisitModel.js
	reviewboard/static/rb/js/views/reviewRequestEditorView.js
	reviewboard/templates/reviews/review_request_actions_secondary.html
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/review_request_visit.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewRequestVisitModel.js
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/views/reviewRequestEditorView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/review_request_visit.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewRequestVisitModel.js
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/views/reviewRequestEditorView.js

reviewboard/accounts/evolutions/visibility.py (Diff revision 3)
Show all issues
```
Col: 5
 E101 indentation contains mixed spaces and tabs
```
reviewboard/accounts/evolutions/visibility.py (Diff revision 3)
Show all issues
```
Col: 5
 W191 indentation contains tabs
```
reviewboard/accounts/evolutions/visibility.py (Diff revision 3)
Show all issues
```
Col: 6
 E128 continuation line under-indented for visual indent
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
 undefined name 'ReviewRequestVisit'
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/webapi/resources/review_request_visit.py (Diff revision 3)
Show all issues
```
Col: 61
 W292 no newline at end of file
```

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table
	+
	+	February 11, 2015
	+	- added archive and mute buttons in template with handlers
	+	- added a reviewRequestVisit JavaScript resource that talks to the webAPI
	+	- began review_request_visit API
	+
	+	February 13, 2015
	+	- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
	+	- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
	+	- began archive banner

Commit:

-	d761bad5a5db274969e67f4c0e85995403c5a49f
+	dd41ef4892b29dd572e8a0055eba13103ed9b9e2

Diff:

Revision 4 (+328 -5)

Show changes

	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
	reviewboard/static/rb/js/views/reviewRequestEditorView.js
	4 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/managers.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js



Tool: Pyflakes
Processed Files:
    reviewboard/reviews/managers.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js

reviewboard/accounts/evolutions/visibility.py (Diff revision 4)
Show all issues
```
Col: 9
 E128 continuation line under-indented for visual indent
```
reviewboard/datagrids/grids.py (Diff revision 4)
Show all issues
```
Col: 33
 W291 trailing whitespace
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 'User' imported but unused
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 'augment_method_from' imported but unused
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 'webapi_check_local_site' imported but unused
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 local variable 'user' is assigned to but never used
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 local variable 'review_request' is assigned to but never used
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 undefined name 'DOES_NOT_EXIST'
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 local variable 'user' is assigned to but never used
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 local variable 'review_request' is assigned to but never used
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 4)
Show all issues
```
 undefined name 'DOES_NOT_EXIST'
```
reviewboard/webapi/resources/muted_review_request.py (Diff revision 4)
Show all issues
```
 'augment_method_from' imported but unused
```
reviewboard/webapi/resources/muted_review_request.py (Diff revision 4)
Show all issues
```
 'webapi_check_local_site' imported but unused
```
reviewboard/webapi/resources/muted_review_request.py (Diff revision 4)
Show all issues
```
 'resources' imported but unused
```
reviewboard/webapi/resources/muted_review_request.py (Diff revision 4)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table

		February 11, 2015
		- added archive and mute buttons in template with handlers
		- added a reviewRequestVisit JavaScript resource that talks to the webAPI
		- began review_request_visit API

		February 13, 2015
		- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
		- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
		- began archive banner
	+
	+	February 16, 2015
	+	- url() and archive() in archivedReviewRequestModel.js
	+	- added decorators in the archived_review_request webAPI resource

Commit:

-	dd41ef4892b29dd572e8a0055eba13103ed9b9e2
+	ec502e8704937ace2508186aeb29686d8c8a93c4

Diff:

Revision 5 (+338 -5)

Show changes

	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
	reviewboard/static/rb/js/views/reviewRequestEditorView.js
	4 more

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/managers.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/managers.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js

reviewboard/accounts/evolutions/visibility.py (Diff revision 5)
Show all issues
```
Col: 9
 E128 continuation line under-indented for visual indent
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)
Show all issues
```
 'User' imported but unused
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)
Show all issues
```
 'augment_method_from' imported but unused
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)
Show all issues
```
Col: 60
 E231 missing whitespace after ','
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)
Show all issues
```
 local variable 'user' is assigned to but never used
```
reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)
Show all issues
```
 undefined name 'archived_review_request'
```

I haven't looked too much at the javascript code because it looks very preliminary.

I'd suggest setting the archive flag for a review request in the database admin and using that to test that the dashboard queries are working correctly.

reviewboard/accounts/models.py (Diff revision 5)

Show all issues

These sorts of methods don't really help that much. It's nicer to just have the calling code set the field directly and call save() when appropriate.

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

Show all issues

I'd not sure if this is temporary test code or not, but we should just be passing filter_archived as a parameter to the existing ReviewRequest.objects.*() methods, rather than replacing the queryset. With this as-is, all the pages of the dashboard will look the same.

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

Show all issues

This isn't quite right. Please see my earlier suggestion at https://reviews.reviewboard.org/r/6830/#comment20262
In this context, it would end up looking something like this:

query = self.filter(query).distinct()

if filter_archived:
    hidden = ReviewRequestVisit.objects.filter(
        user=user, visibility__ne=ReviewRequestVisit.VISIBLE)
    hidden = hidden.values_list('review_request_id', flat=True)

    query = query.exclude(pk__in=hidden)

if with_counts:
...

cristocrat Feb. 17, 2015, 10:09 p.m.

Sorry, only just saw this and I had since found another solution (which seems to be working, please see diff revision 6), but please let me know if I should revert to what you posted in this review.

cristocrat Feb. 18, 2015, 7:45 a.m.

Please let me know if revision 6 is okay. I put the query directly into load_extra_state in grids.py, which avoids having to put "filter_archived=self.filter_archived" in every ReviewRequest.objects.()* method

david Feb. 18, 2015, 2:02 p.m.

So putting it into load_extra_state would work, but the query you have (self.queryset.filter(**{ self.visibility_query_field: 'V', })) won't do the right thing because of the particular kind of join we need to do. Just querying visits__visibility won't take into account the user relation on the ReviewRequestVisit model, and we need to do that. Do you understand what my above code snippit is doing and how it differs from what you have?

cristocrat Feb. 18, 2015, 2:23 p.m.

Oh okay, yes I see the difference. I'll have it in my next diff

reviewboard/webapi/resources/archived_review_request.py (Diff revision 5)

Show all issues

This should probably be more like:

visit, is_new = ReviewRequestVisit.objects.get_or_create(
    user=user, review_request=review_request)
visit.visible = ReviewRequestVisit.ARCHIVED
visit.save()

cristocrat Feb. 18, 2015, 10:35 a.m.

Thanks, this makes things a lot easier. One thing though, for some reason, create isn't even being reached when I try to run things in pdb. From my understanding, I'm calling it with this.save in archiveRequestModel.js, is this correct?

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table

		February 11, 2015
		- added archive and mute buttons in template with handlers
		- added a reviewRequestVisit JavaScript resource that talks to the webAPI
		- began review_request_visit API

		February 13, 2015
		- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
		- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
		- began archive banner

		February 16, 2015
		- url() and archive() in archivedReviewRequestModel.js
		- added decorators in the archived_review_request webAPI resource
	+
	+	February 17, 2015
	+	- finished implementing query that filters archived review requests
	+	- added more functional hyperlink that toggles review request visibility with url query
	+	- still need to figure out how to append to existing, non-"show-archived" url queries

Commit:

-	ec502e8704937ace2508186aeb29686d8c8a93c4
+	15817c2ec1d9510700d4b6db06e70cd9c1078b64

Diff:

Revision 6 (+331 -7)

Show changes

	profile_show_archived.py
	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/managers.py
	reviewboard/static/rb/css/common.less
	6 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/accounts/evolutions/visibility.py
    profile_show_archived.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/reviews/managers.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js

profile_show_archived.py (Diff revision 6)

Show all issues

Col: 26
 E225 missing whitespace around operator

profile_show_archived.py (Diff revision 6)

Show all issues

Col: 5
 W604 backticks are deprecated, use 'repr()'

profile_show_archived.py (Diff revision 6)

Show all issues

Col: 7
 E226 missing whitespace around arithmetic operator

profile_show_archived.py (Diff revision 6)
Show all issues
```
Col: 1
 E112 expected an indented block
```
reviewboard/accounts/evolutions/visibility.py (Diff revision 6)
Show all issues
```
Col: 9
 E128 continuation line under-indented for visual indent
```

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table

		February 11, 2015
		- added archive and mute buttons in template with handlers
		- added a reviewRequestVisit JavaScript resource that talks to the webAPI
		- began review_request_visit API

		February 13, 2015
		- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
		- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
		- began archive banner

		February 16, 2015
		- url() and archive() in archivedReviewRequestModel.js
		- added decorators in the archived_review_request webAPI resource

		February 17, 2015
		- finished implementing query that filters archived review requests
		- added more functional hyperlink that toggles review request visibility with url query
~		- still need to figure out how to append to existing, non-"show-archived" url queries
	~	- works as it should if there are no other existing url queries, but still need to figure out how to append to existing, non-"show-archived" url queries

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table

		February 11, 2015
		- added archive and mute buttons in template with handlers
		- added a reviewRequestVisit JavaScript resource that talks to the webAPI
		- began review_request_visit API

		February 13, 2015
		- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
		- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
		- began archive banner

		February 16, 2015
		- url() and archive() in archivedReviewRequestModel.js
		- added decorators in the archived_review_request webAPI resource

		February 17, 2015
		- finished implementing query that filters archived review requests
		- added more functional hyperlink that toggles review request visibility with url query
		- works as it should if there are no other existing url queries, but still need to figure out how to append to existing, non-"show-archived" url queries
	+
	+	February 19, 2015
	+	- fixed archive query
	+	- completed POST request for archiving
	+	- made "Hide" button disappear when review request is already archived

Commit:

-	15817c2ec1d9510700d4b6db06e70cd9c1078b64
+	00a52a262629140a2a129c64195079dab47d350d

Diff:

Revision 7 (+322 -7)

Show changes

	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
	5 more

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js

reviewboard/accounts/evolutions/visibility.py (Diff revision 7)
Show all issues
```
Col: 9
 E128 continuation line under-indented for visual indent
```

reviewboard/datagrids/grids.py (Diff revision 7)
Show all issues
```
This isn't used anymore.
```

Change Summary:


added the archive banner
half-way through unarchiving

Description:

		Orig: January 25, 2015
		- added visibility field to ReviewRequestVisit with visible (V) and archived (A) states
		- added show_archived attribute to DashboardDataGrid
		- added ReviewRequestVisitManager

		January 31, 2015
		- scrapped ReviewRequestVisitManager idea
		- added a query that filters archived requests to ReviewRequestManager
		- wrote and applied a evolution for visibility in the ReviewRequestVisit table

		February 11, 2015
		- added archive and mute buttons in template with handlers
		- added a reviewRequestVisit JavaScript resource that talks to the webAPI
		- began review_request_visit API

		February 13, 2015
		- scrapped review_request_visit API and decided on two separate archived_request_visit and muted_request_visit APIs to prevent visits from being exposed
		- added archive/unarchive/mute/unmute functions in reviewRequestVisit model
		- began archive banner

		February 16, 2015
		- url() and archive() in archivedReviewRequestModel.js
		- added decorators in the archived_review_request webAPI resource

		February 17, 2015
		- finished implementing query that filters archived review requests
		- added more functional hyperlink that toggles review request visibility with url query
		- works as it should if there are no other existing url queries, but still need to figure out how to append to existing, non-"show-archived" url queries

		February 19, 2015
		- fixed archive query
		- completed POST request for archiving
		- made "Hide" button disappear when review request is already archived
	+
	+	February 28, 2015

Commit:

-	00a52a262629140a2a129c64195079dab47d350d
+	0efec0f2ca7bfc80777956ace53bf5509c324cb1

Diff:

Revision 8 (+350 -22)

Show changes

	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
	7 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/templates/reviews/review_header.html
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js



Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/templates/reviews/review_header.html
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js

reviewboard/webapi/resources/archived_review_request.py (Diff revision 8)
Show all issues
```
Col: 19
 E702 multiple statements on one line (semicolon)
```

Commit:

-	0efec0f2ca7bfc80777956ace53bf5509c324cb1
+	368a6e448f4ca03712cfe8d5d0f53925f24cb916

Diff:

Revision 9 (+377 -39)

Show changes

	reviewboard/staticbundles.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/static/rb/js/models/userSessionModel.js
	10 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/base.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/models/userSessionModel.js

reviewboard/webapi/resources/archived_review_request.py (Diff revision 9)
Show all issues
```
Col: 19
 E702 multiple statements on one line (semicolon)
```

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/accounts/evolutions/visibility.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/datagrids/grids.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/models.py
    reviewboard/staticbundles.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/templates/base.html
    reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/models/userSessionModel.js

reviewboard/accounts/evolutions/__init__.py (Diff revision 9)

Show all issues

Should have the model name before it: "reviewrequestvisit_visibility"

I don't see any profile settings in here?

reviewboard/accounts/evolutions/visibility.py (Diff revision 9)
Show all issues
```
Blank line between these.
```

reviewboard/accounts/models.py (Diff revision 9)

Show all issues

Should use one of the constants instead of hard-coding V.

I'm not sure we want to index this here. If there's 500 archived review requests, then the index will contain 500 entries under 'A', which isn't that useful. The user's own archived ones will still need to be searched within that 500.

If we want to index something, it's probably best to do an index_together = ('user', 'visibility').

reviewboard/datagrids/grids.py (Diff revision 9)
Show all issues
```
This is already defined in ReviewRequestDataGrid.
```

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

Show all issues

I feel like we're doing a bunch of unnecessary work for this 'show_archived' flag. I also think we can't assume that profile is defined here yet. Also, we need to record whether we've changed a profile setting.
How about we instead do:

profile_changed = False

...

default_show_archived = (profile and profile.extra_data.get('show_archived', False))

try:
    self.show_archived = ...
except ValueError:
    self.show_archived = default_show_archived

...

if profile and self.show_archived != default_show_archived:
    profile.extra_data[...] = ...
    profile_changed = True

parent_profile_changed = super(...).load_extra_state(...)

return profile_changed or parent_profile_changed

reviewboard/datagrids/grids.py (Diff revision 9)
Show all issues
```
Best named hidden_q or just queryset.
```

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

Show all issues

Instead of the ~Q(...), use Q(visibility__ne=...)

cristocrat March 5, 2015, 10:51 a.m.

This is giving me a FieldError: Join on field 'visibility' not permitted. Did you misspell 'ne' for the lookup type?

david March 6, 2015, 1:47 p.m.

Unfortunately Christian is making up the __ne operator (see https://code.djangoproject.com/ticket/5763).
This could be ReviewRequestVisit.filter(user=user).exclude(visibility=ReviewRequestVisit.VISIBLE), though.

reviewboard/reviews/views.py (Diff revision 9)
Show all issues
```
Should use the consatnt instead of "P".
```
reviewboard/static/rb/js/resources/models/archivedReviewRequestModel.js (Diff revision 9)
Show all issues
```
I don't think we need this anymore?
```
reviewboard/staticbundles.py (Diff revision 9)
Show all issues
```
Should be added alphabetically.
```

Change Summary:


finished banners
implemented automatic refreshing
finished unarchiving
finished all of muting and unmuting

Commit:

-	368a6e448f4ca03712cfe8d5d0f53925f24cb916
+	3ba7d6d3bb8df2c25aa1678aab9053ad30c7850b

Diff:

Revision 10 (+453 -49)

Show changes

	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/models/userSessionModel.js
	10 more

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
    reviewboard/datagrids/grids.py
    reviewboard/accounts/models.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/templates/base.html
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/models/userSessionModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
    reviewboard/datagrids/grids.py
    reviewboard/accounts/models.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/templates/base.html
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/models/userSessionModel.js

reviewboard/datagrids/grids.py (Diff revision 10)
Show all issues
```
 'Q' imported but unused
```

This is getting to be a pretty big change. How would you feel about splitting this up into several review requests? One to add the database stuff, one for the dashboard queries, one to add API, and one for frontend?

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

Rather than using a  continuation character here, can you put the whole expression in parens?

if (review_request.public and
    review_request.status == review_request.PENDING_REVIEW):

reviewboard/static/rb/js/resources/models/reviewGroupModel.js (Diff revision 10)

Show all issues

These should probably be defined inside the UserSession object (right after this.watchedGroups = ...). We should also wrap these strings in gettext().

reviewboard/static/rb/js/resources/models/reviewRequestModel.js (Diff revision 10)
Show all issues
```
Same comment as above.
```
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 10)
Show all issues
```
Same comment as above.
```
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 10)
Show all issues
```
Same comment as above.
```
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 10)
Show all issues
```
Same comment as above.
```
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 10)
Show all issues
```
Same comment as above.
```
reviewboard/templates/reviews/review_header.html (Diff revision 10)
Show all issues
```
This should say "review request" instead of "change"
```
reviewboard/templates/reviews/review_header.html (Diff revision 10)
Show all issues
```
This should say "review request" instead of "change"
```

Change Summary:


signals for automatic unarchiving
fixed url query

Commit:

-	3ba7d6d3bb8df2c25aa1678aab9053ad30c7850b
+	49668b4aea5e044641aa81265b7e76c809c9f066

Diff:

Revision 11 (+518 -61)

Show changes

	reviewboard/accounts/managers.py
	reviewboard/accounts/models.py
	reviewboard/accounts/evolutions/__init__.py
	reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
	reviewboard/datagrids/grids.py
	reviewboard/datagrids/views.py
	reviewboard/reviews/signals.py
	reviewboard/reviews/views.py
	13 more

Fix it!

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/datagrids/views.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/datagrids/grids.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
    reviewboard/accounts/managers.py
    reviewboard/accounts/models.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py
    reviewboard/reviews/signals.py
    reviewboard/reviews/models/review.py

Ignored Files:
    reviewboard/templates/base.html
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/datagrids/hideable_review_request_listview.html
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/models/userSessionModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/datagrids/views.py
    reviewboard/webapi/resources/muted_review_request.py
    reviewboard/datagrids/grids.py
    reviewboard/accounts/evolutions/__init__.py
    reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
    reviewboard/accounts/managers.py
    reviewboard/accounts/models.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/resources/archived_review_request.py
    reviewboard/reviews/signals.py
    reviewboard/reviews/models/review.py

Ignored Files:
    reviewboard/templates/base.html
    reviewboard/templates/reviews/review_header.html
    reviewboard/templates/datagrids/hideable_review_request_listview.html
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/static/rb/css/common.less
    reviewboard/templates/reviews/review_request_actions_secondary.html
    reviewboard/static/rb/js/models/userSessionModel.js

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

Show all issues

Col: 21
 E126 continuation line over-indented for hanging indent

reviewboard/accounts/models.py (Diff revision 11)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/datagrids/grids.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/datagrids/views.py (Diff revision 11)
Show all issues
```
Col: 7
 E111 indentation is not a multiple of four
```

Fix it!

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

Show all issues

No blank line here (django and djblets are both "3rd party" libraries from Review Board's point of view).

reviewboard/accounts/managers.py (Diff revision 11)
Show all issues
```
Can you add docstrings for the class and method?
```

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

Show all issues

Please add visibility=self.model.ARCHIVED to the filter call (we don't want to update muted review requests).

reviewboard/accounts/models.py (Diff revision 11)
Show all issues
```
Please add a blank line after the docstring.
```

reviewboard/accounts/models.py (Diff revision 11)

Show all issues

review_published and reply_published pass a Review instance as their argument rather than a ReviewRequest. You'll need a separate handler for those signals which accesses review.review_request

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

Show all issues

Since you have parens, put the opening paren on the previous line instead of using \\

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

Show all issues

I think djblets.util.http.get_url_params_except will do what you want.

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

Show all issues

Please don't change the signatures of these, and instead have a separate handler.

cristocrat March 24, 2015, 12:34 p.m.

I don't think I understand how parameters are passed. I get a TypeError: _call_unarchive_all_for_review() takes exactly 2 arguments (1 given) in my @receiver that looks like:
@receiver([review_published, reply_published])

def _call_unarchive_all_for_review(sender, review, **kwargs):

ReviewRequestVisit.objects.unarchive_all(review.review_request)

cristocrat March 24, 2015, 1:49 p.m.

nevermind, I got it to work it with separate handlers. The only thing now is that replying does not auto-refresh, so the "archived" banner does not disappear when you reply to something, only when the page is refreshed, is this okay?

reviewboard/static/rb/js/models/userSessionModel.js (Diff revision 11)

Show all issues

I don't understand what the visibility attribute is used for. Can you explain?

cristocrat March 24, 2015, 12:34 p.m.

visibility is used on line 75 to distinguish between watched items and hidden items for the trigger

reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 11)
Show all issues
```
Please use "review request" instead of "change"
```
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 11)
Show all issues
```
Please use "review request" instead of "change"
```

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

[WIP] Add archived state to review requests

February 28, 2015

Description:

Commit:

Diff:

Groups:

Change Summary:

Commit:

Diff:

Description:

Commit:

Diff:

Description:

Commit:

Diff:

Description:

Commit:

Diff:

Description:

Description:

Commit:

Diff:

Change Summary:

Description:

February 28, 2015

Commit:

Diff:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Status: Discarded