- Change Summary:
-
- Start implementing local_site namespacing for dashboards. - Change Group.name to be unique within a local_site instead of unique within the database. - Add a new unique_together to ReviewRequest to ensure that local_id is unique within a local_site.
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
- This review request is a preview, based on
- http://reviews.reviewboard.org/r/1795/ . The "### RBCOMMONS" comments are - temporary until I finish working through everything in this views.py file. - This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. ~ For the all_review_requests page, changes are:
~ For these pages,
+ - all_review_requests + - submitter_list + - group + - group_members + - submitter + changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url Things left to do before this is ready:
- Fix other URLs ~ - Implement local_site namespacing for dashboard and other views ~ - Implement local_site namespacing for dashboard view - Test review_draft_inline_form view - Test all_review_requests view - Test diff view - Test raw_diff view - Test comment_diff_fragments view - Test diff_fragment view - Test preview_review_request_email view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - Test view_screenshot view ~ - Test search view ~ - Test submitter_list view + - Test group view + - Test group_members view + - Test submitter view + - Test search view + - Fix up any uses of Group.objects.get(name='...') to include local_site - Testing Done:
-
+ - Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
+ - Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
+ - Tested that the new review request redirected to the correct location after
creating a request with a local_site.
+ - Tested local_id addressing and permissions for review requests, with both
local_site set and not.
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
Implement URL namspacing and permissions checking for the reviews app.
Review Request #1813 — Created Oct. 5, 2010 and submitted
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the
URL which is parsed out in the initial regex, which will then be passed as an
optional parameter to any views referenced in reviews/urls.py.
The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view.
When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies
that if a local_site_name is not given, but the review request has one assigned,
that addressing it by its pk will return 403.
For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or
doesn't have access, this will return a 403.
- If a local site name is not given, the NewReviewRequestForm will filter the
list of repositories to include only those which do not have an associated
LocalSite object.
- If a local site name is given, the NewReviewRequestForm will filter the
list of repositories to include only those which have the given site name.
For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes
into account the local_site_name and logged in user.
For these pages,
- all_review_requests
- submitter_list
- group
- group_members
- submitter
changes are:
- Add support to ReviewRequestManager.public() to query only reviews with a
local_site_name
- Queries without a local_site_name will show all review requests that
likewise have no associated site. Queries with a name will only show the
relevant items on the dashboard.
For these pages,
- comment_diff_fragments
- delete_screenshot
- diff
- diff_fragment
- preview_reply_email
- preview_review_email
- preview_review_request_email
- raw_diff
- review_draft_inline_form
- search
- view_screenshot
changes are:
- Look up the review request according to the common lookup logic that takes
into account the local_site_name and logged in user.
I've also updated these:
- ReviewRequest.get_absolute_url
- Screenshot.get_absolute_url
- Group.get_absolute_url
Things left to do before this is ready:
- Fix search form ACTION url
- Fix sidebar links on dashboard
- Fix the review number inline on the page to use local_id
- Tested new review request page, looking at both the bare state and local-site specific ones to make sure the repositories listed were restricted as appropriate. - Tested creating new review requests. Verified that the correct local_site was set (whether named or None) and that the local_id was set properly in ascending order. - Tested that the new review request redirected to the correct location after creating a request with a local_site. - Tested local_id addressing and permissions for review requests, with both local_site set and not. - Tested local_site namespacing for the various pages of the dashboard view. - Tested local_site namespacing for group_list view. - Tested local_site namespacing for watched groups dashboard view. - Tested diff_fragment view. - Tested diff view. - Tested group_members view for both global and site-specific groups. - Tested review_draft_inline_form view. - Tested submitter_list view for both global and site-specific URLs. - Tested group view for both global and site-specific groups (and 404). - Tested all_review_requests view for both global and site-specific URLs. - Tested raw_diff view - Tested preview_review_request_email view - Tested view_screenshot view - Tested submitter view - Tested custom SQL for local_id incrementing across different local sites - Tested comment_diff_fragments view - Tested preview_review_email view - Tested preview_reply_email view - Tested delete_screenshot view - Tested local-site filtering of search results - Ran unit tests.
-
-
This may not be a problem in reality today, but ignoring the queryset provided kind of sucks. Can we instead do: queryset = queryset.filter(local_site=local_site) or whatever? Then we can eliminate the else: and provide queryset to the parent constructor.
-
-
Seems this can be consolidated: if ((local_site_name and repo.local_site and repo.local_site.name == local_site_name) or (not local_site_name and repo.local_site is None)): valid_repos.append(...) It's a little wordy perhaps, but we only have one way to append valid repos, and it ends up being that logic anyway. -
-
Can you shorten that to one statement? Why are we doing this, btw? Forbidden should be doing the right thing, shouldn't it?
-
-
Parens instead of \ Also, instead of "not is_authenticated", you should be able to do "is_anonymous"
-
I'm not sure, but I think this will actually load the data for the user, which is overkill. Maybe use count instead.
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- Change Summary:
-
- Implement namespacing for dashboard views (the main links don't go there, but you can type them in). - Implement namespacing for the group list view. - Fix get_absolute_url for groups to include the site name. - Fix get_absolute_url for screenshots to include the site name. - Return a 404 if any URLs under a non-existent site name are accessed.
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
~ - ReviewRequest.get_absolute_url ~ - ReviewRequest.get_absolute_url + - Screenshot.get_absolute_url + - Group.get_absolute_url Things left to do before this is ready:
~ - Fix other URLs ~ - Implement local_site namespacing for dashboard view ~ - Test local_site namespacing for watched groups dashboard view ~ - Test other URLs - Test review_draft_inline_form view - Test all_review_requests view - Test diff view - Test raw_diff view - Test comment_diff_fragments view - Test diff_fragment view - Test preview_review_request_email view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - Test view_screenshot view - Test submitter_list view - Test group view - Test group_members view - Test submitter view - Test search view - Fix up any uses of Group.objects.get(name='...') to include local_site - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
~ - Ran unit tests.
~ - Tested local_site namespacing for the various pages of the dashboard view
+ - Tested local_site namespacing for group_list view
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
- Change Summary:
-
- Test and fix the submitter list. - Only list relevant member/watched groups in the dashboard sidebar depending on the URL. - Fix group and group_members views.
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url - Screenshot.get_absolute_url - Group.get_absolute_url Things left to do before this is ready:
- - Test local_site namespacing for watched groups dashboard view - Test other URLs - Test review_draft_inline_form view - Test all_review_requests view - Test diff view - Test raw_diff view - Test comment_diff_fragments view - Test diff_fragment view - Test preview_review_request_email view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - Test view_screenshot view - - Test submitter_list view - Test group view - - Test group_members view - Test submitter view - Test search view ~ - Fix up any uses of Group.objects.get(name='...') to include local_site ~ - Fix up any uses of Group.objects.get(name='...') to include local_site + - Restrict local_site names to not conflict with any other URLs - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
~ - Tested local_site namespacing for the various pages of the dashboard view
~ - Tested local_site namespacing for group_list view
~ - Ran unit tests.
~ - Tested local_site namespacing for the various pages of the dashboard view.
~ - Tested local_site namespacing for group_list view.
~ - Tested local_site namespacing for watched groups dashboard view.
+ - Tested group_members view for both global and site-specific groups.
+ - Tested submitter_list view for both global and site-specific URLs.
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
-
-
-
-
-
-
We're doing this all over now. I wonder if we can do: localsite_urlpatterns = patterns(...), url('^dashboard/$', ...), ... ) urlpatterns += patterns('', url(r'^((?P<local_site_name>[...]+)/)?', include(localsite_urlpatterns)), ) I don't know how that works if local_site_name isn't provided, but it might work? Worth trying. It'll be more maintainable if so. Also, make sure everything fits on < 80 columns. -
-
- Change Summary:
-
Get diff views, diff fragments, and commenting working. Note that this doesn't change the API (it's still using the same API URLs).
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url - Screenshot.get_absolute_url - Group.get_absolute_url Things left to do before this is ready:
- Test other URLs - - Test review_draft_inline_form view - Test all_review_requests view - - Test diff view - Test raw_diff view - Test comment_diff_fragments view - - Test diff_fragment view - Test preview_review_request_email view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - Test view_screenshot view - Test group view - Test submitter view - Test search view - Fix up any uses of Group.objects.get(name='...') to include local_site - Restrict local_site names to not conflict with any other URLs - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
- Tested local_site namespacing for the various pages of the dashboard view.
- Tested local_site namespacing for group_list view.
- Tested local_site namespacing for watched groups dashboard view.
~ - Tested group_members view for both global and site-specific groups.
~ - Tested submitter_list view for both global and site-specific URLs.
~ - Ran unit tests.
~ - Tested diff_fragment view.
~ - Tested diff view.
~ - Tested group_members view for both global and site-specific groups.
+ - Tested review_draft_inline_form view.
+ - Tested submitter_list view for both global and site-specific URLs.
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
- Diff:
-
Revision 7 (+546 -252)
-
-
For some of the cache stuff, I had to modify this to take a local_site instead of a local_site_name. It puts the burden on the object fetching on the caller's side, but I think it's actually better, and fixes some hairy lambdas we'd otherwise need in the LocalSiteProfile. The query below becomes simplified as a result, though, which is nice.
- Change Summary:
-
- Update dashboards to merge with the new counts stuff. - Fix groups dashboard to use get_absolute_url instead of a reverse match. - Make changes to various tests to make sure they don't break with the new local_site restriction on fetching groups. - Restrict "groups" and "watched groups" on the dashboard to be only those which apply to the current local-site (or no local-site).
- Diff:
-
Revision 8 (+564 -267)
-
-
-
-
-
-
-
-
That's because get_object_or_none own't find the right LocalSite, and returns None, which is valid. We probably want to use get_object_or_404 for the LocalSite query too. Are there other places where we want to check this too? At some point, we probably should write unit tests that test all our main views with an invalid site name and ensure we get a 404.
-
diffviewer shouldn't know about review requests. Instead, provide an extra_context, and have the wrapper in reviews provide that.
-
-
As much as I'd love to get rid of them, we can't until everyone's off of the current RBTools, which requires the rewrite. As long as this only works with local_site=None, I'm happy. In that case, do we need the _deprecated versions? I'd think we could just use the new ones with local_site=None.
- Change Summary:
-
- Move site-local URLs into ^/s/site-name/ so that we don't have to worry about conflicts with existing URLs. - Fix the to-group dashboard view to show a 404 if there's no group with the given name in the given LocalSite. - Change the dashboard to show 404 if there's an invalid view instead of mapping them all back to "incoming". - Fix all_review_requests, submitter_list and group_list to do the right thing. - Get rid of my TODO in the group view since I realized the reason it wasn't working was that I was testing the dashboard view at the time. - Add the param necessary to make the raw_diff view work. - Fix up other uses of Group.objects.get() to always include a local_site with the name.
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url - Screenshot.get_absolute_url - Group.get_absolute_url Things left to do before this is ready:
- Test other URLs - - Test all_review_requests view - - Test raw_diff view - Test comment_diff_fragments view - - Test preview_review_request_email view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - - Test view_screenshot view - - Test group view - - Test submitter view - Test search view ~ - Fix up any uses of Group.objects.get(name='...') to include local_site ~ - Fix sidebar links on dashboard - - Restrict local_site names to not conflict with any other URLs - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
- Tested local_site namespacing for the various pages of the dashboard view.
- Tested local_site namespacing for group_list view.
- Tested local_site namespacing for watched groups dashboard view.
- Tested diff_fragment view.
- Tested diff view.
- Tested group_members view for both global and site-specific groups.
- Tested review_draft_inline_form view.
- Tested submitter_list view for both global and site-specific URLs.
~ - Ran unit tests.
~ - Tested group view for both global and site-specific groups (and 404).
+ - Tested all_review_requests view for both global and site-specific URLs.
+ - Tested raw_diff view
+ - Tested preview_review_request_email view
+ - Tested view_screenshot view
+ - Tested submitter view
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
- Diff:
-
Revision 10 (+590 -286)
- Change Summary:
-
- Fix a syntax error I introduced - Change the local_id increment to use custom, atomic SQL - Remove the LocalSite.next_id field - Clean up the NewReviewRequestForm.create method to use get_object_or_none
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url - Screenshot.get_absolute_url - Group.get_absolute_url Things left to do before this is ready:
- Test other URLs - Test comment_diff_fragments view - Test preview_review_email view - Test preview_reply_email view - Test delete_screenshot view - Test search view ~ - Fix sidebar links on dashboard ~ - Fix sidebar links on dashboard + - Fix the review number inline on the page to use local_id - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
- Tested local_site namespacing for the various pages of the dashboard view.
- Tested local_site namespacing for group_list view.
- Tested local_site namespacing for watched groups dashboard view.
- Tested diff_fragment view.
- Tested diff view.
- Tested group_members view for both global and site-specific groups.
- Tested review_draft_inline_form view.
- Tested submitter_list view for both global and site-specific URLs.
- Tested group view for both global and site-specific groups (and 404).
- Tested all_review_requests view for both global and site-specific URLs.
- Tested raw_diff view
- Tested preview_review_request_email view
- Tested view_screenshot view
- Tested submitter view
~ - Ran unit tests.
~ - Tested custom SQL for local_id incrementing across different local sites
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
- Diff:
-
Revision 11 (+614 -288)
- Change Summary:
-
- Fixed the "Edit Review" action when a user has a draft - Fixed changes for the diff fragment views - Fixed delete_screenshot view (shouldn't this go into the API?) - Removed RBCOMMONS comments, since they're all done!
- Description:
-
Implement URL namspacing and permissions checking for the reviews app.
This change adds LocalSite URL namespacing and permissions checking to the
reviewboard.reviews app. The namespacing is achieved via an optional item in the URL which is parsed out in the initial regex, which will then be passed as an optional parameter to any views referenced in reviews/urls.py. The views then read that local_site_name variable and react accordingly. The
details of what "accordingly" means depends on the view. When looking up a review request via its ID, if a local site name is given, that
ID is used to query the local_id field instead of the pk. This also verifies that if a local_site_name is not given, but the review request has one assigned, that addressing it by its pk will return 403. For the new_review_request page, changes are:
- If a local site name is given and the requesting user isn't logged in or doesn't have access, this will return a 403. - If a local site name is not given, the NewReviewRequestForm will filter the list of repositories to include only those which do not have an associated LocalSite object. - If a local site name is given, the NewReviewRequestForm will filter the list of repositories to include only those which have the given site name. For the review_detail page ("View Reviews"), changes are:
- Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. For these pages,
- all_review_requests - submitter_list - group - group_members - submitter changes are: - Add support to ReviewRequestManager.public() to query only reviews with a local_site_name - Queries without a local_site_name will show all review requests that likewise have no associated site. Queries with a name will only show the relevant items on the dashboard. For these pages,
- comment_diff_fragments - delete_screenshot - diff - diff_fragment - preview_reply_email - preview_review_email - preview_review_request_email - raw_diff - review_draft_inline_form - search - view_screenshot changes are: - Look up the review request according to the common lookup logic that takes into account the local_site_name and logged in user. I've also updated these:
- ReviewRequest.get_absolute_url - Screenshot.get_absolute_url - Group.get_absolute_url Things left to do before this is ready:
~ - Test other URLs ~ - Fix search form ACTION url - - Test comment_diff_fragments view - - Test preview_review_email view - - Test preview_reply_email view - - Test delete_screenshot view - - Test search view - Fix sidebar links on dashboard - Fix the review number inline on the page to use local_id - Testing Done:
-
- Tested new review request page, looking at both the bare state and local-site
specific ones to make sure the repositories listed were restricted as
appropriate.
- Tested creating new review requests. Verified that the correct local_site was
set (whether named or None) and that the local_id was set properly in
ascending order.
- Tested that the new review request redirected to the correct location after
creating a request with a local_site.
- Tested local_id addressing and permissions for review requests, with both
local_site set and not.
- Tested local_site namespacing for the various pages of the dashboard view.
- Tested local_site namespacing for group_list view.
- Tested local_site namespacing for watched groups dashboard view.
- Tested diff_fragment view.
- Tested diff view.
- Tested group_members view for both global and site-specific groups.
- Tested review_draft_inline_form view.
- Tested submitter_list view for both global and site-specific URLs.
- Tested group view for both global and site-specific groups (and 404).
- Tested all_review_requests view for both global and site-specific URLs.
- Tested raw_diff view
- Tested preview_review_request_email view
- Tested view_screenshot view
- Tested submitter view
- Tested custom SQL for local_id incrementing across different local sites
~ - Ran unit tests.
~ - Tested comment_diff_fragments view
+ - Tested preview_review_email view
+ - Tested preview_reply_email view
+ - Tested delete_screenshot view
+ - Tested local-site filtering of search results
+ - Ran unit tests.
- Tested new review request page, looking at both the bare state and local-site
- Diff:
-
Revision 13 (+579 -308)