Checks run (1 failed, 1 succeeded, 1 failed with error)
JSHint
passed.
PEP8 Style Checker
internal error.
Pyflakes
failed.
Read-only mode docs and unit tests for decorator and redirection
Review Request #8847 — Created March 26, 2017 and submitted
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" |
brennie
|
|
|
"enabling read-only mode" instead of "putting the site in read-only mode" |
brennie
|
|
|
into. No comma here. |
brennie
|
|
|
"Review Board" |
brennie
|
|
|
The code should be aligned with the c in code-block, i.e., three spaces (yes, I know it's weird). |
brennie
|
|
|
"into" |
brennie
|
|
|
Just "API requests". |
brennie
|
|
|
How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to … |
brennie
|
|
|
How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to … |
brennie
|
|
|
Wrap readOnly in double-backtickcs so that gets marked as <code>. |
brennie
|
|
|
Likewise here. |
brennie
|
|
|
In JavaScript, this would actually be: if (conditional) { /* Comment */ } The code here also doesnt fit the … |
brennie
|
|
|
Undo this. |
brennie
|
|
|
We don't require docstrings for setUp, tearDown, etc. |
brennie
|
|
|
This can be moved into setUpClass, which only runs once per class, e.g. @classmethod def setUpClass(cls): super(LoginRequiredTest, cls).setUpClass() # ... |
brennie
|
|
|
This should be _mock_view_function. Also your args are in the wrong order. |
brennie
|
|
|
Care to split this up into user & super-user tests? |
brennie
|
|
|
Instead of "the decorator", say Testing @login_required. Same goes for all the tests here. No trailing periods in test docstrings. |
brennie
|
|
|
This comment is |
brennie
|
|
|
Care to split this up into user & super-user tests? |
brennie
|
|
|
Care to split this up into user & super-user tests? |
brennie
|
|
|
""" should be on next line. |
brennie
|
|
|
redefinition of unused 'test_my_account_does_not_redirect_when_not_read_only' from line 647 |
reviewbot
|
|
|
This should be a phrase in the imperative mode, e.g. """Check response versus the expected value""" Missing args etc |
brennie
|
|
|
Can you split this into new tests? |
brennie
|
-
-
-
-
-
-
-
-
-
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."
-
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."
-
-
-
In JavaScript, this would actually be:
if (conditional) { /* Comment */ }The code here also doesnt fit the above.
-
-
-
This can be moved into
setUpClass, which only runs once per class, e.g.@classmethod def setUpClass(cls): super(LoginRequiredTest, cls).setUpClass() # ... -
-
-
Instead of "the decorator", say
Testing @login_required. Same goes for all the tests here.No trailing periods in test docstrings.
-
-
-
-
-
This should be a phrase in the imperative mode, e.g.
"""Check response versus the expected value"""
Missing args etc
-
KH
Review request changed
- Summary:
-
[WIP] Read-only mode docs and unit testsRead-only mode docs and unit tests for decorator
- Description:
-
~ WIP for read-only mode.
~ ~ Adding some documentation and unit tests for decorator and redirections.
~ 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. - Testing Done:
-
+ Build docs - no problem
+ Run python tests - pass except a couple that will be fixed in next commit - Depends On:
-
KH
Review request changed
- Summary:
-
Read-only mode docs and unit tests for decoratorRead-only mode docs and unit tests for decorator and redirection
- Description:
-
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. ~ decorator and its usage to redirect New Review Request and My + Account when in read-only mode.
KH
Review request changed
- Change Summary:
-
Reponse to comments
- Diff:
-
Revision 2 (+275 -4)
Checks run (2 succeeded, 1 failed with error)
JSHint
passed.
PEP8 Style Checker
internal error.
Pyflakes
passed.