fix: no show status conflict

[MehulZR]

What does this PR do?

• Fixes #17633 (GitHub issue number)
• Fixes CAL-4715 (Linear issue number - should be visible at the bottom of the GitHub issue description)

Before:

https://github.com/user-attachments/assets/0e0e529c-da2e-46f6-b710-b82fb893dc86

After:

https://github.com/user-attachments/assets/bcf92f3f-63b5-4c99-b627-858887490b48

Mandatory Tasks (DO NOT REMOVE)

• [x] I have self-reviewed the code (A decent size PR without self-review might be rejected).
• [x] I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
• [ ] I confirm automated tests are in place that prove my fix is effective or that my feature works.

[vercel[bot]]

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

A member of the Team first needs to authorize it.

[graphite-app[bot]]

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (01/15/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (01/15/25)

1 label was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (02/12/25)

1 label was added to this PR based on Keith Williams's automation.

[MehulZR]

Hey @Praashh, In #17659 the implementation is different and the author seems unresponsive for now to make the desired changes requested by Udit.

[MehulZR]

Working on it.

[MehulZR]

no show status conflict update.webm

As we have moved to tanstack table, we have to batch the mutations of attendees in the noShowAttendeesDialog. Otherwise a single toggle mutation will reset(update the trpc client cache) the table. Resulting in noShowAttendeesDialog being closed preemptively.

[MehulZR]

@Praashh Final touch as in review or a nit?

[Praashh]

@Praashh Final touch as in review or a nit?

yes final review

[github-actions[bot]]

This PR is being marked as stale due to inactivity.

[TusharBhatt1]

@MehulZR can you fix the conflicts , so it can be taken forward , btw LGTM

[MehulZR]

@MehulZR can you fix the conflicts , so it can be taken forward , btw LGTM

Done

[TusharBhatt1]

@MehulZR thanks for the effort , but we have decided to move ahead with #18648

[MehulZR]

@TusharBhatt1 I am not sure why you went ahead with #18648 as it contains some issues. I have left some related comments there.

[TusharBhatt1]

Also can you have a look at this : https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

[MehulZR]

Also can you have a look at this : https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

Wasn't able to reproduce on Chrome and Firefox. Can you provide steps to reproduce this?

https://github.com/user-attachments/assets/0cf3e72a-8804-4ec0-943d-1d4a4cfeabb5

Notice when I scroll to the end, I get the bookings for the next page (updating the booking list state), leading to re-render of components which causes the API request for locationOptions & hasTeamPlan

[TusharBhatt1]

Also can you have a look at this : https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

Wasn't able to reproduce on Chrome and Firefox. Can you provide steps to reproduce this?

no.show.conflict.mp4 Notice when I scroll to the end, I get the bookings for the next page (updating the booking list state), leading to re-render of components which causes the API request for locationOptions & hasTeamPlan

No particular step to reproduce , I am clicked on the attendee/Edit button and getting this weird behavior , as shown in the video I shared , tried multiple times , otherwise LGTM It seems : something is causing rerendering , even the drop down immediately closes after it gets open

[MehulZR]

even the drop down immediately closes after it gets open

Yeah, that also stood out to me too.

Can you still reproduce it?

[TusharBhatt1]

@MehulZR I can still see closing of dropdown as soon as it opens and unnecessary network calls , mentioned above - https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

[CLAassistant]

All committers have signed the CLA.

[MehulZR]

@MehulZR I can still see closing of dropdown as soon as it opens and unnecessary network calls , mentioned above - https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

I will do some testing by freshly cloning this branch.

Update: Wasn't able to produce.

[TusharBhatt1]

@MehulZR I can still see closing of dropdown as soon as it opens and unnecessary network calls , mentioned above - https://www.loom.com/share/e852a67beace463693eedbeb8d043dd2

I will do some testing by freshly cloning this branch.

Update: Wasn't able to produce.

Leave it , I have tested it again , not showing on my device as well , just two enhancements when on noShowModal :

1. When no updation and user clicks Done , don't show any alert.
2. When updation , show alert [email protected] marks as show/no show (for multiple attendees show multiple alerts) , instead of No show status updated for attendees

Once these are done and comments are resolved , let's take this ahead

[MehulZR]

When no updation and user clicks Done , don't show any alert.

This is subjective but generally end user expects a response when they interact with something. The popup notification is the indicator that something happened due to the actions of end user.

Saving up a single API Request isn't worth it by separating the Pop up notification from the mutation result.

When updation , show alert [email protected] marks as show/no show (for multiple attendees show multiple alerts) , instead of No show status updated for attendees

It was there before this PR and I think it's there for a reason.

One use case for Cal.com seats event is managing class. Suppose they have to mark multiple students as No Show. Having 15+ notification would be a bad UX.

Individual mutation works the way you are stating.

Also the goal of the PR is to fix the state & UI sync issue. Not what the UI should be like.

[TusharBhatt1]

When no updation and user clicks Done , don't show any alert.

This is subjective but generally end user expects a response when they interact with something. The popup notification is the indicator that something happened due to the actions of end user.

Saving up a single API Request isn't worth it by separating the Pop up notification from the mutation result.

When updation , show alert [email protected] marks as show/no show (for multiple attendees show multiple alerts) , instead of No show status updated for attendees

It was there before this PR and I think it's there for a reason.

One use case for Cal.com seats event is managing class. Suppose they have to mark multiple students as No Show. Having 15+ notification would be a bad UX.

Individual mutation works the way you are stating.

Also the goal of the PR is to fix the state & UI sync issue. Not what the UI should be like.

Exactly the goal is to fix the issue , but you have changed the UI as well , we can't ignore that ,

1. If we look closely on the modal we have individual alerts.
2. It doesn't show any alert , when no updation Also when no updation , showing an alert , it's kind of misleading showing - updated.

Suppose there's a cancel booking modal , and user closes without canceling and gets an alert - "Booking Canceled Successfully" , wouldn't it be misleading

I have also confirmed in the team Let's not stretch it any further - wrap it up, make the enhancements, and take this ahead.

[MehulZR]

My bad. My explanation wasn't very clear. What I meant was I haven't changed the message returned by the backend, which is used to show the pop up notification.

Exactly the goal is to fix the issue , but you have changed the UI as well , we can't ignore that ,

1. If we look closely on the modal we have individual alerts.

I have to change the dialog to a form. Explained here Due to that we aren't doing individual mutation.

2, It doesn't show any alert , when no updation Also when no updation , showing an alert , it's kind of misleading showing - updated.

Suppose there's a cancel booking modal , and user closes without canceling and gets an alert - "Booking Canceled Successfully" , wouldn't it be misleading

That is a valid point but usually there's also a Close/Cancel button for the modal in that case.

Let's not stretch it any further - wrap it up, make the enhancements, and take this ahead.

I would love that too but reviewing is there to iron out any weird/unintentional behavior.

As you have confirmed with the team.

1. I will add a check to trigger the mutation only if there's a change.
2. Will trigger the notification for multiple user individually.

[MehulZR]

https://github.com/user-attachments/assets/2c0989be-86f8-4179-aebe-c3106a9a8d30

This is on main branch. Grouped Attendees have the same behavior.

1. It mutates even if there isn't any change
2. Doesn't triggers the notification individually

Do we want it to be changed too?

[TusharBhatt1]

no.show.status.conflict.mp4 This is on main branch. Grouped Attendees have the same behavior.

1. It mutates even if there isn't any change
2. Doesn't triggers the notification individually

Do we want it to be changed too?

Not sure , will confirm , meanwhile we can focus on the changes we have discussed

[MehulZR]

I would love the confirmation before moving on.

[TusharBhatt1]

I would love the confirmation before moving on.

I think until the confirmation, we can finish the enhancements we already have , ultimately saving time

[TusharBhatt1]

no.show.status.conflict.mp4 This is on main branch. Grouped Attendees have the same behavior.

1. It mutates even if there isn't any change
2. Doesn't triggers the notification individually

Do we want it to be changed too?

Not sure , will confirm , meanwhile we can focus on the changes we have discussed

Confirmed , we should fix the first one , second one is fine

[MehulZR]

https://github.com/user-attachments/assets/2b5794db-2bdd-48be-93c4-991b82d9b8a4

[TusharBhatt1]

no.show.status.conflict.mp4

When group attendees - can we disable mark-no-show button , until there's a change. Currently it's triggering an API call which is unnecessary. Also a comment is unresolved