🪟 🎉 Connector Builder Landing Page #22122

lmossman · 2023-01-31T01:05:23Z

What

Resolves #21490

Adds a landing page for the connector builder, which is shown if the user doesn't have any connector builder changes saved to browser local storage.

To return to this page after making changes, click the Reset All button in the bottom-left corner of the Builder.

How

As described in the issue, this PR only adds tiles for importing a YAML and starting from scratch. Start from scratch just opens a blank connector builder UI as normal.

The Import a YAML button only allows users to select a single .yaml file from their computer to upload. A few outcomes can occur here:

The uploaded file contains invalid YAML. In this case, we just show a Toast with the YAML validation error message and line number, which is auto-dismissed if they click the import button again.
The uploaded file contains valid YAML, but it can't be converted to Builder UI. In this case, we open the YAML in the YAML editor view of the connector builder.
The uploaded file contains valid YAML and can be converted to Builder UI. In this case, we open the Builder UI with the values filled in from the YAML.
- In this case, the Connector Name is set to the titleCase version of the file name (minus the extension), e.g. poke_api.yaml would result in the connector name set to Poke Api

lmossman · 2023-01-31T01:13:12Z

airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/ConfigMenu.tsx

@@ -62,7 +62,7 @@ export const ConfigMenu: React.FC<ConfigMenuProps> = ({ className, testInputJson
              onClick={() => setIsOpen(true)}
              disabled={
                !jsonManifest.spec ||
-                Object.keys(jsonManifest.spec.connection_specification.properties || {}).length === 0
+                Object.keys(jsonManifest.spec.connection_specification?.properties || {}).length === 0


Small bugfix - the page would crash if the spec did not contain connection_specification, and this change prevents that crash from occurring

lmossman · 2023-01-31T01:16:04Z

airbyte-webapp/src/components/connectorBuilder/types.ts

@@ -99,7 +99,7 @@ export interface BuilderStream {

 export const DEFAULT_BUILDER_FORM_VALUES: BuilderFormValues = {
  global: {
-    connectorName: "",
+    connectorName: "Untitled",


We want this eventually anyway, and this prevents us from showing a Required error on Reset All

lmossman · 2023-01-31T01:17:34Z

airbyte-webapp/src/components/connectorBuilder/useManifestToBuilderForm.ts

@@ -60,7 +61,7 @@ export const convertToBuilderFormValues = async (
  }
  const resolvedManifest = resolveResult.manifest as ConnectorManifest;

-  const builderFormValues = DEFAULT_BUILDER_FORM_VALUES;
+  const builderFormValues = cloneDeep(DEFAULT_BUILDER_FORM_VALUES);


Fixes a bug where the lines below this one were actually updating the DEFAULT_BUILDER_FORM_VALUES constant's fields, causing issues like Reset All not behaving properly

lmossman · 2023-01-31T01:20:07Z

airbyte-webapp/src/pages/ConnectorBuilderPage/LandingPage.tsx

+          <input
+            type="file"
+            accept=".yml,.yaml"
+            ref={fileInputRef}
+            style={{ display: "none" }}
+            onChange={(uploadEvent) => {
+              setImportYamlLoading(true);
+              const file = uploadEvent.target.files?.[0];
+              const reader = new FileReader();
+              reader.onload = (readerEvent) => {
+                handleYamlUpload(readerEvent.target?.result as string);
+              };
+              if (file) {
+                reader.readAsText(file);
+              }
+            }}
+          />


Open to suggestions here, but this seemed like the simplest and most standard way to handle file uploads without using a library. And I couldn't find any libraries that were well supported and lightweight

flash1293

Looks mostly good besides some nits and one question about the code structure. Works like a charm!

flash1293 · 2023-01-31T14:03:48Z

airbyte-webapp/src/pages/ConnectorBuilderPage/ConnectorBuilderPage.tsx

@@ -47,6 +50,11 @@ const ConnectorBuilderPageInner: React.FC = React.memo(() => {
        validationSchema={builderFormValidationSchema}
      >
        {(props) => {
+          if (showLandingPage) {
+            return (
+              <LandingPage hideLandingPage={hideLandingPage} switchToUI={switchToUI} switchToYaml={switchToYaml} />


As this is part of the regular ConnectorBuilderPage, it initializes the form and test state services and tries to do a first fetch which fails with a 400.

I wonder whether we should split out the landing page into its own page with its own path - it seems easier to understand in the code (no "page in a page") and we can better control what side effects are triggered and avoid state leaking from one connector to another in case the user switches (also I think we won't get around that for having multiple builder projects anyway).

This is what I could imagine as interim state:

connector-builder/edit shows the actual connector builder UI (redirects to connector-builder on reset)

connector-builder shows the landing page (for now redirects automatically to connector-builder/edit if there is a non-default state in localstorage)

I think we can have a separate service for the landing page wiring up with local storage (and later on with the APIs to save and load connectors).

Doesn't absolutely need to happen on this PR as this is a meaningful increment, but it seems like a better step towards the targeted feature scope - your call how to go with this.

I like your suggestions here - I went ahead and added those changes now. I moved the local storage handling to its own ConnectorBuilderLocalStorage service/context, and moved the landing page into its own page / updated the routes and used redirects to go between.

I also found and fixed the issue with Reset All not always working properly - this was because it wasn't updating local storage properly, so I made sure it does that before navigating back to the landing page.

flash1293 · 2023-01-31T14:24:03Z

airbyte-webapp/src/pages/ConnectorBuilderPage/LandingPage.tsx

+            ref={fileInputRef}
+            style={{ display: "none" }}
+            onChange={(uploadEvent) => {
+              setImportYamlLoading(true);


nit: Why not moving all of this into the callback? Isolates a bit better the raw API access

I've moved this into the callback. The callback is now pretty long but I agree that keeping the logic together is probably better encapsulation

flash1293 · 2023-01-31T14:28:52Z

airbyte-webapp/src/pages/ConnectorBuilderPage/LandingPage.tsx

+            type="file"
+            accept=".yml,.yaml"
+            ref={fileInputRef}
+            style={{ display: "none" }}


nit: the prop hidden should work as well here and is slightly more "correct" (not a big deal though)

flash1293 · 2023-01-31T14:31:10Z

airbyte-webapp/src/components/connectorBuilder/Builder/BuilderSidebar.tsx

@@ -73,6 +73,7 @@ export const BuilderSidebar: React.FC<BuilderSidebarProps> = React.memo(({ class
      onSubmit: () => {
        setValues(DEFAULT_BUILDER_FORM_VALUES);


Doesn't need to be fixed on this PR, but it seems like this doesn't work reliably - after hidden "Reset all", then reloading the page I'm getting back to the previous state (there is probably another render-cycle of some kind putting it back). A separate page with a separate context for state management (see comment above) might help with this automatically.

See earlier comment, after the refactor and making this properly update local storage, this should now be fixed.

flash1293 · 2023-01-31T14:32:39Z

airbyte-webapp/src/services/connectorBuilder/ConnectorBuilderStateService.tsx

@@ -100,6 +95,11 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren
  const manifestRef = useRef(derivedJsonManifest);
  manifestRef.current = derivedJsonManifest;

+  const [showLandingPage, setShowLandingPage] = useState(


nit: isEqual isn't super cheap, would be nice to wrap it into an initializer function

Moved this into the landing page component since we now only need to do this once on page load. I put it in a useEffect which should only run a single time since I'm using refs for the local storage values

flash1293

Looks great, just left to nits but LGTM otherwise. No need for another round of review

flash1293 · 2023-02-02T14:37:24Z

...ebapp/src/pages/connectorBuilder/ConnectorBuilderLandingPage/ConnectorBuilderLandingPage.tsx

+          buttonText="connectorBuilder.landingPage.importYaml.button"
+          buttonProps={{ isLoading: importYamlLoading }}
+          onClick={() => {
+            unregisterNotificationById(YAML_UPLOAD_ERROR_ID);


We should always unregister this when navigating away from the page, otherwise the user could try to upload a yaml, give up and go with the "from scratch" option:

Can be done via return value of an effect hook

useEffect(() => { return () => unregisterNotificationById(YAML_UPLOAD_ERROR_ID); }, [unregisterNotificationById]);

Good catch! Fixed

flash1293 · 2023-02-02T14:43:58Z

...ebapp/src/pages/connectorBuilder/ConnectorBuilderLandingPage/ConnectorBuilderLandingPage.tsx

+          }
+
+          if (fileName) {
+            convertedFormValues.global.connectorName = startCase(fileName.split(".")[0]);


This used to work really well until #21771 - now it will default to "Manifest" which is not super helpful. I think the next best thing is the url base, but no strong opinion on this.

My suggestion:

If the file name is not "manifest", use the current logic

If we can turn into form values, use the base url

flash1293 · 2023-02-02T14:50:58Z

...ebapp/src/pages/connectorBuilder/ConnectorBuilderLandingPage/ConnectorBuilderLandingPage.tsx

+          } catch (e) {
+            setStoredEditorView("yaml");
+            setStoredManifest(json);
+            navigate(ConnectorBuilderRoutePaths.Edit);


Probably out of scope for this PR, but I think it would be very nice to have telemetry for the landing page as well. Wdyt?

I will put up a separate PR for that 👍

* master: Discover worker starts to use API to write schema result (#21875) 🪟 🎉 Connector Builder Landing Page (#22122) Fix pnpm cache path (#22418) Add additional shorter setup guides (#22318) Source Amazon Ads: fix reports stream records primary keys (#21677) Connector acceptance test: Fix discovered catalog caching for different configs (#22301) 🪟🐛 Make modal scrollable (#21973) only compute diff if the schema discovery actually succeeded (#22377) Source Klaviyo: fix schema (#22071) 🪟 🔧 Switch to `pnpm` for package managing (#22053) Source Sentry: turn on default availability strategy (#22303) Source freshdesk: deduplicate table names (#22164)

* master: (86 commits) Discover worker starts to use API to write schema result (#21875) 🪟 🎉 Connector Builder Landing Page (#22122) Fix pnpm cache path (#22418) Add additional shorter setup guides (#22318) Source Amazon Ads: fix reports stream records primary keys (#21677) Connector acceptance test: Fix discovered catalog caching for different configs (#22301) 🪟🐛 Make modal scrollable (#21973) only compute diff if the schema discovery actually succeeded (#22377) Source Klaviyo: fix schema (#22071) 🪟 🔧 Switch to `pnpm` for package managing (#22053) Source Sentry: turn on default availability strategy (#22303) Source freshdesk: deduplicate table names (#22164) Update connector-acceptance-tests-reference.md (#22370) Update the default security groups for the EC2 runner (#22347) Trace refresh schema operations (#22326) Remove manual docker upgrades from workflows (#22344) Update CODEOWNERS for connector acceptance tests to connector ops (#22341) 🐛 source: airtable - handle singleSelect types (#22311) Source tiktok: chunk advertiser IDs (#22309) 🪟 🧪 E2E Tests for auto-detect schema changes (#20682) ...

* master: (24 commits) Discover worker starts to use API to write schema result (#21875) 🪟 🎉 Connector Builder Landing Page (#22122) Fix pnpm cache path (#22418) Add additional shorter setup guides (#22318) Source Amazon Ads: fix reports stream records primary keys (#21677) Connector acceptance test: Fix discovered catalog caching for different configs (#22301) 🪟🐛 Make modal scrollable (#21973) only compute diff if the schema discovery actually succeeded (#22377) Source Klaviyo: fix schema (#22071) 🪟 🔧 Switch to `pnpm` for package managing (#22053) Source Sentry: turn on default availability strategy (#22303) Source freshdesk: deduplicate table names (#22164) Update connector-acceptance-tests-reference.md (#22370) Update the default security groups for the EC2 runner (#22347) Trace refresh schema operations (#22326) Remove manual docker upgrades from workflows (#22344) Update CODEOWNERS for connector acceptance tests to connector ops (#22341) 🐛 source: airtable - handle singleSelect types (#22311) Source tiktok: chunk advertiser IDs (#22309) 🪟 🧪 E2E Tests for auto-detect schema changes (#20682) ...

lmossman added 7 commits January 27, 2023 14:54

save working version of minimal landing page

5f90275

move landing page to own component and show Toast on invalid yaml

b22d310

add default Untitled connector name

294670b

save styling progress

90c0a8c

style the landing page

3d7a43e

use card component

8f04f76

reword descriptions

e51de2f

octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 31, 2023

lmossman changed the title ~~onnector builder landing page~~ 🪟 🎉 Connector Builder Landing Page Jan 31, 2023

lmossman requested a review from flash1293 January 31, 2023 01:11

lmossman commented Jan 31, 2023

View reviewed changes

undo unnecessary syntax change

2012eee

lmossman commented Jan 31, 2023

View reviewed changes

lmossman and others added 4 commits January 30, 2023 17:34

use file name as connector name

af2eb74

move showLandingPage to global state and set to true on reset

5b09441

fix e2e test

6c3f69a

Merge branch 'master' into lmossman/connector-builder-landing-page

d8feb64

flash1293 reviewed Jan 31, 2023

View reviewed changes

lmossman added 5 commits January 31, 2023 16:04

move local storage hooks into a separate service/context

574bdfd

move landing page to its own page

587458e

update doc

7a4810b

move full file input handling logic into callback

2550eeb

Merge branch 'master' into lmossman/connector-builder-landing-page

0ec4ebc

octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Feb 1, 2023

lmossman requested a review from flash1293 February 1, 2023 02:06

use hidden

a88ae72

flash1293 approved these changes Feb 2, 2023

View reviewed changes

clear out notification on unmount

dc425d2

lmossman added 3 commits February 2, 2023 14:59

use url if filename is 'manifest'

7e13623

add comment

68e2631

merge with master

8cbd12e

lmossman enabled auto-merge (squash) February 2, 2023 23:33

add telemetry for landing page events (#22351)

349748b

lmossman merged commit 0cef7b0 into master Feb 6, 2023

lmossman deleted the lmossman/connector-builder-landing-page branch February 6, 2023 17:19

github-actions bot mentioned this pull request Feb 7, 2023

Bump helm chart version reference to 0.43.28 #22489

Closed

This was referenced Feb 8, 2023

Bump Airbyte version from 0.40.32 to 0.40.33 #22578

Closed

Bump Airbyte version from 0.40.32 to 0.40.33 #22689

Closed

Bump Airbyte version from 0.40.32 to 0.40.33 #22839

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎉 Connector Builder Landing Page #22122

🪟 🎉 Connector Builder Landing Page #22122

lmossman commented Jan 31, 2023 •

edited

Loading

lmossman Jan 31, 2023

lmossman Jan 31, 2023

lmossman Jan 31, 2023

lmossman Jan 31, 2023

flash1293 left a comment

flash1293 Jan 31, 2023

lmossman Feb 1, 2023

flash1293 Jan 31, 2023

lmossman Feb 1, 2023

flash1293 Jan 31, 2023

flash1293 Jan 31, 2023

lmossman Feb 1, 2023

flash1293 Jan 31, 2023

lmossman Feb 1, 2023

flash1293 left a comment

flash1293 Feb 2, 2023

lmossman Feb 2, 2023

flash1293 Feb 2, 2023

flash1293 Feb 2, 2023

lmossman Feb 2, 2023

		@@ -73,6 +73,7 @@ export const BuilderSidebar: React.FC<BuilderSidebarProps> = React.memo(({ class
		onSubmit: () => {
		setValues(DEFAULT_BUILDER_FORM_VALUES);

🪟 🎉 Connector Builder Landing Page #22122

🪟 🎉 Connector Builder Landing Page #22122

Conversation

lmossman commented Jan 31, 2023 • edited Loading

What

How

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

flash1293 left a 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

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

flash1293 left a 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

lmossman commented Jan 31, 2023 •

edited

Loading