-
-
Notifications
You must be signed in to change notification settings - Fork 360
feat: email all interested users on event creation #640
feat: email all interested users on event creation #640
Conversation
Now that I've pushed this up, I realise that it's supposed to trigger on new event creation 🤦 That should be easy enough to implement, but, before I do, is triggering on new event the flow we want? |
@ojeytonwilliams I suppose it depends on if this PR is targeting #107 or #99 #107 seems like it would automatically have the system send emails to members who have previously attended an event. #99 would allow the organizer to email members of a chapter manually and for any reason. |
True. I think what I'll do is target #107, since this only allows you to email a canned message. I'll need to change a few things, but it's not tricky. It shouldn't take much effort (on the server side, at least) to handle #99, but the UI would definitely take some work. That would make sense as a separate PR. |
@ojeytonwilliams we have a couple open MVP issues for the text of emails. #276 is for reminders to go out 2 days prior to the event. We have sample text event for that use case that can likely be used and tweaked for the initial RSVP and possibly for other messages |
@ojeytonwilliams if /as you, or others are working on the email, then the previous conversations about following / unfollowing a Chapter and unsubscribing from emails should provide good context on the type of links and privacy settings we imagined to make for easy opt-in and opt-out. That conversation also goes into interfaces on the website and name of buttons, type of buttons and such. For the emails, we also touched slightly on the security aspects of allowing members to unsubscribe or opt-out without needing to login. For that, something like a secure / unique hash was discussed. Whether that anonymous / hash ideas is the correct approach, or easy to do is up for debate, but I thought to mention it. The core of those email and UI opt-in / out conversations were on #107 and I tried to summarize the conversation at a few points, like: |
Thanks for the context, Jim, that's really helpful. For this PR, I'll keep the scope very limited. It's just going to send emails to users whenever an event is created if they have Does that sound reasonable? As far as I could see, the discussion was around scheduled notifications, but I think that someone who has set "Unfollow {chapters.name} Notifications" to FALSE would assume that that Chapter would stop notifying them entirely. |
@ojeytonwilliams yes, we wanted to account for privacy on emails for an rsvp separately from general updates for the chapter itself, like a new event or even a notice from the organizer. user_chapter_roles.interested and rsvp.interested were added in #374. user_chapters was the original table for the chapter level setting, but Timmy Chen removed user_chapters since user_chapter_roles gave us more flexibility on permissions and we split other instance permissions to user_instance_role. I still see user_chapters in the schema, so I'll have to see if that goes away the next time it's run. I'm wondering if the migrations never deleted user_chapters. |
Okay, cool. I think this is behaving how we want, then. Regarding user_chapters, yes, there's still a migration to create it. One thing I was considering was that, since we're still in development, it might be worth periodically clearing out the migrations once we're happy that |
Sounds good on consolidating the dev migrations. I don't know the best practices on that so I'll defer to others on if / when is a good time to consolidate or clean the migrations. I believe it's safe to remove db/migrations/1575817782077-UserChapter.ts since there are no models and per #335. Do you want to do that as part of this PR, or in a new one? |
Neither do I, to be honest. @Zeko369 do you know what's a reasonable practice here? I know you pointed out that creating migrations so that we can validate them is a good idea. As I think about this, I suspect that periodically running Just for the sake of keeping this in scope, I'll create another PR to remove that migration. |
Each Chapter will have half the users interested, but it'll be a different half for each Chapter. This makes it easier to check that the notifications are working.
98fcb5a
to
8e230aa
Compare
@allella to keep the number of PRs manageable, I'm going to merge this. While it doesn't satisfy the user story, I think it's a step in the right direction. |
The UI is a bit horrible, but this PR adds another button to /dashboard/events/[id] which sends emails to everyone in that chapter.