ui: move jobs details to keyed reducer, simplify polling

This commit moves the jobs details to a keyed reducer
and removes the invalidating receivedJobSaga, in favor of
the component-level refresh interval.

Release note: None

pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts

@@ -19,6 +19,14 @@ export type JobsResponse = cockroach.server.serverpb.JobsResponse;
 
 export type JobRequest = cockroach.server.serverpb.JobRequest;
 export type JobResponse = cockroach.server.serverpb.JobResponse;
+export type JobResponseWithKey = {
+  jobResponse: JobResponse;
+  key: string;
+};
+export type ErrorWithKey = {
+  err: Error;
+  key: string;
+};
 
 export const getJobs = (
   req: JobsRequest,

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx

@@ -76,7 +76,9 @@ export class JobDetails extends React.Component<JobDetailsProps> {
   }
 
   componentDidMount(): void {
-    this.refresh();
+    if (!this.props.job) {
+      this.refresh();
+    }
     // Refresh every 10s.
     this.refreshDataInterval = setInterval(() => this.refresh(), 10 * 1000);
   }
@@ -154,7 +156,7 @@ export class JobDetails extends React.Component<JobDetailsProps> {
 
   render(): React.ReactElement {
     const isLoading = !this.props.job || this.props.jobLoading;
-    const error = this.props.job && this.props.jobError;
+    const error = this.props.jobError;
     return (
       <div className={jobCx("job-details")}>
         <Helmet title={"Details | Job"} />

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetailsConnected.tsx

@@ -12,7 +12,11 @@ import { connect } from "react-redux";
 import { RouteComponentProps, withRouter } from "react-router-dom";
 
 import { AppState } from "src/store";
-import { selectJobState } from "../../store/jobDetails/job.selectors";
+import {
+  selectJob,
+  selectJobLoading,
+  selectJobError,
+} from "../../store/jobDetails/job.selectors";
 import {
   JobDetailsStateProps,
   JobDetailsDispatchProps,
@@ -21,11 +25,13 @@ import {
 import { JobRequest } from "src/api/jobsApi";
 import { actions as jobActions } from "src/store/jobDetails";
 
-const mapStateToProps = (state: AppState): JobDetailsStateProps => {
-  const jobState = selectJobState(state);
-  const job = jobState ? jobState.data : null;
-  const jobLoading = jobState ? jobState.inFlight : false;
-  const jobError = jobState ? jobState.lastError : null;
+const mapStateToProps = (
+  state: AppState,
+  props: RouteComponentProps,
+): JobDetailsStateProps => {
+  const job = selectJob(state, props);
+  const jobLoading = selectJobLoading(state, props);
+  const jobError = selectJobError(state, props);
   return {
     job,
     jobLoading,

pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.reducer.ts

@@ -9,7 +9,12 @@
 // licenses/APL.txt.
 
 import { createSlice, PayloadAction } from "@reduxjs/toolkit";
-import { JobRequest, JobResponse } from "src/api/jobsApi";
+import {
+  ErrorWithKey,
+  JobRequest,
+  JobResponse,
+  JobResponseWithKey,
+} from "src/api/jobsApi";
 import { DOMAIN_NAME } from "../utils";
 
 export type JobState = {
@@ -19,33 +24,46 @@ export type JobState = {
   inFlight: boolean;
 };
 
-const initialState: JobState = {
-  data: null,
-  lastError: null,
-  valid: true,
-  inFlight: false,
+export type JobDetailsReducerState = {
+  cachedData: {
+    [id: string]: JobState;
+  };
+};
+
+const initialState: JobDetailsReducerState = {
+  cachedData: {},
 };
 
 const JobSlice = createSlice({
   name: `${DOMAIN_NAME}/job`,
   initialState,
   reducers: {
-    received: (state, action: PayloadAction<JobResponse>) => {
-      state.data = action.payload;
-      state.valid = true;
-      state.lastError = null;
-      state.inFlight = false;
+    received: (state, action: PayloadAction<JobResponseWithKey>) => {
+      state.cachedData[action.payload.key] = {
+        data: action.payload.jobResponse,
+        valid: true,
+        lastError: null,
+        inFlight: false,
+      };
     },
-    failed: (state, action: PayloadAction<Error>) => {
-      state.valid = false;
-      state.lastError = action.payload;
+    failed: (state, action: PayloadAction<ErrorWithKey>) => {
+      state.cachedData[action.payload.key] = {
+        data: null,
+        valid: false,
+        lastError: action.payload.err,
+        inFlight: false,
+      };
     },
-    invalidated: state => {
-      state.valid = false;
+    invalidated: (state, action: PayloadAction<{ key: string }>) => {
+      state.cachedData[action.payload.key] = {
+        data: null,
+        valid: false,
+        lastError: null,
+        inFlight: false,
+      };
     },
-    refresh: (_, action: PayloadAction<JobRequest>) => {},
-    request: (_, action: PayloadAction<JobRequest>) => {},
-    reset: (_, action: PayloadAction<JobRequest>) => {},
+    refresh: (_, _action: PayloadAction<JobRequest>) => {},
+    request: (_, _action: PayloadAction<JobRequest>) => {},
   },
 });
 

pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.spec.ts

@@ -16,28 +16,46 @@ import {
 } from "redux-saga-test-plan/providers";
 import * as matchers from "redux-saga-test-plan/matchers";
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
-
-import { getJob } from "src/api/jobsApi";
-import { refreshJobSaga, requestJobSaga, receivedJobSaga } from "./job.sagas";
-import { actions, reducer, JobState } from "./job.reducer";
+import {
+  ErrorWithKey,
+  getJob,
+  JobRequest,
+  JobResponseWithKey,
+} from "src/api/jobsApi";
+import { refreshJobSaga, requestJobSaga } from "./job.sagas";
+import { actions, reducer, JobDetailsReducerState } from "./job.reducer";
 import { succeededJobFixture } from "../../jobs/jobsPage/jobsPage.fixture";
 import Long from "long";
+import { PayloadAction } from "@reduxjs/toolkit";
 
 describe("job sagas", () => {
   const payload = new cockroach.server.serverpb.JobRequest({
     job_id: new Long(8136728577, 70289336),
   });
+
+  const jobID = payload.job_id.toString();
+
   const jobResponse = new cockroach.server.serverpb.JobResponse(
     succeededJobFixture,
   );
 
+  const jobResponseWithKey: JobResponseWithKey = {
+    key: jobID,
+    jobResponse: jobResponse,
+  };
+
   const jobAPIProvider: (EffectProviders | StaticProvider)[] = [
     [matchers.call.fn(getJob), jobResponse],
   ];
 
+  const action: PayloadAction<JobRequest> = {
+    payload: payload,
+    type: "request",
+  };
+
   describe("refreshJobSaga", () => {
     it("dispatches refresh job action", () => {
-      return expectSaga(refreshJobSaga, actions.request(payload))
+      return expectSaga(refreshJobSaga, action)
         .provide(jobAPIProvider)
         .put(actions.request(payload))
         .run();
@@ -46,54 +64,44 @@ describe("job sagas", () => {
 
   describe("requestJobSaga", () => {
     it("successfully requests job", () => {
-      return expectSaga(requestJobSaga, actions.request(payload))
+      return expectSaga(requestJobSaga, action)
         .provide(jobAPIProvider)
-        .put(actions.received(jobResponse))
+        .put(actions.received(jobResponseWithKey))
         .withReducer(reducer)
-        .hasFinalState<JobState>({
-          data: jobResponse,
-          lastError: null,
-          valid: true,
-          inFlight: false,
+        .hasFinalState<JobDetailsReducerState>({
+          cachedData: {
+            [jobID]: {
+              data: jobResponseWithKey.jobResponse,
+              lastError: null,
+              valid: true,
+              inFlight: false,
+            },
+          },
         })
         .run();
     });
 
     it("returns error on failed request", () => {
       const error = new Error("Failed request");
-      return expectSaga(requestJobSaga, actions.request(payload))
+      const errorWithKey: ErrorWithKey = {
+        key: jobID,
+        err: error,
+      };
+      return expectSaga(requestJobSaga, action)
         .provide([[matchers.call.fn(getJob), throwError(error)]])
-        .put(actions.failed(error))
+        .put(actions.failed(errorWithKey))
         .withReducer(reducer)
-        .hasFinalState<JobState>({
-          data: null,
-          lastError: error,
-          valid: false,
-          inFlight: false,
+        .hasFinalState<JobDetailsReducerState>({
+          cachedData: {
+            [jobID]: {
+              data: null,
+              lastError: error,
+              valid: false,
+              inFlight: false,
+            },
+          },
         })
         .run();
     });
   });
-
-  describe("receivedJobSaga", () => {
-    it("sets valid status to false after specified period of time", () => {
-      const timeout = 500;
-      return expectSaga(receivedJobSaga, timeout)
-        .delay(timeout)
-        .put(actions.invalidated())
-        .withReducer(reducer, {
-          data: jobResponse,
-          lastError: null,
-          valid: true,
-          inFlight: false,
-        })
-        .hasFinalState<JobState>({
-          data: jobResponse,
-          lastError: null,
-          valid: false,
-          inFlight: false,
-        })
-        .run(1000);
-    });
-  });
 });

pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.ts

@@ -8,43 +8,37 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-import { all, call, delay, put, takeLatest } from "redux-saga/effects";
+import { all, call, put, takeLatest } from "redux-saga/effects";
 
 import { actions } from "./job.reducer";
-import { getJob, JobRequest } from "src/api/jobsApi";
-import { CACHE_INVALIDATION_PERIOD, throttleWithReset } from "../utils";
-import { rootActions } from "../reducers";
+import { getJob, JobRequest, JobResponseWithKey } from "src/api/jobsApi";
 import { PayloadAction } from "@reduxjs/toolkit";
+import { ErrorWithKey } from "../../api";
 
 export function* refreshJobSaga(action: PayloadAction<JobRequest>) {
   yield put(actions.request(action.payload));
 }
 
 export function* requestJobSaga(action: PayloadAction<JobRequest>): any {
+  const key = action.payload.job_id.toString();
   try {
     const result = yield call(getJob, action.payload);
-    yield put(actions.received(result));
+    const resultWithKey: JobResponseWithKey = {
+      key: key,
+      jobResponse: result,
+    };
+    yield put(actions.received(resultWithKey));
   } catch (e) {
-    yield put(actions.failed(e));
+    const err: ErrorWithKey = {
+      err: e,
+      key,
+    };
+    yield put(actions.failed(err));
   }
 }
-
-export function* receivedJobSaga(delayMs: number) {
-  yield delay(delayMs);
-  yield put(actions.invalidated());
-}
-
-export function* jobSaga(
-  cacheInvalidationPeriod: number = CACHE_INVALIDATION_PERIOD,
-) {
+export function* jobSaga() {
   yield all([
-    throttleWithReset(
-      cacheInvalidationPeriod,
-      actions.refresh,
-      [actions.invalidated, rootActions.resetState],
-      refreshJobSaga,
-    ),
+    takeLatest(actions.refresh, refreshJobSaga),
     takeLatest(actions.request, requestJobSaga),
-    takeLatest(actions.received, receivedJobSaga, cacheInvalidationPeriod),
   ]);
 }

pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.selectors.ts

@@ -9,9 +9,53 @@
 // licenses/APL.txt.
 
 import { createSelector } from "reselect";
-import { adminUISelector } from "../utils/selectors";
+import { adminUISelector } from "src/store/utils/selectors";
+import { selectID } from "src/selectors";
+import { JobResponse } from "src/api/jobsApi";
 
-export const selectJobState = createSelector(
-  adminUISelector,
-  adminUiState => adminUiState.job,
+const selectJobState = createSelector(adminUISelector, state => {
+  const jobState = state?.job?.cachedData;
+  const emptyJobCache = !jobState || Object.keys(jobState).length === 0;
+  if (emptyJobCache) {
+    return null;
+  }
+  return jobState;
+});
+
+export const selectJob = createSelector(
+  [adminUISelector, selectJobState, selectID],
+  (adminUIState, jobState, jobID) => {
+    const jobsCache = adminUIState?.jobs?.data;
+    let job: JobResponse;
+    if (!jobID || (!jobsCache && !jobState)) {
+      return null;
+    } else if (jobsCache) {
+      job = Object(jobsCache.jobs.find(job => job.id.toString() === jobID));
+    } else if (jobState) {
+      job = jobState[jobID]?.data;
+    }
+    return job;
+  },
+);
+
+export const selectJobError = createSelector(
+  selectJobState,
+  selectID,
+  (state, jobID) => {
+    if (!state || !jobID) {
+      return null;
+    }
+    return state[jobID]?.lastError;
+  },
+);
+
+export const selectJobLoading = createSelector(
+  selectJobState,
+  selectID,
+  (state, jobID) => {
+    if (!state || !jobID) {
+      return null;
+    }
+    return state[jobID]?.inFlight;
+  },
 );