Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds ability to send user invitation emails from Account Overview #703

Merged
merged 3 commits into from
Apr 5, 2021

Conversation

rhonsby
Copy link
Collaborator

@rhonsby rhonsby commented Apr 2, 2021

Description

Currently, the only way to invite a user to an account is to generate a an invite url from the Account Overview page, copy that url, and send it directly to the user. We can make this process a lot easier by allowing for account admins to input the user's email and letting us send the invitation url to provided user.

Note: there's a lot of duplicated logic, so I've added some todos. I'm going to address them as a follow-up to this diff, but I wanted to keep them separate to keep the size of this diff smaller.

Issue

#591

Screenshots

Video of invite flow

CleanShot.2021-04-02.at.16.48.11.mp4

Email when the sender has a name
CleanShot 2021-04-02 at 16 32 10@2x

Email when the sender doesn't have a name
CleanShot 2021-04-02 at 16 25 19@2x

Checklist

  • [✓] Everything passes when running mix test
  • [✓] Ran mix format
  • [✓] No frontend compilation warnings


@spec send_user_invitation_email_enabled? :: boolean()
def send_user_invitation_email_enabled?() do
case System.get_env("USER_INVITATION_EMAIL_ENABLED") do
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we move this to config/runtime.exs, and we could also use String.to_existing_atom to cast boolean values. For example: https://github.com/plausible/analytics/blob/master/config/runtime.exs#L68

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that's a great idea, but it doesn't seem to be very standard practice in the codebase. We should have a larger discussion about how we should handle environmental variables. FWIW I agree that we should consolidate them into config/runtime.exs.

cc @reichert621

Copy link
Collaborator

Choose a reason for hiding this comment

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

yup, let's handle that consolidation in a separate PR 👍

account = Accounts.get_account!(account_id)
Logger.info("Sending user invitation email to #{to_address}")

ChatApi.Emails.send_user_invitation_email(
Copy link
Contributor

Choose a reason for hiding this comment

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

send_user_invitation_email returns a deliver_result that might contain {:error, binary()}. Should we log the error when it gets returned?

Copy link
Collaborator

@reichert621 reichert621 left a comment

Choose a reason for hiding this comment

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

woohoooo 🎉 🚀 🔥

@reichert621 reichert621 merged commit f5f036a into papercups-io:master Apr 5, 2021
@rhonsby rhonsby deleted the improve-team-invite-ux branch April 5, 2021 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants