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.

Sign up for GitHub

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

Jump to bottom

feat: Gsheet adding sheets component #15598

Closed
wants to merge 7 commits into from
Closed

feat: Gsheet adding sheets component #15598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb5f607
first draft
AAfghahi Jul 8, 2021
459eba3
added table_catalog
AAfghahi Jul 9, 2021
9aba8fc
remove console.log
hughhhh Jul 12, 2021
6b30f79
Update superset-frontend/src/views/CRUD/data/database/DatabaseModal/D…
AAfghahi Jul 13, 2021
962e597
Update superset-frontend/src/views/CRUD/data/database/DatabaseModal/i…
AAfghahi Jul 13, 2021
0e4f249
revisions
AAfghahi Jul 13, 2021
84222b2
revisions
AAfghahi Jul 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
255 changes: 173 additions & 82 deletions superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { FormEvent, useState } from 'react';
import React, { FormEvent, useState, Dispatch, SetStateAction } from 'react';
import { SupersetTheme, JsonObject, t } from '@superset-ui/core';
import { InputProps } from 'antd/lib/input';
import { Switch, Select, Button } from 'src/common/components';
Expand Down Expand Up @@ -46,6 +46,7 @@ export const FormFieldOrder = [
'password',
'database_name',
'credentials_info',
'table_catalog',
'query',
'encryption',
];
Expand All @@ -66,13 +67,16 @@ interface FieldPropTypes {
sslForced?: boolean;
defaultDBName?: string;
editNewDb?: boolean;
setPublic: Dispatch<SetStateAction<boolean>>;
isPublic?: boolean;
}

const CredentialsInfo = ({
changeMethods,
isEditMode,
db,
editNewDb,
isPublic,
}: FieldPropTypes) => {
const [uploadOption, setUploadOption] = useState<number>(
CredentialInfoOptions.jsonUpload.valueOf(),
Expand All @@ -82,7 +86,7 @@ const CredentialsInfo = ({
);
return (
<CredentialInfoForm>
{!isEditMode && (
{!isEditMode && db?.engine === 'bigquery' && (
Copy link
Member

Choose a reason for hiding this comment

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

By making these conditionals by engine, we're heading towards what we tried to avoid originally which was to make a large if/then block with a bunch of engines. Can we instead make this more dynamic and look for some property of the engine that is relevant to the code below, like uploadOption?

Copy link
Member

Choose a reason for hiding this comment

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

We currently return this for BQ:

{
  "available_drivers": [
    "bigquery"
  ],
  "default_driver": "bigquery",
  "engine": "bigquery",
  "name": "Google BigQuery",
  "parameters": {
    "properties": {
      "credentials_info": {
        "description": "Contents of BigQuery JSON credentials.",
        "type": "string",
        "x-encrypted-extra": true
      }
    },
    "required": [
      "credentials_info"
    ],
    "type": "object"
  },
  "preferred": false,
  "sqlalchemy_uri_placeholder": "bigquery://{project_id}"
}

Note how for credentials_info we have extra metadata informing that the value should be sent via encrypted_extra:

      "credentials_info": {
        "description": "Contents of BigQuery JSON credentials.",
        "type": "string",
        "x-encrypted-extra": true
      }

We can add more extra metadata in the BE to inform the FE that this should be an upload/past form, eg:

      "credentials_info": {
        "description": "Contents of BigQuery JSON credentials.",
        "type": "string",
        "x-encrypted-extra": true,
        "x-special-input-types": ["paste", "upload"]
      }

So that the FE knows to render the component without hardcoding the engine.

For GSheets this is a bit more complicated, because we want to have conditional rendering of the component. Not sure how to proceed there.

@eschutho, thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason why we have to use ‘credentials_info’ as the key for the field? For most fields, that key is the return value for the api, but since we’re renaming it, can’t we call it something like encrypted_credentials?

Copy link
Member

Choose a reason for hiding this comment

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

Right, we could call it anything we want.

Copy link
Member

Choose a reason for hiding this comment

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

👍 Then encrypted_credentials could be a paste/upload component.. no need for the api to define it.

<>
<FormLabel required>
{t('How do you want to enter service account credentials?')}
Expand All @@ -104,9 +108,10 @@ const CredentialsInfo = ({
)}
{uploadOption === CredentialInfoOptions.copyPaste ||
isEditMode ||
editNewDb ? (
editNewDb ||
(db?.engine === 'gsheets' && !isPublic) ? (
Copy link
Member

Choose a reason for hiding this comment

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

same thing here. We want to avoid a bunch of if/thens by engine.

<div className="input-container">
<FormLabel required>{t('Service Account')}</FormLabel>
<FormLabel required>{t('Service Account Information')}</FormLabel>
<textarea
className="input-form"
name="credentials_info"
Expand All @@ -115,78 +120,126 @@ const CredentialsInfo = ({
placeholder="Paste content of service credentials JSON file here"
/>
<span className="label-paste">
{t('Copy and paste the entire service account .json file here')}
{t(
'Copy and paste the entire service account .json file here to enable connection',
eschutho marked this conversation as resolved.
Show resolved Hide resolved
)}
</span>
</div>
) : (
<div
className="input-container"
css={(theme: SupersetTheme) => infoTooltip(theme)}
>
<div css={{ display: 'flex', alignItems: 'center' }}>
<FormLabel required>{t('Upload Credentials')}</FormLabel>
<InfoTooltip
tooltip={t(
'Use the JSON file you automatically downloaded when creating your service account in Google BigQuery.',
)}
viewBox="0 0 24 24"
/>
</div>

{!fileToUpload && (
<Button
className="input-upload-btn"
onClick={() => document?.getElementById('selectedFile')?.click()}
>
{t('Choose File')}
</Button>
)}
{fileToUpload && (
<div className="input-upload-current">
{fileToUpload}
<DeleteFilled
onClick={() => {
setFileToUpload(null);
changeMethods.onParametersChange({
target: {
name: 'credentials_info',
value: '',
},
});
}}
db?.engine === 'bigquery' && (
<div
className="input-container"
css={(theme: SupersetTheme) => infoTooltip(theme)}
>
<div css={{ display: 'flex', alignItems: 'center' }}>
<FormLabel required>{t('Upload Credentials')}</FormLabel>
<InfoTooltip
tooltip={t(
'Use the JSON file you automatically downloaded when creating your service account in Google BigQuery.',
)}
viewBox="0 0 24 24"
/>
</div>
)}

<input
id="selectedFile"
className="input-upload"
type="file"
onChange={async event => {
let file;
if (event.target.files) {
file = event.target.files[0];
}
setFileToUpload(file?.name);
changeMethods.onParametersChange({
target: {
type: null,
name: 'credentials_info',
value: await file?.text(),
checked: false,
},
});
(document.getElementById(
'selectedFile',
) as HTMLInputElement).value = null as any;
}}
/>
</div>
)
{!fileToUpload && (
<Button
className="input-upload-btn"
onClick={() =>
document?.getElementById('selectedFile')?.click()
Copy link
Member

Choose a reason for hiding this comment

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

can we use a ref to get the element instead of the dom directly?

}
>
{t('Choose File')}
</Button>
)}
{fileToUpload && (
<div className="input-upload-current">
{fileToUpload}
<DeleteFilled
onClick={() => {
setFileToUpload(null);
changeMethods.onParametersChange({
target: {
name: 'credentials_info',
value: '',
},
});
}}
/>
</div>
)}
<input
id="selectedFile"
className="input-upload"
type="file"
onChange={async event => {
Copy link
Member

Choose a reason for hiding this comment

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

can we put this onChange in a separate function? It's a little long to be inline.

let file;
if (event.target.files) {
file = event.target.files[0];
}
setFileToUpload(file?.name);
changeMethods.onParametersChange({
target: {
type: null,
name: 'credentials_info',
value: await file?.text(),
checked: false,
},
});
(document.getElementById(
Copy link
Member

Choose a reason for hiding this comment

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

can we use a ref instead?

'selectedFile',
) as HTMLInputElement).value = null as any;
}}
/>
</div>
)
)}
</CredentialInfoForm>
);
};

const TableCatalog = ({
required,
changeMethods,
isEditMode,
getValidation,
db,
editNewDb,
validationErrors,
}: FieldPropTypes) => {
const [tableCatalog, setTableCatalog] = useState<Record<string, string>>(
db?.parameters?.table_catalog || {},
);
return (
<div>
{Object.keys(tableCatalog).map(field => (
<>
<ValidatedInput
id="table_catalog_name"
name="table_catalog_name"
required={required}
value={field}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.table_catalog}
placeholder="Create a name for this sheet"
label="Google Sheet name and url"
onChange={changeMethods.onParametersChange}
/>
<ValidatedInput
id="table_catalog_value"
name="table_catalog_value"
required={required}
value={tableCatalog[field]}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.table_catalog}
placeholder="Paste the shareable Google Sheet URL here"
onChange={changeMethods.onParametersChange}
/>
</>
))}
</div>
);
};

const hostField = ({
required,
changeMethods,
Expand Down Expand Up @@ -294,25 +347,56 @@ const passwordField = ({
onChange={changeMethods.onParametersChange}
/>
);
const displayField = ({
const DisplayField = ({
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="database_name"
name="database_name"
required
value={db?.database_name}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.database_name}
placeholder=""
label="Display Name"
onChange={changeMethods.onChange}
helpText={t('Pick a nickname for this database to display as in Superset.')}
/>
);
setPublic,
}: FieldPropTypes) => {
const setBooleanToString = (value: string): boolean => {
if (value === 'true') {
return true;
}
return false;
};
return (
<>
<ValidatedInput
id="database_name"
name="database_name"
required
value={db?.database_name}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.database_name}
placeholder=""
label="Display Name"
onChange={changeMethods.onChange}
helpText={t(
'Pick a nickname for this database to display as in Superset.',
)}
/>

{db?.engine === 'gsheets' && (
<>
<FormLabel required>{t('Type of Google Sheets Allowed')}</FormLabel>
<Select
style={{ width: '100%' }}
onChange={(value: string) => setPublic(setBooleanToString(value))}
defaultValue="true"
>
<Select.Option value="true" key={1}>
Publicly shared sheets only
</Select.Option>
<Select.Option value="false" key={2}>
Public and privately shared sheets
</Select.Option>
</Select>
</>
)}
</>
);
};

const queryField = ({
required,
Expand Down Expand Up @@ -371,10 +455,11 @@ const FORM_FIELD_MAP = {
database: databaseField,
username: usernameField,
password: passwordField,
database_name: displayField,
database_name: DisplayField,
query: queryField,
encryption: forceSSLField,
credentials_info: CredentialsInfo,
table_catalog: TableCatalog,
};

const DatabaseConnectionForm = ({
Expand All @@ -388,10 +473,14 @@ const DatabaseConnectionForm = ({
isEditMode = false,
sslForced,
editNewDb,
setPublic,
isPublic,
}: {
isEditMode?: boolean;
sslForced: boolean;
editNewDb?: boolean;
isPublic?: boolean;
setPublic: Dispatch<SetStateAction<boolean>>;
dbModel: DatabaseForm;
db: Partial<DatabaseObject> | null;
onParametersChange: (
Expand Down Expand Up @@ -434,6 +523,8 @@ const DatabaseConnectionForm = ({
isEditMode,
sslForced,
editNewDb,
setPublic,
isPublic,
}),
)}
</div>
Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const [dbName, setDbName] = useState('');
const [editNewDb, setEditNewDb] = useState<boolean>(false);
const [isLoading, setLoading] = useState<boolean>(false);
const [isPublic, setPublic] = useState<boolean>(true);
const conf = useCommonConf();
const dbImages = getDatabaseImages();
const connectionAlert = getConnectionAlert();
Expand All @@ -360,7 +361,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
t('database'),
addDangerToast,
);

const isDynamic = (engine: string | undefined) =>
availableDbs?.databases.filter(
(DB: DatabaseObject) => DB.backend === engine || DB.engine === engine,
Expand Down Expand Up @@ -799,6 +799,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}
return (
<DatabaseConnectionForm
isPublic={isPublic}
setPublic={setPublic}
isEditMode
sslForced={sslForced}
dbModel={dbModel}
Expand Down Expand Up @@ -912,6 +914,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
) : (
<DatabaseConnectionForm
isEditMode
isPublic={isPublic}
setPublic={setPublic}
sslForced={sslForced}
dbModel={dbModel}
db={db as DatabaseObject}
Expand Down Expand Up @@ -1070,6 +1074,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
)}
<DatabaseConnectionForm
db={db}
setPublic={setPublic}
isPublic={isPublic}
sslForced={sslForced}
dbModel={dbModel}
onParametersChange={({
Expand Down
Loading