Add a Dashboard toggle to show or hide archived review requests
Review Request #7109 — Created March 23, 2015 and submitted
| Information | |
|---|---|
| cristocrat | |
| Review Board | |
| master | |
|
|
|
| 7087 | |
| Reviewers | |
| reviewboard, students | |
This adds a toggle on the Dashboard datagrid to show or hide archived review requests, similar to that of the All Review Requests datagrid for showing and hiding closed review requests.
- a new template block is loaded in the dashboard to expose the toggle button
- when the pressed, a
show-archivedquery is appended to the existing URL query show-archived=0filters the archived review requests (those that are archived or muted)show-archived=1includes all review requests, regardless of their state of visibility
NB: When show-archived is not specified, its default value of is 0.
Manual tests: Archived and muted several review requests. In the dashboard, they disappeared when setting
show-archived=0and reappeared when settingshow-archived=1Unit tests: Added to reviewboard.datagrids.tests to test if archived review requests would appear in the datagrid for a particular user for
show-archived=0andshow-archived=1. Also tested for a different user who had not archived any of the same review requests. All tests passed.
| Description | From | Last Updated |
|---|---|---|
|
Could you alphabetize these? |
 brennie
|
|
|
I have the same question here as in the other review request about why we aren't using default_show_hidden as the … |
brennie
|
|
|
Can you chang the formatting so we don't use a backslash, e.g. parent_profile_changed = super(DashboardDataGrid, self).load_extra_state( profile, allow_hide_closed=False) |
brennie
|
|
|
Will {{datagrid.queries}} expand to have a & in it if there are queries? |
brennie
|
|
|
Can we move this down to be in an else clause of the if profile and self.show_hidden != default_show_hidden: conditional? |
 david
|
|
|
In order to address Barret's question, how about this, which looks somewhat more similar to ShowClosedReviewRequestsMixin if profile and 'show_hidden' … |
david
|
|
|
Can we wrap this a little differently? parent_profile_changed = \ super(DashboardDataGrid, self).load_extra_state( profile, allow_hide_closed=False) |
david
|
|
|
Can we do this in DashboardDataGrid.__init__ instead? |
david
|
|
|
This shouldn't be indented at all. |
david
|
|
|
I think we should call these "Show/Hide archived" rather than "hidden". It's true that it isn't totally correct (since it … |
david
|
|
|
Instead of using \, can you add parens around this and wrap after and? if (profile and self.show_archived != profile.extra_data.get('show_archived')): |
david
|
|
|
This is the only change in this file now--can you revert it? |
david
|
|
|
Col: 13 E113 unexpected indentation |
 reviewbot
|
|
|
Col: 13 E112 expected an indented block |
reviewbot
|
|
|
I think it's just under 80 columns if you were to put this all on one line. |
david
|
-
-
-
reviewboard/datagrids/grids.py (Diff revision 1) I have the same question here as in the other review request about why we aren't using
default_show_hiddenas the default parameter. -
reviewboard/datagrids/grids.py (Diff revision 1) Can you chang the formatting so we don't use a backslash, e.g.
parent_profile_changed = super(DashboardDataGrid, self).load_extra_state( profile, allow_hide_closed=False) -
reviewboard/templates/datagrids/hideable_listview.html (Diff revision 1) Will
{{datagrid.queries}}expand to have a&in it if there are queries?
CR
Summary: |
|
|||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||
Testing Done: |
|
|||||||||||||||||||||||||||
Groups: |
|
|||||||||||||||||||||||||||
Diff: |
Revision 2 (+147 -7) |
-
Tool: Pyflakes Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less
-
-
reviewboard/datagrids/grids.py (Diff revision 2) Can we move this down to be in an else clause of the
if profile and self.show_hidden != default_show_hidden:conditional? -
reviewboard/datagrids/grids.py (Diff revision 2) Can we wrap this a little differently?
parent_profile_changed = \ super(DashboardDataGrid, self).load_extra_state( profile, allow_hide_closed=False) -
reviewboard/datagrids/views.py (Diff revision 2) Can we do this in
DashboardDataGrid.__init__instead? -
reviewboard/templates/datagrids/hideable_listview.html (Diff revision 2) This shouldn't be indented at all.
-
reviewboard/templates/datagrids/hideable_listview.html (Diff revision 2) I think we should call these "Show/Hide archived" rather than "hidden". It's true that it isn't totally correct (since it also shows muted), but calling them "hidden" means that we're introducing a third term.
-
-
reviewboard/datagrids/grids.py (Diff revision 2) In order to address Barret's question, how about this, which looks somewhat more similar to
ShowClosedReviewRequestsMixinif profile and 'show_hidden' in profile.extra_data: self.show_hidden = profile.extra_data['show_hidden'] try: show = self.request.GET.get('show-hidden', self.show_hidden) self.show_hidden = int(show) != 0 except ValueError: pass
-
Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less Tool: Pyflakes Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less
-
Tool: Pyflakes Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less
-
-
reviewboard/datagrids/grids.py (Diff revision 4) Instead of using \, can you add parens around this and wrap after
and?if (profile and self.show_archived != profile.extra_data.get('show_archived')): -
reviewboard/datagrids/views.py (Diff revision 4) This is the only change in this file now--can you revert it?
-
Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less -
-
-
Tool: Pyflakes Processed Files: reviewboard/datagrids/grids.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py reviewboard/datagrids/tests.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less
-
Your summary/description needs to be updated to refer to the new naming.
-
reviewboard/datagrids/grids.py (Diff revision 6) I think it's just under 80 columns if you were to put this all on one line.
CR
Summary: |
|
|||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||
Testing Done: |
|
Tool: Pyflakes Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less Tool: PEP8 Style Checker Processed Files: reviewboard/datagrids/grids.py reviewboard/datagrids/views.py Ignored Files: reviewboard/templates/datagrids/hideable_listview.html reviewboard/static/rb/css/common.less