#9004 closed defect (invalid)
[PATCH] Does not have a link to the comment that generated the email
Reported by: | Ryan J Ollos | Owned by: | Robert Corsaro |
---|---|---|---|
Priority: | normal | Component: | AnnouncerPlugin |
Severity: | normal | Keywords: | |
Cc: | Steffen Hoffmann | Trac Release: | 0.12 |
Attachments (3)
Change History (11)
Changed 13 years ago by
Attachment: | AnnouncerNotification.png added |
---|
Changed 13 years ago by
Attachment: | TracNotification.png added |
---|
comment:1 Changed 13 years ago by
Summary: | Does not have a link to the comment that generated the email → [PATCH] Does not have a link to the comment that generated the email |
---|
Changed 13 years ago by
Attachment: | announcerplugin-comment-in-ticket-link.patch added |
---|
append #comment:N to ticket link in notification mails
comment:2 Changed 13 years ago by
This patch looks pretty good to me. Use of cnum
will result in the issues described in #6930 and #6535, but I think we have no other option on 0.11, and the author has handled the issue appropriately with a try/catch when querying cnum
.
This patch should only be applied to the 0.11 branch and this issue handled differently in 0.12 and beyond right? I haven't looked closely at the implementation in 0.12, but Osimons comments in #6930 and a close look at the th:TracDev/DatabaseSchema should show us the way ...
I'll test it out in the morning and make a couple of style comments, and then maybe we can get it applied pending some feedback from hasienda or doki_pen.
comment:3 Changed 13 years ago by
Cc: | Steffen Hoffmann added; anonymous removed |
---|
comment:4 Changed 13 years ago by
I've tested this patch on my Trac 0.11.7 instance, running the latest 0.11 branch (r10458) of the AnnouncerPlugin. The patch works well.
I'd appreciate if someone with more Trac experience could give this a review and decide if it is appropriate to push to the trunk or even just the 0.11 branch. hasienda or doki_pen?
comment:5 Changed 13 years ago by
Resolution: | → invalid |
---|---|
Status: | new → closed |
The trunk has an option ticket_change_with_comment
that is False
by default. After setting to true, the comment is appended to the URL at the end of the email. I'd suggest setting this option to true by default, in order to mimic the behavior of Trac.
comment:6 Changed 13 years ago by
comment:7 Changed 13 years ago by
Trac Release: | 0.11 → 0.12 |
---|
comment:8 Changed 13 years ago by
I originally reported this against 0.11. I'm on 0.12 now. The option is also available on the 0.11.2dev branch, but doesn't look to be available on the 0.11 branch. I haven't tested either of those, just looked at the source code. The trunk should eventually be backported to 0.11 anyway.
This option is not shown on the Trac wiki page. In the past I would always look to project wiki pages to determine the available options. Then, after developing for a while, I realized I should be looking at the TracIni page after installation to discover the available options. I think many newcomers to Trac do the same, and fail to realize the best place to look for documentation of options is usually the TracIni page.
Now, I tend to think that TracIni is a better place for documentation because it is more likely to be accurate, as opposed to the project's wiki page which may be out of date. I have a dream of writing a plugin for Trac-Hacks that pulls documentation from the source code and displays it on the project's wiki page, but that is just a dream for now ;)
For now, I try to watch the RSS and direct new users to the TracIni page. Occasionally, I put an explicit statement on the project wiki page, as I've done here:
That detail also made me uncomfortable, when trying out the AnnouncerPlugin.
Today I made a patch which for me fixes the issue. Patch follows, against 0.11/ subversion tree, tested on Trac 0.11.7.