[WIP] Figuring out how to affect front-end to enable read-only mode
Review Request #8677 — Created Jan. 29, 2017 and discarded
This is exploration for read-only mode, which is a feature to allow admins to disable functionality which would disrupt RB upgrades and maintenance while allowing users to consume site content.
Blocking API and introducing feature into admin panel are in previous commits.
Changes to Navbar (New Review Request link), Actions in ReviewRequestEditorView, ReviewRequestEditorView description, testing, information, comments, diff/comment box all changed.
Sidebar in Dashboard disabled (possibly not the best way to disable those features).
ETag updated to reflect changes in read-only mode for ReviewRequestEditorView.
| Description | From | Last Updated |
|---|---|---|
|
You'll also need to do work to block off write access to the API, and handle this in RB.apiCall in … |
 chipx86
|
|
|
Another thing that modifies the database today is changing columns for a datagrid (dashboard, All Review Requests, etc.). Users can … |
chipx86
|
|
|
While this change wires off some pages (New Review Request, My Account, etc.), that doesn't stop users from actually going … |
chipx86
|
|
|
Col: 13 W291 trailing whitespace |
 reviewbot
|
|
|
Col: 80 E501 line too long (87 > 79 characters) |
reviewbot
|
|
|
Col: 13 E131 continuation line unaligned for hanging indent |
reviewbot
|
|
|
Col: 80 E501 line too long (83 > 79 characters) |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 80 E501 line too long (87 > 79 characters) |
reviewbot
|
|
|
Col: 13 E131 continuation line unaligned for hanging indent |
reviewbot
|
|
|
Col: 19 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Col: 80 E501 line too long (93 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (112 > 79 characters) |
reviewbot
|
|
|
Col: 18 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Col: 80 E501 line too long (112 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (83 > 79 characters) |
reviewbot
|
|
|
'SiteConfiguration' imported but unused |
reviewbot
|
|
|
Col: 19 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Col: 18 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
local variable 'siteconfig' is assigned to but never used |
reviewbot
|
|
|
local variable 'siteconfig' is assigned to but never used |
reviewbot
|
|
|
Col: 73 W292 no newline at end of file |
reviewbot
|
|
|
'SiteConfiguration' imported but unused |
reviewbot
|
|
|
Col: 19 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Col: 18 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Instead of this, we should have a file-level docstring. There are examples elsewhere in the codebase. |
chipx86
|
|
|
The name makes it sound like the user is read-only, but really we're saying "the user can make changes to … |
chipx86
|
|
|
Needs Args and Returns. |
chipx86
|
|
|
Needs double-backticks. This is in ReStructuredText format, and single backticks is a syntax error. |
chipx86
|
|
|
This method gets called a lot, so it's going to be important for it to be fast and not have … |
chipx86
|
|
|
Should be aligned like: var showMenu = (count > 0 && !RB.UserSession.instance.get(...)); |
chipx86
|
|
|
This should be computed only once. You can do so earlier in the file by defining a variable. |
chipx86
|
|
|
Missing space after the comma. |
chipx86
|
|
|
Indentation on the loop should be fixed. |
chipx86
|
|
|
Blank line between these. |
chipx86
|
|
|
We're going to want the logic to be identical between here (and other template code) and is_user_read_only, now and in … |
chipx86
|
|
|
Col: 1 E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
- Change Summary:
-
Updated WIP
- Description:
-
This is exploration for read-only mode, which is a feature to allow admins to disable functionality which would disrupt RB upgrades and maintenance while allowing users to consume site content.
Blocking API and introducing feature into admin panel are in previous commits.
Looking into which templates to modify and if any .js files need to be changed as well. Potentially looking into passing a read-only value to the UserSession, but might not be necessary. Modifying editable and statusEditable in review_request model seems necessary.
~ Looking into 1) creating a utility method to call on to determine if a user is read-only mode or not, 2) perhaps a decorator or another way to introduce read-only logic into existing code.
~ Review Request Editor is all (as far as I can tell) read-only mode "enabled" now. Toolbar, review request information, and comments section are all disabled when in read-only mode.
+ + TODO:
+ - Refactor read-only logic into a utility method somewhere + - Look into caching issues and eTags + - Move into main menu - Diff:
-
Revision 2 (+53 -12)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/models/base_comment.py reviewboard/reviews/default_actions.py reviewboard/reviews/context.py Ignored Files: reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/models/commentEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/models/base_comment.py reviewboard/reviews/default_actions.py reviewboard/reviews/context.py Ignored Files: reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/models/commentEditorModel.js -
-
-
-
-
-
-
-
-
-
-
- Change Summary:
-
Updated WIP with some refactors and changes to most database-effecting UI
- Diff:
-
Revision 3 (+83 -21)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js -
-
-
- Description:
-
This is exploration for read-only mode, which is a feature to allow admins to disable functionality which would disrupt RB upgrades and maintenance while allowing users to consume site content.
Blocking API and introducing feature into admin panel are in previous commits.
~ Looking into which templates to modify and if any .js files need to be changed as well. Potentially looking into passing a read-only value to the UserSession, but might not be necessary. Modifying editable and statusEditable in review_request model seems necessary.
~ Changes to Navbar (New Review Request link), Actions in ReviewRequestEditorView, ReviewRequestEditorView description, testing, information, comments, diff/comment box all changed.
~ Review Request Editor is all (as far as I can tell) read-only mode "enabled" now. Toolbar, review request information, and comments section are all disabled when in read-only mode.
~ Sidebar in Dashboard disabled (possibly not the best way to disable those features).
~ TODO:
~ ETag updated to reflect changes in read-only mode for ReviewRequestEditorView.
- - Refactor read-only logic into a utility method somewhere - - Look into caching issues and eTags - - Move into main menu
- Change Summary:
-
Updated front-end to cover "My Account"
- Diff:
-
Revision 4 (+104 -22)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/accounts/views.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/accounts/views.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js -
-
-
-
-
-
- Change Summary:
-
Minor fixes
- Diff:
-
Revision 5 (+100 -24)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/accounts/views.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/webapi/tests/test_base.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/accounts/views.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/reviews/default_actions.py reviewboard/webapi/tests/test_base.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js
-
This is a great start! Some comments about next steps and fixes/improvements for the current implementation.
-
You'll also need to do work to block off write access to the API, and handle this in
RB.apiCallin JavaScript. -
Another thing that modifies the database today is changing columns for a datagrid (dashboard, All Review Requests, etc.). Users can reorder columns or add/remove columns. There's no harm in continuing to allow those operations to continue, but you should prevent saving in the database in the backend.
-
While this change wires off some pages (New Review Request, My Account, etc.), that doesn't stop users from actually going to those pages. You'll need a solution for those pages as well.
One option is to show different content completely for the pages, letting the user know the site is in read-only mode and they can't post new changes for review or modify settings.
-
Instead of this, we should have a file-level docstring. There are examples elsewhere in the codebase.
-
The name makes it sound like the user is read-only, but really we're saying "the user can make changes to the site." I'd instead go with:
is_site_read_only_for(user). -
-
-
This method gets called a lot, so it's going to be important for it to be fast and not have to re-calculate all the time.
What you can do is store a caching variable on
userand return it, like this:if not hasattr(user, '_cache_site_is_read_only'): user._cache_site_is_read_only = ... return user._cache_site_is_read_only -
-
-
-
-
-
We're going to want the logic to be identical between here (and other template code) and
is_user_read_only, now and in the future. Instead of doing a check here, you can add a function toreviewboard/admin/context_processors.pythat you'd then add in the list of context processors inreviewboard/settings.py. This would call out tois_user_read_onlyand return the result as a variable that can be used in the templates.
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/webapi/tests/test_base.py reviewboard/reviews/models/base_comment.py reviewboard/webapi/base.py reviewboard/accounts/views.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py reviewboard/admin/context_processors.py reviewboard/settings.py reviewboard/reviews/templatetags/reviewtags.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/webapi/tests/test_base.py reviewboard/reviews/models/base_comment.py reviewboard/webapi/base.py reviewboard/accounts/views.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py reviewboard/admin/context_processors.py reviewboard/settings.py reviewboard/reviews/templatetags/reviewtags.py Ignored Files: reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/pages/views/datagridPageView.js reviewboard/static/rb/js/models/commentEditorModel.js -
-
-
-
-
-
-
-
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/webapi/base.py reviewboard/accounts/views.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py reviewboard/datagrids/grids.py reviewboard/admin/context_processors.py reviewboard/settings.py reviewboard/reviews/templatetags/reviewtags.py Ignored Files: reviewboard/static/rb/js/pages/views/dashboardView.js reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/static/rb/js/utils/apiUtils.es6.js reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/models/commentEditorModel.js -
-
-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/admin/read_only.py reviewboard/reviews/models/base_comment.py reviewboard/webapi/base.py reviewboard/accounts/views.py reviewboard/reviews/default_actions.py reviewboard/reviews/models/review_request.py reviewboard/datagrids/grids.py reviewboard/admin/context_processors.py reviewboard/settings.py reviewboard/reviews/templatetags/reviewtags.py Ignored Files: reviewboard/static/rb/js/pages/views/dashboardView.js reviewboard/static/rb/js/views/starManagerView.js reviewboard/templates/reviews/review_reply_section.html reviewboard/templates/base/navbar.html reviewboard/static/rb/js/utils/apiUtils.es6.js reviewboard/templates/base.html reviewboard/templates/base/_nav_support_menu.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/models/commentEditorModel.js
Tool: Pyflakes Processed Files: reviewboard/reviews/models/base_comment.py reviewboard/reviews/context.py Ignored Files: reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/utils/apiUtils.es6.js reviewboard/templates/base.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/models/base_comment.py reviewboard/reviews/context.py Ignored Files: reviewboard/static/rb/js/models/reviewRequestEditorModel.js reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/utils/apiUtils.es6.js reviewboard/templates/base.html