Skip to content

fix: Clean up filter hooks #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 26, 2024
Merged

fix: Clean up filter hooks #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aed5fe1
fix: Clean up filter hooks
adityachoudhari26 Nov 26, 2024
0a9912d
nit
adityachoudhari26 Nov 26, 2024
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
9 changes: 5 additions & 4 deletions apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/JobTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"use client";

import type { JobCondition } from "@ctrlplane/validators/jobs";
import React from "react";
import { IconFilter, IconLoader2 } from "@tabler/icons-react";
import _ from "lodash";
Expand All @@ -20,25 +21,25 @@ import { JobStatusReadable } from "@ctrlplane/validators/jobs";
import { NoFilterMatch } from "~/app/[workspaceSlug]/(app)/_components/filter/NoFilterMatch";
import { JobConditionBadge } from "~/app/[workspaceSlug]/(app)/_components/job-condition/JobConditionBadge";
import { JobConditionDialog } from "~/app/[workspaceSlug]/(app)/_components/job-condition/JobConditionDialog";
import { useJobFilter } from "~/app/[workspaceSlug]/(app)/_components/job-condition/useJobFilter";
import { useJobDrawer } from "~/app/[workspaceSlug]/(app)/_components/job-drawer/useJobDrawer";
import { JobTableStatusIcon } from "~/app/[workspaceSlug]/(app)/_components/JobTableStatusIcon";
import { useFilter } from "~/app/[workspaceSlug]/(app)/_components/useFilter";
import { api } from "~/trpc/react";

type JobTableProps = {
workspaceId: string;
};

export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
const { filter, setFilter } = useJobFilter();
const { filter, setFilter } = useFilter<JobCondition>();
const { setJobId } = useJobDrawer();
const allReleaseJobTriggers = api.job.config.byWorkspaceId.list.useQuery(
{ workspaceId },
{ refetchInterval: 60_000, placeholderData: (prev) => prev },
);

const releaseJobTriggers = api.job.config.byWorkspaceId.list.useQuery(
{ workspaceId, filter, limit: 100 },
{ workspaceId, filter: filter ?? undefined, limit: 100 },
{ refetchInterval: 10_000, placeholderData: (prev) => prev },
);

Expand Down Expand Up @@ -91,7 +92,7 @@ export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
<NoFilterMatch
numItems={allReleaseJobTriggers.data?.total ?? 0}
itemType="job"
onClear={() => setFilter(undefined)}
onClear={() => setFilter(null)}
/>
)}

Expand Down
5 changes: 4 additions & 1 deletion apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ export default function TargetLayout({
)}

{pathname.includes(`/${params.workspaceSlug}/target-views`) && (
<CreateTargetViewDialog workspaceId={workspace.data?.id ?? ""}>
<CreateTargetViewDialog
workspaceId={workspace.data?.id ?? ""}
filter={null}
>
<Button variant="outline" size="sm" className="gap-1.5">
<IconPlus className="h-4 w-4" /> Add View
</Button>
Expand Down
12 changes: 6 additions & 6 deletions apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/TargetPageContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const TargetPageContent: React.FC<{
view: schema.ResourceView | null;
}> = ({ workspace, view }) => {
const [search, setSearch] = React.useState("");
const { filter, setFilter, setView } = useTargetFilter();
const { filter, setFilter } = useTargetFilter();

useDebounce(
() => {
Expand Down Expand Up @@ -113,13 +113,13 @@ export const TargetPageContent: React.FC<{
limit: 0,
});
const targets = api.resource.byWorkspaceId.list.useQuery(
{ workspaceId, filter },
{ workspaceId, filter: filter ?? undefined },
{ placeholderData: (prev) => prev },
);

const onFilterChange = (condition: ResourceCondition | undefined) => {
const onFilterChange = (condition: ResourceCondition | null) => {
const cond = condition ?? defaultCondition;
if (isEmptyCondition(cond)) setFilter(undefined);
if (isEmptyCondition(cond)) setFilter(null);
if (!isEmptyCondition(cond)) setFilter(cond);
};

Expand Down Expand Up @@ -173,7 +173,7 @@ export const TargetPageContent: React.FC<{
<CreateTargetViewDialog
workspaceId={workspace.id}
filter={filter}
onSubmit={setView}
onSubmit={(v) => setFilter(v.filter, v.id)}
>
<Button className="h-7">Save view</Button>
</CreateTargetViewDialog>
Expand Down Expand Up @@ -207,7 +207,7 @@ export const TargetPageContent: React.FC<{
<NoFilterMatch
numItems={targetsAll.data?.total ?? 0}
itemType="target"
onClear={() => setFilter(undefined)}
onClear={() => setFilter(null)}
/>
)}
{targets.data != null && targets.data.total > 0 && (
Expand Down
11 changes: 8 additions & 3 deletions ...webservice/src/app/[workspaceSlug]/(app)/_components/job-condition/JobConditionDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ import {
import { MAX_DEPTH_ALLOWED } from "@ctrlplane/validators/conditions";
import {
defaultCondition,
isEmptyCondition,
isValidJobCondition,
} from "@ctrlplane/validators/jobs";

import { JobConditionRender } from "./JobConditionRender";

type JobConditionDialogProps = {
condition?: JobCondition;
onChange: (condition: JobCondition | undefined) => void;
condition: JobCondition | null;
onChange: (condition: JobCondition | null) => void;
children: React.ReactNode;
};

Expand Down Expand Up @@ -73,9 +74,13 @@ export const JobConditionDialog: React.FC<JobConditionDialogProps> = ({
);
return;
}
onChange(localCondition);
setOpen(false);
setError(null);
if (isEmptyCondition(localCondition)) {
onChange(null);
return;
}
onChange(localCondition);
}}
>
Save
Expand Down
40 changes: 0 additions & 40 deletions apps/webservice/src/app/[workspaceSlug]/(app)/_components/job-condition/useJobFilter.ts
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions .../webservice/src/app/[workspaceSlug]/(app)/_components/release-channel-drawer/Overview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const getFinalFilter = (filter: ReleaseCondition | null) =>

const getReleaseFilterUrl = (
workspaceSlug: string,
releaseChannelId: string,
systemSlug?: string,
deploymentSlug?: string,
filter?: ReleaseCondition,
Expand All @@ -56,7 +57,7 @@ const getReleaseFilterUrl = (
const filterHash = LZString.compressToEncodedURIComponent(
JSON.stringify(filter),
);
return `${baseUrl}/releases?filter=${filterHash}`;
return `${baseUrl}/releases?filter=${filterHash}&release-channel-id=${releaseChannelId}`;
};

const schema = z.object({
Expand Down Expand Up @@ -108,9 +109,7 @@ export const Overview: React.FC<OverviewProps> = ({ releaseChannel }) => {
.then(() =>
utils.deployment.releaseChannel.byId.invalidate(releaseChannel.id),
)
.then(() => {
if (paramFilter != null) setFilter(releaseFilter);
})
.then(() => paramFilter != null && setFilter(releaseFilter ?? null))
.then(() => router.refresh());
});
Comment on lines 109 to 114
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 26, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for the API mutation

The mutation chain should include error handling to provide feedback to users when updates fail.

  const onSubmit = form.handleSubmit((data) => {
    const releaseFilter = getFinalFilter(data.releaseFilter);
    updateReleaseChannel
      .mutateAsync({ id: releaseChannel.id, data: { ...data, releaseFilter } })
      .then(() => form.reset({ ...data, releaseFilter }))
      .then(() =>
        utils.deployment.releaseChannel.byId.invalidate(releaseChannel.id),
      )
      .then(() => paramFilter != null && setFilter(releaseFilter ?? null))
-     .then(() => router.refresh());
+     .then(() => router.refresh())
+     .catch((error) => {
+       console.error('Failed to update release channel:', error);
+       // Consider adding a toast notification or other user feedback here
+     });
  });

Committable suggestion skipped: line range outside the PR's diff.


Expand All @@ -125,6 +124,7 @@ export const Overview: React.FC<OverviewProps> = ({ releaseChannel }) => {
const releases = releasesQ.data;
const releaseFilterUrl = getReleaseFilterUrl(
workspaceSlug,
releaseChannel.id,
systemSlug,
deploymentSlug,
filter,
Expand Down
17 changes: 11 additions & 6 deletions ...c/app/[workspaceSlug]/(app)/_components/release-channel-drawer/ReleaseChannelDropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import {
} from "@ctrlplane/ui/dropdown-menu";

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

type DeleteReleaseChannelDialogProps = {
releaseChannelId: string;
Expand All @@ -37,16 +36,22 @@ const DeleteReleaseChannelDialog: React.FC<DeleteReleaseChannelDialogProps> = ({
children,
}) => {
const [open, setOpen] = useState(false);
const { removeReleaseChannelId } = useReleaseChannelDrawer();
const { setFilter } = useReleaseFilter();
const { setParams } = useQueryParams();
const router = useRouter();
const deleteReleaseChannel =
api.deployment.releaseChannel.delete.useMutation();
const onDelete = () =>
deleteReleaseChannel
.mutateAsync(releaseChannelId)
.then(() => removeReleaseChannelId())
.then(() => setFilter(undefined, null))
.then(() =>
// setting params one at a time does not work and only makes one change
// so we directly set all params to null at once
setParams({
"release-channel-id": null,
release_channel_id: null,
filter: null,
}),
)
.then(() => router.refresh())
.then(() => setOpen(false));

Expand Down
32 changes: 21 additions & 11 deletions ...ce/src/app/[workspaceSlug]/(app)/_components/release-condition/ReleaseConditionDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@ctrlplane/ui/tabs";
import {
defaultCondition,
isEmptyCondition,
isValidReleaseCondition,
MAX_DEPTH_ALLOWED,
} from "@ctrlplane/validators/releases";
Expand All @@ -36,10 +37,10 @@ import { ReleaseConditionRender } from "./ReleaseConditionRender";
import { useReleaseFilter } from "./useReleaseFilter";

type ReleaseConditionDialogProps = {
condition?: ReleaseCondition;
condition: ReleaseCondition | null;
deploymentId?: string;
onChange: (
condition: ReleaseCondition | undefined,
condition: ReleaseCondition | null,
releaseChannelId?: string | null,
) => void;
releaseChannels?: SCHEMA.ReleaseChannel[];
Expand All @@ -58,16 +59,17 @@ export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
const { setFilter, releaseChannelId } = useReleaseFilter();

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

const [localCondition, setLocalCondition] = useState<
ReleaseCondition | undefined
>(condition ?? defaultCondition);
const [localCondition, setLocalCondition] = useState<ReleaseCondition | null>(
condition ?? defaultCondition,
);
const isLocalConditionValid =
localCondition == null || isValidReleaseCondition(localCondition);
const filter = localCondition ?? undefined;
const releasesQ = api.release.list.useQuery(
{ deploymentId: deploymentId ?? "", filter: localCondition, limit: 5 },
{ deploymentId: deploymentId ?? "", filter, limit: 5 },
{ enabled: deploymentId != null && isLocalConditionValid },
);
const releases = releasesQ.data;
Expand Down Expand Up @@ -107,14 +109,14 @@ export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
</DialogDescription>
</DialogHeader>
<Select
value={localReleaseChannelId}
value={localReleaseChannelId ?? undefined}
onValueChange={(value) => {
const releaseChannel = releaseChannels.find(
(rc) => rc.id === value,
);
if (releaseChannel == null) return;
setLocalReleaseChannelId(value);
setLocalCondition(releaseChannel.releaseFilter ?? undefined);
setLocalCondition(releaseChannel.releaseFilter);
}}
>
<SelectTrigger>
Expand All @@ -141,7 +143,7 @@ export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
);
if (releaseChannel == null) return;
setFilter(
releaseChannel.releaseFilter ?? undefined,
releaseChannel.releaseFilter,
localReleaseChannelId,
);
setOpen(false);
Expand Down Expand Up @@ -189,9 +191,17 @@ export const ReleaseConditionDialog: React.FC<ReleaseConditionDialogProps> = ({
);
return;
}
onChange(localCondition, null);
setOpen(false);
setError(null);

if (
localCondition != null &&
isEmptyCondition(localCondition)
) {
onChange(null, null);
return;
}
onChange(localCondition, null);
}}
>
Save
Expand Down
Loading
Loading