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

feat: add option to provide cancellation reason for email #1587

Merged
merged 6 commits into from
Jan 28, 2022

Conversation

nrademacher
Copy link
Contributor

@nrademacher nrademacher commented Jan 21, 2022

What does this PR do?

Closes #1451 by adding an option to provide a reason when cancelling event, which is added to the cancellation email.

Type of change

  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • Verified the new field is present by logging the email markup that would be sent to the cancelling attendee or organiser

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code and corrected any misspellings
  • I have commented my code, particularly in hard-to-understand areas
    • Not warranted
  • I have made corresponding changes to the documentation
    • Not applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
    • This would involve creating a functionality mocking setup; would skip if possible, considering the scope of the change
  • New and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Jan 21, 2022

@nrademacher is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@zomars zomars added ♻️ autoupdate tells kodiak to keep this branch up-to-date ❗️ migrations contains migration files 🧪 testing needed labels Jan 24, 2022
@vercel
Copy link

vercel bot commented Jan 24, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/cal/calendso/32AWj7hrCCE7QFDVLJUudTPBuLh6
✅ Preview: https://calendso-git-fork-nrademacher-feature-reasonforcancellation-cal.vercel.app

@vercel vercel bot temporarily deployed to Preview January 24, 2022 17:34 Inactive
Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Can we hide the field when the reason is empty, or add a "No reason provided" message?

image

@nrademacher
Copy link
Contributor Author

Can we hide the field when the reason is empty, or add a "No reason provided" message?

image

You're right, blank field looks odd. In case there's no reason given, it would in my opinion be best to omit the field in the email.

@zomars
Copy link
Member

zomars commented Jan 24, 2022

You're right, blank field looks odd. In case there's no reason given, it would in my opinion be best to omit the field in the email.

Agreed!

@alishaz-polymath
Copy link
Member

I think this might a good PR to add Rejection reason as well. Thoughts @zomars, @emrysal ?

@nrademacher nrademacher force-pushed the feature/reason_for_cancellation branch from a3d0730 to bdb2ccb Compare January 28, 2022 13:37
@zomars
Copy link
Member

zomars commented Jan 28, 2022

I think this might a good PR to add Rejection reason as well. Thoughts @zomars, @emrysal ?

We could follow up in another PR 👍🏽

@vercel vercel bot temporarily deployed to Preview January 28, 2022 17:29 Inactive
@zomars zomars enabled auto-merge (squash) January 28, 2022 17:39
Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

I'll merge once checks pass. Thank you for contributing! 🙏🏽

@zomars zomars merged commit 07b75da into calcom:main Jan 28, 2022
buschco pushed a commit to buschco/calendso that referenced this pull request Mar 27, 2022
* feat: add option to provide cancellation reason for email

* chore: move pos of getCancellationReason method in classes

* fix: only show cancellation reason if given
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♻️ autoupdate tells kodiak to keep this branch up-to-date ❗️ migrations contains migration files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reason for cancellation
3 participants