Fix AccountPage to have default form classes on initial page registration

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

Wang Jun Sun
Review Board
master
7099
4a5e110...
reviewboard
chipx86

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.

  • 0
  • 0
  • 8
  • 0
  • 8
Description From Last Updated
Review Bot
  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)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. reviewboard/accounts/tests.py (Diff revision 1)
     
     
    Col: 20
     E225 missing whitespace around operator
    
  4. reviewboard/accounts/tests.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. 
      
Wang Jun Sun
Review Bot
  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. 
      
Xuanyi Lin
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 2)
     
     

    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 Trowbridge
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 2)
     
     
     
     
     

    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)
     
     

    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)
     
     

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

  5. 
      
Wang Jun Sun
Review Bot
  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. 
      
Barret Rennie
  1. 
      
  2. reviewboard/accounts/pages.py (Diff revision 3)
     
     
     

    Blank line between after class docstring.

  3. 
      
Wang Jun Sun
Review Bot
  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 Trowbridge
  1. Ship It!
  2. 
      
Wang Jun Sun
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (d075eb2)
Loading...