Summary:

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

Review Request #8847 — Created March 26, 2017 and submitted Jan. 10, 2018, 2:25 p.m.

Information
Owner:	khp
Repository:	Review Board
Branch:	master
Bugs:
Depends On:	~~8648~~, ~~8824~~, ~~8812~~, ~~8657~~, ~~8810~~, ~~8811~~
Blocks:	8861
Reviewers
Groups:	reviewboard, students
People:

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 and its usage to redirect New Review Request and My
Account when in read-only mode.

Testing Done

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

Issues

Description	From	Last Updated
"recommended that you enable read-only mode"	brennie	March 29, 2017, 12:19 p.m.
"enabling read-only mode" instead of "putting the site in read-only mode"	brennie	March 29, 2017, 12:20 p.m.
into. No comma here.	brennie	March 29, 2017, 12:21 p.m.
"Review Board"	brennie	March 29, 2017, 12:21 p.m.
The code should be aligned with the c in code-block, i.e., three spaces (yes, I know it's weird).	brennie	March 29, 2017, 12:21 p.m.
"into"	brennie	March 29, 2017, 12:22 p.m.
Just "API requests".	brennie	March 29, 2017, 12:22 p.m.
How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to …	brennie	March 29, 2017, 12:23 p.m.
How about "When the site is in read-only mode, only changes made to models by superusers will be propagated to …	brennie	March 29, 2017, 12:23 p.m.
Wrap readOnly in double-backtickcs so that gets marked as <code>.	brennie	March 29, 2017, 12:24 p.m.
Likewise here.	brennie	March 29, 2017, 12:25 p.m.
In JavaScript, this would actually be: if (conditional) { /* Comment */ } The code here also doesnt fit the …	brennie	March 29, 2017, 12:25 p.m.
Undo this.	brennie	March 29, 2017, 12:27 p.m.
We don't require docstrings for setUp, tearDown, etc.	brennie	March 29, 2017, 12:25 p.m.
This can be moved into setUpClass, which only runs once per class, e.g. @classmethod def setUpClass(cls): super(LoginRequiredTest, cls).setUpClass() # ...	brennie	March 29, 2017, 12:30 p.m.
This should be _mock_view_function. Also your args are in the wrong order.	brennie	March 29, 2017, 12:29 p.m.
Care to split this up into user & super-user tests?	brennie	March 29, 2017, 12:48 p.m.
Instead of "the decorator", say Testing @login_required. Same goes for all the tests here. No trailing periods in test docstrings.	brennie	March 30, 2017, 10:29 a.m.
This comment is	brennie	March 29, 2017, 12:49 p.m.
Care to split this up into user & super-user tests?	brennie	March 30, 2017, 10:29 a.m.
Care to split this up into user & super-user tests?	brennie	March 29, 2017, 12:49 p.m.
""" should be on next line.	brennie	March 29, 2017, 12:49 p.m.
redefinition of unused 'test_my_account_does_not_redirect_when_not_read_only' from line 647	reviewbot	March 30, 2017, 10:28 a.m.
This should be a phrase in the imperative mode, e.g. """Check response versus the expected value""" Missing args etc	brennie	March 30, 2017, 10:29 a.m.
Can you split this into new tests?	brennie	March 30, 2017, 10:29 a.m.

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

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

Show all issues

 redefinition of unused 'test_my_account_does_not_redirect_when_not_read_only' from line 647

docs/manual/admin/upgrading/upgrading-reviewboard.rst (Diff revision 1)
Show all issues
```
"recommended that you enable read-only mode"
```

docs/manual/admin/upgrading/upgrading-sites.rst (Diff revision 1)

Show all issues

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

docs/manual/admin/upgrading/upgrading-sites.rst (Diff revision 1)
Show all issues
```
into.
No comma here.
```
docs/manual/extending/extensions/class.rst (Diff revision 1)
Show all issues
```
"Review Board"
```

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

Show all issues

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

docs/manual/extending/extensions/js-extensions.rst (Diff revision 1)
Show all issues
```
"into"
```
docs/manual/extending/extensions/js-extensions.rst (Diff revision 1)
Show all issues
```
Just "API requests".
```

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

Show all issues

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

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

Show all issues

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

docs/manual/extending/extensions/js-extensions.rst (Diff revision 1)
Show all issues
```
Wrap readOnly in double-backtickcs so that gets marked as <code>.
```

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

Show all issues

Likewise here.

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

Show all issues

In JavaScript, this would actually be:
if (conditional) {
    /* Comment */
}


The code here also doesnt fit the above.

reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
Undo this.
```

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

Show all issues

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

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

Show all issues

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

@classmethod
def setUpClass(cls):
    super(LoginRequiredTest, cls).setUpClass()

    # ...

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

Show all issues

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

reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
Care to split this up into user & super-user tests?
```

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

Show all issues

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

No trailing periods in test docstrings.

reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
This comment is
```
1. KH
  
  khp March 30, 2017, 10:45 a.m.
  LOL
reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
Care to split this up into user & super-user tests?
```
reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
Care to split this up into user & super-user tests?
```
reviewboard/accounts/tests.py (Diff revision 1)
Show all issues
```
""" should be on next line.
```

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

Show all issues

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

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

Missing args etc

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

Show all issues

Can you split this into new tests?

Summary:

-	[WIP] Read-only mode docs and unit tests
+	Read-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:

+	~~8648 - Add siteconfig var and button for read-only mode~~
+	~~8810 - Disable front-end features for read-only mode~~
+	~~8811 - Redirect some views in read-only mode, hide links to these views~~
+	~~8812 - Add read-only banner under navbar~~
+	~~8657 - Respond to API requests with 503 when read-only mode~~
+	~~8824 - Adding custom message in banner~~

Summary:

-	Read-only mode docs and unit tests for decorator
+	Read-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.

Change Summary:

Reponse to comments

Diff:

Revision 2 (+275 -4)

Show changes

	docs/manual/admin/upgrading/upgrading-reviewboard.rst
	docs/manual/admin/upgrading/upgrading-sites.rst
	docs/manual/extending/extensions/class.rst
	docs/manual/extending/extensions/js-extensions.rst
	reviewboard/accounts/tests.py
	reviewboard/reviews/tests/test_views.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (768aab4)