Modify

Opened 5 years ago

Closed 4 years ago

#4781 closed defect (fixed)

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

Reported by: JirkaV Owned by: doki_pen
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 5 years ago by anonymous

  • Cc rjollos added

comment:2 in reply to: ↑ description Changed 5 years ago by rjollos

  • Cc rjollos 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 follow-up: Changed 4 years ago by doki_pen

  • Owner changed from ixokai to doki_pen
  • Status changed from new to assigned

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 4 years ago by doki_pen

Making a note not to forget about accountmanager plugin

comment:5 in reply to: ↑ 3 Changed 4 years ago by rjollos

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 4 years ago by doki_pen

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 follow-up: Changed 4 years ago by doki_pen

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 4 years ago by rjollos

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 4 years ago by doki_pen

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:11 in reply to: ↑ 10 Changed 4 years ago by rjollos

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 4 years ago by rjollos

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 4 years ago by rjollos

  • Resolution set to fixed
  • Status changed from assigned to closed

Added a note here: AnnouncerPlugin#Configuration

comment:14 Changed 4 years ago by rjollos

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 4 years ago by doki_pen

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 4 years ago by rjollos

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 4 years ago by doki_pen

  • Resolution set to fixed
  • Status changed from reopened to closed

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.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from doki_pen. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.