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

Review Request #8650 — Created Jan. 21, 2017 and submitted

Information

Review Board
master
8ebbef2...

Reviewers

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.

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

Description From Last Updated

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

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

daviddavid

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 17 E122 continuation line missing indentation or outdented

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 14 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 16 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Do we need the reviewtags library here?

daviddavid

Space between datagrid_filters and %}

brenniebrennie

Col: 28 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 26 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

The load_extra_state method takes a non-optional profile argument.

daviddavid

The other code in this file is old and doesn't match up with our new docstring guidelines. This should include …

daviddavid

Can you make this "Do nothing."?

daviddavid

allow_hide_inactive is always going to be True so we could just get rid of it.

daviddavid

This can all go on one line.

daviddavid

Col: 51 W291 trailing whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 58 W291 trailing whitespace

reviewbotreviewbot

Col: 37 W291 trailing whitespace

reviewbotreviewbot

Col: 51 W291 trailing whitespace

reviewbotreviewbot

The summary should be a single line, followed by a blank line, followed by more stuff. The following stuff should …

daviddavid

Blank line between these.

daviddavid

Should be "bool" instead of "Boolean"

daviddavid

"The user profile..."

brenniebrennie

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

brenniebrennie

Not necessary.

brenniebrennie

Col: 62 E241 multiple spaces after ','

reviewbotreviewbot

Blank line between these.

brenniebrennie

I just noticed that there's an extra </li> added on each of these lines (in addition to the one after …

daviddavid
reviewbot
  1. 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
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  5. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  7. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  8. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 17
     E122 continuation line missing indentation or outdented
    
  11. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  12. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  13. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  14. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  15. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  16. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  17. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  18. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 14
     E251 unexpected spaces around keyword / parameter equals
    
  19. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 16
     E251 unexpected spaces around keyword / parameter equals
    
  20. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  21. 
      
david
  1. 
      
  2. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    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

  3. Show all issues

    Do we need the reviewtags library here?

    1. Yup we do! Without it, the listview doesn't show up at all.

  4. 
      
brennie
  1. 
      
  2. Show all issues

    Space between datagrid_filters and %}

  3. 
      
RK
reviewbot
  1. 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
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 2)
     
     
    Show all issues
    Col: 28
     E251 unexpected spaces around keyword / parameter equals
    
  3. reviewboard/datagrids/grids.py (Diff revision 2)
     
     
    Show all issues
    Col: 26
     E251 unexpected spaces around keyword / parameter equals
    
  4. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
david
  1. 
      
  2. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
    Show all issues

    The load_extra_state method takes a non-optional profile argument.

  3. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
    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/

  4. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
    Show all issues

    Can you make this "Do nothing."?

  5. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
    Show all issues

    allow_hide_inactive is always going to be True so we could just get rid of it.

  6. reviewboard/datagrids/grids.py (Diff revision 3)
     
     
     
    Show all issues

    This can all go on one line.

  7. 
      
RK
reviewbot
  1. 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
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 4)
     
     
    Show all issues
    Col: 51
     W291 trailing whitespace
    
  3. reviewboard/datagrids/grids.py (Diff revision 4)
     
     
    Show all issues
    Col: 1
     W293 blank line contains whitespace
    
  4. reviewboard/datagrids/grids.py (Diff revision 4)
     
     
    Show all issues
    Col: 58
     W291 trailing whitespace
    
  5. reviewboard/datagrids/grids.py (Diff revision 4)
     
     
    Show all issues
    Col: 37
     W291 trailing whitespace
    
  6. 
      
RK
reviewbot
  1. 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
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 5)
     
     
    Show all issues
    Col: 51
     W291 trailing whitespace
    
  3. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
david
  1. 
      
  2. reviewboard/datagrids/grids.py (Diff revision 6)
     
     
     
    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.
        """
    
  3. reviewboard/datagrids/grids.py (Diff revision 6)
     
     
     
    Show all issues

    Blank line between these.

  4. reviewboard/datagrids/grids.py (Diff revision 6)
     
     
    Show all issues

    Should be "bool" instead of "Boolean"

  5. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/datagrids/grids.py (Diff revision 7)
     
     
    Show all issues

    "The user profile..."

  3. reviewboard/datagrids/grids.py (Diff revision 7)
     
     
     
     
     
    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
    
  4. reviewboard/datagrids/grids.py (Diff revision 7)
     
     
    Show all issues

    Not necessary.

  5. 
      
RK
reviewbot
  1. 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
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 8)
     
     
    Show all issues
    Col: 62
     E241 multiple spaces after ','
    
  3. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. One last nitpick!

  2. reviewboard/datagrids/grids.py (Diff revision 9)
     
     
     
    Show all issues

    Blank line between these.

  3. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
david
  1. 
      
  2. 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.

  3. 
      
RK
reviewbot
  1. 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
    
    
  2. 
      
mike_conley
  1. This looks good to me! Great work!

  2. 
      
RK
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (294bb5c)