Skip to content
This repository has been archived by the owner on May 14, 2022. It is now read-only.

Sending state notifications based on role membership #240

Merged
merged 1 commit into from
Dec 11, 2015

Conversation

escowles
Copy link
Member

@escowles escowles commented Dec 9, 2015

Closes #153 and #201

@jpstroop
Copy link
Member

jpstroop commented Dec 9, 2015

I'm 👍 to this approach

@state = state

Role.where(name: "notify_#{state}").first_or_create.users.each do |u|
mail(to: "#{u.email}", subject: "[plum] #{@curation_concern.human_readable_type} #{curation_concern_id}: #{state}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's a problem, but I'm moderately sure this'll send N emails. Would we rather do all of the to: in one line? Or as BCC?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having been on these kinds of list before, it's definitely better if I can see the other people who received the message, so that I can start a reply-all thread if I need to.

@escowles
Copy link
Member Author

I've updated it to put all the addresses in the to: field of a single email.

tpendragon pushed a commit that referenced this pull request Dec 11, 2015
Sending state notifications based on role membership
@tpendragon tpendragon merged commit 098a110 into master Dec 11, 2015
@tpendragon tpendragon deleted the role-based-notification branch December 11, 2015 18:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants