External Service Credentials #12454
External Service Credentials #12454
Conversation
somebody1234
added
CI: No changelog needed
🧪 Storybook is successfully deployed!📊 Dashboard:
|
…ype assertion as they already get `form` from the `FormContext`
- Closes #12011 - Note that since Google API is experimental, the new credentials are currently integrated with Sheets feature. Analytics feature is not yet updated to support them, let's do that as a separate followup PR. - Related PRs: - GUI: #12454 - cloud: https://github.com/enso-org/cloud-v2/pull/1821 - This libs PR can probably be merged before the others, but first we want to test the integration of the full flow.
|
|
||
| /** A modal for creating a credential. */ | ||
| export default function CreateCredentialModal(props: CreateCredentialModalProps) { | ||
| const { noDialog = false, doCreate } = props |
There was a problem hiding this comment.
noDialog should be forbidden, prefer exporting 2 components: CreateCredentialModal and CreateCredentialForm
| const { getText } = useText() | ||
| const toastAndLog = useToastAndLog() | ||
|
|
||
| const form = Form.useForm({ |
There was a problem hiding this comment.
It's better to inline form here:
<Form schema={google.FORM_SCHEMA} method="dialog" className="w-full" onSubmit={....}>
There was a problem hiding this comment.
Same true for the other providers
| form={form} | ||
| name="scopes" | ||
| label={getText('googleCredentialScopes')} | ||
| defaultValue={['sheets']} |
There was a problem hiding this comment.
it's better to set default values on the form element
| @@ -651,6 +651,25 @@ export function useNewSecret(backend: Backend) { | |||
| }) | |||
| } | |||
|
|
|||
| /** A function to create a new credential. */ | |||
| export function useNewCredential(backend: Backend) { | |||
There was a problem hiding this comment.
I don't think we need a hook here, prefer newCredentialMutationOptions and use an appropriate hook in place useMutation/useMutationCallback
| @@ -417,6 +419,59 @@ function AssetPropertiesInternal(props: AssetPropertiesInternalProps) { | |||
| </div> | |||
| )} | |||
|
|
|||
| {isSecret && isCredential && ( | |||
| <div className={styles.section()} {...secretSpotlight.props}> | |||
| <Heading | |||
There was a problem hiding this comment.
<Text variant="h1">{getText('configuration')}</Text>
There was a problem hiding this comment.
what does this comment mean?
There was a problem hiding this comment.
Github doesn't sanitize text bu default... fixed!
There was a problem hiding this comment.
Why Text and not Heading though?
All other variants in this file use Heading, I essentially copy-pasted and adapted another variant. So I need some justification for changing this to be different than the convention in other parts of this file.
app/gui/src/dashboard/data/serviceCredentials/GoogleCredentialsForm.tsx
Outdated
Show resolved
Hide resolved
…` in favor of `backendMutationOptions`
- Closes #12011 - Note that since Google API is experimental, the new credentials are currently integrated with Sheets feature. Analytics feature is not yet updated to support them, let's do that as a separate followup PR. - Related PRs: - GUI: #12454 - cloud: enso-org/cloud-v2#1821 - This libs PR can probably be merged before the others, but first we want to test the integration of the full flow.
- Implement enso-org/cloud-v2#1694 - Add dialog to add an External Service Credential # Important Notes None
Pull Request Description
Important Notes
None
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,or the Snowflake database integration, a run of the Extra Tests has been scheduled.
If applicable, it is suggested to paste a link to a successful run of the Extra Tests.