Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embedding SDK - integration commit #40198

Merged
merged 14 commits into from
Mar 26, 2024
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
/resources/frontend_client/embed.html
/resources/frontend_client/index.html
/resources/frontend_client/public.html
/resources/embedding-sdk/dist/
/resources/i18n/*.edn
/resources/instance_analytics.zip
/resources/license-backend-third-party.txt
Expand Down
8 changes: 8 additions & 0 deletions enterprise/frontend/src/embedding-sdk/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Embedding SDK

### Build
`yarn build`

`yarn build-embedding-sdk`

Build results are located at `<root>/resources/embedding-sdk`
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type * as React from "react";
import { t } from "ttag";

import { DEFAULT_FONT } from "../../config";
import { EmbeddingContext } from "../../context";
import { useInitData } from "../../hooks";
import type { SDKConfigType } from "../../types";

import { SdkContentWrapper } from "./SdkContentWrapper";

interface AppInitializeControllerProps {
children: React.ReactNode;
config: SDKConfigType;
}

export const AppInitializeController = ({
config,
children,
}: AppInitializeControllerProps) => {
const { isLoggedIn, isInitialized } = useInitData({
config,
});

return (
<EmbeddingContext.Provider
value={{
isInitialized,
isLoggedIn,
}}
>
<SdkContentWrapper font={config.font ?? DEFAULT_FONT}>
{!isInitialized ? <div>{t`Loading…`}</div> : children}
</SdkContentWrapper>
</EmbeddingContext.Provider>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import styled from "@emotion/styled";

import { alpha, color } from "metabase/lib/colors";
import { aceEditorStyles } from "metabase/query_builder/components/NativeQueryEditor/NativeQueryEditor.styled";
import { saveDomImageStyles } from "metabase/visualizations/lib/save-chart-image";

export const SdkContentWrapper = styled.div<{ font: string }>`
--default-font-family: "${({ font }) => font}";
--color-brand: ${color("brand")};
--color-brand-alpha-04: ${alpha("brand", 0.04)};
--color-brand-alpha-88: ${alpha("brand", 0.88)};
--color-focus: ${color("focus")};

${aceEditorStyles}
${saveDomImageStyles}

--default-font-size: 0.875em;
--default-font-color: var(--color-text-dark);
--default-bg-color: var(--color-bg-light);

font-family: var(--default-font-family), sans-serif;
font-size: var(--default-font-size);
font-weight: 400;
font-style: normal;
color: var(--color-text-dark);
margin: 0;
height: 100%; /* ensure the entire page will fill the window */
display: flex;
flex-direction: column;
background-color: var(--color-bg-light);

-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import type { AnyAction, Store } from "@reduxjs/toolkit";
import type * as React from "react";
import { memo } from "react";
import { Provider } from "react-redux";

import reducers from "metabase/reducers-main";
import { getStore } from "metabase/store";
import { EmotionCacheProvider } from "metabase/styled-components/components/EmotionCacheProvider";
import { ThemeProvider } from "metabase/ui/components/theme/ThemeProvider";
import type { State } from "metabase-types/store";

import type { SDKConfigType } from "../../types";
import { AppInitializeController } from "../private/AppInitializeController";

import "metabase/css/vendor.css";
import "metabase/css/index.module.css";

const store = getStore(reducers) as unknown as Store<State, AnyAction>;

const MetabaseProviderInternal = ({
children,
config,
}: {
children: React.ReactNode;
config: SDKConfigType;
}): React.JSX.Element => {
return (
<Provider store={store}>
<EmotionCacheProvider>
<ThemeProvider>
<AppInitializeController config={config}>
{children}
</AppInitializeController>
</ThemeProvider>
</EmotionCacheProvider>
</Provider>
);
};

export const MetabaseProvider = memo(MetabaseProviderInternal);
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice, if you include an example how this provider is supposed to be used in enterprise/frontend/src/embedding-sdk/README.md. Now that readme file is pretty barebone.

It's hard to just imagine how this component is supposed to be used without context.

Otherwise, we could have a long comment above this file documenting how it's supposed to be used.

Copy link
Contributor Author

@deniskaber deniskaber Mar 25, 2024

Choose a reason for hiding this comment

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

Example on how to use SDK usage would be self-described in the host app example - https://github.com/metabase/embedding-sdk-customer-zero/blob/pre-alpha/src/App.js#L20 .

  • we will have types for public components added in the next task.

But I will add an action item to add documentation for the SDK at some point.

Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import { useEffect, useState } from "react";

import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
import { useSelector } from "metabase/lib/redux";
import {
onCloseChartType,
onOpenChartSettings,
setUIControls,
} from "metabase/query_builder/actions";
import QueryVisualization from "metabase/query_builder/components/QueryVisualization";
import ChartTypeSidebar from "metabase/query_builder/components/view/sidebars/ChartTypeSidebar";
import { getMetadata } from "metabase/selectors/metadata";
import { CardApi } from "metabase/services";
import { Box, Group, Text } from "metabase/ui";
import { PublicMode } from "metabase/visualizations/click-actions/modes/PublicMode";
import Question from "metabase-lib/v1/Question";
import type { Card, CardId, Dataset } from "metabase-types/api";

import { useEmbeddingContext } from "../../context";

interface QueryVisualizationProps {
questionId: CardId;
showVisualizationSelector?: boolean;
}

type State = {
loading: boolean;
card: Card | null;
cardError?: Card | string | null;
result: Dataset | null;
resultError?: Dataset | string | null;
};

export const QueryVisualizationSdk = ({
questionId,
showVisualizationSelector,
}: QueryVisualizationProps): JSX.Element | null => {
const { isInitialized, isLoggedIn } = useEmbeddingContext();
const metadata = useSelector(getMetadata);

const [{ loading, card, result, cardError, resultError }, setState] =
useState<State>({
loading: false,
card: null,
cardError: null,
result: null,
resultError: null,
});

const loadCardData = async ({ questionId }: { questionId: number }) => {
setState(prevState => ({
WiNloSt marked this conversation as resolved.
Show resolved Hide resolved
...prevState,
loading: true,
}));

Promise.all([
CardApi.get({ cardId: questionId }),
CardApi.query({
cardId: questionId,
}),
])
.then(([card, result]) => {
setState(prevState => ({
...prevState,
card,
result,
loading: false,
cardError: null,
resultError: null,
}));
})
.catch(([cardError, resultError]) => {
setState(prevState => ({
...prevState,
result: null,
card: null,
loading: false,
cardError,
resultError,
}));
});
};

useEffect(() => {
if (!isInitialized || !isLoggedIn) {
setState({
loading: false,
card: null,
result: null,
cardError: null,
resultError: null,
});
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I like the else part more compared to the previous solution. I was thinking something felt off, but I couldn't think of anything else at that time.

loadCardData({ questionId });
}
}, [isInitialized, isLoggedIn, questionId]);

const changeVisualization = (newQuestion: Question) => {
setState({
card: newQuestion.card(),
result: result,
loading: false,
});
};

if (!isInitialized) {
return null;
}

if (!isLoggedIn) {
return (
<div>
<Text>You should be logged in to see this content.</Text>
</div>
);
}

const isLoading = loading || (!result && !resultError);

return (
<LoadingAndErrorWrapper
className="flex-full full-width"
loading={isLoading}
error={cardError || resultError}
noWrapper
>
{() => {
const question = new Question(card, metadata);
const legacyQuery = question.legacyQuery({
useStructuredQuery: true,
});

return (
<Group h="100%" pos="relative" align="flex-start">
{showVisualizationSelector && (
<Box w="355px">
<ChartTypeSidebar
question={question}
result={result}
onOpenChartSettings={onOpenChartSettings}
onCloseChartType={onCloseChartType}
query={legacyQuery}
setUIControls={setUIControls}
updateQuestion={changeVisualization}
/>
</Box>
)}
<QueryVisualization
className="flex full-width"
question={question}
rawSeries={[{ card, data: result?.data }]}
isRunning={isLoading}
isObjectDetail={false}
isResultDirty={false}
isNativeEditorOpen={false}
result={result}
noHeader
mode={PublicMode}
/>
</Group>
);
}}
</LoadingAndErrorWrapper>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { QueryVisualizationSdk } from "./QueryVisualization";
export { MetabaseProvider } from "./MetabaseProvider";
1 change: 1 addition & 0 deletions enterprise/frontend/src/embedding-sdk/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const DEFAULT_FONT = "Lato";
15 changes: 15 additions & 0 deletions enterprise/frontend/src/embedding-sdk/context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { createContext, useContext } from "react";

interface EmbeddingSdkContextData {
isInitialized: boolean;
isLoggedIn: boolean;
}

export const EmbeddingContext = createContext<EmbeddingSdkContextData>({
isInitialized: false,
isLoggedIn: false,
});

export const useEmbeddingContext = () => {
return useContext(EmbeddingContext);
};
2 changes: 2 additions & 0 deletions enterprise/frontend/src/embedding-sdk/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./public";
export * from "./private";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./use-init-data";
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { useEffect, useState } from "react";
import _ from "underscore";

import { reloadSettings } from "metabase/admin/settings/settings";
import api from "metabase/lib/api";
import { useDispatch } from "metabase/lib/redux";
import { refreshCurrentUser } from "metabase/redux/user";
import registerVisualizations from "metabase/visualizations/register";

import type { SDKConfigType } from "../../types";

const registerVisualizationsOnce = _.once(registerVisualizations);

interface InitDataLoaderParameters {
config: SDKConfigType;
}

export const useInitData = ({
config,
}: InitDataLoaderParameters): {
isLoggedIn: boolean;
isInitialized: boolean;
} => {
const dispatch = useDispatch();
const [isInitialized, setIsInitialized] = useState(false);
const [isLoggedIn, setIsLoggedIn] = useState(false);

useEffect(() => {
registerVisualizationsOnce();
}, []);

useEffect(() => {
api.basename = config.metabaseInstanceUrl;

if (config.authType === "apiKey" && config.apiKey) {
api.apiKey = config.apiKey;
} else {
setIsLoggedIn(false);
return;
}

Promise.all([
dispatch(refreshCurrentUser()),
dispatch(reloadSettings()),
]).then(() => {
setIsInitialized(true);
setIsLoggedIn(true);
Comment on lines +46 to +47
Copy link
Member

Choose a reason for hiding this comment

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

would it be possible that either refreshCurrentUser or reloadSettings would fail? i.e. if it returns 401 or something because the token is wrong., should the promise still be resolved? It just feels like we should check the result from calling these endpoints before setting isLoggedIn to true just to make sure we can call these endpoints.

What do you think?

Copy link
Contributor Author

@deniskaber deniskaber Mar 25, 2024

Choose a reason for hiding this comment

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

Here we have Promise.all. API calls inside refreshCurrentUser and reloadSettings have awaits, so in case any call fails, we would not get to .then.

We should add some error handling for sure, something like an ErrorBoundary in QueryVisualizationSdk component, but I don't think this fits in this PR scope.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, I found that we reject anything not within 2xx status, so this is good. I was worried errored requests are still going through .then

if (status >= 200 && status <= 299) {
if (options.transformResponse) {
body = options.transformResponse(body, { data });
}
resolve(body);
} else {
reject({
status: status,
data: body,
isCancelled: isCancelled,
});

});
}, [config, dispatch]);

return {
isLoggedIn,
isInitialized,
};
};
4 changes: 4 additions & 0 deletions enterprise/frontend/src/embedding-sdk/hooks/public/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export * from "./use-current-user";
export * from "./use-application-name";
export * from "./use-question-search";
export * from "./use-available-fonts";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { useSelector } from "metabase/lib/redux";
import { getApplicationName } from "metabase/selectors/whitelabel";

export const useApplicationName = () => useSelector(getApplicationName);
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { useSelector } from "metabase/lib/redux";
import { getSetting } from "metabase/selectors/settings";

export const useAvailableFonts = () => {
Copy link
Member

Choose a reason for hiding this comment

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

I don't see we export setFont here (used here). Should we? This is another reason why I mentioned earlier why I think we should have at least some simple documents how to use each components in the SDK, so we know how they're intended for consumption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remove this usage from embedding-sdk-customer-zero, but forgot to push it there 😄 . Pushed now

return {
availableFonts: useSelector(state => getSetting(state, "available-fonts")),
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the first time I see a hook inside a return 😮 , it makes sense though

Copy link
Member

Choose a reason for hiding this comment

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

};
};
Loading
Loading