Skip to content

fix: Conferencing Label When Installing Apps#10351

Merged
hariombalhara merged 3 commits into
mainfrom
fix/multiinstall-conferencing-apps
Jul 25, 2023
Merged

fix: Conferencing Label When Installing Apps#10351
hariombalhara merged 3 commits into
mainfrom
fix/multiinstall-conferencing-apps

Conversation

@joeauyeung
Copy link
Copy Markdown
Contributor

What does this PR do?

Disables install conferencing apps on teams & orgs until the PR is ready.

Fixes # (issue)

Type of change

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

How should this be tested?

  • Create a team or org
  • Install a video app
    • The only option should be to install the app to the user

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 Jul 24, 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 Jul 25, 2023 0:46am
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 0:46am
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 0:46am
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 0:46am
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 25, 2023 0:46am
qa ⬜️ Ignored (Inspect) Jul 25, 2023 0:46am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 24, 2023

Thank you for following the naming conventions! 🙏

@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Jul 24, 2023

🤖 Meticulous spotted visual differences in 35 of 230 screens tested: view and approve differences detected.

Last updated for commit 4ec5bb5. This comment will update as new commits are pushed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 24, 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 Jul 24, 2023

Current Playwright Test Results Summary

✅ 96 Passing - ❌ 1 Failing - ⚠️ 2 Flaky

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

(Last updated on 07/25/2023 12:47:13am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 4ec5bb5

Started: 07/25/2023 12:45:42am UTC

❌ Failures

📄   packages/embeds/embed-core/playwright/tests/embed-pages.e2e.ts • 1 Failure

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Embed Pages Event Type Page: should not have margin top on embed page
Retry 2Retry 1Initial Attempt
Error: expect(received).toBe(expected) // Object.is equality...
expect(received).toBe(expected) // Object.is equality

Expected: 0
Received: null
5.81% (9) 9 / 155 runs
failed over last 7 days
0.65% (1) 1 / 155 run
flaked over last 7 days

⚠️ Flakes

📄   packages/app-store/routing-forms/playwright/tests/basic.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Routing Forms Seeded Routing Form Routing Link - Reporting and CSV Download
Retry 1Initial Attempt
10.69% (17) 17 / 159 runs
failed over last 7 days
35.22% (56) 56 / 159 runs
flaked over last 7 days

📄   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
1.29% (2) 2 / 155 runs
failed over last 7 days
98.06% (152) 152 / 155 runs
flaked over last 7 days

View Detailed Build Results


zomars
zomars previously requested changes Jul 24, 2023
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Can we check why there are type errors in the PR? 🙏🏽

Comment thread apps/web/components/apps/App.tsx Outdated
joeauyeung and others added 2 commits July 24, 2023 20:35
Co-authored-by: Omar López <zomars@me.com>
Comment on lines +40 to +42
const enabledOnTeams = !app.categories.some(
(category) => category === "calendar" || category === "conferencing"
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zomars, I need to add a check for concurrent meetings when enabling team conferencing apps. This is just a part of the stack of PRs that would be nice to have in prod.

(category) => category === "calendar" || category === "conferencing"
);

const appInstalled = enabledOnTeams && userAdminTeams ? userAdminTeams.length === appAdded : appAdded > 0;
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Jul 25, 2023

Choose a reason for hiding this comment

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

nit:
Creating this variable would make things obvious isAppInstalledForAllTargets = userAdminTeams.length === appAdded

@hariombalhara hariombalhara deleted the fix/multiinstall-conferencing-apps branch July 25, 2023 06:33
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
Co-authored-by: Omar López <zomars@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants