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_ROOT
configuration 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_ROOT
twice 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_url
is 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_url
so that it always returns the server URL - Changing the calls to
get_server_url
that actually want the root site URL to use a new function (get_site_root_url
inreviewboard/admin/server.py
) instead