Modify

Opened 9 years ago

Closed 8 years ago

#4781 closed defect (fixed)

AnnouncerPlugin breaks other plugin notifications (documentation should warn about it)

Reported by: Jirka Vejrazka Owned by: Robert Corsaro
Priority: normal Component: AnnouncerPlugin
Severity: minor Keywords:
Cc: Trac Release: 0.11

Description

Hi,

I just finished debugging an issue where FullBlogNotificationPlugin was not sending emails. It turned out that it was using trac.notification system that was "inactivated" by "change your [notification] to [announcer]". I'm not exactly sure what plugin is to blame here (if any), but AnnouncerPlugin documentation should warn about possible impact to other plugins instead of recommending the change in trac.ini headers blindly.

Just my 2 cents...

Jirka

Attachments (0)

Change History (17)

comment:1 Changed 8 years ago by anonymous

Cc: Ryan J Ollos added; anonymous removed

comment:2 in reply to:  description Changed 8 years ago by Ryan J Ollos

Cc: anonymous added; Ryan J Ollos removed

Replying to JirkaV:

... AnnouncerPlugin documentation should warn about possible impact to other plugins instead of recommending the change in trac.ini headers blindly.

I see your point, as this is causing problems for me as well. There is probably a better way to do this rather than just renaming [notification] to [announcer].

comment:3 Changed 8 years ago by Robert Corsaro

Owner: changed from Stephen Hansen to Robert Corsaro
Status: newassigned

Just a reminder that the AnnouncerPlugin wiki page is a _wiki_ page. Anyone can add any warning they want, I wouldn't be upset. The install instruction clearly list it as an alpha plugin that should be tested.

That said, full blogs notification code would need to be ported to announcer. Another option is we try to implement the notification apis. I haven't looked into this too much, but I will.

comment:4 Changed 8 years ago by Robert Corsaro

Making a note not to forget about accountmanager plugin

comment:5 in reply to:  3 Changed 8 years ago by Ryan J Ollos

Replying to doki_pen:

That said, full blogs notification code would need to be ported to announcer. Another option is we try to implement the notification apis. I haven't looked into this too much, but I will.

I'm not complaining because this is a great plugin, but it would obviously be ideal if the AnnouncerPlugin by default did not cause other plugins to behave differently than they would if the AnnouncerPlugin was not installed. That is, plugins such as the FullBlogNotificationPlugin would continue to use the default trac notification system unless a module is implemented for the plugin.

comment:6 Changed 8 years ago by Robert Corsaro

Not possible. Notification code is does not expose enough extension points. I'm going to write announcer events for these two plugins and send them to the maintainers of fullblogplugin and accountmanager plugin.

comment:7 Changed 8 years ago by Robert Corsaro

I started working on fullblog here:

http://github.com/dokipen/trac-fullblogannouncements-plugin

I decided to make it a completely seperate plugin, since fullblognotification is a seperate plugin.

comment:8 in reply to:  7 Changed 8 years ago by Ryan J Ollos

Replying to doki_pen:

I decided to make it a completely seperate plugin, since fullblognotification is a seperate plugin.

The author also has a sample-plugins directory where he includes the BlogDraftPlugin, so you might have the option of including it with his distribution if you'd prefer to go that route: source:fullblogplugin/0.11/sample-plugins.

comment:9 Changed 8 years ago by Robert Corsaro

Nah, 3 files and a couple of templates. I updated the wiki. Have fun and please report issues (or find me on irc).

http://trac-hacks.org/wiki/FullBlogNotificationPlugin

comment:10 Changed 8 years ago by Robert Corsaro

comment:11 in reply to:  10 Changed 8 years ago by Ryan J Ollos

Replying to doki_pen:

https://trac-hacks.org/wiki/FullBlogAnnouncementsPlugin

I should be able to install in the next week or so and test it out. Thanks!

comment:12 Changed 8 years ago by Ryan J Ollos

Seems like adding something like the following to the wiki page should be sufficient to close this ticket:

This plugin may break notifications from other plugins. Please read the documentation carefully.

comment:13 Changed 8 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

Added a note here: AnnouncerPlugin#Configuration

comment:14 Changed 8 years ago by Ryan J Ollos

I'm getting the impression that I'm the first one to test the FullBlogAnnouncementsPlugin ;)

I'll post some patches for that component shortly.

comment:15 Changed 8 years ago by Robert Corsaro

Resolution: fixed
Status: closedreopened

Awesome! You are the first to really use it. I suspect that it was working better when I wrote it, but that with all the work I've been doing it has regressed somewhat. Also, I don't think it will work with trunk at all. I'll try to get a trunk version together.

comment:16 Changed 8 years ago by Ryan J Ollos

Thanks. I'll be doing all my initial testing with 0.11dev, but once we get that working well I can help with testing the trunk version as well.

comment:17 Changed 8 years ago by Robert Corsaro

Resolution: fixed
Status: reopenedclosed

Alright. I just committed three modules into trunk. Modules for bitten, fullblog and accountmanager. The accountmanager module require a patch to accountmanager to get verify email and reset password working. You can find my patched version of accountmanager, which can be found here: http://github.com/dokipen/trac-accountmanager-plugin. I have sent the patches to the upstream maintainer (pacopablo). Of course, there is some work to backport to 0.11 that I'm not willing to do right now. If anyone wants to take a stab, feel free. I can give my blessing on commit privs to 0.11dev, but you'll have to make a ticket request.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Robert Corsaro.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.