Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {

import { api } from "~/trpc/react";
import { ReleaseConditionRender } from "../release-condition/ReleaseConditionRender";
import { useReleaseFilter } from "../release-condition/useReleaseFilter";
import { ReleaseBadgeList } from "../ReleaseBadgeList";

type OverviewProps = {
Expand Down Expand Up @@ -86,6 +87,7 @@ export const Overview: React.FC<OverviewProps> = ({ releaseChannel }) => {
systemSlug?: string;
deploymentSlug?: string;
}>();
const { filter: paramFilter, setFilter } = useReleaseFilter();

const defaultValues = {
...releaseChannel,
Expand All @@ -106,6 +108,9 @@ export const Overview: React.FC<OverviewProps> = ({ releaseChannel }) => {
.then(() =>
utils.deployment.releaseChannel.byId.invalidate(releaseChannel.id),
)
.then(() => {
if (paramFilter != null) setFilter(releaseFilter);
})
.then(() => router.refresh());
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from "@ctrlplane/ui/dropdown-menu";

import { api } from "~/trpc/react";
import { useReleaseFilter } from "../release-condition/useReleaseFilter";
import { useReleaseChannelDrawer } from "./useReleaseChannelDrawer";

type DeleteReleaseChannelDialogProps = {
Expand All @@ -37,13 +38,15 @@ const DeleteReleaseChannelDialog: React.FC<DeleteReleaseChannelDialogProps> = ({
}) => {
const [open, setOpen] = useState(false);
const { removeReleaseChannelId } = useReleaseChannelDrawer();
const { removeReleaseChannel } = useReleaseFilter();
const router = useRouter();
const deleteReleaseChannel =
api.deployment.releaseChannel.delete.useMutation();
const onDelete = () =>
deleteReleaseChannel
.mutateAsync(releaseChannelId)
.then(() => removeReleaseChannelId())
.then(() => removeReleaseChannel())
.then(() => router.refresh())
.then(() => setOpen(false));
Comment on lines +41 to 51
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Ensure all releaseChannelId references are properly handled upon deletion.

Some instances of releaseChannelId are not associated with removal functions, which may lead to inconsistent state.

  • packages/api/src/router/release.ts
  • packages/api/src/router/environment.ts
  • apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/useReleaseChannelDrawer.ts
  • apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/DeploymentPageContent.tsx
  • apps/webservice/src/app/[workspaceSlug]/_components/release-condition/useReleaseFilter.ts
  • apps/webservice/src/app/[workspaceSlug]/_components/release-condition/ReleaseConditionDialog.tsx
  • apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/ReleaseChannelDropdown.tsx
  • apps/webservice/src/app/[workspaceSlug]/_components/release-channel-drawer/ReleaseChannelDrawer.tsx
  • apps/webservice/src/app/[workspaceSlug]/_components/environment-policy-drawer/ReleaseChannels.tsx
  • apps/webservice/src/app/[workspaceSlug]/_components/environment-drawer/ReleaseChannels.tsx
🔗 Analysis chain

Verify proper cleanup of release channel references.

Let's ensure that all references to the deleted release channel are properly cleaned up.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any potential references to release channels that might need cleanup

# Look for other components using release channels
echo "Checking for components using release channels..."
rg -l "useReleaseChannel|releaseChannelId" --type typescript

# Look for filter-related components that might need updates
echo "Checking for filter-related components..."
rg -l "useReleaseFilter|release-channel.*filter" --type typescript

# Check for any hardcoded release channel IDs
echo "Checking for hardcoded release channel references..."
rg -l "releaseChannel.*Id.*=.*['\"]" --type typescript

Length of output: 612


Script:

#!/bin/bash
# Description: Check for any potential references to release channels that might need cleanup

# Look for other components using release channels
echo "Checking for components using release channels..."
rg "useReleaseChannel|releaseChannelId" --glob "*.ts" --glob "*.tsx"

# Look for filter-related components that might need updates
echo "Checking for filter-related components..."
rg "useReleaseFilter|release-channel.*filter" --glob "*.ts" --glob "*.tsx"

# Check for any hardcoded release channel IDs
echo "Checking for hardcoded release channel references..."
rg "releaseChannel.*Id.*=['\"]" --glob "*.ts" --glob "*.tsx"

Length of output: 9498


Script:

#!/bin/bash
# Description: Verify that all references to releaseChannelId properly handle deletion

# Check if removal functions are called wherever releaseChannelId is used
echo "Verifying removal function calls for releaseChannelId..."
rg "removeReleaseChannelId\(\)|removeReleaseChannel\(\)" --glob "*.ts" --glob "*.tsx"

# Ensure that after deletion, there are no lingering references to releaseChannelId
echo "Checking for lingering releaseChannelId references after deletion..."
rg "releaseChannelId" --glob "*.ts" --glob "*.tsx" | grep -v "removeReleaseChannelId\(\)|removeReleaseChannel\(\)"

Length of output: 5223


Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"use client";

import type * as SCHEMA from "@ctrlplane/db/schema";
import type { ReleaseCondition } from "@ctrlplane/validators/releases";
import React, { useState } from "react";

Expand All @@ -13,6 +14,16 @@ import {
DialogTitle,
DialogTrigger,
} from "@ctrlplane/ui/dialog";
import {
Select,
SelectContent,
SelectGroup,
SelectItem,
SelectLabel,
SelectTrigger,
SelectValue,
} from "@ctrlplane/ui/select";
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@ctrlplane/ui/tabs";
import {
defaultCondition,
isValidReleaseCondition,
Expand All @@ -22,26 +33,37 @@ import {
import { api } from "~/trpc/react";
import { ReleaseBadgeList } from "../ReleaseBadgeList";
import { ReleaseConditionRender } from "./ReleaseConditionRender";
import { useReleaseFilter } from "./useReleaseFilter";

type ReleaseConditionDialogProps = {
condition?: ReleaseCondition;
deploymentId?: string;
onChange: (condition: ReleaseCondition | undefined) => void;
releaseChannels?: SCHEMA.ReleaseChannel[];
children: React.ReactNode;
};

export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
condition,
deploymentId,
onChange,
releaseChannels = [],
children,
}) => {
const [open, setOpen] = useState(false);
const [error, setError] = useState<string | null>(null);
const [localCondition, setLocalCondition] = useState(
condition ?? defaultCondition,
);
const isLocalConditionValid = isValidReleaseCondition(localCondition);
const { releaseChannelId, setReleaseChannel, removeReleaseChannel } =
useReleaseFilter();

const [localReleaseChannelId, setLocalReleaseChannelId] = useState<
string | undefined
>(releaseChannelId);

const [localCondition, setLocalCondition] = useState<
ReleaseCondition | undefined
>(condition ?? defaultCondition);
const isLocalConditionValid =
localCondition == null || isValidReleaseCondition(localCondition);
const releasesQ = api.release.list.useQuery(
{ deploymentId: deploymentId ?? "", filter: localCondition, limit: 5 },
{ enabled: deploymentId != null && isLocalConditionValid },
Expand All @@ -55,45 +77,125 @@ export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
className="min-w-[1000px]"
onClick={(e) => e.stopPropagation()}
>
<DialogHeader>
<DialogTitle>Edit Release Condition</DialogTitle>
<DialogDescription>
Edit the release filter, up to a depth of {MAX_DEPTH_ALLOWED + 1}.
</DialogDescription>
</DialogHeader>
<ReleaseConditionRender
condition={localCondition}
onChange={setLocalCondition}
/>
{releases != null && <ReleaseBadgeList releases={releases} />}
{error && <span className="text-sm text-red-600">{error}</span>}
<DialogFooter>
<Button
variant="outline"
onClick={() => {
<Tabs
defaultValue={
releaseChannels.length > 0 ? "release-channels" : "new-filter"
}
className="space-y-4"
onValueChange={(value) => {
if (value === "new-filter") {
setLocalCondition(defaultCondition);
setError(null);
}}
>
Clear
</Button>
<div className="flex-grow" />
<Button
onClick={() => {
if (!isValidReleaseCondition(localCondition)) {
setError(
"Invalid release condition, ensure all fields are filled out correctly.",
setLocalReleaseChannelId(undefined);
}
}}
>
{releaseChannels.length > 0 && (
<TabsList>
<TabsTrigger value="release-channels">
Release Channels
</TabsTrigger>
<TabsTrigger value="new-filter">New Filter</TabsTrigger>
</TabsList>
)}
<TabsContent value="release-channels" className="space-y-4">
<DialogHeader>
<DialogTitle>Select Release Channel</DialogTitle>
<DialogDescription>
View releases by release channel.
</DialogDescription>
</DialogHeader>
<Select
value={localReleaseChannelId}
onValueChange={(value) => {
const releaseChannel = releaseChannels.find(
(rc) => rc.id === value,
);
return;
}
onChange(localCondition);
setOpen(false);
setError(null);
}}
>
Save
</Button>
</DialogFooter>
if (releaseChannel == null) return;
setLocalReleaseChannelId(value);
setLocalCondition(releaseChannel.releaseFilter ?? undefined);
}}
>
<SelectTrigger>
<SelectValue placeholder="Select release channel..." />
</SelectTrigger>
<SelectContent>
{releaseChannels.length === 0 && (
<SelectGroup>
<SelectLabel>No release channels found</SelectLabel>
</SelectGroup>
)}
{releaseChannels.map((rc) => (
<SelectItem key={rc.id} value={rc.id}>
{rc.name}
</SelectItem>
))}
</SelectContent>
</Select>
<DialogFooter>
<Button
onClick={() => {
const releaseChannel = releaseChannels.find(
(rc) => rc.id === localReleaseChannelId,
);
if (releaseChannel == null) return;
setReleaseChannel(releaseChannel);
setOpen(false);
setError(null);
}}
Comment on lines +136 to +144
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure consistent state updates by invoking onChange when selecting a release channel.

In the "Release Channels" tab, the onChange handler is not called when the Save button is clicked. To maintain consistency and notify the parent component of the condition change, consider invoking onChange with the selected release channel's release filter.

Apply this diff to fix the issue:

onClick={() => {
  const releaseChannel = releaseChannels.find(
    (rc) => rc.id === localReleaseChannelId,
  );
  if (releaseChannel == null) return;
  setReleaseChannel(releaseChannel);
+ onChange(releaseChannel.releaseFilter ?? undefined);
  setOpen(false);
  setError(null);
}}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onClick={() => {
const releaseChannel = releaseChannels.find(
(rc) => rc.id === localReleaseChannelId,
);
if (releaseChannel == null) return;
setReleaseChannel(releaseChannel);
setOpen(false);
setError(null);
}}
onClick={() => {
const releaseChannel = releaseChannels.find(
(rc) => rc.id === localReleaseChannelId,
);
if (releaseChannel == null) return;
setReleaseChannel(releaseChannel);
onChange(releaseChannel.releaseFilter ?? undefined);
setOpen(false);
setError(null);
}}

disabled={localReleaseChannelId == null}
>
Save
</Button>
</DialogFooter>
</TabsContent>
<TabsContent value="new-filter" className="space-y-4">
<DialogHeader>
<DialogTitle>Edit Release Condition</DialogTitle>
<DialogDescription>
Edit the release filter, up to a depth of{" "}
{MAX_DEPTH_ALLOWED + 1}.
</DialogDescription>
</DialogHeader>
<ReleaseConditionRender
condition={localCondition ?? defaultCondition}
onChange={setLocalCondition}
/>
{releases != null && <ReleaseBadgeList releases={releases} />}
{error && <span className="text-sm text-red-600">{error}</span>}
<DialogFooter>
<Button
variant="outline"
onClick={() => {
setLocalCondition(defaultCondition);
setError(null);
}}
>
Clear
</Button>
<div className="flex-grow" />
<Button
onClick={() => {
console.log(">>> localCondition", localCondition);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove debugging console.log statement.

The console.log statement on line 178 appears to be for debugging purposes and should be removed to prevent unnecessary console output in production.

Apply this diff to remove the debug statement:

-                      console.log(">>> localCondition", localCondition);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(">>> localCondition", localCondition);

if (
localCondition != null &&
!isValidReleaseCondition(localCondition)
) {
setError(
"Invalid release condition, ensure all fields are filled out correctly.",
);
return;
}
removeReleaseChannel();
onChange(localCondition);
setOpen(false);
setError(null);
}}
>
Save
</Button>
</DialogFooter>
</TabsContent>
</Tabs>
</DialogContent>
</Dialog>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type * as SCHEMA from "@ctrlplane/db/schema";
import type { ReleaseCondition } from "@ctrlplane/validators/releases";
import { useCallback, useMemo } from "react";
import { useRouter, useSearchParams } from "next/navigation";
Expand All @@ -17,6 +18,11 @@ export const useReleaseFilter = () => {
}
}, [urlParams]);

const releaseChannelId = useMemo<string | undefined>(
() => urlParams.get("release-channel") ?? undefined,
[urlParams],
);
Comment on lines +21 to +24
Copy link
Member

Choose a reason for hiding this comment

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

?

useParams?


const setFilter = useCallback(
(filter: ReleaseCondition | undefined) => {
if (filter == null) {
Expand All @@ -36,5 +42,36 @@ export const useReleaseFilter = () => {
[router],
);

return { filter, setFilter };
const setReleaseChannel = useCallback(
(releaseChannel: SCHEMA.ReleaseChannel) => {
const query = new URLSearchParams(window.location.search);
query.set("release-channel", releaseChannel.id);
if (releaseChannel.releaseFilter != null) {
const filterJson = LZString.compressToEncodedURIComponent(
JSON.stringify(releaseChannel.releaseFilter),
);
query.set("filter", filterJson);
}

router.replace(`?${query.toString()}`);
router.refresh();
},
[router],
);
Comment on lines +45 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding validation and error handling

The implementation has several potential improvements:

  1. Add validation for releaseChannel.id
  2. Handle potential errors during JSON stringification and compression
  3. Consider if router.refresh() is necessary for all cases
  4. Preserve other URL parameters when updating

Consider this improved implementation:

 const setReleaseChannel = useCallback(
   (releaseChannel: SCHEMA.ReleaseChannel) => {
+    if (!releaseChannel?.id) {
+      console.error('Invalid release channel');
+      return;
+    }
     const query = new URLSearchParams(window.location.search);
     query.set("release-channel", releaseChannel.id);
     if (releaseChannel.releaseFilter != null) {
+      try {
         const filterJson = LZString.compressToEncodedURIComponent(
           JSON.stringify(releaseChannel.releaseFilter),
         );
         query.set("filter", filterJson);
+      } catch (error) {
+        console.error('Failed to compress release filter:', error);
+        // Optionally handle the error case
+      }
     }

     router.replace(`?${query.toString()}`);
-    router.refresh();
+    // Consider if refresh is necessary based on your use case
   },
   [router],
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const setReleaseChannel = useCallback(
(releaseChannel: SCHEMA.ReleaseChannel) => {
const query = new URLSearchParams(window.location.search);
query.set("release-channel", releaseChannel.id);
if (releaseChannel.releaseFilter != null) {
const filterJson = LZString.compressToEncodedURIComponent(
JSON.stringify(releaseChannel.releaseFilter),
);
query.set("filter", filterJson);
}
router.replace(`?${query.toString()}`);
router.refresh();
},
[router],
);
const setReleaseChannel = useCallback(
(releaseChannel: SCHEMA.ReleaseChannel) => {
if (!releaseChannel?.id) {
console.error('Invalid release channel');
return;
}
const query = new URLSearchParams(window.location.search);
query.set("release-channel", releaseChannel.id);
if (releaseChannel.releaseFilter != null) {
try {
const filterJson = LZString.compressToEncodedURIComponent(
JSON.stringify(releaseChannel.releaseFilter),
);
query.set("filter", filterJson);
} catch (error) {
console.error('Failed to compress release filter:', error);
// Optionally handle the error case
}
}
router.replace(`?${query.toString()}`);
// Consider if refresh is necessary based on your use case
},
[router],
);


const removeReleaseChannel = useCallback(() => {
const query = new URLSearchParams(window.location.search);
query.delete("release-channel");
query.delete("filter");
router.replace(`?${query.toString()}`);
router.refresh();
}, [router]);

return {
filter,
setFilter,
releaseChannelId,
setReleaseChannel,
removeReleaseChannel,
};
};
Loading
Loading