-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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 rescheduledBy & canceledBy fields in the DB #15337
feat: Add rescheduledBy & canceledBy fields in the DB #15337
Conversation
@Amit91848 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
Graphite Automations"Add community label" took an action on this PR • (06/05/24)1 label was added to this PR based on Keith Williams's automation. "Add consumer team as reviewer" took an action on this PR • (06/05/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add foundation team as reviewer" took an action on this PR • (08/06/24)1 reviewer was added to this PR based on Keith Williams's automation. |
This PR is being marked as stale due to inactivity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type checks and unit test both are failing . Could you fix them?
This PR is being marked as stale due to inactivity. |
did someone fixed this? can i work on it? |
Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
@@ -163,6 +163,7 @@ export default function Success(props: PageProps) { | |||
const [calculatedDuration, setCalculatedDuration] = useState<number | undefined>(undefined); | |||
const [comment, setComment] = useState(""); | |||
const parsedRating = rating ? parseInt(rating, 10) : 3; | |||
const currentUserEmail = searchParams?.get("email") ?? session?.user?.email ?? undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when coming from the email cancel button then we don't have an email
param but the cancelledBy
param
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be both actually. When a booking is made, the booker is redirected to booking page where it also has email param. Forgot to include the cancelledBy for the email
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for @alishaz-polymath for final approval. It looks good to me 👍
Requesting Platform Review as it touches Platform 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good,
tested locally and works as well 🚀
Just waiting on Platform to clear it before we ship.
E2E results are ready! |
* feat: Add rescheduledBy & canceledBy fields in the DB * fix: type check * fix: type check * fix: use session user email for reschedule * fix: unit test * feat: db field email validation * feat: rescheduledBy and cancelledBy in webhooks * revert unrelated changes. * make session user secondary, default to Anonymous * if condition not required * Make cancelledBy optional Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> * update cancel booking type * fix: update cancelledBy in db when requesting reschedule * remove default value for fields * fix: type check * feat: manage fields via api v1 * fix: add fields in booking read api v1 * test: expand to cover new fields * fix: use cancelledBy param on booking page --------- Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com> Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Co-authored-by: Peer Richelsen <peeroke@gmail.com>
* feat: Add rescheduledBy & canceledBy fields in the DB * fix: type check * fix: type check * fix: use session user email for reschedule * fix: unit test * feat: db field email validation * feat: rescheduledBy and cancelledBy in webhooks * revert unrelated changes. * make session user secondary, default to Anonymous * if condition not required * Make cancelledBy optional Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> * update cancel booking type * fix: update cancelledBy in db when requesting reschedule * remove default value for fields * fix: type check * feat: manage fields via api v1 * fix: add fields in booking read api v1 * test: expand to cover new fields * fix: use cancelledBy param on booking page --------- Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com> Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Co-authored-by: Peer Richelsen <peeroke@gmail.com>
What does this PR do?
Steps:
After creating a booking, a bunch of places I have covered from where you can reschedule / cancel bookings and then it is updated in the db:
booking/{bookingUid}
page - Reschedule or Cancel links.Host:
host.mp4
Attendee:
attendee.mp4
Imp for context:
As per the discussion I had with @alishaz-polymath , in its current form this only populates the rescheduledBy and cancelledBy field in the db. It is not shown anywhere yet, but it sent as a part of webhook payload. It lacks validations and guards which makes it susceptible to misuse, so at some point as a follow up, we would have to introduce it.
Mandatory Tasks (DO NOT REMOVE)