Merge pull request #2092 from zetkin/undocumented/typedoc

TypeDoc

.gitignore

@@ -43,4 +43,7 @@ yarn-error.log*
 .idea
 
 # typescript
-tsconfig.tsbuildinfo
+tsconfig.tsbuildinfo
+
+# typedoc
+/typedoc/

docs/index.ts

@@ -0,0 +1,9 @@
+export { type RemoteItem, type RemoteList } from 'utils/storeUtils';
+export {
+  loadItemIfNecessary,
+  loadListIfNecessary,
+} from 'core/caching/cacheUtils';
+export { default as shouldLoad } from 'core/caching/shouldLoad';
+export { default as ZUIFuture, type ZUIFutureProps } from 'zui/ZUIFuture';
+export { type IFuture } from 'core/caching/futures';
+export { removeOffset } from 'utils/dateUtils';

docs/testing/jest.md

@@ -0,0 +1,32 @@
+---
+title: Jest
+category: Testing
+---
+
+# Jest
+
+## Running Jest
+
+### Run all tests
+
+```
+yarn test
+```
+
+It can be useful to run the whole test suite if you've changed a few files as part of whatever you're working on.
+
+### Run one test
+
+```
+yarn test src/utils/dateUtils.spec.ts
+```
+
+When you're working on one particular file, you can run its tests by putting the path to them after `yarn test`.
+
+### Watch mode
+
+```
+yarn test --watch src/utils/dateUtils.spec.ts
+```
+
+During focused work on a single file, it can be helpful to use the `--watch` flag to re-run the tests automatically every time you change something.

docs/testing/playwright.md

@@ -0,0 +1,77 @@
+---
+title: Playwright
+category: Testing
+---
+
+# Playwright
+
+## Running Playwright
+
+### Run all tests
+
+```
+yarn playwright
+```
+
+It can be useful to run the whole playwright suite if you've changed a few features as part of whatever you're working on.
+
+### Run one test
+
+```
+yarn playwright tests/organize/views/detail/display.spec.ts
+```
+
+When you're working on one particular feature, you can run its playwright tests by putting the path to them after `yarn playwright`.
+
+### Skip the build
+
+```
+yarn playwright:skipbuild tests/organize/views/detail/display.spec.ts
+```
+
+The `yarn playwright` script builds the Next.js app before running the tests. This takes several minutes, and isn't useful if you're only changing test code (code in a `.spec.ts` file). You can tell playwright not to include this build step with the `playwright:skipbuild` script.
+
+### Debug mode
+
+```
+yarn playwright:skipbuild --debug tests/organize/views/detail/display.spec.ts
+```
+
+In its default mode of execution, Playwright runs the tests in a headless browser. This means you can't see them running or interact with the browser's developer tools to debug any problems that arise. By adding the `--debug` flag, you can tell Playwright to run the tests visually in a Chrome window so you can see what's happening and debug any problems.
+
+## Writing Playwright tests
+
+### Moxy
+
+Zetkins Playwright tests isolate the Next.js app from the back end very thoroughly by using [moxy](https://github.com/zetkin/moxy) to set up placeholder responses. This means the tests can run without the app ever actually communicating with the real back end. The purpose of this is to make the tests more deterministic, which means they should only break because of a problem with the code, because nobody can break them by mistake by changing something in the dev server's database that they depend on.
+
+The [code search results for `moxy.setZetkinApiMock`](https://github.com/search?q=repo%3Azetkin%2Fapp.zetkin.org%20moxy.setZetkinApiMock&type=code) are a great starting point to learn about setting up these API mocks.
+
+### waitForNavigation
+
+A common step in a Playwright test is to trigger a click on a link and then continue after the page load has completed. The intuitive way to write this code would be like so.
+
+```typescript
+await page.click(`text=Next Page`);
+await page.waitForNavigation();
+```
+
+You won't find any examples of code written that way in Zetkin though. The problem with writing it like that is that the navigation can sometimes complete before the `await page.waitForNavigation()` has even happened, leaving the test stranded waiting for a navigation that's already happened. Instead, we set up both steps simultaneously like this.
+
+```typescript
+await Promise.all([
+  page.waitForNavigation(),
+  page.click(`text=${AllMembers.title}`),
+]);
+```
+
+### Locators
+
+Browser automation tests like these are notorious for sometimes failing randomly. It's difficult to avoid making subtle mistakes and introducing race conditions like the `waitForNavigation` issue above. One general-purpose technique that can help with this is to prefer using [locators](https://playwright.dev/docs/locators) instead of `page.click()`. The first thing Playwright's own [documentation for `page.click`](https://playwright.dev/docs/api/class-page#page-click) says is not to use it.
+
+> **Discouraged**<br />
+> Use locator-based [locator.click()](https://playwright.dev/docs/api/class-locator#locator-click) instead. Read more about [locators](https://playwright.dev/docs/locators).
+
+Using locators instead of `page.click()` maximizes our chances of Playwright's auto-waiting and auto-retrying saving us from a meaningless random test failure resulting from a race condition.
+
+There are lots of examples to learn from in the [code search results for `page.locator`](https://github.com/search?q=repo%3Azetkin%2Fapp.zetkin.org%20page.locator&type=code).

docs/time/time.md

@@ -0,0 +1,14 @@
+---
+title: Time
+category: Time
+---
+
+# Time
+
+## Day.js
+
+Zetkin uses [Day.js](https://day.js.org/) to work with times and dates. Plugins used include [UTC](https://day.js.org/docs/en/plugin/utc), [IsoWeek](https://day.js.org/docs/en/plugin/iso-week), and [CustomParseFormat](https://day.js.org/docs/en/plugin/custom-parse-format).
+
+## Serialization Format
+
+The most commonly used date serialization format around the Zetkin front end is `2024-07-23T12:55:14.279Z`. A code search for [`new Date().toISOString()`](<https://github.com/search?q=repo%3Azetkin%2Fapp.zetkin.org%20%22new%20Date().toISOString()%22&type=code>) shows all the places where the current date & time are being serialized using this format by the front end code.

package.json

@@ -155,6 +155,7 @@
     "ts-mockito": "^2.6.1",
     "ts-node": "^10.9.1",
     "tsconfig-paths-webpack-plugin": "^3.5.2",
+    "typedoc": "^0.26.5",
     "typescript": "^5.4.3"
   },
   "engines": {

src/core/caching/cacheUtils.ts

@@ -10,6 +10,30 @@ import {
 } from './futures';
 import { RemoteItem, RemoteList } from 'utils/storeUtils';
 
+/**
+ * Used by data fetching hooks to manage cache invalidation and fetching for their collection.
+ *
+ * A typical call to `loadListIfNecessary` looks like this one.
+ *
+ * ```typescript
+ * const tasksFuture = loadListIfNecessary(tasksList, dispatch, {
+ *   actionOnLoad: () => tasksLoad(),
+ *   actionOnSuccess: (data) => tasksLoaded(data),
+ *   loader: () =>
+ *     apiClient.get<ZetkinTask[]>(
+ *       `/api/orgs/${orgId}/campaigns/${campId}/tasks`
+ *     ),
+ * });
+ * ```
+ *
+ * Under the hood, {@link shouldLoad shouldLoad} is used for cache invalidation.
+ *
+ * @category Cache
+ * @param {RemoteList} remoteList The remote list to check and load.
+ * @param {AppDispatch} dispatch The Redux dispatch function.
+ * @param {Object} hooks Callbacks to handle the loading process.
+ * @return {IFuture} An {@link IFuture} object that can be used to render a loading spinner or the data itself.
+ */
 export function loadListIfNecessary<
   DataType,
   OnLoadPayload = void,
@@ -18,10 +42,35 @@ export function loadListIfNecessary<
   remoteList: RemoteList<DataType> | undefined,
   dispatch: AppDispatch,
   hooks: {
+    /**
+     * Called when an error occurs while loading the list.
+     * @param err The error that occurred during the loading process.
+     * @return {PayloadAction} The action to dispatch when an error occurs.
+     */
     actionOnError?: (err: unknown) => PayloadAction<unknown>;
+
+    /**
+     * Called when the list begins loading.
+     * @returns {PayloadAction} The action to dispatch when the list is loading.
+     */
     actionOnLoad: () => PayloadAction<OnLoadPayload>;
+
+    /**
+     * Called when the list loads successfully.
+     * @returns {PayloadAction} The action to dispatch when the list has loaded.
+     */
     actionOnSuccess: (items: DataType[]) => PayloadAction<OnSuccessPayload>;
+
+    /**
+     * Optionally override {@link shouldLoad shouldLoad} with a custom function.
+     * @returns {boolean} Whether the list should be loaded.
+     */
     isNecessary?: () => boolean;
+
+    /**
+     * The function that loads the list. Typically an API call.
+     * @returns {Promise<DataType[]>}
+     */
     loader: () => Promise<DataType[]>;
   }
 ): IFuture<DataType[]> {
@@ -69,6 +118,29 @@ export function loadList<
   return new PromiseFuture(promise);
 }
 
+/**
+ * Used by data fetching hooks to manage cache invalidation and fetching for their entity.
+ *
+ * A typical call to `loadItemIfNecessary` looks like this one.
+ *
+ * ```typescript
+ * const future = loadItemIfNecessary(submissionItem, dispatch, {
+ *   actionOnLoad: () => submissionLoad(submissionId),
+ *   actionOnSuccess: (data) => submissionLoaded(data),
+ *   loader: () =>
+ *     apiClient.get(`/api/orgs/${orgId}/survey_submissions/${submissionId}`),
+ * });
+ * ```
+ *
+ * Under the hood, {@link shouldLoad shouldLoad} is used for cache invalidation.
+ *
+ *
+ * @category Cache
+ * @param {RemoteItem} remoteItem The remote item to check and load.
+ * @param {AppDispatch} dispatch The Redux dispatch function.
+ * @param {Object} hooks Callbacks to handle the loading process.
+ * @return {IFuture} An {@link IFuture} object that can be used to render a loading spinner or the data itself.
+ */
 export function loadItemIfNecessary<
   DataType,
   OnLoadPayload = void,
@@ -77,8 +149,22 @@ export function loadItemIfNecessary<
   remoteItem: RemoteItem<DataType> | undefined,
   dispatch: AppDispatch,
   hooks: {
+    /**
+     * Called when the item begins loading.
+     * @returns {PayloadAction} The action to dispatch when the item is loading.
+     */
     actionOnLoad: () => PayloadAction<OnLoadPayload>;
+
+    /**
+     * Called when the item loads successfully.
+     * @returns {PayloadAction} The action to dispatch when the item has loaded.
+     */
     actionOnSuccess: (item: DataType) => PayloadAction<OnSuccessPayload>;
+
+    /**
+     * The function that loads the item. Typically an API call.
+     * @returns {Promise<DataType[]>}
+     */
     loader: () => Promise<DataType>;
   }
 ): IFuture<DataType> {

src/core/caching/futures.ts

@@ -1,8 +1,26 @@
 import { RemoteItem, RemoteList } from 'utils/storeUtils';
 
+/**
+ * Encapsulates the state of an asynchronous operation to fetch a piece of data.
+ * Most commonly, this means a network request.
+ *
+ * @category Async
+ */
 export interface IFuture<DataType> {
+  /**
+   * The payload of the asynchronous operation.
+   */
   data: DataType | null;
+
+  /**
+   * General purpose error object where any error that occurs during the
+   * asynchronous operation can be stored.
+   */
   error: unknown | null;
+
+  /**
+   * Denotes whether the operation is currently in progress.
+   */
   isLoading: boolean;
 }
 

src/core/caching/shouldLoad.ts

@@ -27,13 +27,35 @@ function isItem(
   return 'deleted' in thing;
 }
 
+/**
+ * Determines if a remote object needs to be loaded from the server.
+ *
+ * Returns `false` for items that are deleted, currently loading, or have been
+ * loaded within the TTL.
+ *
+ * Returns `true` for items whose data is older than the TTL or has been marked
+ * as stale.
+ *
+ * @category Cache
+ * @param {RemoteItem|RemoteList} item The remote object to check.
+ * @return {boolean} `true` if the object should be loaded, `false` otherwise.
+ */
 export default function shouldLoad(
+  /**
+   * The remote object to check.
+   */
   item: RemoteItem<unknown> | RemoteList<unknown> | undefined
 ): boolean;
+/**
+ * @category Cache
+ */
 export default function shouldLoad(
   item: RemoteObjectRecord | undefined,
   ids: number[]
 ): boolean;
+/**
+ * @category Cache
+ */
 export default function shouldLoad(
   item: ObjThatNeedsLoading,
   idsOrVoid?: number[]

src/utils/dateUtils.ts

@@ -31,6 +31,14 @@ export const getFirstAndLastEvent = (
  *
  * This is needed because event objects send an offset of +00:00 (UTC Time) from the server.
  * It is not needed if the datetime string coming from the server doesn't contain an offset.
+ *
+ * ```typescript
+ * removeOffset("2000-01-01 00:00:00+00:00") // "2000-01-01 00:00:00"
+ * ```
+ *
+ * @category Time
+ * @param {string} datetime ISO datetime string with +00:00 offset
+ * @return {string} ISO datetime string without the +00:00 offset
  */
 export const removeOffset = (datetime: string): string => {
   return datetime.split('+')[0];