Project settings from trac.ini not in notification email

[Markus Tacker <m@…>]

trac.ini:

[project]
name = my project
descr = A test project
footer = Visit the Trac open source project at<br /><a href="http://trac.edgewall.com/">http://trac.edgewall.com/</a>
url = https://trac.somehost.de/trac/my-project
icon = trac.ico

Notification Mail:

...
-- 
Ticket URL: </ticket/167>
my project <>
A test project

[sid]

Looks like a duplicate of #3965. Fix is set the [trac] base_url

[cboos]

Not for 0.11.

[osimons]

Replying to sid:

Looks like a duplicate of #3965. Fix is set the [trac] base_url

Yup.

[cboos]

Replying to osimons:

Replying to sid:

Looks like a duplicate of #3965. Fix is set the [trac] base_url

Yup.

Hm, no: the reported problem was about the project URL:

[project]
url = https://trac.somehost.de/trac/my-project

(href is not supposed to be used, according to TracIni#project-section)

and the fact that it's missing here:

my project <>
A test project

I haven't verified, maybe it works now, but the issue here was not about base_url.

[osimons]

Replying to cboos:

I haven't verified, maybe it works now, but the issue here was not about base_url.

Well, in a way it is as we use base_url in email footer. If that is set (or inferred ref. other tickets) we use that.

Interesting with project url option though. Didn't remember it was there, and did a quick search in all templates to see where that is used. The only reference I could find is where we take it as input in 'Basic Settings'…:

            <input type="text" name="url" size="48" value="${project.url}" />

Searching current code for where we fetch it from config revealed 3 more uses (search for 'project', 'url':

• notification.py line 284 + 334
• formatter.py line 447

Going to all the trouble we have to make sure base_url works well, I really can't see any good reason for the [project] url setting? Why would it be different from base_url? Likely something I've missed again, but if I could decide I would change these locations to use base_url and drop the option… Would that be a first? :-)

[cboos]

Well, base_url is for the absolute location of the Trac instance, and [project] url (as I understand it) is for the corresponding software project, e.g. here, it's set to http://trac.edgewall.org/.

So it looks like that in order to fix this ticket, we would have to modify the ticket_notify_email.txt so that it includes it:

trac/ticket/templates/ticket_notify_email.txt

@@ -28 +28 @@
 
 --
 Ticket URL: <$ticket.link>
-$project.name <${abs_href()}>
+$project.name <$project.url>
 $project.descr

What we can also do is to make project.url default to base_url if not explicitly set:

trac/web/chrome.py

@@ -566 +566 @@
         d['project'] = {
             'name': self.env.project_name,
             'descr': self.env.project_description,
-            'url': self.env.project_url,
+            'url': self.env.project_url or self.env.abs_href,
             'admin': self.env.project_admin,
         }
         d['chrome'] = {

[osimons]

Right. And, looking at the source of 0.10-stable I suppose it is in fact a regression:

<?cs var:project.name ?> <<?cs var:project.url ?>>

Onto 'to-fix' list then.

As for using abs_href, note this in line 594 (ie. after your use in the proposed patch):

abs_href = req and req.abs_href or self.env.abs_href

Moving the href and abs_href settings up above your project-setting code and then using that, would make us also inherit base_url from request.

[cboos]

Replying to osimons:

Moving the href and abs_href settings up above your project-setting code and then using that, would make us also inherit base_url from request.

But this is not wanted, as this would make the project.url inside the e-mail dependent on the way the particular user which triggered the change has accessed the Trac instance.

[osimons]

Replying to cboos:

Replying to osimons:

Moving the href and abs_href settings up above your project-setting code and then using that, would make us also inherit base_url from request.

But this is not wanted, as this would make the project.url inside the e-mail dependent on the way the particular user which triggered the change has accessed the Trac instance.

Isn't that why you would set the project url then?

'url': self.env.project_url or abs_href # abs_href already set to req or env

For a typical setup with one url for server, that means no settings are needed for all to work as expected. Change settings to tweak default behaviour, not to make default functionality work out-of-the-box.

[cboos]

1. Zero settings (no [trac] base_url, no [project] url):
   • base_url remains unset
   • env.abs_href gets initialized from the first request on the instance (0.10.4 behavior restored in recent 0.11 > r6240, see #5064)
   • $project.url gets initialized from the above (my patch)

2. Minimal setting ([trac] base_url set, no [project] url):
   • env.abs_href gets initialized from base_url
   • $project.url gets initialized from the above (my patch)

3. Complex setting ([trac] base_url set, [project] url set):
   • env.abs_href gets initialized from base_url
   • $project.url is explicitly set

While 1. and 3. would work the same with your change, the situation 2. would be different, in that the base_url would be ignored and the req.abs_href will be used instead (i.e. the one reconstructed from the URL from the change author). In a setup where the Trac instance is reachable from multiple hosts, say one private one public, that would lead to expose the private URL in the mails.

[osimons]

Replying to cboos:

While 1. and 3. would work the same with your change, the situation 2. would be different, in that the base_url would be ignored and the req.abs_href will be used instead (i.e. the one reconstructed from the URL from the change author).

Sorry, but slightly confused. Are you saying that with your patch and 'zero-config', they are all initialised to the same value anyway? That was all I wanted: base_url not set and [project] url not set → it will use the information from the request. For complex setup, add options to make them all as wanted. Adding a base_url setting overrides the one from req, and if you set project url that is always what you get for that value.

This is because we already set env.abs_href from req if base_url isn't present - back in trac.web.main._dispatch_request()?

[cboos]

Replying to osimons:

… Are you saying that with your patch and 'zero-config', they are all initialised to the same value anyway?

To the value of env.abs_href, which was inferred _once_, from the very first request, here:

… because we already set env.abs_href from req if base_url isn't present - back in trac.web.main._dispatch_request()?

That's not perfect of course, but that's for the zero config case and how it used to work in 0.10.4.

… For complex setup, add options to make them all as wanted. Adding a base_url setting overrides the one from req,

And that was the point I tried to make: with your approach, base_url would have instead been overriden by req.abs_href, which is eventually different from base_url: when use_base_url_for_redirect is not set (the default), req.abs_href always correspond to the reconstructed URL, which may end up being different from base_url in multiple host setup or even simply when the access protocol differ.

Example for case 2:

[trac]
base_url = https://public.org/trac
use_base_url_for_redirect = false

Developer x modifies ticket 1, using http://192.168.1.5/trac/ticket/1.

E-mail sent (my patch):

... blah ticket 1 modified blah...

Ticket URL: <https://public.org/trac/ticket/1> 
my project <https://public.org/trac>

E-mail sent (your patch):

... blah ticket 1 modified blah...

Ticket URL: <https://public.org/trac/ticket/1> 
my project <http://192.168.1.5/trac>

[osimons]

Replying to cboos:

And that was the point I tried to make: with your approach, base_url would have instead been overriden by req.abs_href, which is eventually different from base_url: when use_base_url_for_redirect is not set (the default), req.abs_href always correspond to the reconstructed URL, which may end up being different from base_url in multiple host setup or even simply when the access protocol differ.

You are right (as usual). We wanted the same all along - sorry for the confusion :-)

+1 on your patch.

[cboos]

Simpler (and correct!) patch committed as r6657.