Update code to match writeup (#28)

* Add `CheckButton` component

* Update task client functions

* Add `onSubmit` callback prop to `TaskForm` and update comments

* Improve backend comments

* Use `data-testid` instead of querying by label text

backend/src/controllers/task.ts

@@ -8,18 +8,37 @@ import { validationResult } from "express-validator";
 import TaskModel from "src/models/task";
 import validationErrorParser from "src/util/validationErrorParser";
 
+/**
+ * This is an example of an Express API request handler. We'll tell Express to
+ * run this function when our backend receives a request to retrieve a
+ * particular task.
+ *
+ * Request handlers typically have 3 parameters: req, res, and next.
+ *
+ * @param req The Request object from Express. This contains all the data from
+ * the API request. (https://expressjs.com/en/4x/api.html#req)
+ * @param res The Response object from Express. We use this to generate the API
+ * response for Express to send back. (https://expressjs.com/en/4x/api.html#res)
+ * @param next The next function in the chain of middleware. If there's no more
+ * processing we can do in this handler, but we're not completely done handling
+ * the request, then we can pass it along by calling next(). For all of the
+ * handlers defined in `src/controllers`, the next function is the global error
+ * handler in `src/app.ts`.
+ */
 export const getTask: RequestHandler = async (req, res, next) => {
   const { id } = req.params;
 
   try {
-    // if the id doesn't exist, returns null
+    // if the ID doesn't exist, then findById returns null
     const task = await TaskModel.findById(id);
 
     if (task === null) {
-      // 404 is the "not found" error
       throw createHttpError(404, "Task not found.");
     }
 
+    // Set the status code (200) and body (the task object as JSON) of the response.
+    // Note that you don't need to return anything, but you can still use a return
+    // statement to exit the function early.
     res.status(200).json(task);
   } catch (error) {
     // pass errors to the error handler
@@ -28,11 +47,12 @@ export const getTask: RequestHandler = async (req, res, next) => {
 };
 
 export const createTask: RequestHandler = async (req, res, next) => {
-  // fetch errors returned by the validator
+  // extract any errors that were found by the validator
   const errors = validationResult(req);
   const { title, description, isChecked } = req.body;
 
   try {
+    // if there are errors, then this function throws an exception
     validationErrorParser(errors);
 
     const task = await TaskModel.create({

backend/src/models/task.ts

@@ -4,6 +4,11 @@ const taskSchema = new Schema({
   title: { type: String, required: true },
   description: { type: String },
   isChecked: { type: Boolean, default: false },
+  // Note that dateCreated has type Date, which is MongoDB's recommended format
+  // for storing dates (as opposed to, say, strings or numbers--see
+  // https://www.mongodb.com/developer/products/mongodb/bson-data-types-date/).
+  // When we send a Task object in the JSON body of an API response, the date
+  // will automatically get "serialized" into a standard date string.
   dateCreated: { type: Date, required: true },
 });
 

backend/src/validators/task.ts

@@ -4,9 +4,10 @@ import { body } from "express-validator";
 export const createTask = [
   body("title")
     // title must exist, if not this message will be displayed
-    // bail prevents the remainder of the validation chain for this field from being executed
     .exists()
     .withMessage("A title is required.")
+    // bail prevents the remainder of the validation chain for this field from being executed if
+    // there was an error
     .bail()
     .notEmpty()
     .withMessage("title cannot be empty.")

frontend/src/api/tasks.ts

@@ -1,4 +1,4 @@
-import { type APIResult, handleAPIError, post } from "src/api/requests";
+import { type APIResult, get, handleAPIError, post } from "src/api/requests";
 
 /**
  * Defines the "shape" of a Task object (what fields are present and their types) for
@@ -58,8 +58,8 @@ export interface CreateTaskRequest {
 }
 
 /**
- * The implementation of this API client function is provided as part of the
- * MVP. You can use it as a guide for writing the other client functions.
+ * The implementations of these API client functions are provided as part of the
+ * MVP. You can use them as a guide for writing the other client functions.
  */
 export async function createTask(task: CreateTaskRequest): Promise<APIResult<Task>> {
   try {
@@ -71,6 +71,12 @@ export async function createTask(task: CreateTaskRequest): Promise<APIResult<Tas
   }
 }
 
-export async function getAllTasks(): Promise<APIResult<Task[]>> {
-  return { success: true, data: [] };
+export async function getTask(id: string): Promise<APIResult<Task>> {
+  try {
+    const response = await get(`/api/task/${id}`);
+    const json = (await response.json()) as TaskJSON;
+    return { success: true, data: parseTask(json) };
+  } catch (error) {
+    return handleAPIError(error);
+  }
 }

frontend/src/components/CheckButton.module.css

@@ -0,0 +1,4 @@
+.button {
+  background-color: transparent;
+  border: none;
+}

frontend/src/components/CheckButton.tsx

@@ -0,0 +1,36 @@
+import React from "react";
+import styles from "src/components/CheckButton.module.css";
+
+export interface CheckButtonProps {
+  checked: boolean;
+  disabled?: boolean;
+  onPress?: () => void;
+  className?: string;
+}
+
+/**
+ * See `src/components/Button.tsx` for an explanation of `React.forwardRef`.
+ * This component uses two SVG images to display the checked/unchecked icons.
+ * Both come from the `public` folder--thanks to Create React App, anything in
+ * that folder is automatically made available as a public file, so it's a good
+ * place to store static icons and images. (You can also view the icons directly
+ * in your browser by visiting, say, `localhost:3000/checkButton.checked.svg`.)
+ */
+export const CheckButton = React.forwardRef<HTMLButtonElement, CheckButtonProps>(
+  function CheckButton({ checked, disabled, onPress, className }: CheckButtonProps, ref) {
+    let buttonClass = styles.button;
+    if (className) {
+      buttonClass += ` ${className}`;
+    }
+    return (
+      <button ref={ref} disabled={disabled} onClick={onPress} className={buttonClass}>
+        <img
+          src={checked ? "/checkButton.checked.svg" : "/checkButton.unchecked.svg"}
+          alt=""
+          width={28}
+          height={28}
+        />
+      </button>
+    );
+  },
+);

frontend/src/components/TaskForm.test.tsx

@@ -4,6 +4,10 @@ import "@testing-library/jest-dom";
 import type { CreateTaskRequest, Task } from "src/api/tasks";
 import { TaskForm, type TaskFormProps } from "src/components/TaskForm";
 
+const TITLE_INPUT_ID = "task-title-input";
+const DESCRIPTION_INPUT_ID = "task-description-input";
+const SAVE_BUTTON_ID = "task-save-button";
+
 /**
  * Some of our tests will verify that `TaskForm` calls the correct functions
  * when the user takes certain actions, like clicking the Save button. This mock
@@ -112,8 +116,8 @@ describe("TaskForm", () => {
       task: mockTask,
     });
     expect(screen.queryByText("Edit task")).toBeInTheDocument();
-    expect(screen.queryByLabelText("Title")).toHaveValue("My task");
-    expect(screen.queryByLabelText("Description")).toHaveValue("Very important");
+    expect(screen.queryByTestId(TITLE_INPUT_ID)).toHaveValue("My task");
+    expect(screen.queryByTestId(DESCRIPTION_INPUT_ID)).toHaveValue("Very important");
   });
 
   it("calls submit handler with edited fields", async () => {
@@ -123,11 +127,11 @@ describe("TaskForm", () => {
       mode: "edit",
       task: mockTask,
     });
-    fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Updated title" } });
-    fireEvent.change(screen.getByLabelText("Description"), {
+    fireEvent.change(screen.getByTestId(TITLE_INPUT_ID), { target: { value: "Updated title" } });
+    fireEvent.change(screen.getByTestId(DESCRIPTION_INPUT_ID), {
       target: { value: "Updated description" },
     });
-    const saveButton = screen.getByText("Save");
+    const saveButton = screen.getByTestId(SAVE_BUTTON_ID);
     fireEvent.click(saveButton);
     expect(mockCreateTask).toHaveBeenCalledTimes(1);
     expect(mockCreateTask).toHaveBeenCalledWith({
@@ -149,7 +153,7 @@ describe("TaskForm", () => {
 
   it("catches invalid title", async () => {
     mountComponent({ mode: "create" });
-    const saveButton = screen.getByText("Save");
+    const saveButton = screen.getByTestId(SAVE_BUTTON_ID);
     fireEvent.click(saveButton);
     expect(mockCreateTask).not.toHaveBeenCalled();
     await waitFor(() => {

frontend/src/components/TaskForm.tsx

@@ -6,6 +6,7 @@ import styles from "src/components/TaskForm.module.css";
 export interface TaskFormProps {
   mode: "create" | "edit";
   task?: Task;
+  onSubmit?: (task: Task) => void;
 }
 
 /**
@@ -16,8 +17,9 @@ export interface TaskFormProps {
  * corresponding input component will show its error state if the field is true.
  * Look at where the `errors` object appears below for demonstration.
  *
- * In the MVP, the only possible error in this form is that the title is blank.
- * You'll add another field which will have more complex potential errors.
+ * In the MVP, the only possible error in this form is that the title is blank,
+ * so this is slightly overengineered. However, a more complex form would need
+ * a similar system.
  */
 interface TaskFormErrors {
   title?: boolean;
@@ -30,8 +32,10 @@ interface TaskFormErrors {
  * @param props.mode Controls how the form renders and submits
  * @param props.task Optional initial data to populate the form with (such as
  * when we're editing an existing task)
+ * @param props.onSubmit Optional callback to run after the user submits the
+ * form and the request succeeds
  */
-export function TaskForm({ mode, task }: TaskFormProps) {
+export function TaskForm({ mode, task, onSubmit }: TaskFormProps) {
   const [title, setTitle] = useState<string>(task?.title || "");
   const [description, setDescription] = useState<string>(task?.description || "");
   const [isLoading, setLoading] = useState<boolean>(false);
@@ -50,6 +54,9 @@ export function TaskForm({ mode, task }: TaskFormProps) {
         // clear the form
         setTitle("");
         setDescription("");
+        // onSubmit may be undefined, in which case we can't call it--this is
+        // a neat way to only call it if it's NOT undefined
+        onSubmit && onSubmit(result.data);
       } else {
         // You should always clearly inform the user when something goes wrong.
         // In this case, we're just doing an `alert()` for brevity, but you'd
@@ -75,16 +82,20 @@ export function TaskForm({ mode, task }: TaskFormProps) {
       we are making a form, so we should use `<form>` */}
       <span className={styles.formTitle}>{formTitle}</span>
       <div className={styles.formRow}>
+        {/* `data-testid` is used by React Testing Library--see the tests in
+        `TaskForm.test.tsx` */}
         <TextField
           className={styles.textField}
+          data-testid="task-title-input"
           label="Title"
           value={title}
           onChange={(event) => setTitle(event.target.value)}
           error={errors.title}
         />
         <TextField
           className={`${styles.textField} ${styles.stretch}`}
-          label="Description"
+          data-testid="task-description-input"
+          label="Description (optional)"
           value={description}
           onChange={(event) => setDescription(event.target.value)}
         />
@@ -93,6 +104,7 @@ export function TaskForm({ mode, task }: TaskFormProps) {
         <Button
           kind="primary"
           type="button"
+          data-testid="task-save-button"
           label="Save"
           disabled={isLoading}
           onClick={handleSubmit}

frontend/src/components/index.ts

@@ -1,6 +1,7 @@
 // the no-relative-import-paths eslint plugin doesn't check exports
 // IMO this is fine for an index file
 export { Button } from "./Button";
+export { CheckButton } from "./CheckButton";
 export { HeaderBar } from "./HeaderBar";
 export { Page } from "./Page";
 export { TaskForm } from "./TaskForm";

frontend/src/pages/Home.tsx

@@ -1,7 +1,7 @@
 import React from "react";
 import { Helmet } from "react-helmet-async";
 import { Link } from "react-router-dom";
-import { Page, TaskForm } from "src/components/";
+import { Page, TaskForm } from "src/components";
 
 export function Home() {
   return (