Feature #2659

Refactor Mailer delivery methods out of controller and into observers

Added by Eric Davis almost 9 years ago. Updated over 8 years ago.

Status:ClosedStart date:2009-02-03
Priority:NormalDue date:
Assignee:Eric Davis% Done:

100%

Category:Email notifications
Target version:0.9.0
Resolution:Fixed

Description

There are a few Mailer method calls buried inside of controllers that would be better off as Observers (e.g. IssuesController#add). I'll do the refactoring, just wanted to open an issue so it appears in the release notes.

Associated revisions

Revision 2637
Added by Eric Davis over 8 years ago

Added observers to watch model objects for mail delivery instead of calling Mailer.

  • Added an IssueObserver to watch when Issues are created
  • Added a JournalObserver to watch when Journals are created (Issue updates)
  • Added a NewsObserver for News items.
  • Added a DocumentObserver for Document notifications.
  • Setup IssuesController#new to use the IssueObserver.
  • Setup IssuesController#edit to use the IssueObserver.
  • Setup IssuesController#bulk_edit to use the JournalObserver.
  • Removed the Mailer call in Changeset#scan_commit_for_issue_ids, the
    JournalObserver will handle it.
  • Removed Mailer calls in MailHandler in favor of the Observers.

    #2659

History

#1 Updated by Jean-Philippe Lang almost 9 years ago

Before doing the refactoring, here are a few thoughts:
  • how to do a single notification when several objects are added/updated (just how it occurs today when adding several files at the same time)? A notification should be also added when bulk editing issues. Sending a notification for each modified issue would be insane and pretty slow.
  • maybe one day, we'd like to display the name of the users that were notified (eg. in the flash message). How to retrieve it from the observer?

#2 Updated by Eric Davis almost 9 years ago

Jean-Philippe Lang wrote:

Before doing the refactoring, here are a few thoughts:
  • how to do a single notification when several objects are added/updated (just how it occurs today when adding several files at the same time)?

I'll watch out for those. My main concern is the issue notifications. The add_files action isn't very complex so it won't need refactoring right now.

A notification should be also added when bulk editing issues. Sending a notification for each modified issue would be insane and pretty slow.

Bulk edits already send one email per issue:

# app/controllers/issues_controller.rb L252
Mailer.deliver_issue_edit(journal) if journal.details.any? && Setting.notified_events.include?('issue_updated')
  • maybe one day, we'd like to display the name of the users that were notified (eg. in the flash message). How to retrieve it from the observer?

Would that be the responsibility of the Observer, the Mailer, or the Controller though?

#3 Updated by Eric Davis over 8 years ago

  • Status changed from 7 to Resolved
  • Target version set to 0.8.3
  • % Done changed from 0 to 100
  • Resolution set to Fixed

I've added some observers to watch model objects for mail delivery instead of calling Mailer in r2637.

#4 Updated by Jean-Philippe Lang over 8 years ago

  • Status changed from Resolved to Closed
  • Target version changed from 0.8.3 to 0.9.0

Thanks, I prefer to keep this refactoring for 0.9.

Also available in: Atom PDF