Read-only mode docs and unit tests for decorator and redirection

Review Request #8847 — Created March 26, 2017 and submitted

Information

khp
Review Board
master

Reviewers

Read-only mode is a setting an admin can enable to prevent writes to
the database. This can be used when the site is under maintenence or
being upgraded. This commit adds some new documentation on how
extension authors can utilize read-only mode in js and django.
This commit also includes unit testing for the wrapped login_required
decorator and its usage to redirect New Review Request and My
Account when in read-only mode.

Build docs - no problem
Run python tests - pass except a couple that will be fixed in next commit

Description From Last Updated

"recommended that you enable read-only mode"

brenniebrennie

"enabling read-only mode" instead of "putting the site in read-only mode"

brenniebrennie

into. No comma here.

brenniebrennie

"Review Board"

brenniebrennie

The code should be aligned with the c in code-block, i.e., three spaces (yes, I know it's weird).

brenniebrennie

"into"

brenniebrennie

Just "API requests".

brenniebrennie

How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to …

brenniebrennie

How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to …

brenniebrennie

Wrap readOnly in double-backtickcs so that gets marked as <code>.

brenniebrennie

Likewise here.

brenniebrennie

In JavaScript, this would actually be: if (conditional) { /* Comment */ } The code here also doesnt fit the …

brenniebrennie

Undo this.

brenniebrennie

We don't require docstrings for setUp, tearDown, etc.

brenniebrennie

This can be moved into setUpClass, which only runs once per class, e.g. @classmethod def setUpClass(cls): super(LoginRequiredTest, cls).setUpClass() # ...

brenniebrennie

This should be _mock_view_function. Also your args are in the wrong order.

brenniebrennie

Care to split this up into user & super-user tests?

brenniebrennie

Instead of "the decorator", say Testing @login_required. Same goes for all the tests here. No trailing periods in test docstrings.

brenniebrennie

This comment is

brenniebrennie

Care to split this up into user & super-user tests?

brenniebrennie

Care to split this up into user & super-user tests?

brenniebrennie

""" should be on next line.

brenniebrennie

redefinition of unused 'test_my_account_does_not_redirect_when_not_read_only' from line 647

reviewbotreviewbot

This should be a phrase in the imperative mode, e.g. """Check response versus the expected value""" Missing args etc

brenniebrennie

Can you split this into new tests?

brenniebrennie
Checks run (1 failed, 1 succeeded, 1 failed with error)
JSHint passed.
PEP8 Style Checker internal error.
Pyflakes failed.

Pyflakes

brennie
  1. 
      
  2. "recommended that you enable read-only mode"

  3. "enabling read-only mode" instead of "putting the site in read-only mode"

  4. into.

    No comma here.

  5. "Review Board"

  6. docs/manual/extending/extensions/class.rst (Diff revision 1)
     
     
     
     
     
     
     

    The code should be aligned with the c in code-block, i.e., three spaces (yes, I know it's weird).

  7. Just "API requests".

  8. How about

    "When the site is in read-only mode, only changes made to models by superusers will be propagated to the server; changes made by all other users will be discarded."

  9. How about

    "When the site is in read-only mode, only changes made to models by superusers will be propagated to the server; changes made by all other users will be discarded."

  10. Wrap readOnly in double-backtickcs so that gets marked as <code>.

  11. docs/manual/extending/extensions/js-extensions.rst (Diff revision 1)
     
     
     
     
     
     
     
     

    Likewise here.

  12. In JavaScript, this would actually be:

    if (conditional) {
        /* Comment */
    }
    

    The code here also doesnt fit the above.

  13. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    Undo this.

  14. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    We don't require docstrings for setUp, tearDown, etc.

  15. reviewboard/accounts/tests.py (Diff revision 1)
     
     
     

    This can be moved into setUpClass, which only runs once per class, e.g.

    @classmethod
    def setUpClass(cls):
        super(LoginRequiredTest, cls).setUpClass()
    
        # ...
    
  16. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    This should be _mock_view_function. Also your args are in the wrong order.

  17. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    Care to split this up into user & super-user tests?

  18. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    Instead of "the decorator", say Testing @login_required. Same goes for all the tests here.

    No trailing periods in test docstrings.

  19. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    This comment is

  20. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    Care to split this up into user & super-user tests?

  21. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    Care to split this up into user & super-user tests?

  22. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    """ should be on next line.

  23. reviewboard/accounts/tests.py (Diff revision 1)
     
     

    This should be a phrase in the imperative mode, e.g.

    """Check response versus the expected value"""

    Missing args etc

  24. reviewboard/reviews/tests/test_views.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Can you split this into new tests?

  25. 
      
KH
KH
KH
KH
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (768aab4)
Loading...