Edgewall Software
Modify

Opened 17 years ago

Closed 16 years ago

#3932 closed defect (fixed)

Project settings from trac.ini not in notification email

Reported by: Markus Tacker <m@…> Owned by: Christian Boos
Priority: normal Milestone: 0.11
Component: general Version: devel
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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

Attachments (0)

Change History (15)

comment:1 by Christian Boos, 17 years ago

Milestone: 0.11
Owner: changed from Jonas Borgström to Christian Boos
Severity: normalminor

comment:2 by sid, 17 years ago

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

comment:3 by Christian Boos, 17 years ago

Keywords: notification email added
Milestone: 0.110.12

Not for 0.11.

in reply to:  2 ; comment:4 by osimons, 16 years ago

Component: ticket systemgeneral
Keywords: base_url added; notification email removed
Milestone: 0.12
Resolution: duplicate
Status: newclosed

Replying to sid:

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

Yup.

in reply to:  4 ; comment:5 by Christian Boos, 16 years ago

Keywords: verify added; base_url removed
Milestone: 0.11.1
Resolution: duplicate
Status: closedreopened

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.

in reply to:  5 comment:6 by osimons, 16 years ago

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? :-)

comment:7 by Christian Boos, 16 years ago

Keywords: verify removed

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:

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

  • trac/web/chrome.py

     
    566566        d['project'] = {
    567567            'name': self.env.project_name,
    568568            'descr': self.env.project_description,
    569             'url': self.env.project_url,
     569            'url': self.env.project_url or self.env.abs_href,
    570570            'admin': self.env.project_admin,
    571571        }
    572572        d['chrome'] = {

comment:8 by osimons, 16 years ago

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.

in reply to:  8 ; comment:9 by Christian Boos, 16 years ago

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.

in reply to:  9 comment:10 by osimons, 16 years ago

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.

comment:11 by Christian Boos, 16 years ago

  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)
  1. 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)
  1. 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.

in reply to:  11 ; comment:12 by osimons, 16 years ago

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()?

in reply to:  12 ; comment:13 by Christian Boos, 16 years ago

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>

in reply to:  13 comment:14 by osimons, 16 years ago

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.

comment:15 by Christian Boos, 16 years ago

Milestone: 0.11.10.11
Resolution: fixed
Status: reopenedclosed

Simpler (and correct!) patch committed as r6657.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.