feat: app install flow followup #15193

SomayChauhan · 2024-05-24T12:02:14Z

What does this PR do?

Fixes CAL-3603
Fixes CAL-3604

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected)
(N/A) I have added a Docs issue here if this PR makes changes that would require a documentation change
I have added or modified automated tests that prove my fix is effective or that my feature works (PRs might be rejected if logical changes are not properly tested)

How should this be tested?

Test Cases

Conferencing apps
- LOOM
- OAuth (Zoom)
- Link based (Campfire)
Calendar apps (GCal)
- LOOM
Messaging
- No Change
Payment
- LOOM
Analytics
- LOOM
Others
- LOOM

…flow

* refactor InstallAppButtonChild

* send location data to the handler * add the newly installed to the locations dropdown (prefillLocation)

* only add app if not already added

github-actions · 2024-05-24T12:02:28Z

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

SomayChauhan · 2024-05-24T12:38:10Z

packages/features/ee/teams/lib/getUserAdminTeams.ts

instead of querying all teams and then looking weather this team has current user as admin/owner (which is slow because we have to look at all the teams in db)

queried the user using userId got all his teams and then filter all the teams that have the current user as admin/owner
(this should be faster because user_id is the primary key for the table so getting all the teams of user should be fast)

Love the refactor here. Feels like this would be a good time to move this to the user repository.

SomayChauhan · 2024-05-24T12:38:34Z

packages/features/ee/teams/lib/getUserAdminTeams.ts

          accepted: true,
          role: { in: [MembershipRole.ADMIN, MembershipRole.OWNER] },
        },
+        select: { teamId: true },


only select the required fields

SomayChauhan · 2024-05-24T12:40:38Z

apps/web/pages/apps/installation/[[...step]].tsx

@@ -155,47 +182,46 @@ const OnboardingPage = ({
  });

  const handleSelectAccount = async (teamId?: number) => {


decided to remove this api call and use useAddAppMutation instead

SomayChauhan · 2024-05-24T12:40:48Z

apps/web/pages/apps/installation/[[...step]].tsx

-            },
+    mutation.mutate(
+      {
+        isOmniInstall: true,


setting isOmniInstall to true so that we can be redirected back to /apps/installation/[step]?slug
if not set it redirects to apps/installed/[category] and the redirection can't be controlled

I wonder if we still need this prop. All instances are set to true.

Fair point @joeauyeung

pushed a refactor in

b2f478c and 6937803

@joeauyeung @zomars

SomayChauhan · 2024-05-24T12:41:21Z

packages/app-store/_utils/oauth/decodeOAuthState.ts

@@ -7,9 +7,6 @@ export function decodeOAuthState(req: NextApiRequest) {
    return undefined;
  }
  const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state);
-  if (state.appOnboardingRedirectUrl) {


no longer needed as we now use returnTo only

apps/web/components/apps/InstallAppButtonChild.tsx

apps/web/playwright/apps/conferencing/conferencingApps.e2e.ts

- tests weren't actually running - Refactored so tests can run in parallel

zomars · 2024-07-08T14:50:57Z

Made some changes to the conferencing apps tests to they can run in parallel. Also fixed some flaky tests. f0a5e90 (#15193)

SomayChauhan · 2024-07-08T15:23:42Z

Made some changes to the conferencing apps tests to they can run in parallel.

love the refactor here, pushed a similar refactor for analytics app tests too 3bdabfe

zomars

Thanks again @SomayChauhan. Good enough for now. There's still some areas of opportunity but we can address them in more follow ups.

updated

SomayChauhan added 21 commits May 24, 2024 12:06

feat: remove dropdown from app-store and redirect to new-app-install-…

c1b8399

…flow

removed extra code

a61e4b6

fix: account select loading issue

b60c67a

removed getPaymentCredential (unused)

67bdd9e

fix: only redirect after the app has been added to all the event-types

a3cb931

remove overflowIndicatorStyles

698a118

refactor getUserAdminTeams

d95c2c8

send teamId instead of id

0306433

seperate locations component

618750f

for conferencing apps skip select account page

997e44a

* refactor InstallAppButtonChild

installing conferencing apps shows locations dropdown in configure step

6231b87

send location data to the handler

6bed163

* send location data to the handler * add the newly installed to the locations dropdown (prefillLocation)

fix: type errors

b4968ab

fix: handle es-lint errors

1b09f89

fix: app is added again on submit

5da5814

* only add app if not already added

fix: type erros

ac8f31a

filter out managed events for now

0b1d8d7

fix: show installed count badge

374dbdf

remove 2 toast message

4719144

feat: added tests for conferencing apps

11652e6

fix: loading indicator

dabe466

keithwillcode added consumer core area: core, team members only labels May 24, 2024

SomayChauhan commented May 24, 2024

View reviewed changes

apps/web/components/apps/InstallAppButtonChild.tsx Show resolved Hide resolved

vercel bot deployed to Preview – api July 5, 2024 09:52 View deployment

vercel bot deployed to Preview – dev July 5, 2024 10:09 View deployment

fix: analytics apps test

572a239

vercel bot deployed to Preview – api July 5, 2024 10:18 View deployment

vercel bot deployed to Preview – dev July 5, 2024 10:29 View deployment

SomayChauhan marked this pull request as ready for review July 5, 2024 13:18

Merge branch 'main' into app-install-flow-followup

199b9d6

vercel bot deployed to Preview – ai July 8, 2024 13:09 View deployment

fix: failing test

5db02d5

vercel bot deployed to Preview – api July 8, 2024 13:20 View deployment

zomars requested changes Jul 8, 2024

View reviewed changes

apps/web/playwright/apps/conferencing/conferencingApps.e2e.ts Outdated Show resolved Hide resolved

vercel bot had a problem deploying to Preview – dev July 8, 2024 13:37 Failure

Update conferencingApps.e2e.ts

05d8cdd

vercel bot deployed to Preview – api July 8, 2024 13:47 View deployment

vercel bot deployed to Preview – dev July 8, 2024 14:13 View deployment

Merge branch 'main' into app-install-flow-followup

a3d45af

vercel bot deployed to Preview – ai July 8, 2024 14:35 View deployment

vercel bot deployed to Preview – api July 8, 2024 14:39 View deployment

fix: simplified app install tests

f0a5e90

- tests weren't actually running - Refactored so tests can run in parallel

vercel bot deployed to Preview – api July 8, 2024 14:55 View deployment

vercel bot deployed to Preview – dev July 8, 2024 15:09 View deployment

fix: simplified analytics app install tests

3bdabfe

vercel bot deployed to Preview – api July 8, 2024 15:27 View deployment

vercel bot deployed to Preview – dev July 8, 2024 15:40 View deployment

zomars approved these changes Jul 8, 2024

View reviewed changes

zomars merged commit f1b4d7d into main Jul 8, 2024
36 checks passed

zomars deleted the app-install-flow-followup branch July 8, 2024 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: app install flow followup #15193

feat: app install flow followup #15193

SomayChauhan commented May 24, 2024 •

edited

Loading

github-actions bot commented May 24, 2024 •

edited

Loading

SomayChauhan May 24, 2024

joeauyeung Jun 3, 2024

SomayChauhan May 24, 2024

SomayChauhan May 24, 2024

SomayChauhan May 24, 2024

joeauyeung Jun 3, 2024

zomars Jun 25, 2024

SomayChauhan Jul 5, 2024 •

edited

Loading

SomayChauhan May 24, 2024

zomars commented Jul 8, 2024

SomayChauhan commented Jul 8, 2024 •

edited

Loading

zomars left a comment

		@@ -155,47 +182,46 @@ const OnboardingPage = ({
		});

		const handleSelectAccount = async (teamId?: number) => {

feat: app install flow followup #15193

feat: app install flow followup #15193

Conversation

SomayChauhan commented May 24, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

Test Cases

github-actions bot commented May 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SomayChauhan Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zomars commented Jul 8, 2024

SomayChauhan commented Jul 8, 2024 • edited Loading

zomars left a comment

Choose a reason for hiding this comment

SomayChauhan commented May 24, 2024 •

edited

Loading

github-actions bot commented May 24, 2024 •

edited

Loading

SomayChauhan Jul 5, 2024 •

edited

Loading

SomayChauhan commented Jul 8, 2024 •

edited

Loading