Merge pull request #801 from MuckRock/allanlasser/strictNull

Stricter type checking for null and undefined values

jsconfig.json

@@ -10,6 +10,8 @@
     "sourceMap": true,
     "strict": false,
     "moduleResolution": "bundler",
+    "noUncheckedIndexedAccess": true,
+    "strictNullChecks": true,
     "types": ["@testing-library/jest-dom"]
   },
   // Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias and https://kit.svelte.dev/docs/configuration#files

src/addons/AddOnPin.svelte

@@ -19,6 +19,10 @@
     event.preventDefault();
 
     const csrftoken = getCsrfToken();
+    if (!csrftoken) {
+      console.error("No CSRF token found");
+      return;
+    }
     const options: RequestInit = {
       credentials: "include",
       method: "PATCH", // this component can only update whether an addon is active or not

src/addons/types.ts

@@ -60,8 +60,8 @@ interface AddOnParameters {
 // https://api.www.documentcloud.org/api/addons/
 export interface AddOnListItem {
   id: number;
-  user: number;
-  organization: number;
+  user: null | number;
+  organization: null | number;
   access: "public" | "private";
   name: string;
   repository: string;

src/api/session.ts

@@ -25,7 +25,7 @@ export function getCsrfToken() {
       ?.split(";")
       ?.map((c) => c.split("="))
       // in case there's spaces in the cookie string, trim the key
-      ?.find(([k, v]) => k.trim() === CSRF_COOKIE_NAME) ?? [];
+      ?.find(([k, v]) => k?.trim() === CSRF_COOKIE_NAME) ?? [];
 
   return token;
 }

src/api/types/project.ts

@@ -7,7 +7,7 @@ export interface Project {
   private: boolean;
   created_at: string;
   updated_at: string;
-  edit_access: boolean;
-  add_remove_access: boolean;
+  edit_access: null | boolean;
+  add_remove_access: null | boolean;
   pinned?: boolean;
 }

src/common/Button.svelte

@@ -23,7 +23,7 @@
   export let type: "submit" | "reset" | "button" = "submit";
   export let label = "Submit";
 
-  export let disabledReason = null;
+  export let disabledReason = "";
 </script>
 
 <Tooltip caption={disabledReason}>

src/common/RelativeTime.svelte

@@ -3,7 +3,7 @@
 
   export let date: Date;
 
-  const relativeFormatter = new Intl.RelativeTimeFormat($locale, {
+  const relativeFormatter = new Intl.RelativeTimeFormat($locale ?? "en", {
     style: "long",
   });
 
@@ -24,23 +24,30 @@
 
   function formatTimeAgo(date: Date): string {
     let duration = (date.getTime() - new Date().getTime()) / 1000;
+    let formatted: string = "";
 
     for (let i = 0; i < DIVISIONS.length; i++) {
-      const division = DIVISIONS[i];
+      const division = DIVISIONS[i]!;
       if (Math.abs(duration) < division.amount) {
-        return relativeFormatter.format(Math.round(duration), division.name);
+        formatted = relativeFormatter.format(
+          Math.round(duration),
+          division.name,
+        );
+        break;
       }
       duration /= division.amount;
     }
+
+    return formatted;
   }
 </script>
 
+<time datetime={date.toISOString()} title={date.toISOString()}>
+  {formatTimeAgo(date)}
+</time>
+
 <style>
   time {
     cursor: default;
   }
 </style>
-
-<time datetime={date.toISOString()} title={date.toISOString()}>
-  {formatTimeAgo(date)}
-</time>

src/langs/json/en.json

@@ -1046,7 +1046,8 @@
       "memberMessage": "This Premium Add-On uses AI to perform advanced analysis. Contact your organization admin about upgrading your plan."
     },
     "dispatch": "Dispatch",
-    "history": "History"
+    "history": "History",
+    "noHistory": "You have not run this add-on before"
   },
   "addonBrowserDialog": {
     "title": "Browse Add-Ons",
@@ -1108,6 +1109,7 @@
   "mailkey": {
     "title": "Upload via email",
     "description": "<p>You can upload documents to your account by sending them to to a special email address as attachments.</p><p><strong>For security reasons, this email is only shown to you once. Please copy it to a secure location.</strong></p><p>This address will accept attachments from any email account. Documents are uploaded as private. Generating a new upload address will disable any previously created addresses.</p><p>We’d love your feedback and to hear about creative use cases at <a href=\"mailto:[email protected]\" target=\"_blank\">[email protected]</a>.</p>",
+    "missing_csrf": "Missing CSRF token",
     "create": {
       "button": "Create new email address",
       "success": "<strong>Upload email address succesfully created</strong>:<br> <a href=\"mailto:{mailkey}@uploads.documentcloud.org\" target=\"_blank\">{mailkey}@uploads.documentcloud.org</a>",
@@ -1142,6 +1144,7 @@
   "processing": {
     "addons": "Add-Ons",
     "documents": "Documents",
+    "unknown": "Unknown",
     "totalCount": "{n} active {n, plural, one {process} other {processes}}"
   },
   "feedback": {

src/lib/api/addons.ts

@@ -69,10 +69,11 @@ export async function getAddon(
   const repository = [owner, repo].join("/");
   const { data: addons, error } = await getAddons({ repository }, fetch);
   // there should only be one result, if the addon exists
-  if (error || addons.results.length < 1) {
+
+  if (error || !addons || addons.results.length < 1) {
     return null;
   }
-  return addons.results[0];
+  return addons.results[0] ?? null;
 }
 
 export async function getEvent(
@@ -298,17 +299,15 @@ export function buildPayload(
   const payload: AddOnPayload = { addon: addon.id, parameters };
 
   const selection = formData.get("selection");
+  const documents = formData.get("documents");
+  const query = formData.get("query");
 
-  if (selection === "selected") {
-    payload.documents = formData
-      .get("documents")
-      .toString()
-      .split(",")
-      .map(Number);
+  if (selection === "selected" && documents) {
+    payload.documents = documents.toString().split(",").map(Number);
   }
 
-  if (selection === "query") {
-    payload.query = formData.get("query").toString();
+  if (selection === "query" && query) {
+    payload.query = query.toString();
   }
 
   if (formData.has("event")) {

src/lib/api/documents.ts

@@ -252,7 +252,7 @@ export async function process(
   }[],
   csrf_token: string,
   fetch = globalThis.fetch,
-): Promise<APIResponse<"OK", unknown>> {
+): Promise<APIResponse<null, unknown>> {
   const endpoint = new URL("documents/process/", BASE_API_URL);
 
   const resp = await fetch(endpoint, {
@@ -561,9 +561,9 @@ export function pageFromHash(hash: string): Nullable<number> {
   const re = /^#document\/p(\d+)/; // match pages and notes
   const match = re.exec(hash);
 
-  if (!match) return null;
+  if (!match || !match[1]) return null;
 
-  return +match[1] || null;
+  return +match[1];
 }
 
 /**

src/lib/api/notes.ts

@@ -4,6 +4,8 @@ import type {
   Document,
   Note,
   NoteResults,
+  Nullable,
+  Page,
   ValidationError,
 } from "./types";
 
@@ -22,18 +24,19 @@ import { getApiResponse, isErrorCode } from "../utils";
  * @example https://api.www.documentcloud.org/api/documents/2622/notes/
  * @deprecated
  */
-export async function list(doc_id: number, fetch = globalThis.fetch) {
+export async function list(
+  doc_id: number,
+  fetch = globalThis.fetch,
+): Promise<APIResponse<Page<Note>>> {
   const endpoint = new URL(`documents/${doc_id}/notes/`, BASE_API_URL);
 
   endpoint.searchParams.set("expand", DEFAULT_EXPAND);
 
-  const resp = await fetch(endpoint, { credentials: "include" });
-
-  if (isErrorCode(resp.status)) {
-    throw new Error(resp.statusText);
-  }
+  const resp = await fetch(endpoint, { credentials: "include" }).catch(
+    console.error,
+  );
 
-  return resp.json();
+  return getApiResponse<Page<Note>>(resp);
 }
 
 /**
@@ -176,13 +179,12 @@ export function noteHashUrl(note: Note): string {
  * To get the page number, use pageFromHash
  * @param hash
  */
-export function noteFromHash(hash: string): number {
+export function noteFromHash(hash: string): Nullable<number> {
   const re = /^#document\/p(\d+)\/a(\d+)$/;
   const match = re.exec(hash);
 
-  if (!match) return null;
-
-  return +match[2] || null;
+  if (!match || !match[2]) return null;
+  return +match[2];
 }
 
 /** Width of a note, relative to the document */

src/lib/api/sections.ts

@@ -74,11 +74,17 @@ export async function get(
 export async function create(
   doc_id: string | number,
   section: { page_number: number; title: string },
-  csrf_token: string,
+  csrf_token: string | undefined,
   fetch = globalThis.fetch,
 ): Promise<APIResponse<Section, ValidationError>> {
   const endpoint = new URL(`documents/${doc_id}/sections/`, BASE_API_URL);
 
+  if (!csrf_token) {
+    return Promise.reject({
+      error: { status: 403, message: "CSRF token required" },
+    });
+  }
+
   const resp = await fetch(endpoint, {
     credentials: "include",
     body: JSON.stringify(section),
@@ -100,14 +106,20 @@ export async function update(
   doc_id: string | number,
   section_id: string | number,
   section: { page_number?: number; title?: string },
-  csrf_token: string,
+  csrf_token: string | undefined,
   fetch = globalThis.fetch,
 ): Promise<APIResponse<Section, ValidationError>> {
   const endpoint = new URL(
     `documents/${doc_id}/sections/${section_id}/`,
     BASE_API_URL,
   );
 
+  if (!csrf_token) {
+    return Promise.reject({
+      error: { status: 403, message: "CSRF token required" },
+    });
+  }
+
   const resp = await fetch(endpoint, {
     credentials: "include",
     body: JSON.stringify(section),
@@ -128,14 +140,20 @@ export async function update(
 export async function remove(
   doc_id: string | number,
   section_id: string | number,
-  csrf_token: string,
+  csrf_token: string | undefined,
   fetch = globalThis.fetch,
 ): Promise<APIResponse<null, unknown>> {
   const endpoint = new URL(
     `documents/${doc_id}/sections/${section_id}/`,
     BASE_API_URL,
   );
 
+  if (!csrf_token) {
+    return Promise.reject({
+      error: { status: 403, message: "CSRF token required" },
+    });
+  }
+
   const resp = await fetch(endpoint, {
     credentials: "include",
     method: "DELETE",

src/lib/api/tests/addons.test.ts

@@ -103,7 +103,7 @@ describe("getAddon", async () => {
 
 describe("addon payloads", () => {
   test("buildPayload single dispatch", () => {
-    const scraper = addonsList.results.find((a) => a.name === "Scraper");
+    const scraper = addonsList.results.find((a) => a.name === "Scraper")!;
     const parameters = {
       site: "https://www.documentcloud.org",
       project: "test",
@@ -118,7 +118,7 @@ describe("addon payloads", () => {
   });
 
   test("buildPayload scheduled event", () => {
-    const scraper = addonsList.results.find((a) => a.name === "Scraper");
+    const scraper = addonsList.results.find((a) => a.name === "Scraper")!;
     const parameters = {
       site: "https://www.documentcloud.org",
       project: "test",
@@ -140,7 +140,7 @@ describe("addon payloads", () => {
   test("buildPayload array param", () => {
     const siteSnapshot = addonsList.results.find(
       (a) => a.name === "Site Snapshot",
-    );
+    )!;
     const parameters = {
       sites: ["https://www.muckrock.com", "https://www.documentcloud.org"],
       project_id: 1,
@@ -159,7 +159,7 @@ describe("addon payloads", () => {
   });
 
   test("buildPayload remove blank values", () => {
-    const scraper = addonsList.results.find((a) => a.name === "Scraper");
+    const scraper = addonsList.results.find((a) => a.name === "Scraper")!;
 
     const parameters = {
       site: "https://www.documentcloud.org",
@@ -187,7 +187,7 @@ describe("addon payloads", () => {
   test("buildPayload with documents", () => {
     const translate = addonsList.results.find(
       (a) => a.name === "Translate Documents",
-    );
+    )!;
     const parameters = {
       access_level: "public",
       project_id: 1,
@@ -219,7 +219,7 @@ describe("addon payloads", () => {
   test("buildPayload with query", () => {
     const translate = addonsList.results.find(
       (a) => a.name === "Translate Documents",
-    );
+    )!;
     const parameters = {
       access_level: "public",
       project_id: 1,

src/lib/api/tests/collaborators.test.ts

@@ -74,7 +74,7 @@ describe("manage project users", () => {
 
     const { data } = await collaborators.add(
       project.id,
-      { email: me.email, access: "admin" },
+      { email: me.email!, access: "admin" },
       "token",
       mockFetch,
     );
@@ -105,7 +105,7 @@ describe("manage project users", () => {
       const { user, access } = JSON.parse(options.body);
       const updated: ProjectUser = users.results.find(
         (u) => u.user.id === 1020,
-      );
+      )!;
 
       return {
         ok: true,
@@ -119,7 +119,7 @@ describe("manage project users", () => {
       };
     });
 
-    const me = users.results.find((u) => u.user.id === 1020);
+    const me = users.results.find((u) => u.user.id === 1020)!;
 
     const { data: updated } = await collaborators.update(
       project.id,
@@ -154,7 +154,7 @@ describe("manage project users", () => {
       };
     });
 
-    const me = users.results.find((u) => u.user.id === 1020);
+    const me = users.results.find((u) => u.user.id === 1020)!;
     const { data } = await collaborators.remove(
       project.id,
       me.user.id,