diff --git a/.gitignore b/.gitignore index 6a7d6d8..54ddd35 100644 --- a/.gitignore +++ b/.gitignore @@ -127,4 +127,11 @@ dist .yarn/unplugged .yarn/build-state.yml .yarn/install-state.gz -.pnp.* \ No newline at end of file +.pnp.* + +# Reviewer notes +ANSWERS.md +PRIORITY_FEATURE.md +planIrfan.md +learning.md +bug.md \ No newline at end of file diff --git a/INSTRUCTION.md b/INSTRUCTION.md new file mode 100644 index 0000000..a8dc19b --- /dev/null +++ b/INSTRUCTION.md @@ -0,0 +1,84 @@ +# 1-Week Sprint: Git, Docker, and a Real Codebase + +**Repo:** Fork of [`docker/getting-started-todo-app`](https://github.com/docker/getting-started-todo-app) +**Goal:** Ship 4 features through PR review. Then rebuild the Compose stack solo. + +--- + +## Setup + +- You're added as contributors. Clone the repo directly. +- `main` is protected: PR required, 1 reviewer, no direct pushes, no force-pushes. +- Verify `docker compose up --watch` works. Flag if not. + +--- + +## Start + +- 1: Priority levels (low/med/high). DB column, API field, colored badge. +- 2: Due dates. DB column, validation, date picker, overdue styling. +- 3: Categories (work/personal/shopping). Enum column, filter dropdown. +- 4: Search + sort by created/priority/due. No schema, heavy UI edits. + +You will all touch the same migration, API handler, and `TodoList` component. **Don't coordinate to avoid this.** Conflicts are the lesson. + +**Review, merge, conflict.** PRs go up. You review someone else's before yours merges. Merge order = ready order. After the first merge, everyone else rebases and resolves their own conflicts. Delete merged branches. + +**Rebuild the Compose stack solo.** New branch `compose/`. Rename the existing `Dockerfile` and `compose.yaml` to `Dockerfile.example` and `compose.example.yaml`. Rebuild from scratch. + +1-hour walkthrough, we cherry-pick the best ideas into a team `compose.yaml`. + +--- + +## Rules + +- No direct commits to `main`. No self-approving PRs. +- AI is encouraged but please try and use git and docker yourself. +- PR description: what changed, how to test, screenshot. +- Commit messages in imperative mood. "Add priority field," not "Added priority field." +- Clear and separated branches +- use feat/, fix/, etc branch conventions (https://conventional-branch.github.io/) + +--- + +## What I'll review on your Compose stack + +**Backend Dockerfile.** Multi-stage. Non-root. `npm install` shouldn't re-run on source-only changes. Healthcheck that proves the app is _up_, not just _alive_. + +**Frontend Dockerfile.** Build stage produces static assets. Serve stage is tiny. No `node_modules` in the final image. + +**compose.yaml.** Frontend, backend, db, phpMyAdmin. Backend doesn't start before the DB is _ready_. DB data survives `docker compose down`. No hardcoded secrets. + +**Dev ergonomics.** One command to a working app with hot reload. `.env.example` tells me what I need without leaking yours. + +--- + +## Deliverables (Friday EOD) + +1. Merged feature PR. +2. `compose/` branch. + +--- + +## The existing stack is your starting point, not your target + +The checked-in `compose.yaml` and `Dockerfile` work, but they fail parts of the rubric above on purpose. Keep them as `.example` files and read them. Your rebuild should beat them on at least: + +- Containers run as root. +- Backend has no healthcheck. Compose has no way to know it's actually serving. +- Secrets are inline in `compose.yaml`. There is no `.env.example`. +- One Dockerfile builds both client and backend, and the prod client is baked into the backend image. Split them. The frontend should build to static assets and be served by something that isn't Node. + +--- + +## Hints (not answers) + +- `git rebase` and `git merge` solve the same problem differently. Pick one. Know why. +- `git pull --rebase` saves you unnecessary merge commits. +- Layers that change often go _after_ layers that don't. +- `depends_on` alone doesn't wait for a DB to be ready. Look up `condition`. +- `COPY package*.json ./` before `COPY . .`. There's a reason. +- A static site doesn't need a Node runtime to be served. `nginx:alpine` is ~50MB. +- A container running as root is one `USER` line away from not being. +- `mysqladmin ping` proves MySQL is alive. What proves your API is _serving_? +- `env_file:` and `${VAR}` substitution exist for a reason. So does `.env.example`. diff --git a/backend/spec/persistence/sqlite.spec.js b/backend/spec/persistence/sqlite.spec.js index 996d451..6b94528 100644 --- a/backend/spec/persistence/sqlite.spec.js +++ b/backend/spec/persistence/sqlite.spec.js @@ -1,11 +1,18 @@ +const os = require('os'); +const path = require('path'); + +process.env.SQLITE_DB_LOCATION = path.join(os.tmpdir(), 'test-todo.db'); + const db = require('../../src/persistence/sqlite'); const fs = require('fs'); -const location = process.env.SQLITE_DB_LOCATION || '/etc/todos/todo.db'; +const location = process.env.SQLITE_DB_LOCATION; const ITEM = { id: '7aef3d7c-d301-4846-8358-2a91ec9d6be3', name: 'Test', completed: false, + priority: 'medium', + category: 'personal', }; beforeEach(() => { @@ -38,10 +45,11 @@ test('it can update an existing item', async () => { await db.updateItem( ITEM.id, - Object.assign({}, ITEM, { completed: !ITEM.completed }), + Object.assign({}, ITEM, { completed: !ITEM.completed, category: 'shopping', }), ); const items = await db.getItems(); + expect(items[0].category).toBe('shopping'); expect(items.length).toBe(1); expect(items[0].completed).toBe(!ITEM.completed); }); @@ -63,3 +71,13 @@ test('it can get a single item', async () => { const item = await db.getItem(ITEM.id); expect(item).toEqual(ITEM); }); + +test('it stores and retrieves the priority field', async () => { + await db.init(); + + const item = { ...ITEM, priority: 'high' }; + await db.storeItem(item); + + const items = await db.getItems(); + expect(items[0].priority).toBe('high'); +}); diff --git a/backend/spec/routes/addItem.spec.js b/backend/spec/routes/addItem.spec.js index 2901f5a..86d9ca5 100644 --- a/backend/spec/routes/addItem.spec.js +++ b/backend/spec/routes/addItem.spec.js @@ -1,6 +1,5 @@ const db = require('../../src/persistence'); const addItem = require('../../src/routes/addItem'); -const ITEM = { id: 12345 }; const { v4: uuid } = require('uuid'); jest.mock('uuid', () => ({ v4: jest.fn() })); @@ -11,20 +10,43 @@ jest.mock('../../src/persistence', () => ({ getItem: jest.fn(), })); -test('it stores item correctly', async () => { - const id = 'something-not-a-uuid'; - const name = 'A sample item'; - const req = { body: { name } }; +const ID = 'something-not-a-uuid'; +const NAME = 'A sample item'; + +beforeEach(() => { + jest.clearAllMocks(); + uuid.mockReturnValue(ID); +}); + +test('it stores item with default priority when none is provided', async () => { + const req = { body: { name: NAME } }; const res = { send: jest.fn() }; - uuid.mockReturnValue(id); + await addItem(req, res); + + const expectedItem = { id: ID, name: NAME, completed: false, priority: 'medium' }; + expect(db.storeItem).toHaveBeenCalledWith(expectedItem); + expect(res.send).toHaveBeenCalledWith(expectedItem); +}); + +test('it stores item with the provided valid priority', async () => { + const req = { body: { name: NAME, priority: 'high' } }; + const res = { send: jest.fn() }; await addItem(req, res); - const expectedItem = { id, name, completed: false }; + const expectedItem = { id: ID, name: NAME, completed: false, priority: 'high' }; + expect(db.storeItem).toHaveBeenCalledWith(expectedItem); + expect(res.send).toHaveBeenCalledWith(expectedItem); +}); + +test('it defaults to medium when an invalid priority is provided', async () => { + const req = { body: { name: NAME, priority: 'critical' } }; + const res = { send: jest.fn() }; + + await addItem(req, res); - expect(db.storeItem.mock.calls.length).toBe(1); - expect(db.storeItem.mock.calls[0][0]).toEqual(expectedItem); - expect(res.send.mock.calls[0].length).toBe(1); - expect(res.send.mock.calls[0][0]).toEqual(expectedItem); + expect(db.storeItem).toHaveBeenCalledWith( + expect.objectContaining({ priority: 'medium' }), + ); }); diff --git a/backend/spec/routes/updateItem.spec.js b/backend/spec/routes/updateItem.spec.js index 640e56e..b43a158 100644 --- a/backend/spec/routes/updateItem.spec.js +++ b/backend/spec/routes/updateItem.spec.js @@ -10,7 +10,7 @@ jest.mock('../../src/persistence', () => ({ test('it updates items correctly', async () => { const req = { params: { id: 1234 }, - body: { name: 'New title', completed: false }, + body: { name: 'New title', completed: false, category: 'personal' }, }; const res = { send: jest.fn() }; @@ -23,6 +23,8 @@ test('it updates items correctly', async () => { expect(db.updateItem.mock.calls[0][1]).toEqual({ name: 'New title', completed: false, + priority: 'medium', + category: 'personal' }); expect(db.getItem.mock.calls.length).toBe(1); diff --git a/backend/src/categories.js b/backend/src/categories.js new file mode 100644 index 0000000..00a0c16 --- /dev/null +++ b/backend/src/categories.js @@ -0,0 +1,12 @@ +const CATEGORIES = ['work', 'personal', 'shopping']; +const DEFAULT_CATEGORY = 'personal'; + +function normalizeCategory(category) { + return CATEGORIES.includes(category) ? category : DEFAULT_CATEGORY; +} + +module.exports = { + CATEGORIES, + DEFAULT_CATEGORY, + normalizeCategory, +}; diff --git a/backend/src/persistence/mysql.js b/backend/src/persistence/mysql.js index 39dc075..cf76dbf 100644 --- a/backend/src/persistence/mysql.js +++ b/backend/src/persistence/mysql.js @@ -1,6 +1,7 @@ const waitPort = require('wait-port'); const fs = require('fs'); const mysql = require('mysql2'); +const { DEFAULT_CATEGORY, normalizeCategory } = require('../categories'); const { MYSQL_HOST: HOST, @@ -39,12 +40,29 @@ async function init() { return new Promise((acc, rej) => { pool.query( - 'CREATE TABLE IF NOT EXISTS todo_items (id varchar(36), name varchar(255), completed boolean) DEFAULT CHARSET utf8mb4', + `CREATE TABLE IF NOT EXISTS todo_items ( + id varchar(36), + name varchar(255), + completed boolean, + priority VARCHAR(20), + category ENUM('work', 'personal', 'shopping') NOT NULL DEFAULT '${DEFAULT_CATEGORY}' + ) DEFAULT CHARSET utf8mb4`, (err) => { if (err) return rej(err); - - console.log(`Connected to mysql db at host ${HOST}`); - acc(); + pool.query( + `ALTER TABLE todo_items ADD COLUMN priority VARCHAR(20) DEFAULT 'medium'`, + (err2) => { + // 1060 = duplicate column — column already exists, which is fine + if (err2 && err2.errno !== 1060) return rej(err2); + pool.query( + `ALTER TABLE todo_items ADD COLUMN category ENUM('work', 'personal', 'shopping') NOT NULL DEFAULT '${DEFAULT_CATEGORY}'`, + (err3) => { + if (err3 && err3.errno !== 1060) return rej(err3); + acc(); + }, + ); + }, + ); }, ); }); @@ -67,6 +85,7 @@ async function getItems() { rows.map((item) => Object.assign({}, item, { completed: item.completed === 1, + category: normalizeCategory(item.category) }), ), ); @@ -82,6 +101,7 @@ async function getItem(id) { rows.map((item) => Object.assign({}, item, { completed: item.completed === 1, + category: normalizeCategory(item.category) }), )[0], ); @@ -92,8 +112,8 @@ async function getItem(id) { async function storeItem(item) { return new Promise((acc, rej) => { pool.query( - 'INSERT INTO todo_items (id, name, completed) VALUES (?, ?, ?)', - [item.id, item.name, item.completed ? 1 : 0], + 'INSERT INTO todo_items (id, name, completed, priority, category) VALUES (?, ?, ?, ?, ?)', + [item.id, item.name, item.completed ? 1 : 0, item.priority, normalizeCategory(item.category)], (err) => { if (err) return rej(err); acc(); @@ -105,8 +125,8 @@ async function storeItem(item) { async function updateItem(id, item) { return new Promise((acc, rej) => { pool.query( - 'UPDATE todo_items SET name=?, completed=? WHERE id=?', - [item.name, item.completed ? 1 : 0, id], + 'UPDATE todo_items SET name=?, completed=?, category=? WHERE id=?', + [item.name, item.completed ? 1 : 0, normalizeCategory(item.category), id], (err) => { if (err) return rej(err); acc(); diff --git a/backend/src/persistence/sqlite.js b/backend/src/persistence/sqlite.js index cf4a81b..a72f029 100644 --- a/backend/src/persistence/sqlite.js +++ b/backend/src/persistence/sqlite.js @@ -1,5 +1,10 @@ const sqlite3 = require('sqlite3').verbose(); const fs = require('fs'); +const { + CATEGORIES, + DEFAULT_CATEGORY, + normalizeCategory, +} = require('../categories'); const location = process.env.SQLITE_DB_LOCATION || '/etc/todos/todo.db'; let db, dbAll, dbRun; @@ -18,10 +23,37 @@ function init() { console.log(`Using sqlite database at ${location}`); db.run( - 'CREATE TABLE IF NOT EXISTS todo_items (id varchar(36), name varchar(255), completed boolean)', - (err, result) => { + `CREATE TABLE IF NOT EXISTS todo_items ( + id varchar(36), + name varchar(255), + priority varchar(20), + completed boolean, + category text NOT NULL DEFAULT '${DEFAULT_CATEGORY}' + CHECK(category IN (${CATEGORIES.map((value) => `'${value}'`).join(', ')})) + )`, + (err) => { if (err) return rej(err); - acc(); + + db.all('PRAGMA table_info(todo_items)', (tableErr, rows) => { + if (tableErr) return rej(tableErr); + + const hasCategoryColumn = rows.some( + (column) => column.name === 'category', + ); + + if (hasCategoryColumn) { + acc(); + return; + } + + db.run( + `ALTER TABLE todo_items ADD COLUMN category text NOT NULL DEFAULT '${DEFAULT_CATEGORY}'`, + (alterErr) => { + if (alterErr) return rej(alterErr); + acc(); + }, + ); + }); }, ); }); @@ -45,6 +77,7 @@ async function getItems() { rows.map((item) => Object.assign({}, item, { completed: item.completed === 1, + category: normalizeCategory(item.category), }), ), ); @@ -60,6 +93,7 @@ async function getItem(id) { rows.map((item) => Object.assign({}, item, { completed: item.completed === 1, + category: normalizeCategory(item.category), }), )[0], ); @@ -70,8 +104,8 @@ async function getItem(id) { async function storeItem(item) { return new Promise((acc, rej) => { db.run( - 'INSERT INTO todo_items (id, name, completed) VALUES (?, ?, ?)', - [item.id, item.name, item.completed ? 1 : 0], + 'INSERT INTO todo_items (id, name, completed, priority, category) VALUES (?, ?, ?, ?, ?)', + [item.id, item.name, item.completed ? 1 : 0, item.priority, normalizeCategory(item.category),], (err) => { if (err) return rej(err); acc(); @@ -83,8 +117,8 @@ async function storeItem(item) { async function updateItem(id, item) { return new Promise((acc, rej) => { db.run( - 'UPDATE todo_items SET name=?, completed=? WHERE id = ?', - [item.name, item.completed ? 1 : 0, id], + 'UPDATE todo_items SET name=?, category = ?, completed=? WHERE id = ?', + [item.name, normalizeCategory(item.category), item.completed ? 1 : 0, id], (err) => { if (err) return rej(err); acc(); diff --git a/backend/src/routes/addItem.js b/backend/src/routes/addItem.js index a865030..f06200d 100644 --- a/backend/src/routes/addItem.js +++ b/backend/src/routes/addItem.js @@ -1,11 +1,16 @@ const db = require('../persistence'); const { v4: uuid } = require('uuid'); +const { normalizeCategory } = require('../categories'); +const VALID_PRIORITIES = ['low', 'medium', 'high']; module.exports = async (req, res) => { + const priority = VALID_PRIORITIES.includes(req.body.priority) ? req.body.priority : 'medium'; const item = { id: uuid(), name: req.body.name, completed: false, + priority, + category: normalizeCategory(req.body.category), }; await db.storeItem(item); diff --git a/backend/src/routes/getGreeting.js b/backend/src/routes/getGreeting.js index a9b997e..0ad5947 100644 --- a/backend/src/routes/getGreeting.js +++ b/backend/src/routes/getGreeting.js @@ -1,7 +1,11 @@ -const GREETING = 'Hello world!'; +const GREETINGS = [ + "Welcome1", + "dsadad!", + "yoyo!", +]; module.exports = async (req, res) => { - res.send({ - greeting: GREETING, - }); + res.send({ + greeting: GREETINGS[Math.floor(Math.random() * GREETINGS.length)], + }); }; diff --git a/backend/src/routes/updateItem.js b/backend/src/routes/updateItem.js index c2f5871..e788a8c 100644 --- a/backend/src/routes/updateItem.js +++ b/backend/src/routes/updateItem.js @@ -1,10 +1,13 @@ const db = require('../persistence'); +const VALID_PRIORITIES = ['low', 'medium', 'high']; +const { normalizeCategory } = require('../categories'); module.exports = async (req, res) => { - await db.updateItem(req.params.id, { - name: req.body.name, - completed: req.body.completed, - }); + const updates = { + ...req.body, + priority: VALID_PRIORITIES.includes(req.body.priority) ? req.body.priority : 'medium' + }; + await db.updateItem(req.params.id, updates); const item = await db.getItem(req.params.id); res.send(item); }; diff --git a/client/src/categories.js b/client/src/categories.js new file mode 100644 index 0000000..8ac837e --- /dev/null +++ b/client/src/categories.js @@ -0,0 +1,6 @@ +export const CATEGORIES = ['work', 'personal', 'shopping']; +export const DEFAULT_CATEGORY = 'personal'; + +export function formatCategoryLabel(category) { + return category.charAt(0).toUpperCase() + category.slice(1); +} \ No newline at end of file diff --git a/client/src/components/AddNewItemForm.jsx b/client/src/components/AddNewItemForm.jsx index b057ce1..79ad629 100644 --- a/client/src/components/AddNewItemForm.jsx +++ b/client/src/components/AddNewItemForm.jsx @@ -1,12 +1,13 @@ import { useState } from 'react'; import PropTypes from 'prop-types'; -import Button from 'react-bootstrap/Button'; -import Form from 'react-bootstrap/Form'; -import InputGroup from 'react-bootstrap/InputGroup'; +import { InputGroup, Form, Button } from 'react-bootstrap'; +import { CATEGORIES, DEFAULT_CATEGORY } from '../categories'; export function AddItemForm({ onNewItem }) { const [newItem, setNewItem] = useState(''); + const [category, setCategory] = useState(DEFAULT_CATEGORY); const [submitting, setSubmitting] = useState(false); + const [priority, setPriority] = useState('medium'); const submitNewItem = (e) => { e.preventDefault(); @@ -14,17 +15,20 @@ export function AddItemForm({ onNewItem }) { const options = { method: 'POST', - body: JSON.stringify({ name: newItem }), + body: JSON.stringify({ name: newItem, category, priority }), headers: { 'Content-Type': 'application/json' }, }; fetch('/api/items', options) - .then((r) => r.json()) - .then((item) => { + .then(r => r.json()) + .then(item => { onNewItem(item); setSubmitting(false); setNewItem(''); - }); + setCategory(DEFAULT_CATEGORY); + setPriority(''); + }) + .catch(() => setSubmitting(false)) }; return ( @@ -37,6 +41,17 @@ export function AddItemForm({ onNewItem }) { placeholder="New Item" aria-label="New item" /> + setCategory(e.target.value)} + aria-label="Select category" + > + {CATEGORIES.map((categoryOption) => ( + + ))} + - {item.name} + +
{item.name}
+ + {item.category} +