-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add dynamic secrets modal form on secrets overview page #3241
Conversation
WalkthroughThis pull request integrates new dynamic secret management and subscription handling features into the secret manager pages. The Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (18)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx (1)
658-658
: Consider using actual environment data instead of placeholder valuesThe current implementation uses placeholder values for the environment
name
andid
fields ("not-used"). This approach is error-prone and makes the code less maintainable. Consider using the actual environment data fromcurrentWorkspace.environments
that you're already accessing on line 304.-environments={[{ slug: environment, name: "not-used", id: "not-used" }]} +environments={[currentWorkspace.environments.find((env) => env.slug === environment)!]}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (1)
120-149
: Improve error handling in the Promise.all implementationThe current implementation maps over environments and executes requests in parallel, but error handling could be improved. If one environment request fails, the error notification is displayed, but the function continues trying other environments.
Consider using Promise.allSettled for better granular error handling:
-const promises = selectedEnvs.map(async (environment) => { - try { - await createDynamicSecret.mutateAsync({ - provider: { type: DynamicSecretProviders.ElasticSearch, inputs: provider }, - maxTTL, - name, - path: secretPath, - defaultTTL, - projectSlug, - environmentSlug: environment.slug - }); - onCompleted(); - } catch { - createNotification({ - type: "error", - text: "Failed to create dynamic secret" - }); - } -}); -await Promise.all(promises); +const results = await Promise.allSettled(selectedEnvs.map(environment => + createDynamicSecret.mutateAsync({ + provider: { type: DynamicSecretProviders.ElasticSearch, inputs: provider }, + maxTTL, + name, + path: secretPath, + defaultTTL, + projectSlug, + environmentSlug: environment.slug + }) +)); + +const hasFailures = results.some(result => result.status === "rejected"); +const hasSuccesses = results.some(result => result.status === "fulfilled"); + +if (hasFailures) { + createNotification({ + type: hasSuccesses ? "warning" : "error", + text: hasSuccesses + ? "Some dynamic secrets were created successfully, but others failed" + : "Failed to create dynamic secrets" + }); +} + +if (hasSuccesses) { + onCompleted(); +}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx (2)
94-94
: Consider setting an initial default valueWhen multiple environments are available but no environments are initially selected, users might miss that they need to select environments before submitting the form. Consider setting a default value even when there are multiple environments available.
- environments: environments.length === 1 ? environments : [] + environments: environments.length === 1 ? environments : [environments[0]]
109-129
: Improve error handling for multi-environment operationsThe current implementation calls
onCompleted()
after each successful environment creation, which could be misleading if some environments succeed while others fail. Consider moving the success notification outside the loop to indicate all operations completed successfully.const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; const promises = selectedEnvs.map(async (env) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.AwsElastiCache, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: env.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", text: "Failed to create dynamic secret" }); } }); await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (3)
89-89
: Consider setting an initial default valueWhen multiple environments are available but no environments are initially selected, users might miss that they need to select environments before submitting the form. Consider setting a default value even when there are multiple environments available.
- environments: environments.length === 1 ? environments : [] + environments: environments.length === 1 ? environments : [environments[0]]
104-124
: Improve error handling for multi-environment operationsThe current implementation calls
onCompleted()
after each successful environment creation, which could be misleading if some environments succeed while others fail. Consider moving the success notification outside the loop to indicate all operations completed successfully.const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.SapHana, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", text: "Failed to create dynamic secret" }); } }); await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };
327-350
: Ensure consistency in menu placement across formsThe
menuPlacement="top"
prop is used here but isn't consistent across all form components. If this is intentional based on UI layout, that's fine, but it's worth ensuring consistency for a better user experience.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx (3)
179-179
: Consider setting an initial default valueWhen multiple environments are available but no environments are initially selected, users might miss that they need to select environments before submitting the form. Consider setting a default value even when there are multiple environments available.
- environments: environments.length === 1 ? environments : [] + environments: environments.length === 1 ? environments : [environments[0]]
197-217
: Improve error handling for multi-environment operationsThe current implementation calls
onCompleted()
after each successful environment creation, which could be misleading if some environments succeed while others fail. Consider moving the success notification outside the loop to indicate all operations completed successfully.const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; const promises = selectedEnvs.map(async (env) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.SqlDatabase, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: env.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", text: "Failed to create dynamic secret" }); } }); await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };
665-688
: Consider validation for empty environment selectionWhen the user has multiple environments but doesn't select any, the form will submit with an empty array, which might not be the intended behavior. Consider adding validation to ensure at least one environment is selected.
- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "Please select at least one environment")frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (3)
121-121
: Consider setting an initial default valueWhen multiple environments are available but no environments are initially selected, users might miss that they need to select environments before submitting the form. Consider setting a default value even when there are multiple environments available.
- environments: environments.length === 1 ? environments : [] + environments: environments.length === 1 ? environments : [environments[0]]
138-158
: Improve error handling for multi-environment operationsThe current implementation calls
onCompleted()
after each successful environment creation, which could be misleading if some environments succeed while others fail. Consider moving the success notification outside the loop to indicate all operations completed successfully.const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.Ldap, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", text: "Failed to create dynamic secret" }); } }); await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };
392-415
: Consider extracting the environment selection componentThis environment selection component is repeated across multiple forms with identical implementation. Consider extracting it to a reusable component to improve maintainability.
You could create a new component like
EnvironmentSelector.tsx
:import { Controller } from "react-hook-form"; import { FilterableSelect, FormControl } from "@app/components/v2"; import { WorkspaceEnv } from "@app/hooks/api/types"; type Props = { control: any; environments: WorkspaceEnv[]; name?: string; menuPlacement?: "top" | "bottom" | "auto"; }; export const EnvironmentSelector = ({ control, environments, name = "environments", menuPlacement = "auto" }: Props) => { if (environments.length <= 1) return null; return ( <Controller control={control} name={name} render={({ field: { value, onChange }, fieldState: { error } }) => ( <FormControl label="Environments" isError={Boolean(error)} errorText={error?.message} > <FilterableSelect isMulti options={environments} value={value} onChange={onChange} placeholder="Select environments to create secret in..." getOptionLabel={(option) => option.name} getOptionValue={(option) => option.slug} menuPlacement={menuPlacement} /> </FormControl> )} /> ); };Then use it in each form:
<EnvironmentSelector control={control} environments={environments} menuPlacement="top" />frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (1)
330-353
: Consider standardizing menu placement.The menuPlacement prop is set to "top" for the FilterableSelect component. Make sure this is consistent with the UX design across all similar dropdowns in the application.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
315-338
: Consider adding a helper text for environment selection.While the placeholder text provides guidance, consider adding a helper text below the environment select field to further explain the implications of selecting multiple environments (e.g., "Dynamic secrets will be created in all selected environments").
<FormControl label="Environments" isError={Boolean(error)} errorText={error?.message} + helperText="Dynamic secrets will be created in all selected environments" >
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (1)
1-383
: Consider adding form validation for empty environment selection.When there are multiple environments, the form will initialize with an empty environments array. It would be good to add validation to ensure that at least one environment is selected before submission.
You could update the schema validation to include a minimum length check:
environments: z.object({ name: z.string(), slug: z.string() }).array() + .min(1, "Please select at least one environment")
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (2)
100-129
: Well-implemented multi-environment creation processThe function has been refactored to handle multiple environments by mapping over the selected environments and creating promises for each. Using Promise.all to wait for all environments to be processed is a good approach.
However, consider enhancing the error handling to provide more specific feedback about which environments failed during creation.
const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.Cassandra, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); return { success: true, environment }; } catch { - createNotification({ - type: "error", - text: "Failed to create dynamic secret" - }); return { success: false, environment }; } }); const results = await Promise.all(promises); const successes = results.filter(r => r.success); const failures = results.filter(r => !r.success); if (failures.length > 0) { createNotification({ type: "error", text: `Failed to create dynamic secret in ${failures.length} environment${failures.length > 1 ? 's' : ''}: ${failures.map(f => f.environment.name).join(', ')}` }); } if (successes.length > 0) { onCompleted(); }
361-384
: Good conditional rendering of environment selectorThe FilterableSelect component is properly implemented to allow users to select multiple environments when more than one is available. The component is correctly configured with appropriate props for multi-selection.
Consider adding validation to ensure at least one environment is selected when there are multiple available environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx
(1 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CreateDynamicSecretForm.tsx
(18 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
[error] 869-869: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check Frontend Type and Lint check
🔇 Additional comments (35)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CreateDynamicSecretForm.tsx (3)
38-44
: The Props type has been updated to support multiple environmentsThe change from a single environment string to an array of WorkspaceEnv objects enables the multi-environment selection feature. This is well aligned with the PR objective of allowing users to select specific environments when generating dynamic secrets.
129-135
: Props interface has been properly updatedThe component signature has been updated to accept the new environments prop instead of the previous environment prop. This change is consistent with the updated Props type.
196-202
: Dynamic secret provider forms properly receive environments arrayThe SqlDatabaseInputForm now receives the environments array, which is consistent with the component's updated interface.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (3)
78-79
: Form schema appropriately updated to include environmentsThe form schema has been correctly updated to include the environments field, ensuring proper validation of the multi-environment selection.
114-114
: Sensible default environments assignment based on array lengthWhen only one environment is available, it's automatically selected as the default, which improves the user experience. This is a good design decision.
422-445
: Multi-environment selection UI is well implementedThe FilterableSelect component for environment selection is appropriately conditionally rendered only when multiple environments are available. The implementation includes proper validation, labels, and error handling.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
385-408
: Environment selection implementation is consistent across formsThe FilterableSelect for environment selection is consistent with the implementation in ElasticSearchInputForm and other forms. The conditional rendering based on environment count improves the UX by only showing the selection when needed.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx (1)
315-337
: LGTM: Multi-environment selection implementationThe implementation correctly handles the conditional rendering of the environment selector component when multiple environments are available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (4)
44-45
: Good validation for secret names.The addition of lowercase validation for the name field is a good practice to ensure consistent naming conventions across secrets.
71-72
: Smart default environment handling.Setting default environments based on array length is a good UX decision. It automatically selects the environment when only one is available, reducing unnecessary user interaction.
86-105
: Well-implemented concurrent environment processing.Using Promise.all for handling multiple environment creation requests is efficient and ensures all operations complete before proceeding. The proper error handling for each environment is also well-implemented.
303-326
: Good conditional rendering of environment selector.Only showing the environment selector when multiple environments are available improves the UX by not displaying unnecessary UI elements.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (1)
119-124
: Enhanced error handling with specific error messages.Good job on improving the error notification to display the actual error message when available. This provides more context to users when something goes wrong.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx (2)
140-159
: Robust implementation for multi-environment support.The promise-based implementation for handling multiple environments is well-structured and includes proper error handling. This ensures a reliable user experience even when creating secrets across multiple environments simultaneously.
454-477
: Consistent UI pattern for environment selection.The implementation follows the same pattern used in other forms, maintaining consistency across the application. This helps users build a mental model of how the application works.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
110-129
: Comprehensive error handling with detailed error messages.The error handler properly checks if the error is an instance of Error to provide more specific error messages when available. This is good practice for improving user experience during failure scenarios.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (5)
10-18
: Import addition for multi-environment support.The FilterableSelect component has been correctly added to support the new multi-environment selection feature.
62-63
: Schema updated for environment selection.The form schema has been properly extended to include the environments field with appropriate validation.
72-72
: Props type updated for multiple environments.The Props type is correctly updated to accept an array of WorkspaceEnv objects instead of a single environment string.
103-103
: Default environment value handling.Good approach to initialize the environments array conditionally based on the number of available environments.
428-451
: Multi-environment selection UI added.Good implementation of the conditional rendering for the environment selector. The component only shows when there are multiple environments available, which is a sensible approach.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (2)
55-56
: Schema updated with environments field.The form schema has been properly extended to include the environments field with appropriate validation.
329-352
: Multi-environment selection UI added.The FilterableSelect component is correctly implemented with proper properties and only displays when there are multiple environments available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx (1)
307-330
: Multi-environment selection UI added.The FilterableSelect component implementation looks good. It's conditionally rendered and provides a clear interface for users to select multiple environments.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (1)
344-367
: Multi-environment selection UI added.Good implementation of the conditional rendering for the FilterableSelect component. It only appears when there are multiple environments to choose from.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (4)
22-22
: Good addition of WorkspaceEnv type importNice work importing the WorkspaceEnv type to properly type the environments array in the component props.
57-58
: Well-structured schema change for multi-environment supportThe form schema has been properly updated to support multiple environments, with each environment correctly typed as an object containing name and slug properties.
67-67
: Props updated correctly for multi-environment supportThe component props now properly accept an array of WorkspaceEnv objects instead of a single environment string, which aligns with the new multi-environment functionality.
94-94
: Smart default values for environmentsGood approach to automatically use the environment when there's only one available, or default to an empty array when multiple environments are present, requiring explicit user selection.
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx (6)
22-22
: Good addition of UpgradePlanModalThe import of the UpgradePlanModal component is appropriate for prompting users to upgrade when attempting to access subscription-limited features.
77-77
: Good component import for CreateDynamicSecretFormImporting the CreateDynamicSecretForm component from the correct location follows good code organization principles.
138-138
: Good use of subscription hookUsing the useSubscription hook to check subscription status is the correct approach for conditionally enabling feature access.
186-196
: Well-implemented permission-based environment filteringThe filtering of environments based on permissions ensures users can only create dynamic secrets in environments they have access to, which is a good security practice.
1207-1213
: Good integration of CreateDynamicSecretForm with modal systemThe CreateDynamicSecretForm component is correctly integrated with the popup system and properly receives all necessary props including the filtered environments with appropriate permissions.
1214-1224
: Well-implemented conditional upgrade modalThe UpgradePlanModal is correctly implemented with conditional rendering based on subscription status and appropriate messaging based on the user's current subscription plan.
...r/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
Outdated
Show resolved
Hide resolved
...nager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
Outdated
Show resolved
Hide resolved
...-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
Outdated
Show resolved
Hide resolved
...manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
Outdated
Show resolved
Hide resolved
...anager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (6)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
116-150
:⚠️ Potential issueNested promise handling and missing success callback
This code repeats the pattern of a nestedmap
call, which was flagged previously for not returning the inner promises, and only callsonCompleted()
on error. Please ensure the inner async calls are awaited and thatonCompleted()
also runs on success.Example fix:
let hasErrors = false; - const promises = selectedEnvs.map(async (env) => { - try { - selectedUsers.map(async (user) => { - await createDynamicSecret.mutateAsync({ ... }); - }); - } catch { - ... - hasErrors = true; - } - }); - await Promise.all(promises); + if (createDynamicSecret.isPending) return; + + const allPromises: Promise<unknown>[] = []; + for (const env of selectedEnvs) { + for (const user of selectedUsers) { + allPromises.push( + createDynamicSecret.mutateAsync({ ... }) + ); + } + } + try { + await Promise.all(allPromises); + } catch { + hasErrors = true; + } + onCompleted();frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (1)
139-141
:⚠️ Potential issueFix the completion logic condition
The current implementation calls
onCompleted()
when there are errors, which is counterintuitive. The completion handler should be called when there are no errors.await Promise.all(promises); -if (hasErrors) { +if (!hasErrors) { onCompleted(); }frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (1)
124-126
:⚠️ Potential issueFix the completion logic condition
The current implementation calls
onCompleted()
when there are errors, which is counterintuitive. The completion handler should be called when there are no errors.await Promise.all(promises); -if (hasErrors) { +if (!hasErrors) { onCompleted(); }frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx (1)
95-128
:⚠️ Potential issueFix inverted logic in completion handler.
The current implementation only calls
onCompleted()
when there are errors, which is counterintuitive. This logic needs to be inverted to callonCompleted()
only when there are no errors.const promises = selectedEnvs.map(async (env) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.SapAse, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: env.slug }); } catch { createNotification({ type: "error", text: `Failed to create dynamic secret in environment ${env.name}` }); hasErrors = true; } }); await Promise.all(promises); - if (hasErrors) { + if (!hasErrors) { onCompleted(); }frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (1)
101-141
:⚠️ Potential issueFix inverted logic in completion handler.
The current implementation only calls
onCompleted()
when there are errors, which is counterintuitive. This logic needs to be inverted to callonCompleted()
only when there are no errors.const promises = selectedEnvs.map(async (env) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.MongoDB, inputs: { ...provider, port: provider?.port ? provider.port : undefined, roles: provider.roles.map((el) => el.roleName) } }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: env.slug }); } catch { createNotification({ type: "error", text: `Failed to create dynamic secret in environment ${env.name}` }); hasErrors = true; } }); await Promise.all(promises); - if (hasErrors) { + if (!hasErrors) { onCompleted(); }frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (1)
100-133
:⚠️ Potential issueFix inverted logic in completion handler.
The current implementation only calls
onCompleted()
when there are errors, which is counterintuitive. This logic needs to be inverted to callonCompleted()
only when there are no errors.const promises = selectedEnvs.map(async (env) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.Cassandra, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: env.slug }); } catch { createNotification({ type: "error", text: `Failed to create dynamic secret in environment ${env.name}` }); hasErrors = true; } }); await Promise.all(promises); - if (hasErrors) { + if (!hasErrors) { onCompleted(); }
🧹 Nitpick comments (6)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (1)
78-79
: Consider requiring at least one environment
If every secret must be tied to at least one environment, add.min(1)
to ensure the array is never empty.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
64-65
: Validate secret name and environment array
Including a lowercase check forname
is good. However, consider using.min(1)
inenvironments
if at least one environment must be selected.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (1)
55-56
: Enforce lowercase name and define environments
Consider adding.min(1)
toenvironments
if you require at least one environment.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (1)
8-8
: Consider adding validation to ensure at least one environment is selected.While the implementation for multi-environment support is good, there's no explicit validation ensuring that at least one environment is selected when multiple environments are available. Consider adding this validation to prevent form submission with an empty environment selection.
You could modify the schema validation to require at least one environment:
- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "At least one environment must be selected")Also applies to: 11-11, 45-45, 54-54, 71-72, 77-110, 307-330
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx (1)
16-16
: Consider extracting the duplicated environment selection component.The environment selection implementation is duplicated across all the form components. Consider extracting this into a reusable component to improve maintainability.
You could create a component like
EnvironmentSelector
that takes the necessary props:type EnvironmentSelectorProps = { control: Control<any>; environments: WorkspaceEnv[]; }; const EnvironmentSelector = ({ control, environments }: EnvironmentSelectorProps) => { if (environments.length <= 1) return null; return ( <Controller control={control} name="environments" render={({ field: { value, onChange }, fieldState: { error } }) => ( <FormControl label="Environments" isError={Boolean(error)} errorText={error?.message} > <FilterableSelect isMulti options={environments} value={value} onChange={onChange} placeholder="Select environments to create secret in..." getOptionLabel={(option) => option.name} getOptionValue={(option) => option.slug} menuPlacement="top" /> </FormControl> )} /> ); };This would reduce code duplication and make future changes to the environment selection UI easier to implement consistently.
Also applies to: 26-26, 69-69, 78-78, 115-115, 131-164, 458-481
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (1)
100-133
: Consider extracting common environment handling to a reusable utility.This pattern of handling multiple environments is identical across several form components. Consider extracting this logic into a reusable hook or utility function to reduce code duplication and ensure consistent behavior.
Example of how this could be implemented:
// useMultiEnvironmentMutation.ts import { createNotification } from "@app/components/notifications"; import { WorkspaceEnv } from "@app/hooks/api/types"; type MutationFn<T> = (params: T) => Promise<any>; export function useMultiEnvironmentMutation<T extends { environmentSlug: string }>( mutationFn: MutationFn<T>, onCompleted: () => void ) { const executeForEnvironments = async ( params: Omit<T, "environmentSlug">, environments: WorkspaceEnv[] ) => { let hasErrors = false; const promises = environments.map(async (env) => { try { await mutationFn({ ...params, environmentSlug: env.slug } as T); } catch { createNotification({ type: "error", text: `Failed to execute operation in environment ${env.name}` }); hasErrors = true; } }); await Promise.all(promises); if (!hasErrors) { onCompleted(); } }; return executeForEnvironments; }Usage in forms:
const mutateMultipleEnvironments = useMultiEnvironmentMutation( createDynamicSecret.mutateAsync, onCompleted ); const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; await mutateMultipleEnvironments( { provider: { type: DynamicSecretProviders.Cassandra, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug }, selectedEnvs ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx
(1 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx
- frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
- frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check Frontend Type and Lint check
🔇 Additional comments (41)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (6)
12-12
: ImportingFilterableSelect
is correct
This import aligns with the multi-environment selection feature.
23-23
: Type import forWorkspaceEnv
Importing the workspace environment type ensures strong typing for multi-environment functionality.
88-88
: Props interface updated
Switchingenvironment
to an arrayWorkspaceEnv[]
is consistent with the new multi-environment approach.
94-94
: Correct destructuring of environments
Destructuring the newenvironments
prop grants immediate access to the environment array for the form.
114-115
: Providing default environments
Automatically selecting the sole environment when only one is present is a convenient user experience.
426-449
: Conditional multi-environment selection
ShowingFilterableSelect
only if there is more than one environment helps keep the UI uncluttered.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (6)
15-15
: ImportingFilterableSelect
This change supports multi-environment selection for Azure Entra ID secrets.
26-26
: WorkspaceEnv import
Adding theWorkspaceEnv
type is necessary for handling multiple environments.
74-74
: Props updated for multi-environment
Replacingenvironment: string
withenvironments: WorkspaceEnv[]
is correctly aligned with the new flow.
80-80
: Destructuring newenvironments
Ensures direct access to the array of environments in the component.
90-93
: Default multiple environments
Automatically setting environments to a single default if only one is available helps expedite the form-filling process.
389-412
: Multi-environment dropdown
Displaying multiple environments viaFilterableSelect
caters to complex setups and improves usability.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (6)
14-14
: FilterableSelect import
Import aligns with the component's multi-environment feature.
22-22
: ImportingWorkspaceEnv
Essential for representing multiple environments in the SAP HANA form.
65-65
: Type definition forenvironments
Shifting to an array-based approach facilitates multi-environment support.
71-71
: Destructuring theenvironments
prop
Helps pass multiple environment data directly to the form.
88-89
: Setting default environment array
Auto-fills a single environment if only one exists, otherwise defaults to an empty array for user selection.
331-354
: Conditional environment select
The conditional rendering ofFilterableSelect
for multiple environments provides a clean user experience while supporting flexible configurations.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx (1)
15-15
: Multi-environment support implementation looks good.The changes to enable the selection of multiple environments for creating dynamic secrets are well implemented. The code correctly handles:
- Form schema validation for environment selection
- Default selection of a single environment when only one is available
- Parallel processing of secret creation across multiple environments
- Error handling with proper user feedback for each environment
Also applies to: 26-26, 84-85, 94-94, 179-179, 188-219, 669-692
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (1)
11-18
: Implementation of multi-environment support looks good.The changes to support creating dynamic secrets across multiple environments are well-implemented. The code correctly manages:
- Environment selection via the FilterableSelect component
- Default selection when only one environment is available
- Concurrent secret creation with Promise.all
- Proper error handling with environment-specific error messages
Also applies to: 21-21, 81-82, 92-92, 121-121, 129-162, 396-481
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (3)
106-107
: Updated method signature to handle multiple environmentsThe function now correctly accepts an array of environments from the form, allowing secrets to be created across multiple environments at once.
111-130
: Logic for creating secrets across multiple environments looks goodThe implementation properly maps over selected environments and creates a dynamic secret for each one, with good error handling for individual environments.
319-342
: Environment selector UI implementation looks goodThe conditional rendering of the environment selector only when multiple environments are available is a good UX decision.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (2)
118-136
: Good error handling with environment-specific error messagesThe error notification now includes the environment name in the error message, which provides better context for troubleshooting.
432-455
: Consistent implementation of environment selectorThe environment selector implementation is consistent with other forms, providing a good user experience when working with multiple environments.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (2)
108-126
: Good implementation of multi-environment supportThe code properly handles creating dynamic secrets across multiple environments with appropriate error handling for each environment.
334-357
: Well-implemented environment selector UIThe FilterableSelect implementation for environment selection provides a good user experience and is consistent with other forms.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (2)
118-119
: Improved error message with environment contextThe error notification now includes the environment name, providing better context for troubleshooting.
333-356
: Consistent environment selector implementationThe environment selector implementation is consistent with other forms in the application, providing a unified experience.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx (4)
53-54
: LGTM: Form schema properly updated for multi-environment support.The schema is correctly defined with the new
environments
field as an array of objects containing name and slug properties.
63-63
: Props type properly updated to support multiple environments.The update from a single environment string to an array of WorkspaceEnv objects is consistent with the multi-environment functionality being implemented.
89-90
: Good default environment handling for single vs multiple environments.The default value is sensibly set - when only one environment is available, it's automatically selected, otherwise it starts with an empty array requiring user selection.
311-334
: Good implementation of environment selection with FilterableSelect.The FilterableSelect component is appropriately conditional - only rendered when multiple environments are available. The implementation includes proper handling of value, onChange, error states, and provides clear user instructions.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (4)
58-59
: LGTM: Form schema properly updated for multi-environment support.The schema is correctly defined with the new
environments
field as an array of objects containing name and slug properties.
68-68
: Props type properly updated to support multiple environments.The update from a single environment string to an array of WorkspaceEnv objects is consistent with the multi-environment functionality being implemented.
90-91
: Good default environment handling for single vs multiple environments.The default value is sensibly set - when only one environment is available, it's automatically selected, otherwise it starts with an empty array requiring user selection.
348-371
: Good implementation of environment selection with FilterableSelect.The FilterableSelect component is appropriately conditional - only rendered when multiple environments are available. The implementation includes proper handling of value, onChange, error states, and provides clear user instructions.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (4)
57-58
: LGTM: Form schema properly updated for multi-environment support.The schema is correctly defined with the new
environments
field as an array of objects containing name and slug properties.
67-67
: Props type properly updated to support multiple environments.The update from a single environment string to an array of WorkspaceEnv objects is consistent with the multi-environment functionality being implemented.
94-95
: Good default environment handling for single vs multiple environments.The default value is sensibly set - when only one environment is available, it's automatically selected, otherwise it starts with an empty array requiring user selection.
365-388
: Good implementation of environment selection with FilterableSelect.The FilterableSelect component is appropriately conditional - only rendered when multiple environments are available. The implementation includes proper handling of value, onChange, error states, and provides clear user instructions.
.../SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
Outdated
Show resolved
Hide resolved
...anager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
Outdated
Show resolved
Hide resolved
...t-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
Outdated
Show resolved
Hide resolved
...ager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
121-151
:⚠️ Potential issueFix the promise chain for multiple users across environments
The current implementation has a critical issue where promises created in the inner
selectedUsers.map()
are not being properly awaited or returned. This means the system will not wait for all user secrets to be created before considering the operation complete, potentially leading to incomplete operations or unhandled errors.Apply this fix to properly handle both environments and users:
-const promises = selectedEnvs.map(async (env) => { - try { - selectedUsers.map(async (user: { id: string; name: string; email: string }) => { - await createDynamicSecret.mutateAsync({ - provider: { - type: DynamicSecretProviders.AzureEntraId, - inputs: { - userId: user.id, - tenantId: provider.tenantId, - email: user.email, - applicationId: provider.applicationId, - clientSecret: provider.clientSecret - } - }, - maxTTL, - name: `${name}-${user.name}`, - path: secretPath, - defaultTTL, - projectSlug, - environmentSlug: env.slug - }); - }); - } catch { - createNotification({ - type: "error", - text: `Failed to create dynamic secret in environment ${env.name}` - }); - hasErrors = true; - } -}); -await Promise.all(promises); +const allPromises = []; + +for (const env of selectedEnvs) { + for (const user of selectedUsers) { + allPromises.push( + createDynamicSecret.mutateAsync({ + provider: { + type: DynamicSecretProviders.AzureEntraId, + inputs: { + userId: user.id, + tenantId: provider.tenantId, + email: user.email, + applicationId: provider.applicationId, + clientSecret: provider.clientSecret + } + }, + maxTTL, + name: `${name}-${user.name}`, + path: secretPath, + defaultTTL, + projectSlug, + environmentSlug: env.slug + }).catch(error => { + createNotification({ + type: "error", + text: `Failed to create dynamic secret for user ${user.name} in environment ${env.name}` + }); + hasErrors = true; + return error; // Prevent promise rejection from stopping other creations + }) + ); + } +} + +await Promise.all(allPromises);frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (1)
125-152
: 🛠️ Refactor suggestionEnsure completion callback is consistently invoked
Currently,
onCompleted()
is only called if no errors occurred. This could lead to UI inconsistencies when some, but not all, secret creations fail. Even with partial failures, the UI should proceed to the next step or update to reflect completion.await Promise.all(promises); if (!hasErrors) { onCompleted(); +} else { + // Still call onCompleted even if there were some errors + // since we already notified the user of specific failures + onCompleted(); }Alternatively, simplify to:
await Promise.all(promises); -if (!hasErrors) { onCompleted(); -}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx (1)
130-133
: 🛠️ Refactor suggestionEnsure completion callback is consistently invoked
Currently,
onCompleted()
is only called if no errors occurred. This could lead to UI inconsistencies when some, but not all, secret creations fail. Even with partial failures, the UI should proceed to the next step or update to reflect completion.await Promise.all(promises); if (!hasErrors) { onCompleted(); +} else { + // Still call onCompleted even if there were some errors + // since we already notified the user of specific failures + onCompleted(); }Alternatively, simplify to:
await Promise.all(promises); -if (!hasErrors) { onCompleted(); -}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx (1)
218-220
: 🛠️ Refactor suggestionEnsure completion callback is consistently invoked
Currently,
onCompleted()
is only called if no errors occurred. This could lead to UI inconsistencies when some, but not all, secret creations fail. Even with partial failures, the UI should proceed to the next step or update to reflect completion.await Promise.all(promises); if (!hasErrors) { onCompleted(); +} else { + // Still call onCompleted even if there were some errors + // since we already notified the user of specific failures + onCompleted(); }Alternatively, simplify to:
await Promise.all(promises); -if (!hasErrors) { onCompleted(); -}
🧹 Nitpick comments (5)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (3)
88-90
: Pre-select environments for better UXWhen multiple environments are available, the form initializes with an empty selection (
environments: environments.length === 1 ? environments : []
), requiring users to manually select environments before submission. This creates extra work for users.Consider pre-selecting all available environments as the default:
- environments: environments.length === 1 ? environments : [] + environments: environments
56-56
: Add validation to ensure at least one environment is selectedThe form schema defines
environments
as an array but doesn't enforce a minimum selection. Users could potentially submit the form without selecting any environments.Add array length validation to ensure at least one environment is selected:
- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "Select at least one environment")
331-354
: Consider moving environment selection higher in the formThe environment selection is currently placed at the bottom of the form. Since it's a critical part of the form that determines where secrets will be created, consider moving it higher for better visibility.
Move the environment selection component to appear right after the secret name field (around line 150).
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (1)
432-455
: Consider extracting common FilterableSelect patternSince this same FilterableSelect implementation appears in multiple forms, consider extracting it to a reusable component to reduce code duplication and ensure consistent behavior across all forms.
+ // In a new file, e.g., EnvironmentSelector.tsx + import { Controller } from "react-hook-form"; + import { FilterableSelect, FormControl } from "@app/components/v2"; + import { WorkspaceEnv } from "@app/hooks/api/types"; + + type EnvironmentSelectorProps = { + control: Control<any>; + name: string; + environments: WorkspaceEnv[]; + error?: FieldError; + }; + + export const EnvironmentSelector = ({ + control, + name, + environments, + error + }: EnvironmentSelectorProps) => { + if (environments.length <= 1) return null; + + return ( + <Controller + control={control} + name={name} + render={({ field: { value, onChange }, fieldState: { error: fieldError } }) => ( + <FormControl + label="Environments" + isError={Boolean(error || fieldError)} + errorText={error?.message || fieldError?.message} + > + <FilterableSelect + isMulti + options={environments} + value={value} + onChange={onChange} + placeholder="Select environments to create secret in..." + getOptionLabel={(option) => option.name} + getOptionValue={(option) => option.slug} + menuPlacement="top" + /> + </FormControl> + )} + /> + ); + }; - {environments.length > 1 && ( - <Controller - control={control} - name="environments" - render={({ field: { value, onChange }, fieldState: { error } }) => ( - <FormControl - label="Environments" - isError={Boolean(error)} - errorText={error?.message} - > - <FilterableSelect - isMulti - options={environments} - value={value} - onChange={onChange} - placeholder="Select environments to create secret in..." - getOptionLabel={(option) => option.name} - getOptionValue={(option) => option.slug} - menuPlacement="top" - /> - </FormControl> - )} - /> - )} + <EnvironmentSelector + control={control} + name="environments" + environments={environments} + />frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (1)
57-59
: Consider extracting environment schema definition into a shared constant.Since the environment schema is identical across multiple form components, consider extracting it into a shared constant to reduce duplication and ensure consistency.
+// In a shared file or at the top of the file +const environmentSchema = z.object({ name: z.string(), slug: z.string() }).array(); const formSchema = z.object({ // ...other fields name: z.string().refine((val) => val.toLowerCase() === val, "Must be lowercase"), - environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: environmentSchema });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Check Frontend Type and Lint check
- GitHub Check: GitGuardian Security Checks
🔇 Additional comments (30)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (1)
104-127
: Consistent error handling implementationThe implementation correctly handles errors for each environment and only completes the form when no errors occur. This resolves the previous review comment about calling
onCompleted()
on success.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (4)
81-82
: Add validation to ensure at least one environment is selectedThe form schema defines
environments
as an array but doesn't enforce a minimum selection. Users could potentially submit the form without selecting any environments.- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "Select at least one environment")
121-121
: Pre-select environments for better UXWhen multiple environments are available, the form initializes with an empty selection (
environments: environments.length === 1 ? environments : []
), requiring users to manually select environments before submission. This creates extra work for users.- environments: environments.length === 1 ? environments : [] + environments: environments
138-161
: Consistent error handling implementationThe implementation correctly handles errors for each environment and only completes the form when no errors occur. The error handling approach is consistent with other form components.
396-419
: Consider moving environment selection higher in the formThe environment selection is currently placed at the bottom of the form. Since it's a critical part of the form that determines where secrets will be created, consider moving it higher for better visibility.
Move the environment selection component to appear right after the basic form fields (around line 185).
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx (4)
68-69
: Add validation to ensure at least one environment is selectedThe form schema defines
environments
as an array but doesn't enforce a minimum selection. Users could potentially submit the form without selecting any environments.- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "Select at least one environment")
115-115
: Pre-select environments for better UXWhen multiple environments are available, the form initializes with an empty selection (
environments: environments.length === 1 ? environments : []
), requiring users to manually select environments before submission. This creates extra work for users.- environments: environments.length === 1 ? environments : [] + environments: environments
140-163
: Consistent error handling implementationThe implementation correctly handles errors for each environment and only completes the form when no errors occur. The error handling approach is consistent with other form components.
458-481
: Consider moving environment selection higher in the formThe environment selection is currently placed at the bottom of the form after the Advanced accordion section. Since it's a critical part of the form that determines where secrets will be created, consider moving it higher for better visibility.
Move the environment selection component to appear near the beginning of the form configuration section (around line 225).
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (4)
63-64
: Add validation to ensure at least one environment is selectedThe form schema defines
environments
as an array but doesn't enforce a minimum selection. Users could potentially submit the form without selecting any environments.- environments: z.object({ name: z.string(), slug: z.string() }).array() + environments: z.object({ name: z.string(), slug: z.string() }).array().min(1, "Select at least one environment")
95-95
: Pre-select environments for better UXWhen multiple environments are available, the form initializes with an empty selection (
environments: environments.length === 1 ? environments : []
), requiring users to manually select environments before submission. This creates extra work for users.- environments: environments.length === 1 ? environments : [] + environments: environments
110-134
: Error handling correctly implementedThe implementation correctly handles errors for each environment and only completes the form when no errors occur. The code now properly calls
onCompleted()
only on success, addressing the previous review comment about the completion logic condition.
319-342
: Consider moving environment selection higher in the formThe environment selection is currently placed at the bottom of the form. Since it's a critical part of the form that determines where secrets will be created, consider moving it higher for better visibility.
Move the environment selection component to appear right after the secret name field (around line 157).
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (6)
8-11
: Good implementation of multi-environment supportThe import of
FilterableSelect
andWorkspaceEnv
type is appropriate for the multi-environment feature being implemented.
44-45
: LGTM on schema validationThe lowercase validation for secret names and the new environments array field are well defined. This ensures consistent naming conventions and proper typing for the multi-environment feature.
54-54
: Props interface updated correctly for multi-environment supportThe change from
environment: string
toenvironments: WorkspaceEnv[]
properly reflects the new capability to handle multiple environments.
70-72
: Smart default initialization for environmentsGood implementation of conditional default values based on the environments array length. This ensures a smooth user experience when only one environment is available.
77-110
: Robust implementation of multi-environment secret creationThe updated handler function properly processes multiple environments with good error handling. The approach of using
Promise.all
allows for concurrent processing while maintaining individual error handling for each environment.I especially like the specific error messages that include the environment name, which will help users troubleshoot issues more effectively.
307-330
: Well-implemented environment selection UIThe conditional rendering of the FilterableSelect component only when multiple environments are available is a good UX decision. The component is configured correctly with appropriate props for label display and value selection.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (4)
59-60
: Consistent schema implementationThe lowercase validation and environments array field follow the same pattern as in other form components, maintaining consistency across the codebase.
92-92
: Default values correctly initializedGood implementation of conditional default values for environments based on length, ensuring a seamless experience when only one environment is available.
98-131
: Error handling correctly implementedThe error handling for multiple environments is well implemented. The fix from the previous review (changing
if (hasErrors)
toif (!hasErrors)
) has been correctly applied, ensuring thatonCompleted()
is only called when all operations succeed.
334-357
: Consistent UI implementation across componentsThe environment selection UI is consistent with other form components, following the same pattern of conditional rendering and configuration. This maintains a uniform experience throughout the application.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (2)
103-127
: Fixed completion handling as suggestedThe completion handling has been refactored as suggested in the previous review. The code now correctly tracks errors and only calls
onCompleted()
once after all operations are complete, rather than for each environment.
333-356
: Consistent FilterableSelect implementationThe environment selection component implementation is consistent with other form components, providing a uniform user experience across the application.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (1)
109-142
: Fixed completion handling as suggested in previous reviewThe completion handling has been refactored as suggested in the previous review. The code now correctly tracks errors and only calls
onCompleted()
once after all operations complete successfully.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (2)
101-141
: Good implementation of multi-environment support with proper error handling.The implementation correctly addresses the previous review comment by tracking errors and only calling
onCompleted()
once after all operations complete. The error messages now include the environment name, providing better context for troubleshooting.
348-371
: Well-implemented environment selection component.The environment selector is properly implemented and only rendered when multiple environments are available. The component includes appropriate labels, error handling, and placeholder text.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (2)
100-133
: Good implementation of multi-environment support with proper error handling.The implementation correctly processes multiple environments with appropriate error handling. The code tracks errors and only calls
onCompleted()
once after all operations complete, with specific error messages for each environment.
365-388
: Well-implemented environment selection component.The environment selector is properly implemented and only rendered when multiple environments are available. The component includes appropriate labels, error handling, and placeholder text.
...r/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
121-140
:⚠️ Potential issueFix the promise handling for multiple Azure Entra ID users
The current implementation uses
map
to iterate through selected users, but doesn't properly collect or await these promises. This will cause only the last user to be properly processed while others may be ignored if they take longer.Apply this diff to fix the issue:
- selectedUsers.map(async (user: { id: string; name: string; email: string }) => { - await createDynamicSecret.mutateAsync({ - provider: { - type: DynamicSecretProviders.AzureEntraId, - inputs: { - userId: user.id, - tenantId: provider.tenantId, - email: user.email, - applicationId: provider.applicationId, - clientSecret: provider.clientSecret - } - }, - maxTTL, - name: `${name}-${user.name}`, - path: secretPath, - defaultTTL, - projectSlug, - environmentSlug: environment.slug - }); - }); - onCompleted(); + const userPromises = selectedUsers.map(async (user: { id: string; name: string; email: string }) => { + return createDynamicSecret.mutateAsync({ + provider: { + type: DynamicSecretProviders.AzureEntraId, + inputs: { + userId: user.id, + tenantId: provider.tenantId, + email: user.email, + applicationId: provider.applicationId, + clientSecret: provider.clientSecret + } + }, + maxTTL, + name: `${name}-${user.name}`, + path: secretPath, + defaultTTL, + projectSlug, + environmentSlug: environment.slug + }); + }); + await Promise.all(userPromises); + onCompleted();
♻️ Duplicate comments (5)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (1)
101-107
:⚠️ Potential issueImplementation doesn't process multiple environments
The current implementation only processes a single environment object rather than iterating through multiple selected environments, which doesn't match the intended functionality and previous review feedback.
const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, - environment + environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; + const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.MongoDB, inputs: { ...provider, port: provider?.port ? provider.port : undefined, roles: provider.roles.map((el) => el.roleName) } }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", - text: "Failed to create dynamic secret" + text: `Failed to create dynamic secret in environment ${environment.name}` }); } + }); + await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (4)
63-64
:⚠️ Potential issueSchema needs multi-environment support
The form schema defines a single environment object, but based on past review comments and intended functionality, it should support an array of selected environments.
- environment: z.object({ name: z.string(), slug: z.string() }) + environments: z + .object({ name: z.string(), slug: z.string() }) + .array() + .min(1, "At least one environment must be selected")
103-104
:⚠️ Potential issueUpdate default value for multi-environment support
The current default value approach only sets a single environment, but should be updated to support an array of environments.
- environment: environments.length === 1 ? environments[0] : undefined + environments: environments.length === 1 ? [environments[0]] : []
109-115
:⚠️ Potential issueImplementation doesn't process multiple environments
The current implementation only processes a single environment object rather than iterating through multiple selected environments, which doesn't match the intended functionality and previous review feedback.
const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, - environment + environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; + const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.RabbitMq, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", - text: "Failed to create dynamic secret" + text: `Failed to create dynamic secret in environment ${environment.name}` }); } + }); + await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };
425-447
:⚠️ Potential issueUpdate environment selection for multi-environment support
The current environment selection UI only allows selecting one environment, but should be updated to support selecting multiple environments.
-{environments.length > 1 && ( +{environments.length > 0 && ( <Controller control={control} - name="environment" + name="environments" render={({ field: { value, onChange }, fieldState: { error } }) => ( <FormControl label="Environment" isError={Boolean(error)} errorText={error?.message} > <FilterableSelect options={environments} value={value} onChange={onChange} + isMulti - placeholder="Select the environment to create secret in..." + placeholder="Select environments to create secret in..." getOptionLabel={(option) => option.name} getOptionValue={(option) => option.slug} menuPlacement="top" /> </FormControl> )} /> )}
🧹 Nitpick comments (2)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (1)
77-96
: Remove unnecessary empty lineThere's an unnecessary empty line in the function that should be removed for consistency.
const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environment }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; - try {
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
95-95
: Fix redundant default value assignmentThe current default value logic is redundant and potentially confusing. It's using
environments[0]
in both the condition and the fallback.- environment: environments.length === 1 ? environments[0] : environments[0] + environment: environments.length >= 1 ? environments[0] : undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx
(9 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx
(9 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check Frontend Type and Lint check
🔇 Additional comments (49)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
382-404
: LGTM! Good implementation of environment selectionThe conditional rendering of the environment selection component is well implemented, showing it only when there are multiple environments to choose from.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (4)
59-60
: LGTM! Form schema properly extendedThe form schema has been correctly updated to include the environment field with appropriate validation.
92-93
: LGTM! Smart default environment implementationGood approach to set the default environment when there's only one environment available, which improves the user experience by reducing unnecessary selections.
107-117
: LGTM! Proper dynamic secret creation with environmentThe dynamic secret creation code correctly uses the selected environment's slug.
327-349
: LGTM! Consistent implementation of environment selectionThe environment selection component is consistent with the implementation in other forms, maintaining a uniform user experience.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (3)
55-56
: LGTM! Form schema properly structuredThe form schema correctly includes the environment field with appropriate validation.
94-112
: LGTM! Proper error handling in dynamic secret creationThe function has good error handling with appropriate notifications to the user when creating dynamic secrets.
326-348
: LGTM! Well-implemented environment selection UIThe environment selection component is well-implemented with good placement at the bottom of the form.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (1)
301-323
: LGTM! Consistent environment selection implementationThe environment selection UI is consistently implemented across all forms, providing a uniform experience.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx (1)
14-14
: Multi-environment selection feature implemented correctlyThe changes to support multiple environments are well-implemented. The component now:
- Uses a
WorkspaceEnv
array instead of a single environment string- Automatically selects the environment when only one is available
- Provides a filterable dropdown when multiple environments are available
- Updates form validation to include environment as an object with name and slug
Also applies to: 22-22, 55-56, 65-65, 72-72, 94-94, 100-118, 312-333
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx (1)
15-15
: Multi-environment selection properly implementedThe code correctly implements multi-environment support with good user experience:
- When only one environment exists, it's pre-selected
- When multiple environments exist, a filterable dropdown is shown
- The dropdown is positioned at the top (
menuPlacement="top"
) to prevent UI issues with the form's positionAlso applies to: 26-26, 84-85, 94-94, 151-151, 179-179, 188-207, 663-685
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
10-10
: Multi-environment support correctly implementedThe changes to support multiple environments follow the same pattern as other form components, ensuring consistency across the application. The form now properly validates that secret names must be lowercase and correctly handles environment selection.
Also applies to: 18-18, 63-64, 74-74, 80-80, 103-103, 114-114, 308-330
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx (1)
16-16
: Multi-environment selection implemented consistentlyThe changes to support multiple environments are implemented consistently with the other form components:
- The props interface now accepts an array of environments
- The form validation now includes an environment object with name and slug
- The mutation call correctly uses environment.slug
- The UI conditionally renders a filterable select when multiple environments are available
Also applies to: 26-26, 68-69, 78-78, 99-99, 115-115, 131-149, 451-473
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (9)
10-18
: Imports look consistent.
They align with the newFilterableSelect
usage and there's no sign of conflict with existing imports.
21-21
: NewWorkspaceEnv
import is aligned with multi-environment support.
Good addition for type safety.
81-82
: Validate absence handling forenvironment
.
The schema enforcesenvironment
as a required object. If an environment isn't selected, validation will fail. Confirm that the UI flow or user instructions cover selection.
92-92
:environments: WorkspaceEnv[];
The type definition is correct and aligns with multi-environment needs.
100-100
: Prop usage ofenvironments
.
No issues with how it's passed into the component.
120-121
: Potential undefinedenvironment
.
Whenenvironments.length > 1
or is 0,environment
could remainundefined
, which is still required by the schema. Ensure users must explicitly select an environment.
129-135
: Extended function signature forenvironment
.
This change is coherent with the updated schema and form defaults.
146-146
: Accessingenvironment.slug
.
Ensureenvironment
is always defined before accessing a property to avoid runtime errors.
389-411
:FilterableSelect
for environment selection.
Implementing a dropdown for multi-environment scenarios is a good approach to scalability.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx (9)
14-14
: ImportedFilterableSelect
.
No concerns with the addition.
21-21
: New import forWorkspaceEnv
.
Consistent with multi-environment adoption.
53-54
: Handling requiredenvironment
Similar to previous file’s schema changes.
63-63
:environments
prop introduction
Same logic shift to multi-env.
69-69
: Readingenvironments
Aligned with multi-environment design.
89-89
: Defaultenvironment
selection
Same single-env fallback pattern.
95-101
: ExpandedhandleCreateDynamicSecret
Environment param usage mirrors the approach in LdapInputForm.
112-112
:environmentSlug
fromenvironment.slug
Validated usage.
305-326
:FilterableSelect
for environment
Implementation mirrors that of Ldap form.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (9)
14-14
: AddFilterableSelect
import
Straightforward addition.
22-22
: ImportWorkspaceEnv
Follows the multi-environment pattern.
55-56
: Environment required in schema
Same approach as prior forms.
65-65
:environments
in props
Consistent with multi-env usage.
71-71
: Reading the environment array
Matches the refactor for multi-environments.
89-89
: Default environment
Same single-env fallback logic.
95-101
: Extended signature
Environment param included, similar to other forms.
112-112
:environmentSlug: environment.slug
No changes from prior approach.
324-346
: Dynamic environment selection
Reflects the repeated multi-env block.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (9)
14-14
:FilterableSelect
import
No concerns found.
22-22
: AddedWorkspaceEnv
Maintains the multi-env approach.
57-58
:environment
required
Same schema addition as seen before.
67-67
: New propenvironments
Conforms to the multi-environment refactor.
82-82
: Usingenvironments
Identical extension from earlier forms.
93-94
: Initialize defaultprovider
andenvironment
Mirrors the single-environment fallback.
100-106
: ExtendedhandleCreateDynamicSecret
Includes environment param, same pattern as other forms.
118-118
: Applyingenvironment.slug
Replicates previous usage.
359-381
:FilterableSelect
block
Consistent multi-env UI pattern.
...anager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
Show resolved
Hide resolved
...anager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
Outdated
Show resolved
Hide resolved
...anager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments but works well, tested with postgres 👍
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
Outdated
Show resolved
Hide resolved
...SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (1)
112-150
:⚠️ Potential issueFix the multi-user creation logic to properly handle promises.
The code doesn't properly handle promises when creating multiple user secrets. The map function creates promises but doesn't collect or await them properly, which could lead to the completion callback being triggered before all secrets are created.
Apply this diff to properly handle multiple user secret creation:
const handleCreateDynamicSecret = async ({ name, selectedUsers, provider, maxTTL, defaultTTL, environment }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; + const promises = []; try { - selectedUsers.map(async (user: { id: string; name: string; email: string }) => { - await createDynamicSecret.mutateAsync({ + for (const user of selectedUsers) { + promises.push(createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.AzureEntraId, inputs: { userId: user.id, tenantId: provider.tenantId, email: user.email, applicationId: provider.applicationId, clientSecret: provider.clientSecret } }, maxTTL, name: `${name}-${user.name}`, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug - }); - }); + })); + } + await Promise.all(promises); onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", text: "Failed to create dynamic secret" }); } };
♻️ Duplicate comments (1)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (1)
103-109
: 🛠️ Refactor suggestionFunction signature updated for environment object support.
The function signature has been updated to handle the environment as an object, but should be modified to support multiple environments based on past review comments.
const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, - environment + environments: selectedEnvs }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; + let hasErrors = false; + const promises = selectedEnvs.map(async (environment) => { try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.MongoDB, inputs: { ...provider, port: provider?.port ? provider.port : undefined, roles: provider.roles.map((el) => el.roleName) } }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); - onCompleted(); } catch { + hasErrors = true; createNotification({ type: "error", - text: "Failed to create dynamic secret" + text: `Failed to create dynamic secret in environment ${environment.name}` }); } + }); + await Promise.all(promises); + if (!hasErrors) { + onCompleted(); + } };
🧹 Nitpick comments (6)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (1)
96-122
: Improve error message specificity for better user feedback.While the function correctly handles the single environment case, the error message could be more specific to help users understand which environment had the issue.
} catch { createNotification({ type: "error", - text: "Failed to create dynamic secret" + text: `Failed to create dynamic secret in environment ${environment.name}` }); }frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (1)
123-123
: Consider adding null check for environments arrayThe default value for
environment
assumes thatenvironments
array has at least one item whenisSingleEnvironmentMode
is true. However, if the array is empty,environments[0]
will be undefined, potentially causing issues during form submission.- environment: isSingleEnvironmentMode ? environments[0] : undefined + environment: isSingleEnvironmentMode && environments.length > 0 ? environments[0] : undefinedfrontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
97-97
: Consider adding null check for environments arrayThe default value for
environment
assumes thatenvironments
array has at least one item whenisSingleEnvironmentMode
is true. However, if the array is empty,environments[0]
will be undefined, potentially causing issues during form submission.- environment: isSingleEnvironmentMode ? environments[0] : undefined + environment: isSingleEnvironmentMode && environments.length > 0 ? environments[0] : undefinedfrontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (1)
94-94
: Consider adding null check for environments arrayThe default value for
environment
assumes thatenvironments
array has at least one item whenisSingleEnvironmentMode
is true. However, if the array is empty,environments[0]
will be undefined, potentially causing issues during form submission.- environment: isSingleEnvironmentMode ? environments[0] : undefined + environment: isSingleEnvironmentMode && environments.length > 0 ? environments[0] : undefinedfrontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (1)
96-96
: Consider adding null check for environments arrayThe default value for
environment
assumes thatenvironments
array has at least one item whenisSingleEnvironmentMode
is true. However, if the array is empty,environments[0]
will be undefined, potentially causing issues during form submission.- environment: isSingleEnvironmentMode ? environments[0] : undefined + environment: isSingleEnvironmentMode && environments.length > 0 ? environments[0] : undefinedfrontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (1)
111-137
: Consider updating error message to include environment details.The error message is generic, but could be more informative by including the environment name when the creation fails.
catch { createNotification({ type: "error", - text: "Failed to create dynamic secret" + text: `Failed to create dynamic secret for environment "${environment.name}"` }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
(8 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx
(1 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx
(5 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CreateDynamicSecretForm.tsx
(18 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx
(9 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx
(7 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx
(6 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx
(9 hunks)frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check Frontend Type and Lint check
🔇 Additional comments (72)
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/ActionBar.tsx (1)
658-660
: Consistent environment structure implementation.The environment string is now wrapped in a WorkspaceEnv-compatible object and passed as an array to maintain the correct typing for the CreateDynamicSecretForm component. The isSingleEnvironmentMode flag properly indicates this is operating in a single environment context.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CreateDynamicSecretForm.tsx (3)
38-45
: Properly updated Props interface to support multi-environment feature.The environment string has been replaced with an array of WorkspaceEnv objects and a new isSingleEnvironmentMode flag. This change correctly implements the requirement to support dynamic secret creation across multiple environments.
130-137
: Component parameters correctly updated to match new props interface.The destructuring of props properly receives the new environments array and isSingleEnvironmentMode flag.
203-205
: Properly propagating environments to child components.All instances of environment passing have been updated to pass the new environments array and isSingleEnvironmentMode flag, ensuring consistent implementation across all form types.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AzureEntraIdInputForm.tsx (3)
55-66
: Updated form schema and props to support multi-environment feature.The form schema correctly includes the environment object and the Props type has been updated to accept an array of WorkspaceEnv objects.
77-96
: Default environment value conditionally set based on mode.When in single environment mode, the form is initialized with the first environment, which is a good optimization that simplifies the user experience in this case.
384-406
: Well-implemented environment selector for multi-environment mode.The FilterableSelect component is appropriately conditionally rendered only when not in single environment mode. The implementation correctly uses the WorkspaceEnv structure with proper option labelling and value mapping.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RedisInputForm.tsx (4)
55-57
: Form schema properly updated for multi-environment support.The form schema now includes the environment object with the proper structure matching the WorkspaceEnv type.
60-76
: Props type correctly updated for multi-environment feature.The Props type has been updated to replace the single environment string with an array of WorkspaceEnv objects and added the isSingleEnvironmentMode flag.
89-91
: Default environment conditionally set based on mode.The form's defaultValues correctly sets the environment to the first environment when in single environment mode, which provides a good user experience by reducing unnecessary steps.
328-350
: Well-implemented environment selector with proper placement.The FilterableSelect component is correctly implemented and only appears when not in single environment mode. The menuPlacement="top" is a good choice to ensure the dropdown is visible in the form's context.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsElastiCacheInputForm.tsx (5)
55-56
: Schema field addition for environment selection looks good.The new environment field in the form schema correctly defines the structure with name and slug properties, which aligns with the WorkspaceEnv type.
65-66
: Props interface update appropriately handles multiple environments.Replacing the single environment string with an array of WorkspaceEnv objects and adding the isSingleEnvironmentMode flag provides good flexibility for different usage scenarios.
96-96
: Good default value handling for environments.Setting a default environment when in single environment mode ensures the form is pre-populated correctly without requiring user selection.
102-108
: Function signature update correctly captures environment parameter.The handleCreateDynamicSecret function now properly accepts the environment object and uses its slug property for the API call, maintaining compatibility with the backend.
Also applies to: 119-119
314-335
: Improve visibility of selected environment in single environment mode.When in single environment mode, the environment selector is hidden, but users may not know which environment is being used.
Consider displaying the selected environment name in a non-editable way when in single environment mode, to ensure users know which environment they're creating the secret in:
-{!isSingleEnvironmentMode && ( +{isSingleEnvironmentMode ? ( + <FormControl + label="Environment" + > + <div className="py-2 px-3 border border-mineshaft-600 bg-mineshaft-800 rounded text-mineshaft-300"> + {environments[0].name} + </div> + </FormControl> +) : ( <Controller control={control} name="environment" render={({ field: { value, onChange }, fieldState: { error } }) => ( <FormControl label="Environment" isError={Boolean(error)} errorText={error?.message} > <FilterableSelect options={environments} value={value} onChange={onChange} placeholder="Select the environment to create secret in..." getOptionLabel={(option) => option.name} getOptionValue={(option) => option.slug} /> </FormControl> )} /> )}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SqlDatabaseInputForm.tsx (6)
84-85
: Schema field addition for environment selection looks good.The new environment field in the form schema correctly defines the structure with name and slug properties, which aligns with the WorkspaceEnv type.
94-95
: Props interface update appropriately handles multiple environments.Replacing the single environment string with an array of WorkspaceEnv objects and adding the isSingleEnvironmentMode flag provides good flexibility for different usage scenarios.
181-181
: Good default value handling for environments.Setting a default environment when in single environment mode ensures the form is pre-populated correctly without requiring user selection.
190-196
: Function signature update correctly captures environment parameter.The handleCreateDynamicSecret function now properly accepts the environment object and uses its slug property for the API call, maintaining compatibility with the backend.
Also applies to: 208-208
665-687
: Consider showing environment name even in single environment mode.Similar to other forms in this PR, when in single environment mode, users may not know which environment they're creating the secret in.
Consider displaying the environment name in a non-editable way for clarity.
682-682
: Good user experience enhancement with menuPlacement.Setting menuPlacement to "top" is a thoughtful addition since this control appears at the bottom of a long form, ensuring the dropdown menu doesn't extend beyond the viewport.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/ElasticSearchInputForm.tsx (5)
78-79
: Schema field addition for environment selection looks good.The new environment field in the form schema correctly defines the structure with name and slug properties, which aligns with the WorkspaceEnv type.
88-89
: Props interface update appropriately handles multiple environments.Replacing the single environment string with an array of WorkspaceEnv objects and adding the isSingleEnvironmentMode flag provides good flexibility for different usage scenarios.
116-116
: Good default value handling for environments.Setting a default environment when in single environment mode ensures the form is pre-populated correctly without requiring user selection.
122-128
: Function signature update correctly captures environment parameter.The handleCreateDynamicSecret function now properly accepts the environment object and uses its slug property for the API call, maintaining compatibility with the backend.
Also applies to: 139-139
421-443
: Consider showing environment name even in single environment mode.Similar to other forms in this PR, when in single environment mode, users may not know which environment they're creating the secret in.
Consider displaying the environment name in a non-editable way for clarity.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/AwsIamInputForm.tsx (5)
44-45
: Schema field addition for environment selection looks good.The new environment field in the form schema correctly defines the structure with name and slug properties, which aligns with the WorkspaceEnv type.
54-55
: Props interface update appropriately handles multiple environments.Replacing the single environment string with an array of WorkspaceEnv objects and adding the isSingleEnvironmentMode flag provides good flexibility for different usage scenarios.
72-74
: Simplified default values in form initialization.The form has been simplified to only initialize the environment field, which is good practice. However, ensure that any other required default values are being set elsewhere or the form will work correctly without them.
Verify that this form works properly with the minimal defaults compared to other forms that have more extensive default values.
79-85
: Function signature update correctly captures environment parameter.The handleCreateDynamicSecret function now properly accepts the environment object and uses its slug property for the API call, maintaining compatibility with the backend.
Also applies to: 97-97
303-325
: Consider showing environment name even in single environment mode.Similar to other forms in this PR, when in single environment mode, users may not know which environment they're creating the secret in.
Consider displaying the environment name in a non-editable way for clarity.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/LdapInputForm.tsx (1)
391-413
: Good implementation of environment selectionThe conditional rendering of the environment selection component is well implemented. The FilterableSelect component provides a good user experience for selecting environments when multiple options are available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/TotpInputForm.tsx (1)
310-332
: Good implementation of environment selectionThe conditional rendering of the environment selection component is well implemented. The FilterableSelect component provides a good user experience for selecting environments when multiple options are available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SnowflakeInputForm.tsx (1)
329-351
: Good implementation of environment selectionThe conditional rendering of the environment selection component is well implemented. The FilterableSelect component provides a good user experience for selecting environments when multiple options are available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/CassandraInputForm.tsx (1)
361-383
: Good implementation of environment selectionThe conditional rendering of the environment selection component is well implemented. The FilterableSelect component provides a good user experience for selecting environments when multiple options are available.
frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapHanaInputForm.tsx (8)
14-14
: Import added for environment selector component.The
FilterableSelect
component has been correctly added to the imports to support the new environment selection functionality.
22-22
: Type import added for workspace environments.The
WorkspaceEnv
type is correctly imported to support the new environment selection feature.
55-56
: Form schema updated to support environment selection.The
name
field validation and newenvironment
field have been properly added to the form schema.
65-66
: Props updated to support multi-environment functionality.The props interface has been updated to replace the single
environment
string with an array ofWorkspaceEnv
objects and add an optionalisSingleEnvironmentMode
flag.
91-91
: Default environment handling improved.The form now correctly sets the default environment based on whether the component is in single environment mode, using the first environment from the array.
97-103
: Function signature updated for environment object support.The
handleCreateDynamicSecret
function signature has been updated to handle the environment as an object withname
andslug
properties.
114-114
: Environment slug usage in API call.The function correctly uses
environment.slug
when making the API call, aligning with the updated data structure.
306-348
: Environment selector component added.A
FilterableSelect
component has been added for environment selection when not in single environment mode. This properly handles environment selection and integrates with the form control.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoAtlasInputForm.tsx (8)
16-16
: Import added for environment selector component.The
FilterableSelect
component has been correctly added to the imports to support the new environment selection functionality.
26-26
: Type import added for workspace environments.The
WorkspaceEnv
type is correctly imported to support the new environment selection feature.
68-69
: Form schema updated to support environment selection.The
name
field validation and newenvironment
field have been properly added to the form schema.
78-79
: Props updated to support multi-environment functionality.The props interface has been updated to replace the single
environment
string with an array ofWorkspaceEnv
objects and add an optionalisSingleEnvironmentMode
flag.
117-117
: Default environment handling improved.The form now correctly sets the default environment based on whether the component is in single environment mode, using the first environment from the array.
133-139
: Function signature updated for environment object support.The
handleCreateDynamicSecret
function signature has been updated to handle the environment as an object withname
andslug
properties.
150-150
: Environment slug usage in API call.The function correctly uses
environment.slug
when making the API call, aligning with the updated data structure.
453-475
: Environment selector component added.A
FilterableSelect
component has been added for environment selection when not in single environment mode. This properly handles environment selection and integrates with the form control.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/SapAseInputForm.tsx (9)
14-14
: Import added for environment selector component.The
FilterableSelect
component has been correctly added to the imports to support the new environment selection functionality.
21-21
: Type import added for workspace environments.The
WorkspaceEnv
type is correctly imported to support the new environment selection feature.
53-54
: Form schema updated to support environment selection.The
name
field validation and newenvironment
field have been properly added to the form schema.
63-64
: Props updated to support multi-environment functionality.The props interface has been updated to replace the single
environment
string with an array ofWorkspaceEnv
objects and add an optionalisSingleEnvironmentMode
flag.
91-91
: Default environment handling improved.The form now correctly sets the default environment based on whether the component is in single environment mode, using the first environment from the array.
97-103
: Function signature updated for environment object support.The
handleCreateDynamicSecret
function signature has been updated to handle the environment as an object withname
andslug
properties.
114-115
: Environment slug usage in API call.The function correctly uses
environment.slug
when making the API call, aligning with the updated data structure.
97-123
: CallonCompleted()
only after all operations complete.Based on past review comments, it would be better to handle completions after all operations finish. This ensures the form is only completed once, even if creating secrets across multiple environments.
const handleCreateDynamicSecret = async ({ name, maxTTL, provider, defaultTTL, environment }: TForm) => { // wait till previous request is finished if (createDynamicSecret.isPending) return; try { await createDynamicSecret.mutateAsync({ provider: { type: DynamicSecretProviders.SapAse, inputs: provider }, maxTTL, name, path: secretPath, defaultTTL, projectSlug, environmentSlug: environment.slug }); onCompleted(); } catch { createNotification({ type: "error", text: "Failed to create dynamic secret" }); } };
306-328
: Environment selector component added.A
FilterableSelect
component has been added for environment selection when not in single environment mode. This properly handles environment selection and integrates with the form control.frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/MongoDBInputForm.tsx (6)
10-18
: Imports rearranged for better organization.The imports have been properly reorganized, importing the
FilterableSelect
component needed for environment selection functionality.
21-21
: Type import added for workspace environments.The
WorkspaceEnv
type is correctly imported to support the new environment selection feature.
59-59
: Schema needs multi-environment support.The form schema defines a single environment object, but based on past review comments and intended functionality, it should support an array of selected environments.
- environment: z.object({ name: z.string(), slug: z.string() }) + environments: z + .object({ name: z.string(), slug: z.string() }) + .array() + .min(1, "At least one environment must be selected")
68-69
: Props updated to support multi-environment functionality.The props interface has been updated to replace the single
environment
string with an array ofWorkspaceEnv
objects and add an optionalisSingleEnvironmentMode
flag.
92-92
: Update default value for multi-environment support.The current default value approach only sets a single environment, but should be updated to support an array of environments.
- environment: isSingleEnvironmentMode ? environments[0] : undefined + environments: isSingleEnvironmentMode ? [environments[0]] : []
343-365
: Update environment selection for multi-environment support.The current environment selection UI only allows selecting one environment, but should be updated to support selecting multiple environments.
-{!isSingleEnvironmentMode && ( +{!isSingleEnvironmentMode && ( <Controller control={control} - name="environment" + name="environments" render={({ field: { value, onChange }, fieldState: { error } }) => ( <FormControl label="Environment" isError={Boolean(error)} errorText={error?.message} > <FilterableSelect options={environments} value={value} onChange={onChange} + isMulti - placeholder="Select the environment to create secret in..." + placeholder="Select environments to create secret in..." getOptionLabel={(option) => option.name} getOptionValue={(option) => option.slug} menuPlacement="top" /> </FormControl> )} /> )}frontend/src/pages/secret-manager/SecretDashboardPage/components/ActionBar/CreateDynamicSecretForm/RabbitMqInputForm.tsx (5)
62-63
: Great schema update for environment validation.The schema change now properly validates the environment object structure, ensuring both name and slug properties are present, which aligns well with the new multi-environment support.
72-73
: Good implementation of multi-environment support.The component now properly accepts an array of environments and has an optional flag to control whether it's in single-environment mode. This flexible approach allows for both backward compatibility and new multi-environment functionality.
Also applies to: 79-83
105-105
: Appropriate default value handling for environments.The conditional default value setting is well-implemented - when in single environment mode, it automatically selects the first environment, otherwise it leaves it undefined for the user to select.
427-449
: Excellent UI enhancement for environment selection.The conditional rendering of the FilterableSelect component provides a good user experience for multi-environment selection. The component is only shown when needed, keeping the interface clean when in single environment mode.
111-137
: Simplify implementation to work with multiple environments.The current implementation handles one environment at a time. Consider refactoring to handle multiple environments like in other forms, which would be consistent with the pattern suggested in previous reviews.
Let's check if other dynamic secret forms handle multiple environments:
#!/bin/bash # Check if other dynamic secret forms handle multiple environments similar to previous review suggestion rg -A 5 "\bselectedEnvs\.map\b" --type=tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description 📣
Add an option on the Secrets Overview page to create dynamic secrets, enabling users to choose which environments should be included.
CleanShot.2025-03-14.at.12.01.23.mp4
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets
Summary by CodeRabbit