-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add validation hooks to surface prerequisites for installation mutations #1388
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,11 +6,14 @@ import { useIntegrationQuery } from "src/hooks/query/useIntegrationQuery"; | |||||
| import { useInstallationProps } from "../InstallationProvider"; | ||||||
|
|
||||||
| import { useInstallation } from "./useInstallation"; | ||||||
| import { useInstallationValidation } from "./useInstallationValidation"; | ||||||
|
|
||||||
| /** | ||||||
| * delete installation hook | ||||||
| * @returns {Object} An object containing: | ||||||
| * - `deleteInstallation` (function): A function to delete the installation. | ||||||
| * - `canDelete` (boolean): Whether the installation can be deleted. | ||||||
| * - `validationErrors` (Object): Detailed validation error information. | ||||||
| * - `isIdle` (boolean): Whether the mutation is idle. | ||||||
| * - `isPending` (boolean): Whether the mutation is pending. | ||||||
| * - `error` (Error | null): The error object, if any. | ||||||
|
|
@@ -21,6 +24,7 @@ export function useDeleteInstallation() { | |||||
| const { integrationNameOrId } = useInstallationProps(); | ||||||
| const { data: integrationObj } = useIntegrationQuery(integrationNameOrId); | ||||||
| const { installation } = useInstallation(); | ||||||
| const { canDelete, validationErrors } = useInstallationValidation(); | ||||||
|
||||||
| const { canDelete, validationErrors } = useInstallationValidation(); | |
| const { canDelete, validationErrors } = useInstallationValidation(integrationObj, installation); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { useIntegrationQuery } from "src/hooks/query/useIntegrationQuery"; | ||
|
|
||
| import { useInstallationProps } from "../InstallationProvider"; | ||
|
|
||
| import { useInstallation } from "./useInstallation"; | ||
|
|
||
| /** | ||
| * Validation hook that checks prerequisites for installation mutations | ||
| * @returns {Object} An object containing: | ||
| * - `canCreate` (boolean): Whether a new installation can be created | ||
| * - `canUpdate` (boolean): Whether an existing installation can be updated | ||
| * - `canDelete` (boolean): Whether an existing installation can be deleted | ||
| * - `validationErrors` (Object): Detailed validation error information | ||
| */ | ||
| export function useInstallationValidation() { | ||
| const { integrationNameOrId } = useInstallationProps(); | ||
| const { data: integrationObj } = useIntegrationQuery(integrationNameOrId); | ||
| const { installation } = useInstallation(); | ||
|
|
||
| const validationErrors = { | ||
| noIntegration: !integrationObj, | ||
| installationExists: !!installation, | ||
| noInstallation: !installation, | ||
| }; | ||
|
|
||
| return { | ||
| canCreate: !validationErrors.installationExists && !validationErrors.noIntegration, | ||
| canUpdate: !validationErrors.noInstallation && !validationErrors.noIntegration, | ||
| canDelete: !validationErrors.noInstallation && !validationErrors.noIntegration, | ||
| validationErrors, | ||
| installation, | ||
| integrationObj, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,11 +12,14 @@ import { toUpdateConfigContent } from "../config/types"; | |||||
| import { useInstallationProps } from "../InstallationProvider"; | ||||||
|
|
||||||
| import { useInstallation } from "./useInstallation"; | ||||||
| import { useInstallationValidation } from "./useInstallationValidation"; | ||||||
|
|
||||||
| /** | ||||||
| * update installation hook | ||||||
| * @returns {Object} An object containing: | ||||||
| * - `updateInstallation` (function): A function to update the installation. | ||||||
| * - `canUpdate` (boolean): Whether the installation can be updated. | ||||||
| * - `validationErrors` (Object): Detailed validation error information. | ||||||
| * - `isIdle` (boolean): Whether the mutation is idle. | ||||||
| * - `isPending` (boolean): Whether the mutation is pending. | ||||||
| * - `error` (Error | null): The error object, if any. | ||||||
|
|
@@ -27,6 +30,7 @@ export function useUpdateInstallation() { | |||||
| const { integrationNameOrId } = useInstallationProps(); | ||||||
| const { data: integrationObj } = useIntegrationQuery(integrationNameOrId); | ||||||
| const { installation } = useInstallation(); | ||||||
| const { canUpdate, validationErrors } = useInstallationValidation(); | ||||||
|
||||||
| const { canUpdate, validationErrors } = useInstallationValidation(); | |
| const { canUpdate, validationErrors } = useInstallationValidation(integrationObj, installation); |
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.
The
useInstallationValidationhook internally callsuseIntegrationQueryanduseInstallation, resulting in duplicate hook calls since these are already invoked on lines 32 and 34. This creates redundant queries and state management. Consider refactoringuseInstallationValidationto acceptintegrationObjandinstallationas parameters to avoid duplicate hook invocations.