Summary

Added a filter on the users page for viewing inactive/removed users

Review Request #8650 — Created Jan. 21, 2017 and submitted Jan. 24, 2017, 1:33 p.m.

Information

Owner

rkdhatt

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

8ebbef2...

Reviewers

Groups

reviewboard, students

People

Description

There's been a request to add a filter to the Users page that allows for showing those who are no longer part of the team. This was done by updating class 'UsersDataGrid' to introduce show/hide inactive users functionality. I also created templates/datagrids/user_listview.html to display the show/hide inactive toggle.

Testing Done

This was tested by changing a user from active to inactive via admin page. Then, I checked on the users page if an inactive user disappears after clicking 'Hide inactive', and whether an inactive user also appears after clicking 'Show inactive'.

Issues

Description	From	Last Updated
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Jan. 21, 2017, 1:48 p.m.
Because there's only one UsersDataGrid, we don't really need a mixin class. We can just put this implementation into UsersDataGrid.load_extra_state	david	Jan. 21, 2017, 1:49 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Jan. 21, 2017, 1:48 p.m.
Col: 17 E122 continuation line missing indentation or outdented	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 14 E251 unexpected spaces around keyword / parameter equals	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 16 E251 unexpected spaces around keyword / parameter equals	reviewbot	Jan. 21, 2017, 1:49 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Jan. 21, 2017, 1:49 p.m.
Do we need the reviewtags library here?	david	Jan. 21, 2017, 1:58 p.m.
Space between datagrid_filters and %}	brennie	Jan. 21, 2017, 1:49 p.m.
Col: 28 E251 unexpected spaces around keyword / parameter equals	reviewbot	Jan. 21, 2017, 1:55 p.m.
Col: 26 E251 unexpected spaces around keyword / parameter equals	reviewbot	Jan. 21, 2017, 1:55 p.m.
The load_extra_state method takes a non-optional profile argument.	david	Jan. 21, 2017, 2:45 p.m.
The other code in this file is old and doesn't match up with our new docstring guidelines. This should include …	david	Jan. 21, 2017, 2:45 p.m.
Can you make this "Do nothing."?	david	Jan. 21, 2017, 2:45 p.m.
allow_hide_inactive is always going to be True so we could just get rid of it.	david	Jan. 21, 2017, 2:45 p.m.
This can all go on one line.	david	Jan. 21, 2017, 2:45 p.m.
Col: 51 W291 trailing whitespace	reviewbot	Jan. 21, 2017, 2:51 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Jan. 21, 2017, 2:51 p.m.
Col: 58 W291 trailing whitespace	reviewbot	Jan. 21, 2017, 2:51 p.m.
Col: 37 W291 trailing whitespace	reviewbot	Jan. 21, 2017, 2:51 p.m.
Col: 51 W291 trailing whitespace	reviewbot	Jan. 21, 2017, 2:53 p.m.
The summary should be a single line, followed by a blank line, followed by more stuff. The following stuff should …	david	Jan. 21, 2017, 3:02 p.m.
Blank line between these.	david	Jan. 21, 2017, 3:02 p.m.
Should be "bool" instead of "Boolean"	david	Jan. 21, 2017, 3:02 p.m.
"The user profile..."	brennie	Jan. 22, 2017, 10:47 a.m.
So a few things here: self.show_inactive is initially False and int(False) will raise ValueError. Instead of self.show_inactive, lets use 0. …	brennie	Jan. 22, 2017, 10:47 a.m.
Not necessary.	brennie	Jan. 22, 2017, 10:47 a.m.
Col: 62 E241 multiple spaces after ','	reviewbot	Jan. 22, 2017, 10:51 a.m.
Blank line between these.	brennie	Jan. 23, 2017, 2:21 p.m.
I just noticed that there's an extra </li> added on each of these lines (in addition to the one after …	david	Jan. 23, 2017, 3:07 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E101 indentation contains mixed spaces and tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E101 indentation contains mixed spaces and tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 17
 E122 continuation line missing indentation or outdented
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E101 indentation contains mixed spaces and tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 14
 E251 unexpected spaces around keyword / parameter equals
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 16
 E251 unexpected spaces around keyword / parameter equals
```
reviewboard/datagrids/grids.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W191 indentation contains tabs
```

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

The issue has been resolved. Show all issues

Because there's only one UsersDataGrid, we don't really need a mixin class. We can just put this implementation into UsersDataGrid.load_extra_state

reviewboard/templates/datagrids/user_listview.html (Diff revision 1)
The issue has been dropped. Show all issues
```
Do we need the reviewtags library here?
```
1. RK
  
  rkdhatt Jan. 21, 2017, 1:38 p.m.
  Yup we do! Without it, the listview doesn't show up at all.

reviewboard/templates/datagrids/user_listview.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Space between datagrid_filters and %}
```

Description:

~		Included a class 'ShowInactiveUsersMixin' to show/hide inactive users (datagrids/grids.py).
	~	There's been a request to add a filter to the Users page that allows for showing those who are no longer part of the team. This was done by updating class 'UsersDataGrid' to introduce show/hide inactive users functionality. I also created templates/datagrids/user_listview.html to display the show/hide inactive toggle.
-		Updated class 'UsersDataGrid' to introduce show/hide inactive users functionality.
-		Created templates/datagrids/user_listview.html to display show or hide inactive toggle.

Testing Done:

~		Changed a user from active to inactive via admin page.
	~	This was tested by changing a user from active to inactive via admin page. Then, I checked on the users page if an inactive user disappears after clicking 'Hide inactive', and whether an inactive user also appears after clicking 'Show inactive'.
-		Checked on users page if inactive user disappears after clicking 'Hide inactive'.
-		Checked on users page if inactive user appears after clicking 'Show inactive'.

Commit:

-	f9b83bf91dc0ac7cf918a0f28d54bca6041156b3
+	4c4b58762feda868003c1086047ac3dfbd14a00e

Diff:

Revision 2 (+31 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 28
 E251 unexpected spaces around keyword / parameter equals
```
reviewboard/datagrids/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 26
 E251 unexpected spaces around keyword / parameter equals
```

Commit:

-	4c4b58762feda868003c1086047ac3dfbd14a00e
+	f175345340c784e75002f83c3a7ffd5ea174bab4

Diff:

Revision 3 (+31 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 3)
The issue has been resolved. Show all issues
```
The load_extra_state method takes a non-optional profile argument.
```

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

The issue has been resolved. Show all issues

The other code in this file is old and doesn't match up with our new docstring guidelines. This should include "Args" and "Returns" sections as per https://www.reviewboard.org/docs/codebase/dev/docs/writing-codebase-docs/

reviewboard/datagrids/grids.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can you make this "Do nothing."?
```
reviewboard/datagrids/grids.py (Diff revision 3)
The issue has been resolved. Show all issues
```
allow_hide_inactive is always going to be True so we could just get rid of it.
```
reviewboard/datagrids/grids.py (Diff revision 3)
The issue has been resolved. Show all issues
```
This can all go on one line.
```

Commit:

-	f175345340c784e75002f83c3a7ffd5ea174bab4
+	20822660ea0b25ecf43f47ae7289575cb251362d

Diff:

Revision 4 (+40 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Col: 51
 W291 trailing whitespace
```
reviewboard/datagrids/grids.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/datagrids/grids.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Col: 58
 W291 trailing whitespace
```
reviewboard/datagrids/grids.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Col: 37
 W291 trailing whitespace
```

Commit:

-	20822660ea0b25ecf43f47ae7289575cb251362d
+	3848bba91de066afcc7c60113b6fee84e1eafe98

Diff:

Revision 5 (+40 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Col: 51
 W291 trailing whitespace
```

Commit:

-	3848bba91de066afcc7c60113b6fee84e1eafe98
+	52ff6553335a2b22fa296bb04515131cce0c134b

Diff:

Revision 6 (+40 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

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

The issue has been resolved. Show all issues

The summary should be a single line, followed by a blank line, followed by more stuff. The following stuff should be aligned to the starting column of the """. Something like this:


def load_extra_state(self, profile):
    """Load extra state for the datagrid.

    This handles hiding or showing inactive users.

    Args:
        profile (reviewboard.accounts.models.Profile):
            User profile which contains some basic configurable
            settings.

    Returns:
        bool:
        Always returns False.
    """

reviewboard/datagrids/grids.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/datagrids/grids.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Should be "bool" instead of "Boolean"
```

Commit:

-	52ff6553335a2b22fa296bb04515131cce0c134b
+	4815b4aaa5fac8c2531d4eed7ba80ea4a34948bd

Diff:

Revision 7 (+42 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 7)
The issue has been resolved. Show all issues
```
"The user profile..."
```

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

The issue has been resolved. Show all issues

So a few things here:

self.show_inactive is initially False and int(False) will raise ValueError. Instead of self.show_inactive, lets use 0.
If you pull the request.GET.get(...) code into a separate variable, it will look slightly cleaner, i.e. less indents:

show_inactive = self.request.GET.get('show-inactive',  0)

try:
    self.show_inactive = int(show_inactive)
except ValueError:
    pass

reviewboard/datagrids/grids.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Not necessary.
```

Commit:

-	4815b4aaa5fac8c2531d4eed7ba80ea4a34948bd
+	cb413a2b8ce0622b3202e35a074ae1294f1bcf3e

Diff:

Revision 8 (+40 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/datagrids/grids.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Col: 62
 E241 multiple spaces after ','
```

Commit:

-	cb413a2b8ce0622b3202e35a074ae1294f1bcf3e
+	41b05a9370a879f71d29c4ee1a3d1ab624bd311b

Diff:

Revision 9 (+40 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

Ship it!

```
One last nitpick!
```
reviewboard/datagrids/grids.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Blank line between these.
```

Commit:

-	41b05a9370a879f71d29c4ee1a3d1ab624bd311b
+	5170d17ba4615322017295c9fd9e3c7f04f59461

Diff:

Revision 10 (+41 -1)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/user_listview.html

reviewboard/templates/datagrids/user_listview.html (Diff revision 10)

The issue has been resolved. Show all issues

I just noticed that there's an extra </li> added on each of these lines (in addition to the one after the {% endif %}). This is also a bug in the code you modeled this on, so it's totally not your fault.

We should at least fix it here, and if you wanted to fix up the one in review_request_listview.html as well, that would be awesome.

Commit:

-	5170d17ba4615322017295c9fd9e3c7f04f59461
+	8ebbef260edd69fbe46f9ec8925a008e503ba00b

Diff:

Revision 11 (+43 -3)

Show changes

	reviewboard/datagrids/grids.py
	reviewboard/templates/datagrids/review_request_listview.html
	reviewboard/templates/datagrids/user_listview.html

Tool: Pyflakes
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/review_request_listview.html
    reviewboard/templates/datagrids/user_listview.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/datagrids/grids.py

Ignored Files:
    reviewboard/templates/datagrids/review_request_listview.html
    reviewboard/templates/datagrids/user_listview.html

Ship it!

```
This looks good to me! Great work!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (294bb5c)