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

fix: noShow status conflict #17659

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

fix: noShow status conflict #17659

wants to merge 17 commits into from

Conversation

sanjeevs9
Copy link

What does this PR do?

This PR fixes an issue where two components were performing the same task but were not synced. As a result, updates made in one component were not reflected in the other, leading to inconsistencies. The fix ensures that both components are properly synchronized, so changes made in one are instantly reflected in the other without requiring a page refresh.

20241115163001.mp4

-Removed unnecessary state logic: Simplified state management by removing redundant useState for noShow.
-Added type definitions: Improved type safety and maintainability with explicit type annotations.
-Defined a global state for mutation: Implemented a global state to manage mutation handling across components.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • 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.

How should this be tested?

  • What is expected (happy path) to have (input and output)?

apps\web\components\booking\BookingListItem.tsx

  • Any other important info that could help to test that PR

Checklist

Copy link

vercel bot commented Nov 15, 2024

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

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Nov 15, 2024
@graphite-app graphite-app bot requested a review from a team November 15, 2024 11:16
@CLAassistant
Copy link

CLAassistant commented Nov 15, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the bookings area: bookings, availability, timezones, double booking label Nov 15, 2024
Copy link
Contributor

github-actions bot commented Nov 15, 2024

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:

Unknown release type "Fix" found in pull request title "Fix: noShow Status conflict". 

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@github-actions github-actions bot added Medium priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Nov 15, 2024
@dosubot dosubot bot added this to the v4.8 milestone Nov 15, 2024
Copy link

graphite-app bot commented Nov 15, 2024

Graphite Automations

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

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

"Add community label" took an action on this PR • (11/15/24)

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

"Add ready-for-e2e label" took an action on this PR • (11/17/24)

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

@sanjeevs9
Copy link
Author

This is my first PR on Cal.com. I would appreciate your reviews.

@sanjeevs9 sanjeevs9 changed the title Fix/status Fix: noShow Status conflict Nov 15, 2024
@vikaspatil0021
Copy link
Contributor

@Udit-takkar

@Praashh
Copy link
Contributor

Praashh commented Nov 15, 2024

This is my first PR on Cal.com. I would appreciate your reviews.

can you please sign CLA ?

@Praashh Praashh self-assigned this Nov 15, 2024
@sanjeevs9 sanjeevs9 changed the title Fix: noShow Status conflict fix: noShow status conflict Nov 15, 2024
Praashh
Praashh previously approved these changes Nov 17, 2024
Copy link
Contributor

@Praashh Praashh left a comment

Choose a reason for hiding this comment

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

LGTM!!

Copy link
Contributor

github-actions bot commented Nov 17, 2024

E2E results are ready!

@sanjeevs9
Copy link
Author

LGTM!!

fixed the type error!!

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

I think we can go with a much simpler approach of just updating the attendee no show status inside booking.get infinite query cache.

you can checkout apps/web/modules/event-types/views/event-types-listing-view.tsx to check how can we do that

@sanjeevs9
Copy link
Author

I think we can go with a much simpler approach of just updating the attendee no show status inside booking.get infinite query cache.

you can checkout apps/web/modules/event-types/views/event-types-listing-view.tsx to check how can we do that

Earlier, we were managing three local states for attendee, group attendee, and no-show attendee dialogue boxes. The issue was that they were not synchronized . I removed all the individual logic and created a single global state to maintain consistency. Additionally, the attendee order in the dialogue box now matches the attendee order on the card, as both are sorted based on the ID

and also i checked this
apps/web/modules/event-types/views/event-types-listing-view.tsx
the 'Hide from Profile' button on the card is synced with the dialogue box using global mutation only.

if i am missing something pls let me know or if you want some another approach
@Praashh @Udit-takkar

@Praashh
Copy link
Contributor

Praashh commented Nov 21, 2024

I think we can go with a much simpler approach of just updating the attendee no show status inside booking.get infinite query cache.
you can checkout apps/web/modules/event-types/views/event-types-listing-view.tsx to check how can we do that

Earlier, we were managing three local states for attendee, group attendee, and no-show attendee dialogue boxes. The issue was that they were not synchronized . I removed all the individual logic and created a single global state to maintain consistency. Additionally, the attendee order in the dialogue box now matches the attendee order on the card, as both are sorted based on the ID

and also i checked this apps/web/modules/event-types/views/event-types-listing-view.tsx the 'Hide from Profile' button on the card is synced with the dialogue box using global mutation only.

if i am missing something pls let me know or if you want some another approach @Praashh @Udit-takkar

If you recheck the issue, the problem will be in the dialog box's state. I don't know if you noticed or not. but previously if you marked a booking as a no-show and refreshed then the booking was marked as a no-show. I believe it was just for the dialog box state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working community Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-4715] No show status not updated
5 participants