chore: text and diagram updates#20
Conversation
WalkthroughAdds and restructures documentation in Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
README.md (7)
16-17: Fix MD007: nested list indentation under “API documentation”.Indent the two sub-bullets by 2 spaces (not 3) to satisfy markdownlint.
Apply:
- - Static: <https://ph-regions.vercel.app> - - Interactive: <https://ph-regions.vercel.app/docs> + - Static: <https://ph-regions.vercel.app> + - Interactive: <https://ph-regions.vercel.app/docs>
117-119: Fix MD034: replace bare URLs in Quickstart step 5.Use Markdown links to avoid “bare URL” warnings.
- - REST API documentation: http://localhost:3001 - - REST API endpoints: http://localhost:3001/api + - REST API documentation: [http://localhost:3001](http://localhost:3001) + - REST API endpoints: [http://localhost:3001/api](http://localhost:3001/api)
315-316: Avoid brittle “Line #21” references in docker-compose notes.Line numbers drift. Refer to the mount path/name instead.
-> 💡 **NOTE:** Comment out **Line #21, under [public folder volume]** in the `docker-compose.yml` file ... +> 💡 **NOTE:** Temporarily comment out the host bind-mount for the public folder in `docker-compose.yml` +> (the volume that maps `./server/public` -> `/server/public`) before generating OpenAPI assets, then restore it.If helpful, we can add a short yq snippet or exact compose volume key instead of a line number.
Also applies to: 325-326, 331-332
101-115: Make the seed command resilient to container/service names.Hardcoding the container name can break on forks. Prefer docker compose exec against the service.
-`docker exec -it weaponsforge-ph-regions npm run seed` +`docker compose exec <server-service-name> npm run seed` +# Example (if the service is named "server"): +# docker compose exec server npm run seedPlease confirm the actual service name in docker-compose and update the example accordingly.
28-33: Optional: add loading="lazy" to screenshots for quicker page paint.Small UX win on GitHub web rendering.
- <img src="assets/ph_regions_ss_01.png" width="45%" alt="Static API docs screenshot" /> - <img src="assets/ph_regions_ss_02.png" width="45%" alt="Interactive API docs screenshot" /> + <img src="assets/ph_regions_ss_01.png" width="45%" loading="lazy" alt="Static API docs screenshot" /> + <img src="assets/ph_regions_ss_02.png" width="45%" loading="lazy" alt="Interactive API docs screenshot" />
93-94: Clarify the OpenAPI docs path in folder structure.“🌐 openapi” points at “/scripts/openapi/docs”; consider aligning the label/path to avoid confusion.
-- 🌐 **openapi** - Contains OpenAPI definitions using Zod schemas (folder: `/scripts/openapi/docs`) +- 🌐 **scripts/openapi/docs** - Contains OpenAPI definitions generated from Zod schemasOr keep the label “openapi” but remove the parenthetical path.
385-397: Don’t instruct adding unvalidated routes, even temporarily.Recommend creating routes with validation middleware from the start to avoid accidental commits exposing unvalidated endpoints.
-Add new routes ... **without input validation** for now. +Add new routes with the validation middleware wired immediately to avoid exposing unvalidated endpoints.assets/README.md (2)
7-7: Capture how to regenerate/export the PNG from the .drawio source.Add a one‑liner so contributors keep assets/architecture.png in sync with assets/architecture.drawio.
-### Notes +### Notes +- Export `assets/architecture.drawio` to `assets/architecture.png` (File → Export as → PNG) whenever the diagram changes.
7-10: Optional: add license/source notes for embedded icons.If draw.io shapes/icons with specific licenses are used, link the license or note “draw.io default shapes”.
assets/architecture.drawio (3)
54-59: Add arrowheads to seed→model edge for directionality.Edge xfzBbhjO1vl4Opf90gaK-3 has no arrow; consider
startArrow=classicThin;startFill=1(or endArrow) to show data flow.
51-53: Unify label casing and add a legend.Standardize node titles (e.g., “Model”, “Schema (Zod)”, “Router”, “Controller”, “Middleware (Validation)”) and add a small legend for shapes/colors.
Also applies to: 57-59, 61-65
23-27: Link diagram nodes to code paths (optional).Where supported, add hyperlinks:
- seed script → server/src/scripts/seed.ts
- schema (Zod) → server/src/schemas/...
- model → server/src/models/...
Also applies to: 51-53, 57-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/architecture.pngis excluded by!**/*.pngassets/ph_regions_ss_01.pngis excluded by!**/*.pngassets/ph_regions_ss_02.pngis excluded by!**/*.png
📒 Files selected for processing (3)
README.md(8 hunks)assets/README.md(1 hunks)assets/architecture.drawio(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: weaponsforge
PR: weaponsforge/ph-regions#16
File: server/public/openapi.json:64-64
Timestamp: 2025-09-10T18:53:14.581Z
Learning: The user weaponsforge prefers to use full server URLs (like "http://localhost:3001" or Vercel URLs) in OpenAPI specifications rather than relative URLs, even when relative URLs could make the spec more portable across environments.
📚 Learning: 2025-07-27T22:53:58.000Z
Learnt from: weaponsforge
PR: weaponsforge/ph-regions#1
File: server/src/controllers/region.ts:0-0
Timestamp: 2025-07-27T22:53:58.000Z
Learning: In the ph-regions project, route parameters like regionId, provinceId, and municipalityId are validated by the zod validate() middleware at the route level in files like server/src/routes/region.ts, server/src/routes/province.ts, etc. The validate middleware in server/src/middlewares/validate.ts automatically validates request parameters against MongoIdSchema, ensuring they are valid MongoDB ObjectIDs before reaching the controller functions. This eliminates the need for additional parameter validation within the controller functions themselves.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.18.1)
README.md
16-16: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
117-117: Bare URL used
(MD034, no-bare-urls)
118-118: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (3)
assets/architecture.drawio (2)
54-65: Nice addition: seed script and developer flow.The new nodes and dashed link improve clarity of the setup flow.
23-27: Confirm architecture.png was regenerated to reflect the updated .drawio.README shows assets/architecture.png; ensure it matches this source.
README.md (1)
389-401: Confirmed: referenced OpenAPI files exist and are registered.server/src/scripts/openapi/docs/api.schema.ts is present; server/src/scripts/openapi/main.ts creates an OpenAPIRegistry and calls registry.register(...) to register the schemas.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
117-118: Consider wrapping bare URLs in angle bracketsStatic analysis flagged bare URLs that should be wrapped in angle brackets for proper Markdown formatting.
- - REST API documentation: http://localhost:3001 - - REST API endpoints: http://localhost:3001/api + - REST API documentation: <http://localhost:3001> + - REST API endpoints: <http://localhost:3001/api>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(8 hunks)assets/README.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/README.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: weaponsforge
PR: weaponsforge/ph-regions#16
File: server/public/openapi.json:64-64
Timestamp: 2025-09-10T18:53:14.581Z
Learning: The user weaponsforge prefers to use full server URLs (like "http://localhost:3001" or Vercel URLs) in OpenAPI specifications rather than relative URLs, even when relative URLs could make the spec more portable across environments.
📚 Learning: 2025-07-27T22:53:58.000Z
Learnt from: weaponsforge
PR: weaponsforge/ph-regions#1
File: server/src/controllers/region.ts:0-0
Timestamp: 2025-07-27T22:53:58.000Z
Learning: In the ph-regions project, route parameters like regionId, provinceId, and municipalityId are validated by the zod validate() middleware at the route level in files like server/src/routes/region.ts, server/src/routes/province.ts, etc. The validate middleware in server/src/middlewares/validate.ts automatically validates request parameters against MongoIdSchema, ensuring they are valid MongoDB ObjectIDs before reaching the controller functions. This eliminates the need for additional parameter validation within the controller functions themselves.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.18.1)
README.md
117-117: Bare URL used
(MD034, no-bare-urls)
118-118: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (9)
README.md (9)
12-18: LGTM! Well-structured Online Demo sectionThe new Online Demo section provides clear access to both the REST API and documentation resources with appropriate URLs.
28-33: LGTM! Good visual presentation of screenshotsThe section rename from "Online Deployments" to "Local Development Screenshots" is more accurate, and the centered layout with proper sizing provides good visual appeal.
74-76: Align README dependency versions with server/package.jsonThe dependency versions in the README don't match the actual versions in server/package.json, creating potential confusion for developers.
101-119: LGTM! Excellent Quickstart sectionThe new Quickstart section provides a clear, streamlined path for developers to get started quickly with Docker-based setup. The step-by-step approach covers all essential tasks from environment setup to accessing resources.
125-125: Good prioritization of Docker workflowThe updated installation instructions appropriately emphasize Docker as the primary approach while still providing the Node alternative.
149-155: LGTM! Clear Docker-first seeding instructionsThe updated seeding instructions properly reference the Docker workflow and provide clear guidance on when to run the seed command.
378-400: Excellent enhancement to endpoint creation guidelinesThe expanded "Adding New Endpoints" section significantly improves the development workflow by:
- Requiring Zod schemas to include metadata descriptions and examples
- Emphasizing proper type casting from Zod to Mongoose schemas
- Adding validation middleware steps
- Clarifying HTTP-specific terminology (query, response, params, body)
These improvements will help maintain consistency and quality across the API.
315-315: Confirmed: docker-compose.yml line 21 (public folder volume) is correctLine 21 contains
- /opt/server/publicas referenced in the README — no documentation change required.
40-40: Quickstart section exists — no changes needed.
Found "## 🆕 Quickstart" in README.md at line 101.
1 participant
Summary by CodeRabbit