Skip to content

fix: If Invalid Calendar Credentials Then Skip Listing Calendars [CAL-2340]#10720

Merged
emrysal merged 2 commits intomainfrom
fix/deleting-invalid-calendars
Aug 11, 2023
Merged

fix: If Invalid Calendar Credentials Then Skip Listing Calendars [CAL-2340]#10720
emrysal merged 2 commits intomainfrom
fix/deleting-invalid-calendars

Conversation

@joeauyeung
Copy link
Copy Markdown
Contributor

What does this PR do?

This PR wraps listing deleted selected calendars in a try/catch block so if the credentials are invalid the user are still able to delete the calendar app.

Fixes #10719

Requirement/Documentation

  • If there is a requirement document, please, share it here.
  • If there is ab UI/UX design document, please, share it here.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • Add GCal
  • In the DB manipulate the access token to make it an invalid credential
  • Delete the integration

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@vercel
Copy link
Copy Markdown

vercel bot commented Aug 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 11, 2023 8:42pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 11, 2023 8:42pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 11, 2023 8:42pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 11, 2023 8:42pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Aug 11, 2023 8:42pm
qa ⬜️ Ignored (Inspect) Visit Preview Aug 11, 2023 8:42pm

@joeauyeung joeauyeung requested a review from a team August 11, 2023 13:58
@joeauyeung joeauyeung self-assigned this Aug 11, 2023
@github-actions github-actions bot added the 🐛 bug Something isn't working label Aug 11, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 11, 2023

Thank you for following the naming conventions! 🙏

@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous bot commented Aug 11, 2023

🤖 Meticulous spotted visual differences in 27 of 114 screens tested: view and approve differences detected.

Last updated for commit 74e3e66. This comment will update as new commits are pushed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 11, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel bot commented Aug 11, 2023

Current Playwright Test Results Summary

✅ 116 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 08/11/2023 08:42:25pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 74e3e66

Started: 08/11/2023 08:41:01pm UTC

⚠️ Flakes

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
5.02% (16) 16 / 319 runs
failed over last 7 days
94.98% (303) 303 / 319 runs
flaked over last 7 days

View Detailed Build Results


@keithwillcode keithwillcode requested a review from a team August 11, 2023 14:19
Copy link
Copy Markdown
Contributor

@pumfleet pumfleet left a comment

Choose a reason for hiding this comment

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

Read through code and it looks good to me

@zomars
Copy link
Copy Markdown
Contributor

zomars commented Aug 11, 2023

so if the credentials are invalid the user are still able to delete the calendar app.

What are invalid credentials in this context? Does this means that invalid credentials cannot be deleted anymore?

@emrysal
Copy link
Copy Markdown
Contributor

emrysal commented Aug 11, 2023

@zomars rather the other way around in this case, when deleting a credential, selectedCalendar would be removed first which triggered an error - crashing credential deletion. @joeauyeung how does a user get rid of the selectedCalendar?

sean-brydon pushed a commit that referenced this pull request Aug 14, 2023
Co-authored-by: Omar López <zomars@me.com>
@joeauyeung
Copy link
Copy Markdown
Contributor Author

@zomars rather the other way around in this case, when deleting a credential, selectedCalendar would be removed first which triggered an error - crashing credential deletion. @joeauyeung how does a user get rid of the selectedCalendar?

Let's say that a user sees an error on the calendar page that there was an error fetching their calendar and to reinstall the integration. If there was a problem with fetching their calendars to begin without, without the try/catch block it would throw an error saying that calendars could not be retrieved. @zomars @emrysal

sean-brydon added a commit that referenced this pull request Aug 18, 2023
* Init + get timezone + offset data agh

* Add 12/24h mode - style correctly

* User users timezone + working hours. Still some stuff to figure out

* Multiple working hours

* move calc to once per day

* Demo with two users and differnt timezones

* availabillity control tab via search params

* WIP hover overlay

* THIS WORKS ISH

* fix: multiple duration getSchedule calls [CAL-2336] (#10709)

Co-authored-by: Omar López <zomars@me.com>

* New Crowdin translations by Github Action

* fix: If the input type "Name" is selected, the label can't be changed from our default label "Your Name" (#10618)

Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>

* New Crowdin translations by Github Action

* test: Create unit tests for react components in packages/ui/components/form/step (#10442)

Co-authored-by: gitstart-calcom <gitstart@users.noreply.github.com>
Co-authored-by: Keith Williams <keithwillcode@gmail.com>

* feat: element call app added (#10585)

Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>

* New Crowdin translations by Github Action

* New Crowdin translations by Github Action

* fix: e2e test for rescheduling overlapping time (#10721)

Co-authored-by: CarinaWolli <wollencarina@gmail.com>

* feat: mailhog fixture (#10606)

* feat: mailhog fixture

* fix: nodemailer to dispatch emails with e2e env

* fix: remove space from email subject

* feat: fixture getFirstEventAsOwner

* feat: assert email subjects

* fix: and enable dynamic booking test (#10642)

* fix and enable dynamic booking test

* remove page pause

---------

Co-authored-by: Alex van Andel <me@alexvanandel.com>

* fix: Broken team events if a user with the same name exists (#10724)

* fix: Broken team events if a user with the same name exists

* Fix tests + fix usernameList optionality

* Try to list calendars, if not continue (#10720)

Co-authored-by: Omar López <zomars@me.com>

* v3.1.9

* link to org settings (#10718)

* feat: app paypal payment (#8797)

Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com>
Co-authored-by: Omar López <zomars@me.com>

* fix: RTL issues on booking pages + email confirmation (#10526)

Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* fix: merge conflict

* Fixing org slug (#10538)

* fix: paypal build fixes

* Fix avatar for org in Shell top (#10712)

* feat: add range of dates for availability over-ride (#10462)

* feat: add range of dates for availability over-ride

* chore: changed range select to multiple select

---------

Co-authored-by: Alex van Andel <me@alexvanandel.com>

* fix: border issue for time slots (#10577)

Co-authored-by: Raghul D <v-raghuld@microsoft.com>

* style: Fix text wrapping issue in button (#10725)

Co-authored-by: Omar López <zomars@me.com>

* New Crowdin translations by Github Action

* fix: App Install Dropdown Sort Properly [CAL-2285] (#10672)

Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* fix: link escaping in booking page (#10360)

Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>

* chore: fix refund i18n message (#10731)

* chore: remove tailwind-scrollbar warning (#10523)

Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>

* New Crowdin translations by Github Action

* chore: Simplified date overrides (#10728)

* chore: Simplified date overrides

* Fixed a test that had a date override that wasn't at midnight utc

* Wrote test that showed a fixed Europe/Brussels

* Lint fix

* New Crowdin translations by Github Action

* Fix offset time + fetching correct dates

* Deal with awkward minute offsets

* remove store overhead

* Format H based on tz

* Cleanup store logic

* Cleanup

* Remove comments

* Remove comments

* Remove yarn.lock

* Dark mode & v-align text fixes

* Move ButtonGroup to the left to prevent chevron from jumping

* Shift based on timezone (non-hour) and have 15min granularity in hour display background color

---------

Co-authored-by: Leo Giovanetti <hello@leog.me>
Co-authored-by: Omar López <zomars@me.com>
Co-authored-by: Crowdin Bot <support+bot@crowdin.com>
Co-authored-by: Zain Gulbaz <zaingulbaz8@gmail.com>
Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>
Co-authored-by: GitStart-Cal.com <121884634+gitstart-calcom@users.noreply.github.com>
Co-authored-by: gitstart-calcom <gitstart@users.noreply.github.com>
Co-authored-by: Keith Williams <keithwillcode@gmail.com>
Co-authored-by: Suyash Srivastava <suyashsrivastava5053@gmail.com>
Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: Shivam Kalra <shivamkalra98@gmail.com>
Co-authored-by: alannnc <alannnc@gmail.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Joe Au-Yeung <65426560+joeauyeung@users.noreply.github.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
Co-authored-by: Pradumn Kumar <pradumn@tealfeed.com>
Co-authored-by: Raghul <123321540+Raghul18@users.noreply.github.com>
Co-authored-by: Raghul D <v-raghuld@microsoft.com>
Co-authored-by: ABDERRAHMANI IDRISSI HAMZA <97639117+idrissi-hamza@users.noreply.github.com>
Co-authored-by: Nafees Nazik <84864519+G3root@users.noreply.github.com>
Co-authored-by: Danila <daniil.demidovich@gmail.com>
Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working core area: core, team members only

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

[CAL-2340] Unable to remove failed GCal connection

4 participants