Skip to content

Comments

fix(bookings): allow filter changes when permission errors occur#24202

Closed
KartikLabhshetwar wants to merge 27 commits intocalcom:mainfrom
KartikLabhshetwar:fix/booking-filter-permission-error-recovery
Closed

fix(bookings): allow filter changes when permission errors occur#24202
KartikLabhshetwar wants to merge 27 commits intocalcom:mainfrom
KartikLabhshetwar:fix/booking-filter-permission-error-recovery

Conversation

@KartikLabhshetwar
Copy link
Contributor

@KartikLabhshetwar KartikLabhshetwar commented Oct 1, 2025

What does this PR do?

Video demo:

before:

Screen.Recording.2025-10-10.at.8.39.19.PM.mov

after:

Screen.Recording.2025-10-10.at.8.34.22.PM.mov

When a user's role changes from ADMIN to MEMBER after creating filter segments that include other users' bookings, they encounter a permission error and become stuck because:

  1. Error message "You do not have permissions to fetch bookings for specified userIds" appears
  2. All filter controls are hidden when an error occurs
  3. Users cannot change filter segments, clear filters, or modify any filters
  4. This creates a deadlock where users cannot recover from the permission issue

Solution

  • Always show filter controls: Removed conditional rendering that hides DataTableWrapper on error
  • Display error above filters: Error alert now appears above the filter interface instead of replacing it
  • Enable recovery: Users can now change filter segments, clear filters, or modify individual filters to resolve permission issues

Changes Made

  • Modified apps/web/modules/bookings/views/bookings-listing-view.tsx
  • Removed {query.status !== "error" && (...)} wrapper around DataTableWrapper
  • Moved error alert outside conditional block
  • Preserved WipeMyCalActionButton logic to only show when no errors

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?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@vercel
Copy link

vercel bot commented Oct 1, 2025

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

The PR updates apps/web/modules/bookings/views/bookings-listing-view.tsx. DataTableWrapper is now always rendered, removing its previous dependency on a non-error state. WipeMyCalActionButton rendering is separated and gated by: there are upcoming bookings for today, the status is "upcoming", and the query status is not "error". ToolbarLeft, ToolbarRight, LoaderView, and EmptyView remain as before. No exported/public interfaces were altered.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the core change by indicating that booking filters remain adjustable when permission errors occur, directly reflecting the main fix implemented in the pull request.
Linked Issues Check ✅ Passed The pull request implements the linked issues’ requirements by removing the {query.status !== "error"} guard so filters remain visible on errors and moving the error alert above the filter interface, thereby allowing users to adjust booking filters on permission errors as specified in [#24167] and [CAL-6490].
Out of Scope Changes Check ✅ Passed All code modifications are focused on the bookings-listing-view’s error handling and filter rendering logic to address the stated permission-error recovery objectives, with no unrelated or extraneous changes detected.
Description Check ✅ Passed The provided pull request description clearly outlines the problem context, the specific issues addressed, the proposed solution, and the exact changes made to the bookings-listing-view.tsx file, including links to issue trackers and video demos, making it directly relevant to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Oct 1, 2025
@graphite-app graphite-app bot requested a review from a team October 1, 2025 09:34
@github-actions github-actions bot added bookings area: bookings, availability, timezones, double booking consumer High priority Created by Linear-GitHub Sync 🐛 bug Something isn't working 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Oct 1, 2025
Copy link
Member

@dhairyashiil dhairyashiil left a comment

Choose a reason for hiding this comment

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

Please attach before and after loom video, making it draft until then.

@KartikLabhshetwar KartikLabhshetwar marked this pull request as ready for review October 10, 2025 15:12
@KartikLabhshetwar
Copy link
Contributor Author

Please attach before and after loom video, making it draft until then.

hi @dhairyashiil please check the before and after demo. and see if everything is okay here.

@KartikLabhshetwar
Copy link
Contributor Author

@Udit-takkar @CarinaWolli

dhairyashiil
dhairyashiil previously approved these changes Oct 16, 2025
Copy link
Member

@dhairyashiil dhairyashiil left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼 , Thank you for your contribution Kartik 🎉

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

E2E results are ready!

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the ❗️ migrations contains migration files label Oct 25, 2025
@KartikLabhshetwar KartikLabhshetwar marked this pull request as draft October 25, 2025 10:53
@KartikLabhshetwar KartikLabhshetwar force-pushed the fix/booking-filter-permission-error-recovery branch from 4716b54 to e16ee37 Compare October 25, 2025 10:57
@KartikLabhshetwar KartikLabhshetwar marked this pull request as ready for review October 25, 2025 10:58
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Copy link
Member

@dhairyashiil dhairyashiil left a comment

Choose a reason for hiding this comment

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

Please resolve merge conflicts

@dhairyashiil dhairyashiil marked this pull request as draft October 25, 2025 13:09
@anikdhabal
Copy link
Contributor

Fixed by this:- #24194

@anikdhabal anikdhabal closed this Nov 4, 2025
@KartikLabhshetwar KartikLabhshetwar deleted the fix/booking-filter-permission-error-recovery branch December 23, 2025 17:33
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 consumer High priority Created by Linear-GitHub Sync 🧹 Improvements Improvements to existing features. Mostly UX/UI ❗️ migrations contains migration files ready-for-e2e size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow adjusting booking filters when "No permission" error is shown

4 participants