From 9f87cf009fbcec6cc414439ac73235e7e4ead143 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Thu, 8 May 2025 12:45:09 -0500 Subject: [PATCH] fix frontend type issues with split Job type Issue #347 details how #344 broke some key parts of the frontend due to missing fields in the job list payloads (errors, logs, and metadata were removed from the job payload for the list API). The root issue is that the TypeScript types were not adjusted to reflect the new split Job type, so several parts of the code made reference to things they could no longer access (and didn't actually need anyway). In particular this was true for deserializing job payloads where the same logic was applied to all job responses. To fix this, update the types to have a separate `JobMinimal` and `Job` type just as the API backend does. Update all code that uses them to ensure the correct one is utilized. It's only list view stuff that needs `JobMinimal` for now. Fixes #347. --- src/components/JobList.stories.ts | 4 +-- src/components/JobList.tsx | 12 +++---- src/routes/jobs/index.tsx | 6 ++-- src/services/jobs.ts | 58 ++++++++++++++++++++----------- src/test/factories/job.ts | 48 ++++++++++++++++++++++++- 5 files changed, 95 insertions(+), 33 deletions(-) diff --git a/src/components/JobList.stories.ts b/src/components/JobList.stories.ts index 6827f424..0059c985 100644 --- a/src/components/JobList.stories.ts +++ b/src/components/JobList.stories.ts @@ -1,7 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { JobState } from "@services/types"; -import { jobFactory } from "@test/factories/job"; +import { jobMinimalFactory } from "@test/factories/job"; import JobList from "./JobList"; @@ -16,7 +16,7 @@ type Story = StoryObj; export const Running: Story = { args: { - jobs: jobFactory.running().buildList(10), + jobs: jobMinimalFactory.running().buildList(10), setJobRefetchesPaused: () => {}, state: JobState.Running, }, diff --git a/src/components/JobList.tsx b/src/components/JobList.tsx index f5387d67..c9a57175 100644 --- a/src/components/JobList.tsx +++ b/src/components/JobList.tsx @@ -17,7 +17,7 @@ import { } from "@heroicons/react/24/outline"; import { useSelected } from "@hooks/use-selected"; import { useShiftSelected } from "@hooks/use-shift-selected"; -import { Job } from "@services/jobs"; +import { JobMinimal } from "@services/jobs"; import { StatesAndCounts } from "@services/states"; import { JobState } from "@services/types"; import { Link } from "@tanstack/react-router"; @@ -39,7 +39,7 @@ const states: { [key in JobState]: string } = { [JobState.Scheduled]: "text-rose-400 bg-rose-400/10", }; -const timestampForRelativeDisplay = (job: Job): Date => { +const timestampForRelativeDisplay = (job: JobMinimal): Date => { switch (job.state) { case JobState.Completed: return job.finalizedAt ? job.finalizedAt : new Date(); @@ -50,7 +50,7 @@ const timestampForRelativeDisplay = (job: Job): Date => { } }; -const JobTimeDisplay = ({ job }: { job: Job }): React.JSX.Element => { +const JobTimeDisplay = ({ job }: { job: JobMinimal }): React.JSX.Element => { return ( { type JobListItemProps = { checked: boolean; - job: Job; + job: JobMinimal; onChangeSelect: ( checked: boolean, event: React.ChangeEvent, @@ -159,7 +159,7 @@ export type JobRowsProps = { canShowMore: boolean; deleteJobs: (jobIDs: bigint[]) => void; initialFilters?: Filter[]; - jobs: Job[]; + jobs: JobMinimal[]; onFiltersChange?: (filters: Filter[]) => void; retryJobs: (jobIDs: bigint[]) => void; setJobRefetchesPaused: (value: boolean) => void; @@ -174,7 +174,7 @@ type JobListProps = { canShowMore: boolean; deleteJobs: (jobIDs: bigint[]) => void; initialFilters?: Filter[]; - jobs: Job[]; + jobs: JobMinimal[]; loading?: boolean; onFiltersChange?: (filters: Filter[]) => void; retryJobs: (jobIDs: bigint[]) => void; diff --git a/src/routes/jobs/index.tsx b/src/routes/jobs/index.tsx index dbee116e..63fdcee3 100644 --- a/src/routes/jobs/index.tsx +++ b/src/routes/jobs/index.tsx @@ -5,7 +5,7 @@ import { defaultValues, jobSearchSchema } from "@routes/jobs/index.schema"; import { cancelJobs, deleteJobs, - Job, + JobMinimal, listJobs, ListJobsKey, listJobsKey, @@ -312,9 +312,9 @@ const jobsQueryOptions = ( opts?: { pauseRefetches: boolean; refetchInterval: number }, ) => { const keepPreviousDataUnlessStateChanged: PlaceholderDataFunction< - Job[], + JobMinimal[], Error, - Job[], + JobMinimal[], ListJobsKey > = (previousData, previousQuery) => { if (!previousQuery) return undefined; diff --git a/src/services/jobs.ts b/src/services/jobs.ts index 49966fce..caddc780 100644 --- a/src/services/jobs.ts +++ b/src/services/jobs.ts @@ -24,25 +24,45 @@ export type Job = { : JobFromAPI[Key] extends AttemptErrorFromAPI[] ? AttemptError[] : JobFromAPI[Key]; -} & { +}; + +export type JobFromAPI = { + errors: AttemptErrorFromAPI[]; logs: JobLogs; + metadata: KnownMetadata | object; +} & JobMinimalFromAPI; + +export type JobLogEntry = { + attempt: number; + log: string; +}; + +// New type for better organized logs +export type JobLogs = { + [attempt: number]: string; +}; + +export type JobMinimal = { + [Key in keyof JobMinimalFromAPI as SnakeToCamelCase]: Key extends + | StringEndingWithUnderscoreAt + | undefined + ? Date + : JobMinimalFromAPI[Key]; }; // Represents a Job as received from the API. This just like Job, except with // string dates instead of Date objects and keys as snake_case instead of // camelCase. -export type JobFromAPI = { +export type JobMinimalFromAPI = { args: object; attempt: number; attempted_at?: string; attempted_by: string[]; created_at: string; - errors: AttemptErrorFromAPI[]; finalized_at?: string; id: bigint; kind: string; max_attempts: number; - metadata: KnownMetadata | object; priority: number; queue: string; scheduled_at: string; @@ -50,16 +70,6 @@ export type JobFromAPI = { tags: string[]; }; -export type JobLogEntry = { - attempt: number; - log: string; -}; - -// New type for better organized logs -export type JobLogs = { - [attempt: number]: string; -}; - export type JobWithKnownMetadata = { metadata: KnownMetadata; } & Omit; @@ -87,19 +97,18 @@ type RiverJobLogEntry = { log: string; }; -export const apiJobToJob = (job: JobFromAPI): Job => ({ +export const apiJobMinimalToJobMinimal = ( + job: JobMinimalFromAPI, +): JobMinimal => ({ args: job.args, attempt: job.attempt, attemptedAt: job.attempted_at ? new Date(job.attempted_at) : undefined, attemptedBy: job.attempted_by, createdAt: new Date(job.created_at), - errors: apiAttemptErrorsToAttemptErrors(job.errors), finalizedAt: job.finalized_at ? new Date(job.finalized_at) : undefined, id: BigInt(job.id), kind: job.kind, - logs: extractJobLogs(job.metadata), maxAttempts: job.max_attempts, - metadata: job.metadata, priority: job.priority, queue: job.queue, scheduledAt: new Date(job.scheduled_at), @@ -107,6 +116,13 @@ export const apiJobToJob = (job: JobFromAPI): Job => ({ tags: job.tags, }); +export const apiJobToJob = (job: JobFromAPI): Job => ({ + ...apiJobMinimalToJobMinimal(job), + errors: apiAttemptErrorsToAttemptErrors(job.errors), + logs: extractJobLogs(job.metadata), + metadata: job.metadata, +}); + const apiAttemptErrorsToAttemptErrors = ( errors: AttemptErrorFromAPI[], ): AttemptError[] => { @@ -196,7 +212,7 @@ export const listJobsKey = (args: ListJobsFilters): ListJobsKey => { ]; }; -export const listJobs: QueryFunction = async ({ +export const listJobs: QueryFunction = async ({ queryKey, signal, }) => { @@ -222,13 +238,13 @@ export const listJobs: QueryFunction = async ({ } }); - return API.get>( + return API.get>( { path: "/jobs", query }, { signal }, ).then( // Map from JobFromAPI to Job: // TODO: there must be a cleaner way to do this given the type definitions? - (response) => response.data.map(apiJobToJob), + (response) => response.data.map(apiJobMinimalToJobMinimal), ); }; diff --git a/src/test/factories/job.ts b/src/test/factories/job.ts index e5cf4f80..efd797a6 100644 --- a/src/test/factories/job.ts +++ b/src/test/factories/job.ts @@ -1,5 +1,5 @@ import { faker } from "@faker-js/faker"; -import { AttemptError, Job } from "@services/jobs"; +import { AttemptError, Job, JobMinimal } from "@services/jobs"; import { JobState } from "@services/types"; import { add, sub } from "date-fns"; import { Factory } from "fishery"; @@ -28,6 +28,52 @@ export const attemptErrorFactory = AttemptErrorFactory.define(({ params }) => { }; }); +// Helper type to extract only JobMinimal fields from Job: +type JobMinimalFields = Pick; + +class JobMinimalFactory extends Factory { + available() { + return this.params(jobFactory.available().build()); + } + + cancelled() { + return this.params(jobFactory.cancelled().build()); + } + + completed() { + return this.params(jobFactory.completed().build()); + } + + discarded() { + return this.params(jobFactory.discarded().build()); + } + + pending() { + return this.params(jobFactory.pending().build()); + } + + retryable() { + return this.params(jobFactory.retryable().build()); + } + + running() { + return this.params(jobFactory.running().build()); + } + + scheduled() { + return this.params(jobFactory.scheduled().build()); + } + + scheduledSnoozed() { + return this.params(jobFactory.scheduledSnoozed().build()); + } +} + +export const jobMinimalFactory = JobMinimalFactory.define(({ sequence }) => { + const job = jobFactory.build({ id: BigInt(sequence) }); + return job as JobMinimalFields; +}); + class JobFactory extends Factory { available() { return this.params({});