-
-
Notifications
You must be signed in to change notification settings - Fork 839
Tags listing now uses ClickHouse #2576
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 |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { type LoaderFunctionArgs } from "@remix-run/server-runtime"; | ||
import { z } from "zod"; | ||
import { timeFilters } from "~/components/runs/v3/SharedFilters"; | ||
import { $replica } from "~/db.server"; | ||
import { RunTagListPresenter } from "~/presenters/v3/RunTagListPresenter.server"; | ||
import { requireUserId } from "~/services/session.server"; | ||
|
||
const Params = z.object({ | ||
envId: z.string(), | ||
}); | ||
|
||
const SearchParams = z.object({ | ||
name: z.string().optional(), | ||
period: z.preprocess((value) => (value === "all" ? undefined : value), z.string().optional()), | ||
from: z.coerce.number().optional(), | ||
to: z.coerce.number().optional(), | ||
}); | ||
|
||
export async function loader({ request, params }: LoaderFunctionArgs) { | ||
const userId = await requireUserId(request); | ||
const { envId } = Params.parse(params); | ||
|
||
const environment = await $replica.runtimeEnvironment.findFirst({ | ||
select: { | ||
id: true, | ||
projectId: true, | ||
organizationId: true, | ||
}, | ||
where: { id: envId, organization: { members: { some: { userId } } } }, | ||
}); | ||
|
||
if (!environment) { | ||
throw new Response("Not Found", { status: 404 }); | ||
} | ||
|
||
const search = new URL(request.url).searchParams; | ||
const name = search.get("name"); | ||
|
||
const parsedSearchParams = SearchParams.safeParse({ | ||
name: name ? decodeURIComponent(name) : undefined, | ||
period: search.get("period") ?? undefined, | ||
from: search.get("from") ?? undefined, | ||
to: search.get("to") ?? undefined, | ||
}); | ||
|
||
if (!parsedSearchParams.success) { | ||
throw new Response("Invalid search params", { status: 400 }); | ||
} | ||
|
||
const { period, from, to } = timeFilters({ | ||
period: parsedSearchParams.data.period, | ||
from: parsedSearchParams.data.from, | ||
to: parsedSearchParams.data.to, | ||
}); | ||
|
||
const presenter = new RunTagListPresenter(); | ||
const result = await presenter.call({ | ||
environmentId: environment.id, | ||
projectId: environment.projectId, | ||
organizationId: environment.organizationId, | ||
name: parsedSearchParams.data.name, | ||
period, | ||
from, | ||
to, | ||
}); | ||
return result; | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import { | |
type ListedRun, | ||
type RunListInputOptions, | ||
type RunsRepositoryOptions, | ||
type TagListOptions, | ||
convertRunListInputOptionsToFilterRunsOptions, | ||
} from "./runsRepository.server"; | ||
|
||
|
@@ -104,6 +105,32 @@ export class PostgresRunsRepository implements IRunsRepository { | |
return Number(result[0].count); | ||
} | ||
|
||
async listTags({ projectId, query, offset, limit }: TagListOptions) { | ||
const tags = await this.options.prisma.taskRunTag.findMany({ | ||
select: { | ||
name: true, | ||
}, | ||
where: { | ||
projectId, | ||
name: query | ||
? { | ||
startsWith: query, | ||
mode: "insensitive", | ||
} | ||
: undefined, | ||
}, | ||
orderBy: { | ||
id: "desc", | ||
}, | ||
take: limit + 1, | ||
skip: offset, | ||
}); | ||
|
||
return { | ||
tags: tags.map((tag) => tag.name), | ||
}; | ||
} | ||
Comment on lines
+108
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical type mismatch and missing filter implementations. The method signature destructures only
Apply this diff to align with the interface contract: - async listTags({ projectId, query, offset, limit }: TagListOptions) {
+ async listTags({ organizationId, environmentId, projectId, query, period, from, to, offset, limit }: TagListOptions) {
+ // Build time filter conditions
+ const timeConditions: Prisma.TaskRunTagWhereInput = {};
+
+ if (period) {
+ const periodMs = parseDuration(period);
+ if (periodMs) {
+ timeConditions.createdAt = {
+ gte: new Date(Date.now() - periodMs),
+ };
+ }
+ }
+
+ if (from) {
+ timeConditions.createdAt = {
+ ...timeConditions.createdAt,
+ gte: new Date(from),
+ };
+ }
+
+ if (to) {
+ timeConditions.createdAt = {
+ ...timeConditions.createdAt,
+ lte: new Date(to),
+ };
+ }
+
const tags = await this.options.prisma.taskRunTag.findMany({
select: {
name: true,
},
where: {
+ project: {
+ organizationId,
+ },
projectId,
+ taskRun: {
+ runtimeEnvironmentId: environmentId,
+ },
name: query
? {
- startsWith: query,
+ contains: query,
mode: "insensitive",
}
: undefined,
+ ...timeConditions,
},
orderBy: {
id: "desc",
},
- take: limit + 1,
+ take: limit,
skip: offset,
});
return {
tags: tags.map((tag) => tag.name),
};
} Note: You'll need to import
🤖 Prompt for AI Agents
|
||
|
||
#buildRunIdsQuery( | ||
filterOptions: FilterRunsOptions, | ||
page: { size: number; cursor?: string; direction?: "forward" | "backward" } | ||
|
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.
Honor query and pagination when listing tags.
listTags
ignoresoptions.query
,options.limit
, andoptions.offset
, so/runs/tags
always scans everything and thename=
filter from the presenter is a no-op. That’s a regression from the previous implementation and makeshasMore
meaningless. You need to push the filtering and pagination into ClickHouse before executing the query. For example, apply the name filter:Make sure the ClickHouse builder’s
limit
call (or equivalent) honours bothlimit
andoffset
so the response aligns with the requested page.📝 Committable suggestion
🤖 Prompt for AI Agents