fix: persist requiresCancellationReason selection#29282
Conversation
|
Welcome to Cal.diy, @Maheshkumarjena! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThis PR exposes the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Hi @romitg2 , this PR was marked stale due to inactivity. Whenever you get a chance, could you please review it? I’d appreciate any feedback or requested changes. Thank you! |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
@Maheshkumarjena can u also include the cancellation too in the demo showing the where it is required and where not? Once added, please open the PR again. |
|
Hi @bandhan-majumder ! I've attached the demo video to show the cancellation flow functionality , demonstrating both cases where a cancellation reason is required and where it is not required based on the selected setting. below are the video and screenshot. Please let me know if there's anything else you'd like me to include. cancellation.reason.functional.mp4
Thanks! |
bandhan-majumder
left a comment
There was a problem hiding this comment.
Awesome @Maheshkumarjena . Tested locally, works fine. Approving!
Thank you for the review and for helping get this merged. I appreciate it. |
|
Hi @bandhan-majumder , When you have a chance, could you please take a look at #29493 ? It fixes #28986 and addresses a significant issue affecting customers using Office365 integrations. As mentioned by Aroman , attendees can receive misleading Outlook cancellation emails for seated bookings, which can cause confusion. I've added the fix along with tests and would appreciate your review whenever you're available. Thanks!
|


What does this PR do?
This PR fixes an event type settings persistence issue for
Require cancellation reasonon the event type edit page.When a host selected
Mandatory for attendee onlyand saved the event, the value was not being rehydrated correctly after refresh/reopen. The saved setting existed, but the Advanced Settings UI fell back to the defaultMandatory for host only.This PR is intentionally scoped only to the dropdown persistence issue. It ensures the saved
requiresCancellationReasonvalue is:It also adds a regression test covering the event type fetch path for
requiresCancellationReason.Video Demo
before.fix.mp4
Shows:
Mandatory for attendee onlyMandatory for host onlyff1.mp4
Shows:
Mandatory for attendee onlyMandatory Tasks (DO NOT REMOVE)
N/A
How should this be tested?
Test steps:
Advanced.Require cancellation reason, selectMandatory for attendee onlyor any other options.Mandatory for attendee onlyor any other choosen option that persists after refresh .Expected result:
Automated coverage:
requiresCancellationReasonis returned when configured.Checklist