-
-
-
-
reviewboard/reviews/context.py (Diff revision 1) Col: 13 E131 continuation line unaligned for hanging indent
-
reviewboard/reviews/models/base_comment.py (Diff revision 1) Col: 80 E501 line too long (83 > 79 characters)
-
reviewboard/reviews/models/base_comment.py (Diff revision 1) Col: 1 W293 blank line contains whitespace
[WIP] Figuring out how to affect front-end to enable read-only mode
Review Request #8677 — Created Jan. 29, 2017 and discarded
Information | |
---|---|
khp | |
Review Board | |
master | |
|
|
Reviewers | |
reviewboard, students | |
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 … |
|
|
Another thing that modifies the database today is changing columns for a datagrid (dashboard, All Review Requests, etc.). Users can … |
|
|
While this change wires off some pages (New Review Request, My Account, etc.), that doesn't stop users from actually going … |
|
|
Col: 13 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (87 > 79 characters) |
![]() |
|
Col: 13 E131 continuation line unaligned for hanging indent |
![]() |
|
Col: 80 E501 line too long (83 > 79 characters) |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 80 E501 line too long (87 > 79 characters) |
![]() |
|
Col: 13 E131 continuation line unaligned for hanging indent |
![]() |
|
Col: 19 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 80 E501 line too long (93 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (112 > 79 characters) |
![]() |
|
Col: 18 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 80 E501 line too long (112 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (103 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (86 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (103 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (83 > 79 characters) |
![]() |
|
'SiteConfiguration' imported but unused |
![]() |
|
Col: 19 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 18 E127 continuation line over-indented for visual indent |
![]() |
|
local variable 'siteconfig' is assigned to but never used |
![]() |
|
local variable 'siteconfig' is assigned to but never used |
![]() |
|
Col: 73 W292 no newline at end of file |
![]() |
|
'SiteConfiguration' imported but unused |
![]() |
|
Col: 19 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 18 E127 continuation line over-indented for visual indent |
![]() |
|
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 … |
|
|
Needs Args and Returns. |
|
|
Needs double-backticks. This is in ReStructuredText format, and single backticks is a syntax error. |
|
|
This method gets called a lot, so it's going to be important for it to be fast and not have … |
|
|
Should be aligned like: var showMenu = (count > 0 && !RB.UserSession.instance.get(...)); |
|
|
This should be computed only once. You can do so earlier in the file by defining a variable. |
|
|
Missing space after the comma. |
|
|
Indentation on the loop should be fixed. |
|
|
Blank line between these. |
|
|
We're going to want the logic to be identical between here (and other template code) and is_user_read_only, now and in … |
|
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
'django_reset' imported but unused |
![]() |
|
'from settings_local import *' used; unable to detect undefined names |
![]() |
|
'django_reset' imported but unused |
![]() |
|
'from settings_local import *' used; unable to detect undefined names |
![]() |

Change Summary:
Updated WIP
Description: |
|
|||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
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
-
-
reviewboard/reviews/context.py (Diff revision 2) Col: 13 E131 continuation line unaligned for hanging indent
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 19 E127 continuation line over-indented for visual indent
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (93 > 79 characters)
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (112 > 79 characters)
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 18 E127 continuation line over-indented for visual indent
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (112 > 79 characters)
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (103 > 79 characters)
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (86 > 79 characters)
-
reviewboard/reviews/default_actions.py (Diff revision 2) Col: 80 E501 line too long (103 > 79 characters)
-
reviewboard/reviews/models/base_comment.py (Diff revision 2) Col: 80 E501 line too long (83 > 79 characters)
Change Summary:
Updated WIP with some refactors and changes to most database-effecting UI

-
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
-
-
reviewboard/reviews/default_actions.py (Diff revision 3) Col: 19 E127 continuation line over-indented for visual indent
-
reviewboard/reviews/default_actions.py (Diff revision 3) Col: 18 E127 continuation line over-indented for visual indent
Description: |
|
---|
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
-
reviewboard/accounts/views.py (Diff revision 4) local variable 'siteconfig' is assigned to but never used
-
reviewboard/accounts/views.py (Diff revision 4) local variable 'siteconfig' is assigned to but never used
-
-
-
reviewboard/reviews/default_actions.py (Diff revision 4) Col: 19 E127 continuation line over-indented for visual indent
-
reviewboard/reviews/default_actions.py (Diff revision 4) Col: 18 E127 continuation line over-indented for visual indent
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.apiCall
in 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.
-
reviewboard/admin/read_only.py (Diff revision 5) Instead of this, we should have a file-level docstring. There are examples elsewhere in the codebase.
-
reviewboard/admin/read_only.py (Diff revision 5) 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)
. -
-
reviewboard/admin/read_only.py (Diff revision 5) Needs double-backticks. This is in ReStructuredText format, and single backticks is a syntax error.
-
reviewboard/admin/read_only.py (Diff revision 5) 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
user
and 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
-
reviewboard/static/rb/js/pages/views/datagridPageView.js (Diff revision 5) Should be aligned like:
var showMenu = (count > 0 && !RB.UserSession.instance.get(...));
-
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 5) This should be computed only once. You can do so earlier in the file by defining a variable.
-
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 5) Missing space after the comma.
-
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 5) Indentation on the loop should be fixed.
-
-
reviewboard/templates/base.html (Diff revision 5) 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.py
that you'd then add in the list of context processors inreviewboard/settings.py
. This would call out tois_user_read_only
and 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
-
reviewboard/admin/context_processors.py (Diff revision 6) Col: 1 E302 expected 2 blank lines, found 1
-
-
reviewboard/admin/context_processors.py (Diff revision 6) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
-
-
-
reviewboard/settings.py (Diff revision 6) 'from settings_local import *' used; unable to detect undefined names

-
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
-
-
reviewboard/settings.py (Diff revision 7) 'from settings_local import *' used; unable to detect undefined names

-
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