4612: Email notifications generate incorrect URLs when SITE_ROOT is not '/'
- Fixed
- Review Board
| jhominal | |
|
|
What version are you running?
3.0
What's the URL of the page containing the problem?
The problem manifests in all generated emails
What steps will reproduce the problem?
- Configure a Reviewboard site, with a non-empty
SITE_ROOTconfiguration setting- Create a review
- Open the email notification for the created review
What is the expected output? What do you see instead?
The link for the review should be the link to the full site (e.g.
https://<domain>:<port>/reviews/r/<review_request_id>/withSITE_ROOT = '/reviews/') but will containSITE_ROOTtwice instead (e.g.https://<domain>:<port>/reviews//reviews/r/<review_request_id>/).What operating system are you using? What browser?
Not significant
Please provide any additional information below.
After inspecting the code, I have seen that version 2.5.x of Reviewboard was writing this in its email notification templates:
<a href="{{domain_method}}://{{domain}}{{review_request.get_absolute_url}}">{{domain_method}}://{{domain}}{{review_request.get_absolute_url}}</a>While the 3.0 is written like that:
<a href="{{site_url}}{{review_request.get_absolute_url}}">{{site_url}}{{review_request.get_absolute_url}}</a>
site_urlis the result of a call toadmin.server.get_server_url, which is used for two distinct notions in the reviewboard code:
- The root Reviewboard URL (
https://<domain>:<port>/reviews/in my case) - The server URL (
https://<domain>:<port>in my case)
Currently, get_server_url's implementation returns the root RB URL, but in the case of the email notification, we actually want the server URL without a path. (However, that bug was shipped because there is no difference in output in the default configuration.)
I would propose:
- Modifying
get_server_urlso that it always returns the server URL - Changing the calls to
get_server_urlthat actually want the root site URL to use a new function (get_site_root_urlinreviewboard/admin/server.py) instead