Fix AccountPage to have default form classes on initial page registration

Review Request #7103 — Created March 22, 2015 and submitted

Information

Review Board
master
4a5e110...

Reviewers

When an extension uses AccountPagesHook with a pre-built list of forms (form_classes), the list form_classes is cleared when the extension's AccountPage is unregistered after the extension reloads. This causes the extension's AccountPage(s) to not show up in the account page after.

This fix keeps track of the initial form_classes defined by the AccountPage with _default_form_classes, such that the original defined list of form_classes will be restored after an extension reload.

Wrote two unit tests which passed alongside existing tests. Also verified that an extension which uses AccountPagesHook still shows up in account pages after a reload.

Description From Last Updated

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 20 E225 missing whitespace around operator

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

I'm really confused by this. We initialize one from the other and vice versa? Can we keep the flow of …

daviddavid

Since we are comparing between empty or not empty list, I think this is possible as well. elif not page_cls.form_classes:

XU xuanyi

No period at the end of test case docstrings (nose will add a ... when printing the names)

daviddavid

No period at the end of test case docstrings (nose will add a ... when printing the names)

daviddavid

Blank line between after class docstring.

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
  2. reviewboard/accounts/pages.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. reviewboard/accounts/tests.py (Diff revision 1)
     
     
    Show all issues
    Col: 20
     E225 missing whitespace around operator
    
  4. reviewboard/accounts/tests.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. 
      
SU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
  2. 
      
XU
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 2)
     
     
    Show all issues

    Since we are comparing between empty or not empty list, I think this is possible as well.

    elif not page_cls.form_classes:
    
    1. Yes, that is more Pythonic, but there are also occurrences of the explicit condition elsewhere in the code base, so I am not sure what the standard is.

  3. 
      
david
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    I'm really confused by this. We initialize one from the other and vice versa? Can we keep the flow of data one direction?

    1. We initialize _default_form_classes only when it first registers. I have separated the block to make this clearer.

  3. reviewboard/accounts/tests.py (Diff revision 2)
     
     
    Show all issues

    No period at the end of test case docstrings (nose will add a ... when printing the names)

  4. reviewboard/accounts/tests.py (Diff revision 2)
     
     
    Show all issues

    No period at the end of test case docstrings (nose will add a ... when printing the names)

  5. 
      
SU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 3)
     
     
     
    Show all issues

    Blank line between after class docstring.

  3. 
      
SU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/tests.py
        reviewboard/accounts/pages.py
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
SU
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (d075eb2)