TS types for Redux state and selectors in Releases UI (#5488)

* feature: better TS types for API responses

* feature: redux types and fix types in selectors and there's still more to do :'(

* feature: make type more good?

* chore: replace jsonClone calls with builtin structuredClone

* fix: stupid type

* chore: migrate selectors tests to TS

* fix: make Releases.tsx mergeable

* chore: comment docs for some of the types

Author: edisile

static/js/publisher/pages/Releases/Releases.tsx

@@ -5,6 +5,8 @@ import Release from "./Release";
 import { setPageTitle } from "../../utils";
 import Loader from "../../components/Loader";
 
+import { ReleasesAPIResponse } from "../../types/releaseTypes";
+
 function Releases(): React.JSX.Element {
   const { snapId } = useParams();
   const { isLoading, isFetched, data } = useQuery({
@@ -16,7 +18,7 @@ function Releases(): React.JSX.Element {
         throw new Error("There was a problem fetching releases data");
       }
 
-      const responseData = await response.json();
+      const responseData = (await response.json()) as ReleasesAPIResponse;
 
       if (!responseData.success) {
         throw new Error("There was a problem fetching releases data");
@@ -43,6 +45,7 @@ function Releases(): React.JSX.Element {
             flags: {
               isProgressiveReleaseEnabled: true,
             },
+            snapName: data.snap_name,
           }}
         />
       )}

static/js/publisher/pages/Releases/__tests__/helpers.test.js

@@ -90,31 +90,6 @@ describe("isSameVersion", () => {
   });
 });
 
-describe("jsonClone", () => {
-  it("should make a copy of a JS Object Literal", () => {
-    const testJson = {
-      string: "string",
-      number: 12,
-      boolean: true,
-      array: ["string", 12, true],
-    };
-    const result = jsonClone(testJson);
-    expect(result).toEqual(testJson);
-    expect(result).not.toBe(testJson);
-  });
-
-  it("should remove methods", () => {
-    const testJson = {
-      string: "string",
-      function: function () {
-        return "test";
-      },
-    };
-
-    expect(jsonClone(testJson)).toEqual({ string: "string" });
-  });
-});
-
 describe("getTrackGuardrails", () => {
   beforeEach(() => {
     fetch.mockClear();

static/js/publisher/pages/Releases/actions/channelMap.js

@@ -41,6 +41,11 @@ export function releaseRevisionSuccess(revision, channel) {
   };
 }
 
+/**
+ * 
+ * @param {string} channel Channel to close
+ * @returns {{type: string, "payload": {channel: string}}}
+ */
 export function closeChannelSuccess(channel) {
   return {
     type: CLOSE_CHANNEL_SUCCESS,

static/js/publisher/pages/Releases/actions/releases.ts

@@ -26,36 +26,19 @@ import {
   initReleasesData,
 } from "../releasesState";
 import { updateFailedRevisions } from "./failedRevisions";
+import { PendingReleaseItem, Release, ReleasesAPIResponse, DispatchFn, ReleasesReduxState } from "../../../types/releaseTypes";
 
 export const UPDATE_RELEASES = "UPDATE_RELEASES";
 
-interface FetchReleaseResponse {
-  data: {
-    release_history: {
-      revisions: any[];
-      releases: any[];
-    };
-    channel_map: any;
-    snap_name: string;
-  };
-}
-
-function updateReleasesData(apiData: FetchReleaseResponse) {
+// returns a Redux thunk callback that unpacks the API response into the state
+function updateReleasesData(apiData: ReleasesAPIResponse) {
   const {
     release_history: releasesData,
     channel_map: channelMap,
     snap_name: snapName,
   } = apiData.data;
   return (
-    dispatch: (arg0: {
-      type: string;
-      payload:
-        | { releases: any }
-        | { revisions: any }
-        | { architectures: any[] }
-        | { channelMap: any }
-        | { failedRevisions: any };
-    }) => void,
+    dispatch: DispatchFn,
   ) => {
     const revisionsList = releasesData.revisions;
     const releases = releasesData.releases;
@@ -75,7 +58,7 @@ function updateReleasesData(apiData: FetchReleaseResponse) {
   };
 }
 
-export function handleCloseResponse(dispatch: any, json: any, channels: any) {
+export function handleCloseResponse(dispatch: DispatchFn, json: any, channels: any) {
   if (json.success) {
     if (json.closed_channels && json.closed_channels.length > 0) {
       json.closed_channels.forEach((channel: string) => {
@@ -126,7 +109,7 @@ export function getErrorMessage(error: {
 }
 
 export function handleReleaseResponse(
-  dispatch: any,
+  dispatch: DispatchFn,
   json: any,
   release: any,
   revisions: any,
@@ -174,16 +157,12 @@ export function handleReleaseResponse(
 }
 
 export function releaseRevisions() {
-  const mapToRelease = (pendingRelease: {
-    progressive: { percentage: number };
-    revision: { revision: any };
-    channel: any;
-  }) => {
+  const mapToRelease = (pendingRelease: PendingReleaseItem) => {
     let progressive = null;
 
     if (
-      pendingRelease.progressive &&
-      pendingRelease.progressive.percentage < 100
+      pendingRelease?.progressive?.percentage &&
+      pendingRelease?.progressive?.percentage < 100
     ) {
       progressive = pendingRelease.progressive;
     }
@@ -197,23 +176,18 @@ export function releaseRevisions() {
   };
 
   return (
-    dispatch: any,
-    getState: () => {
-      pendingReleases: any;
-      pendingCloses: any;
-      revisions: any;
-      options: any;
-    },
+    dispatch: DispatchFn,
+    getState: () => ReleasesReduxState,
   ) => {
     const { pendingReleases, pendingCloses, revisions, options } = getState();
     const { snapName } = options;
 
     // To dedupe releases
     const progressiveReleases: {
-      id: any;
-      revision: { revision: any };
-      channels: any[];
-      progressive: { percentage: number } | null;
+      id: number;
+      revision: PendingReleaseItem["revision"];
+      channels: PendingReleaseItem["channel"][];
+      progressive: PendingReleaseItem["progressive"] | null;
     }[] = [];
     const regularReleases: Array<any> = [];
     Object.keys(pendingReleases).forEach((revId) => {
@@ -222,7 +196,6 @@ export function releaseRevisions() {
 
         if (pendingRelease.progressive) {
           // first move progressive releases out
-
           progressiveReleases.push(mapToRelease(pendingRelease));
         } else {
           const releaseIndex = regularReleases.findIndex(
@@ -241,17 +214,17 @@ export function releaseRevisions() {
     const releases = progressiveReleases.concat(regularReleases);
 
     const _handleReleaseResponse = (
-      json: { success: any; channel_map_tree: any; errors?: any },
+      json: { success: boolean; channel_map_tree: any; errors?: any },
       release: { id: number; revision: number; channels: string[] }[],
     ) => {
       return handleReleaseResponse(dispatch, json, release, revisions);
     };
 
     const _handleCloseResponse = (json: {
-      success?: any;
-      closed_channels?: any;
-      error?: boolean | undefined;
-      json?: string | undefined;
+      success?: boolean;
+      closed_channels?: string[];
+      error?: boolean;
+      json?: string;
     }) => {
       return handleCloseResponse(dispatch, json, pendingCloses);
     };
@@ -261,7 +234,7 @@ export function releaseRevisions() {
       .then(() => fetchCloses(_handleCloseResponse, snapName, pendingCloses))
       .then(() => fetchSnapReleaseStatus(snapName))
       .then((json) =>
-        dispatch(updateReleasesData(json as unknown as FetchReleaseResponse)),
+        dispatch(updateReleasesData(json)),
       )
       .catch((error) =>
         dispatch(
@@ -277,7 +250,7 @@ export function releaseRevisions() {
   };
 }
 
-export function updateReleases(releases: { revision: number }[]) {
+export function updateReleases(releases: Release[]) {
   return {
     type: UPDATE_RELEASES,
     payload: { releases },

static/js/publisher/pages/Releases/api/releases.js

@@ -1,7 +1,15 @@
+import { ReleasesAPIResponse } from "../../../types/releaseTypes";
 import { DEFAULT_ERROR_MESSAGE as ERROR_MESSAGE } from "../constants";
 
+/**
+ *
+ * @param {*} onComplete
+ * @param {*} releases
+ * @param {string} snapName
+ * @returns
+ */
 export function fetchReleases(onComplete, releases, snapName) {
-  let queue = Promise.resolve(); // Q() in q
+  let queue = Promise.resolve();
 
   // handle releases as a queue
   releases.forEach((release) => {
@@ -10,14 +18,19 @@ export function fetchReleases(onComplete, releases, snapName) {
         snapName,
         release.id,
         release.channels,
-        release.progressive,
+        release.progressive
       ).then((json) => onComplete(json, release));
     }));
   });
 
   return queue;
 }
 
+/**
+ *
+ * @param {string} snapName
+ * @returns {Promise<ReleasesAPIResponse>}
+ */
 export function fetchSnapReleaseStatus(snapName) {
   return fetch(`/api/${snapName}/releases`, {
     method: "GET",
@@ -37,6 +50,11 @@ export function fetchSnapReleaseStatus(snapName) {
     });
 }
 
+/**
+ *
+ * @param {string} snapName
+ * @returns
+ */
 export function fetchRelease(snapName, revision, channels, progressive) {
   const body = {
     name: snapName,
@@ -67,6 +85,13 @@ export function fetchRelease(snapName, revision, channels, progressive) {
     });
 }
 
+/**
+ *
+ * @param {*} onComplete
+ * @param {string} snapName
+ * @param {string[]} channels
+ * @returns
+ */
 export function fetchCloses(onComplete, snapName, channels) {
   if (channels && channels.length) {
     return fetchClose(snapName, channels).then((json) => {
@@ -77,6 +102,12 @@ export function fetchCloses(onComplete, snapName, channels) {
   }
 }
 
+/**
+ *
+ * @param {string} snapName
+ * @param {string[]} channels
+ * @returns
+ */
 export function fetchClose(snapName, channels) {
   return fetch(`/${snapName}/releases/close-channel`, {
     method: "POST",