Skip to content

Commit 38313f3

Browse files
committed
fix(webapp): address PR review feedback on region filter
Move regions to the org-layout loader and expose them via a useRegions hook (mirroring useEnvironment), so every TaskRunsTable consumer (runs, errors, schedules, waitpoints) renders the column with friendly names and flag icons. Drops the resource loader and per-route region fetch. Also fixes the "Clear all filters" button not appearing when only a region filter is set (regions was missing from the hasFilters check).
1 parent 6ccc3bf commit 38313f3

6 files changed

Lines changed: 47 additions & 120 deletions

File tree

apps/webapp/app/components/runs/v3/RunFilters.tsx

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import { useShortcutKeys } from "~/hooks/useShortcutKeys";
6262
import { ShortcutKey } from "~/components/primitives/ShortcutKey";
6363
import { type loader as tagsLoader } from "~/routes/resources.environments.$envId.runs.tags";
6464
import { type loader as queuesLoader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues";
65-
import { type loader as regionsLoader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions";
65+
import { useRegions } from "~/hooks/useRegions";
6666
import { RegionLabel } from "./RegionLabel";
6767
import { type loader as versionsLoader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.versions";
6868
import { Button } from "../../primitives/Buttons";
@@ -383,6 +383,7 @@ export function RunsFilters(props: RunFiltersProps) {
383383
searchParams.has("runId") ||
384384
searchParams.has("scheduleId") ||
385385
searchParams.has("queues") ||
386+
searchParams.has("regions") ||
386387
searchParams.has("machines") ||
387388
searchParams.has("versions") ||
388389
searchParams.has("errorId") ||
@@ -1292,10 +1293,8 @@ function RegionsDropdown({
12921293
searchValue: string;
12931294
onClose?: () => void;
12941295
}) {
1295-
const organization = useOrganization();
1296-
const project = useProject();
1297-
const environment = useEnvironment();
12981296
const { values, replace } = useSearchParams();
1297+
const regions = useRegions();
12991298

13001299
const handleChange = (values: string[]) => {
13011300
clearSearchValue();
@@ -1308,41 +1307,29 @@ function RegionsDropdown({
13081307

13091308
const selected = values("regions").filter((v) => v !== "");
13101309

1311-
const fetcher = useFetcher<typeof regionsLoader>();
1312-
1313-
useEffect(() => {
1314-
if (fetcher.state === "idle" && fetcher.data === undefined) {
1315-
fetcher.load(
1316-
`/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/regions`
1317-
);
1318-
}
1319-
}, [fetcher.state, fetcher.data, organization.slug, project.slug, environment.slug]);
1320-
13211310
const filtered = useMemo(() => {
13221311
type RegionItem = { masterQueue: string; name: string; location?: string };
13231312
const items: RegionItem[] = [];
13241313

13251314
for (const masterQueue of selected) {
1326-
const known = fetcher.data?.regions.find((r) => r.masterQueue === masterQueue);
1315+
const known = regions.find((r) => r.masterQueue === masterQueue);
13271316
if (!known) {
13281317
items.push({ masterQueue, name: masterQueue });
13291318
}
13301319
}
13311320

1332-
if (fetcher.data) {
1333-
for (const region of fetcher.data.regions) {
1334-
if (!items.some((i) => i.masterQueue === region.masterQueue)) {
1335-
items.push({
1336-
masterQueue: region.masterQueue,
1337-
name: region.name,
1338-
location: region.location,
1339-
});
1340-
}
1321+
for (const region of regions) {
1322+
if (!items.some((i) => i.masterQueue === region.masterQueue)) {
1323+
items.push({
1324+
masterQueue: region.masterQueue,
1325+
name: region.name,
1326+
location: region.location,
1327+
});
13411328
}
13421329
}
13431330

13441331
return matchSorter(items, searchValue, { keys: ["name", "masterQueue"] });
1345-
}, [searchValue, fetcher.data, selected.join(",")]);
1332+
}, [searchValue, regions, selected.join(",")]);
13461333

13471334
return (
13481335
<SelectProvider value={selected} setValue={handleChange} virtualFocus={true}>
@@ -1362,7 +1349,6 @@ function RegionsDropdown({
13621349
render={(props) => (
13631350
<div className="flex items-center justify-stretch">
13641351
<input {...props} placeholder={"Filter by region..."} />
1365-
{fetcher.state === "loading" && <Spinner color="muted" />}
13661352
</div>
13671353
)}
13681354
/>
@@ -1378,9 +1364,7 @@ function RegionsDropdown({
13781364
</SelectItem>
13791365
))
13801366
: null}
1381-
{filtered.length === 0 && fetcher.state !== "loading" && (
1382-
<SelectItem disabled>No regions found</SelectItem>
1383-
)}
1367+
{filtered.length === 0 && <SelectItem disabled>No regions found</SelectItem>}
13841368
</SelectList>
13851369
</SelectPopover>
13861370
</SelectProvider>
@@ -1389,33 +1373,11 @@ function RegionsDropdown({
13891373

13901374
function AppliedRegionsFilter() {
13911375
const { values, del } = useSearchParams();
1392-
const organization = useOrganization();
1393-
const project = useProject();
13941376
const environment = useEnvironment();
1395-
const fetcher = useFetcher<typeof regionsLoader>();
1377+
const knownRegions = useRegions();
13961378

13971379
const regions = values("regions");
13981380

1399-
useEffect(() => {
1400-
if (
1401-
regions.length > 0 &&
1402-
!regions.every((v) => v === "") &&
1403-
fetcher.state === "idle" &&
1404-
fetcher.data === undefined
1405-
) {
1406-
fetcher.load(
1407-
`/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/regions`
1408-
);
1409-
}
1410-
}, [
1411-
regions.join(","),
1412-
fetcher.state,
1413-
fetcher.data,
1414-
organization.slug,
1415-
project.slug,
1416-
environment.slug,
1417-
]);
1418-
14191381
if (environment.type === "DEVELOPMENT") {
14201382
return null;
14211383
}
@@ -1425,7 +1387,7 @@ function AppliedRegionsFilter() {
14251387
}
14261388

14271389
const labels = regions.map((mq) => {
1428-
const match = fetcher.data?.regions.find((r) => r.masterQueue === mq);
1390+
const match = knownRegions.find((r) => r.masterQueue === mq);
14291391
return match?.name ?? mq;
14301392
});
14311393

apps/webapp/app/components/runs/v3/TaskRunsTable.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { useSelectedItems } from "~/components/primitives/SelectedItemsProvider"
2323
import { SimpleTooltip } from "~/components/primitives/Tooltip";
2424
import { TruncatedCopyableValue } from "~/components/primitives/TruncatedCopyableValue";
2525
import { useEnvironment } from "~/hooks/useEnvironment";
26+
import { useRegions } from "~/hooks/useRegions";
2627
import { useFeatures } from "~/hooks/useFeatures";
2728
import { useOrganization } from "~/hooks/useOrganizations";
2829
import { useProject } from "~/hooks/useProject";
@@ -73,7 +74,6 @@ type RunsTableProps = {
7374
variant?: TableVariant;
7475
disableAdjacentRows?: boolean;
7576
additionalTableState?: Record<string, string>;
76-
regions?: { masterQueue: string; name: string; location?: string }[];
7777
};
7878

7979
export function TaskRunsTable({
@@ -87,11 +87,9 @@ export function TaskRunsTable({
8787
allowSelection = false,
8888
variant = "dimmed",
8989
additionalTableState,
90-
regions,
9190
}: RunsTableProps) {
92-
const regionByMasterQueue = new Map(
93-
(regions ?? []).map((r) => [r.masterQueue, r] as const)
94-
);
91+
const regions = useRegions();
92+
const regionByMasterQueue = new Map(regions.map((r) => [r.masterQueue, r] as const));
9593
const organization = useOrganization();
9694
const project = useProject();
9795
const environment = useEnvironment();
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { type UIMatch } from "@remix-run/react";
2+
import { type UseDataFunctionReturn } from "remix-typedjson";
3+
import type { loader as orgLoader } from "~/routes/_app.orgs.$organizationSlug/route";
4+
import { organizationMatchId } from "./useOrganizations";
5+
import { useTypedMatchesData } from "./useTypedMatchData";
6+
7+
export type MatchedRegion = UseDataFunctionReturn<typeof orgLoader>["regions"][number];
8+
9+
export function useRegions(matches?: UIMatch[]): MatchedRegion[] {
10+
const routeMatch = useTypedMatchesData<typeof orgLoader>({
11+
id: organizationMatchId,
12+
matches,
13+
});
14+
15+
return routeMatch?.regions ?? [];
16+
}

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import { findProjectBySlug } from "~/models/project.server";
4444
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
4545
import { getRunFiltersFromRequest } from "~/presenters/RunFilters.server";
4646
import { NextRunListPresenter } from "~/presenters/v3/NextRunListPresenter.server";
47-
import { RegionsPresenter } from "~/presenters/v3/RegionsPresenter.server";
4847
import { clickhouseClient } from "~/services/clickhouseInstance.server";
4948
import {
5049
setRootOnlyFilterPreference,
@@ -95,18 +94,6 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
9594
...filters,
9695
});
9796

98-
const regionsPresenter = new RegionsPresenter();
99-
const regions = await regionsPresenter
100-
.call({ userId, projectSlug: project.slug })
101-
.then(({ regions }) =>
102-
regions.map((r) => ({
103-
masterQueue: r.masterQueue,
104-
name: r.name,
105-
location: r.location,
106-
}))
107-
)
108-
.catch(() => [] as { masterQueue: string; name: string; location?: string }[]);
109-
11097
// Only persist rootOnly when no tasks are filtered. While a task filter is active,
11198
// the toggle's URL value can be a temporary auto-flip (or a user override scoped to
11299
// the current task filter), and we don't want either bleeding into the saved
@@ -125,14 +112,13 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
125112
data: list,
126113
rootOnlyDefault: filters.rootOnly,
127114
filters,
128-
regions,
129115
},
130116
headers ? { headers } : undefined
131117
);
132118
};
133119

134120
export default function Page() {
135-
const { data, rootOnlyDefault, filters, regions } = useTypedLoaderData<typeof loader>();
121+
const { data, rootOnlyDefault, filters } = useTypedLoaderData<typeof loader>();
136122
const { isConnected } = useDevPresence();
137123
const project = useProject();
138124
const environment = useEnvironment();
@@ -191,7 +177,6 @@ export default function Page() {
191177
selectedItems={selectedItems}
192178
rootOnlyDefault={rootOnlyDefault}
193179
filters={filters}
194-
regions={regions}
195180
/>
196181
);
197182
}}
@@ -209,13 +194,11 @@ function RunsList({
209194
selectedItems,
210195
rootOnlyDefault,
211196
filters,
212-
regions,
213197
}: {
214198
list: Awaited<UseDataFunctionReturn<typeof loader>["data"]>;
215199
selectedItems: Set<string>;
216200
rootOnlyDefault: boolean;
217201
filters: TaskRunListSearchFilters;
218-
regions: { masterQueue: string; name: string; location?: string }[];
219202
}) {
220203
const navigation = useNavigation();
221204
const isLoading = navigation.state !== "idle";
@@ -324,7 +307,6 @@ function RunsList({
324307
isLoading={isLoading}
325308
allowSelection
326309
rootOnlyDefault={rootOnlyDefault}
327-
regions={regions}
328310
/>
329311
</div>
330312
)}

apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { prisma } from "~/db.server";
77
import { useOptionalOrganization } from "~/hooks/useOrganizations";
88
import { useTypedMatchesData } from "~/hooks/useTypedMatchData";
99
import { OrganizationsPresenter } from "~/presenters/OrganizationsPresenter.server";
10+
import { RegionsPresenter, type Region } from "~/presenters/v3/RegionsPresenter.server";
1011
import { getImpersonationId } from "~/services/impersonation.server";
1112
import { getCachedUsage, getCurrentPlan } from "~/services/platform.v3.server";
1213
import { requireUser } from "~/services/session.server";
@@ -88,7 +89,10 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
8889
firstDayOfNextMonth.setUTCDate(1);
8990
firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);
9091

91-
const [plan, usage, customDashboards] = await Promise.all([
92+
const shouldLoadRegions =
93+
!!projectParam && !!environment && environment.type !== "DEVELOPMENT";
94+
95+
const [plan, usage, customDashboards, regions] = await Promise.all([
9296
getCurrentPlan(organization.id),
9397
getCachedUsage(organization.id, { from: firstDayOfMonth, to: firstDayOfNextMonth }),
9498
prisma.metricsDashboard.findMany({
@@ -100,6 +104,12 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
100104
},
101105
orderBy: { createdAt: "desc" },
102106
}),
107+
shouldLoadRegions
108+
? new RegionsPresenter()
109+
.call({ userId: user.id, projectSlug: projectParam! })
110+
.then(({ regions }) => regions)
111+
.catch(() => [] as Region[])
112+
: Promise.resolve([] as Region[]),
103113
]);
104114

105115
let hasExceededFreeTier = false;
@@ -147,6 +157,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
147157
organization,
148158
project,
149159
environment,
160+
regions,
150161
isImpersonating: !!impersonationId,
151162
currentPlan: { ...plan, v3Usage: { ...usage, hasExceededFreeTier, usagePercentage } },
152163
customDashboards: customDashboardsWithWidgetCount,

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions.ts

Lines changed: 0 additions & 42 deletions
This file was deleted.

0 commit comments

Comments
 (0)