Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI - Also adds Fathom app by hariombalhara · Pull Request #4727 · calcom/cal.diy

hariombalhara · 2022-09-28T03:13:51Z

What does this PR do?

Also, review https://app.gitbook.com/o/6snd8PyPYMhg0wUw6CeQ/s/VXRprBTuMlihk37NQgUU/~/changes/6xkqZ4qvJ3Xh9k8UaWaZ/engineering/product-specs/app-store#user-apps docs. Will keep on updating it as and when required.

Note: This PR also deletes some V1 files whose V2 versions I was modifying.

Also adds fathom app through the merge of #4804

Important

Because we are parsing metadata schema with zod now, it's possible that some users might have metadata(already existing) that might not be meeting zod schema and thus that might fail eventTypes loading. Though zod schema is as per the current code but it's possible that code might have evolved and there might be data not meeting current code expectations.
Recovery Plan in case the issue comes
- If many people are being impacted, revert. Because there are no migrations in it, it is completely safe to revert this change. Any event types that make an update to Apps in the mean time, wouldn't have that change reflected but as soon as we make the change live again, they should be able to see their configurations back.
- If few cases appear, go and fix their data as per the schema

Design Principle-1: One App shouldn't break another App.

Thanks to ErrorBoundaries, error in one AppCard won't impact another.
e.g. why should Stripe App issue make other Apps unavailable?
Each app can only modify its respective slot under the metadata.apps namespace. This avoids the chances of one app breaking the other app with name conflicts on properties.
Moves rainbow, stripe and giphy app-related code from EventAppsTab to their respective app folders.
- Deprecates eventType.price and eventType.currency as these are specific to stripe only. Migration to drop the columns will follow later.
Replaced all occurrences of eventType.price and eventType.currency to stripeAppData.price and stripeData.currency - This requires testing of all pages involved as payment is an important flow.

Design Principle-2: While adding a new app, the developer shouldn't worry about code beyond the app directory

So, the entire app code exists in app-store/APP_NAME only.

Design Principle-3: Adding new apps and maintenance should be easy. See Loom

Adding a new EventType app should be super easy and quick.
Limited the ways app data can be accessed. getEventTypeAppData is the only accepted entry point to access event-type app data. This allows capabilities like denying access to data if the app is disabled.
AppStore CLI has the support to instantly create such an app which can be toggled
Introduced the concept of extensions in AppStore(to extend EventTypes, Routing Forms and other features) that can be specified using extendsFeature property. Currently it supports only EventType.
- EventTypeAppCard.tsx file would automatically be generated for Apps with extendsFeature set to EventType
- zod.ts would also be generated that would allow the app to expose the data it wants to store in eventType.metadata as appDataSchema export
- apps.browser.generated.tsx would have the required exports automatically generated for ease of use.

Design Principle-4: Easy discoverability of apps(See Loom)

Not installed apps, would also show now under AppsTab allowing users to easily discover apps relevant to EventType within that view itself.
User would be able to do mostly a one-click install of apps using OmniInstallAppButton from the AppsTab page itself. Giphy and Rainbow would be one click but because stripe requires authentication it can't be one click.
AppCard now links to the app in app-store so that users can get more details about the app quickly.

Other Fixes

UX Improvement: Disabling doesn't discard app data now, that you would have spent time in configuring. See this loom for a demo
Updated EventTypeDescription to V2 on event-types listing page. This is consistent with booking pages now.
Before:

After:

Before:

After:

Type of change

Bug fix (non-breaking change which fixes an issue)
Chore (refactoring code, technical debt, workflow improvements)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How should this be tested?

Install Rainbow, Stripe, Giphy apps and verify their impact on booking pages.
Enable these apps and then verify their impact
Configure these apps with various options.
Test that stripe installation n correctly resets price to 0 in metadata.apps

Checklist

I haven't added tests that prove my fix is effective or that my feature works

… doesnt work correctly

vercel · 2022-09-28T03:13:54Z

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

Name	Status	Preview	Updated
cal	✅ Ready (Inspect)	Visit Preview	Oct 14, 2022 at 11:25AM (UTC)

…d models consistent in expecting scalars only

…ype-extensions

…pe of the PR huge

…ype-extensions

* added fathom app wrapper, needs script injection to public booking page * new logo * Add Fathom script support on booking pages and add it as an eventTypeapp * Add automation and analytics apps * Add missing pieces for analytics category * Rename BookingPageScripts to BookingPageTagManager Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>

…ype-extensions

…ons' into app-store-cli-event-type-extensions

…ype-extensions

leog

Left some comments, nothing blocking I think. Great changes!

leog · 2022-10-14T12:34:50Z

+ * Use this component to allow installing an app from anywhere on the app.
+ * Use of this component requires you to remove custom InstallAppButtonComponent so that it can manage the redirection itself
+ */
+export default function OmniInstallAppButton({ appId, className }: { appId: string; className: string }) {


Not sure if I get why we need this sort of button. Does it makes it easier to include anywhere in the app? Like in the app store single category page?

Yeah, it allows you to install an app from any page on our webapp

leog · 2022-10-14T12:37:47Z

      icon: Icon.FiGrid,
-      info: `${enabledAppsNumber} ${t("active")}`,
+      //TODO: Handle proper translation with count handling
+      info: `${installedAppsNumber} apps, ${enabledAppsNumber} ${t("active")}`,


This should be easy to do. You could also add a comment if possible next to the translation in common.js to let the translators know where does the string apply, as it is difficult to infer context.

leog · 2022-10-14T12:39:56Z

+    })
+  ).map((eventType) => ({
+    ...eventType,
+    metadata: EventTypeMetaDataSchema.parse(eventType.metadata),


Just to be clear, as I saw you renamed metadata to appData at the beginning, metadata in this context is the same as appData? If so, should we rename it everywhere else? I like appData the most as it clearly states the entity that holds that data.

leog · 2022-10-14T12:41:38Z

+            InstalledAppVariants.calendar,
+            InstalledAppVariants.analytics,
+            InstalledAppVariants.automation,
+          ]}


I like the explicitness here 👍

leog · 2022-10-14T12:43:49Z

    },
  });

+  const appsMetadata = formMethods.getValues("metadata")?.apps;


Same question about having now two names for the same thing if that's what it is. IMHO introducing new names should rename everything, otherwise we will end up confusing names.

Agreed, eventType.metadata.apps should be called appData

zomars

This PR is a bold move. But a welcome one nonetheless. Let's ship this to staging and test the heck out of it before going to prod on Monday.

Impressive work @hariombalhara 🙏

hariombalhara added 6 commits September 27, 2022 13:42

Add OmniInstall button

c8758e0

Make AppCards configurable by the app itself

8412968

Make OmniInstallAppButton not redirect

564f55a

Fixes

4a545bd

Add extendsFeature support to CLI

ee31a93

Move to automatic file generation approach as dynamic import checking…

667b690

… doesnt work correctly

hariombalhara changed the title ~~Add OmniInstall button~~ Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI Sep 28, 2022

zomars mentioned this pull request Sep 28, 2022

Fixing active vs existing apps in event type #4405

Closed

1 task

Use zod everywhere consistenly for metadata and fix all TS issues

73d0561

vercel Bot had a problem deploying to Preview September 28, 2022 13:20 Failure

leog linked an issue Sep 28, 2022 that may be closed by this pull request

/event-types-detail - Active apps count is wrong #4221

Closed

hariombalhara added 3 commits September 29, 2022 17:41

Fix viewer.eventTypes endpoint. Make prisma base select and _ prefixe…

80a3a1e

…d models consistent in expecting scalars only

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

fa20e5e

…ype-extensions

Remove unnecessary zod parsing of event-types as it is making the sco…

e04625d

…pe of the PR huge

vercel Bot had a problem deploying to Preview September 29, 2022 15:45 Failure

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

c1b39b3

…ype-extensions

vercel Bot had a problem deploying to Preview September 29, 2022 16:11 Failure

Fix UI TS errors

785e6eb

vercel Bot had a problem deploying to Preview September 29, 2022 17:30 Failure

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

ea5c95a

…ype-extensions

vercel Bot had a problem deploying to Preview September 30, 2022 05:04 Failure

hariombalhara added 3 commits September 30, 2022 10:53

wip

8792bff

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

80341ec

…ype-extensions

Add zod types support in EventTypeAppCard.tsx

afce272

vercel Bot had a problem deploying to Preview September 30, 2022 11:49 Failure

Fixes during PR review and other failing tests

1ae50c6

hariombalhara force-pushed the app-store-cli-event-type-extensions branch from cfe9494 to 1ae50c6 Compare September 30, 2022 12:54

vercel Bot deployed to Preview September 30, 2022 13:05 View deployment

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

bd453a7

…ype-extensions

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

930ad76

…ype-extensions

vercel Bot deployed to Preview October 4, 2022 14:32 View deployment

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

35a4677

…ype-extensions

vercel Bot deployed to Preview October 6, 2022 11:40 View deployment

Merge branch 'main' into app-store-cli-event-type-extensions

084b7b4

vercel Bot deployed to Preview October 6, 2022 14:14 View deployment

hariombalhara self-assigned this Oct 7, 2022

hariombalhara added this to the v.2.1 milestone Oct 7, 2022

hariombalhara linked an issue Oct 7, 2022 that may be closed by this pull request

Rainbow App not visible under Apps in EventType even if installed #4792

Closed

Merge branch 'main' into app-store-cli-event-type-extensions

1ec20b9

vercel Bot deployed to Preview October 9, 2022 11:46 View deployment

hariombalhara mentioned this pull request Oct 10, 2022

added fathom app #4804

Merged

3 tasks

hariombalhara changed the title ~~Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI~~ Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI - Also adds Fathom app Oct 10, 2022

hariombalhara added 2 commits October 10, 2022 20:21

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

fe07635

…ype-extensions

Merge remote-tracking branch 'origin/app-store-cli-event-type-extensi…

f10b953

…ons' into app-store-cli-event-type-extensions

vercel Bot deployed to Preview October 10, 2022 14:59 View deployment

hariombalhara added 3 commits October 10, 2022 20:35

Fix lint error

7f22acb

Fix runtime error with legayAppData being undefined

f9c7146

Remove duplicate automation key

2e31cf4

vercel Bot deployed to Preview October 10, 2022 15:23 View deployment

vercel Bot had a problem deploying to Preview October 14, 2022 11:14 Failure

Merge remote-tracking branch 'origin/main' into app-store-cli-event-t…

ace72e4

…ype-extensions

hariombalhara force-pushed the app-store-cli-event-type-extensions branch from 8973423 to ace72e4 Compare October 14, 2022 11:18

vercel Bot deployed to Preview October 14, 2022 11:25 View deployment

leog approved these changes Oct 14, 2022

View reviewed changes

zomars approved these changes Oct 14, 2022

View reviewed changes

zomars merged commit 271d431 into main Oct 14, 2022

zomars deleted the app-store-cli-event-type-extensions branch October 14, 2022 16:24

PeerRich added the core area: core, team members only label Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI - Also adds Fathom app#4727

Introduce EventTypeAppCard in app-store and make it super easy to add it through CLI - Also adds Fathom app#4727
zomars merged 41 commits intomainfrom
app-store-cli-event-type-extensions

hariombalhara commented Sep 28, 2022 •

edited

Loading

Uh oh!

vercel Bot commented Sep 28, 2022 •

edited

Loading

Uh oh!

leog left a comment

Uh oh!

leog Oct 14, 2022

Uh oh!

hariombalhara Oct 14, 2022

Uh oh!

leog Oct 14, 2022

Uh oh!

leog Oct 14, 2022

Uh oh!

leog Oct 14, 2022

Uh oh!

leog Oct 14, 2022

Uh oh!

hariombalhara Oct 14, 2022

Uh oh!

zomars left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

hariombalhara commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Also adds fathom app through the merge of #4804

Important

Design Principle-1: One App shouldn't break another App.

Design Principle-2: While adding a new app, the developer shouldn't worry about code beyond the app directory

Design Principle-3: Adding new apps and maintenance should be easy. See Loom

Design Principle-4: Easy discoverability of apps(See Loom)

Other Fixes

Type of change

How should this be tested?

Checklist

Uh oh!

vercel Bot commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leog left a comment

Choose a reason for hiding this comment

Uh oh!

leog Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

hariombalhara Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

leog Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

leog Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

leog Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

leog Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

hariombalhara Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

zomars left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hariombalhara commented Sep 28, 2022 •

edited

Loading

vercel Bot commented Sep 28, 2022 •

edited

Loading