Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

feat: send emails to rsvps on event cancel #842

Merged

Conversation

gikf
Copy link
Member

@gikf gikf commented Jan 4, 2022

  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the main branch of Chapter.

Still required:

  • Email text.
  • Tests?
  • Anything else?

Related to #100


Draft adding sending emails to rsvps when event is cancelled.

  • Email is send to rsvps that are not cancelled. That means those confirmed or in waitlist. Note in generated data there might be generated rsvps with cancelled: true, but this doesn't seem to be displayed in the client right now.
  • I've initially added permission check, similar to one in other resolver, based on user.user_event_roles, but as that's going to be handled by authorization I've removed it.
  • Tested on local fork.

@gitpod-io
Copy link

gitpod-io bot commented Jan 4, 2022

@ghost
Copy link

ghost commented Jan 4, 2022

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

Don't worry about the text for this PR, placeholders are fine for now.

Tests would be great, though!

it('emails interested users when an event is created', () => {

has some pretty similar logic, so might be worth a look.

One thing I was thinking about was changing RSVPs to be in a single 'state' (Going, OnWaitlist, Cancelled, etc), rather than having multiple boolean states. Something like this: https://dbdiagram.io/d/61d5605b3205b45b73d7b04a Reason being, the RSVP can only be in a single state at once (you can't be on_waitlist and cancelled, say), so I think a single state should be easier to reason about. Since you've just been looking at rsvps, I wondered if you had any thoughts on this.

Sorry for the slight tangent and thanks for working on this!

@gikf
Copy link
Member Author

gikf commented Jan 5, 2022

Thanks for the lead on tests, I'll take a look there.

I saw your issue about RSVPs status #783, I'd agree having status as a single field most likely will simplify some things and help avoid some awkwardness. Like determining if somebody is confirmed/going by checking both on_waitlist and cancelled.

@ojeytonwilliams
Copy link
Contributor

No problem, thanks for taking a look.

I saw your issue about RSVPs status

Cool, I wasn't sure if you'd have seen that, since there's been a lot of discussion. Anyways, it's not something to worry about here, but I appreciate the feedback.

@gikf gikf force-pushed the feat/organizer-events-cancel-emails branch from 7f5e739 to e791389 Compare January 8, 2022 21:37
@gikf gikf marked this pull request as ready for review January 8, 2022 21:47
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

Nice one, @gikf. This LGTM 👍

@ojeytonwilliams ojeytonwilliams merged commit 45caf70 into freeCodeCamp:main Jan 10, 2022
@gikf gikf deleted the feat/organizer-events-cancel-emails branch January 10, 2022 08:52
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.

2 participants