docs: service request architecture with mermaid diagrams #182

Merged

ldraney merged 3 commits from service-request-docs into main 2026-06-11 03:12:07 +00:00

Conversation 1 Commits 3 Files changed 1 +171

ldraney commented 2026-06-08 05:08:02 +00:00

Owner

Copy link

Summary

• Adds docs/service-requests.md — focused architecture doc for the service request workflow
• Status lifecycle state diagram, data model ERD, client/admin flow charts (all mermaid)
• Role access matrix, Stripe integration plan, property comments architecture

Changes

• docs/service-requests.md (new): standalone reference for agents building #123, #176, #179, #180

Test Plan

• Mermaid diagrams render correctly in Forgejo
• No broken cross-references
• Data model matches current schema (post #122 merge)

Review Checklist

• Passed automated review-fix loop
• No secrets committed
• No unnecessary file changes
• Commit messages are descriptive
• Feature flag needed? No — documentation only

Related Notes

• Closes #181
• References #123, #176, #179, #180 — tickets that should use this doc
• Extracted from docs/user-stories-auth.md into a standalone agent-readable reference

## Summary - Adds `docs/service-requests.md` — focused architecture doc for the service request workflow - Status lifecycle state diagram, data model ERD, client/admin flow charts (all mermaid) - Role access matrix, Stripe integration plan, property comments architecture ## Changes - `docs/service-requests.md` (new): standalone reference for agents building #123, #176, #179, #180 ## Test Plan - [ ] Mermaid diagrams render correctly in Forgejo - [ ] No broken cross-references - [ ] Data model matches current schema (post #122 merge) ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive - [ ] Feature flag needed? No — documentation only ## Related Notes - Closes #181 - References #123, #176, #179, #180 — tickets that should use this doc - Extracted from `docs/user-stories-auth.md` into a standalone agent-readable reference

ldraney added 1 commit 2026-06-08 05:08:03 +00:00

docs: add service-requests architecture with mermaid diagrams ... 0ae4f0295d

Focused reference for the service request workflow — status lifecycle,
data model (as built), client/admin flows, role access matrix, and
Stripe integration plan. Agents building #123, #176, #179, and #180
should reference this doc.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented 2026-06-08 05:10:38 +00:00

Author

Owner

Copy link

PR #182 Review

DOMAIN REVIEW

Domain: Documentation (Markdown + Mermaid diagrams). Ruby on Rails codebase context.

This is a single new file docs/service-requests.md (171 lines) serving as an architecture reference for the service request workflow. The doc contains four mermaid diagrams (stateDiagram-v2, erDiagram, two flowcharts), a role access matrix, and cross-references to five tickets.

1. Data Model vs Codebase -- "as built" framing needs caution

The ERD section is titled "Data Model (as built)" which is a strong claim. Verification against the current db/schema.rb on main reveals several discrepancies:

• service_requests table: Not present in db/schema.rb. PR #178 ("Add ServiceRequest model with status transition validation") shows as closed, and the ServiceRequest model file does not exist at app/models/service_request.rb on this checkout. If #178 was merged, the local main may be stale -- but the doc's "as built" label should only apply to what is actually on main.
• owner_sub on properties: The doc's PROPERTY entity shows owner_sub ("Keycloak UUID of owning client"). The db/schema.rb has no such column. Git status shows two related migrations were deleted (add_owner_username_to_properties and rename_owner_username_to_owner_sub), suggesting this column was reverted. If owner_sub does not exist on main, the ERD is inaccurate.
• monthly_price on services: The doc's SERVICE entity shows decimal monthly_price. The actual services table only has name and timestamps -- no price column.
• crew_members table: The doc's CREW_MEMBER entity implies this table exists. The model file app/models/crew_member.rb does exist with validations, but db/schema.rb has no crew_members table. Same stale-checkout caveat applies.
• property_comments table: The doc's PROPERTY_COMMENT entity implies this table exists. Neither the model file nor the schema table exist on this checkout.

Recommendation: Either (a) rename the section to "Data Model (target)" or "Data Model (as of PR #178)" to avoid the "as built" claim if the tables are not yet on main, or (b) verify that PR #178 actually delivered all five entities shown in the ERD and update the doc to match exactly what shipped. The doc's note about accepted being planned for #179 is good -- apply the same honesty to the rest of the ERD.

2. Status Transitions

The doc lists 6 statuses: requested, quoted, paid, scheduled, completed, declined. It also shows an accepted state in the state diagram with a note that accepted is planned for #179. This is well-handled -- the note clearly delineates current vs planned. However, since I cannot verify the model's validates :status, inclusion: on this checkout, the 6-status claim is unverifiable from here.

3. Mermaid Diagram Syntax

All four diagrams use valid mermaid syntax:

• stateDiagram-v2 -- valid state transitions, proper [*] start/end markers
• erDiagram -- valid entity definitions with PK/FK annotations, proper relationship notation (||--o{, }o--o{)
• Two flowchart TD blocks -- valid node syntax, decision diamonds, proper arrow notation

One minor note: the erDiagram uses the }o--o{ (many-to-many) notation for PROPERTY }o--o{ SERVICE : "subscribed via property_services" which correctly represents the join table relationship. Good.

4. Cross-Reference Accuracy

Verified against the Forgejo issue list:

• #122 -- Not visible in the first 50 issues (numbered below 133). The doc says "#122 (model, DONE)". Cannot verify state from here, but #178 ("Add ServiceRequest model") references #122 by convention and is closed.
• #123 -- Not visible in the first 50 issues. The doc says "client request UI". Referenced in docs/user-stories-auth.md as "#123 -- Client request UI (My Property view)". Consistent.
• #176 -- Exists, open: "Property detail page: Projects section (depends on #122)". Doc says "admin property projects display". Accurate.
• #179 -- Exists, open: "Admin project workflow: create, bid, accept, invoice on property page". Doc says "admin project workflow". Accurate.
• #180 -- Exists, open: "Property comments: timestamped discussion for all roles". Doc says "property comments". Accurate.
• #125 (mentioned in Stripe section) -- Not visible in first 50 issues. Referenced in docs/user-stories-auth.md as "#125 -- Stripe integration (payment links + webhook)". Consistent.

5. Role Access Matrix

The matrix has 5 columns: Client, Member, Lead, Admin. Compared against docs/user-stories-auth.md role permission matrix:

• "Schedule work" shows Lead + Admin. The user-stories doc says leads can "Manage own daily list" but does not explicitly say leads can schedule service request work. This could be intentional evolution or a discrepancy -- worth confirming.
• "Mark completed" shows Lead + Admin. Same consideration as scheduling.
• "Create a request (admin on behalf of client)" is Admin-only. Reasonable and consistent with the admin-initiated flow section.
• Missing super_admin column. The user-stories doc includes Super Admin as a separate column. The service-requests doc collapses it, which is fine for a focused doc but should be noted.

The matrix omits Member/Lead from "View own property's requests" (showing --). If leads and members can also be clients of the business they work for (as stated in docs/user-stories-auth.md), they would view requests through their client role, not their crew role. The -- is technically correct (they don't have member/lead-specific access) but could confuse readers.

BLOCKERS

None. This is a documentation-only PR -- no code changes, no security implications, no test requirements.

NITS

1. "as built" label is premature -- The ERD section title "Data Model (as built)" implies these tables are on main right now. If PR #178 landed the service_requests and crew_members tables, this is fine. But property_comments (ticket #180) is open and not built yet. The ERD shows PROPERTY_COMMENT as if it exists. Consider labeling it "Data Model (current + planned)" or adding a note to the PROPERTY_COMMENT entity indicating it is planned (#180), similar to how accepted status is labeled as planned for #179.

2. monthly_price on SERVICE -- The actual services table has no monthly_price column. If this is planned, label it. If it was added by a recent PR, verify.

3. owner_sub on PROPERTY -- Same concern. If this column was reverted (the deleted migrations suggest it was), remove it from the ERD or label it as planned.

4. Super Admin omitted from role matrix -- Minor, but the user-stories doc distinguishes Super Admin. A footnote like "Super Admin has all Admin permissions" would close the gap.

5. Stripe section references #125 -- The doc's header "Related tickets" at the top references #122, #123, #176, #179, #180 but not #125. Since the Stripe section explicitly depends on #125, consider adding it to the top-level references for completeness.

SOP COMPLIANCE

• Branch named after issue -- service-request-docs is descriptive; parent issue is #181. Strictly, the convention is {issue-number}-{kebab-case} (e.g., 181-service-request-docs). The branch omits the issue number prefix.
• PR body follows template -- Has Summary, Changes, Test Plan, Review Checklist, Related Notes sections. Uses Closes #181.
• Related references plan slug -- No plan slug provided (caller confirmed "No plan slug"). Not blocking for a docs PR.
• No secrets committed -- Documentation only, no credentials.
• No unnecessary file changes -- Single file, scoped to the issue.
• Commit messages are descriptive -- PR title follows docs: convention.

PROCESS OBSERVATIONS

• Documentation-first approach is strong. Writing the architecture doc before building #123, #176, #179, #180 gives agents a clear reference. This reduces rework risk.
• "As built" vs "as planned" distinction matters. Agents reading this doc will assume the ERD entities exist in the schema. If property_comments or monthly_price do not exist yet, an agent could write code referencing nonexistent tables. Label planned items clearly.
• Branch naming convention drift. The branch is service-request-docs instead of 181-service-request-docs. Minor, but consistency helps when correlating branches to issues in CI logs.

VERDICT: APPROVED

The doc is well-structured, the mermaid syntax is valid, cross-references are accurate for the tickets that are verifiable, and the role matrix is reasonable. The "as built" labeling concern is a nit, not a blocker -- the doc explicitly notes which features are planned vs current in the status lifecycle section, and should apply the same discipline to the ERD. No code changes means no test or security concerns.

## PR #182 Review ### DOMAIN REVIEW **Domain:** Documentation (Markdown + Mermaid diagrams). Ruby on Rails codebase context. This is a single new file `docs/service-requests.md` (171 lines) serving as an architecture reference for the service request workflow. The doc contains four mermaid diagrams (stateDiagram-v2, erDiagram, two flowcharts), a role access matrix, and cross-references to five tickets. **1. Data Model vs Codebase -- "as built" framing needs caution** The ERD section is titled **"Data Model (as built)"** which is a strong claim. Verification against the current `db/schema.rb` on main reveals several discrepancies: - **`service_requests` table**: Not present in `db/schema.rb`. PR #178 ("Add ServiceRequest model with status transition validation") shows as closed, and the `ServiceRequest` model file does not exist at `app/models/service_request.rb` on this checkout. If #178 was merged, the local main may be stale -- but the doc's "as built" label should only apply to what is actually on main. - **`owner_sub` on properties**: The doc's PROPERTY entity shows `owner_sub` ("Keycloak UUID of owning client"). The `db/schema.rb` has no such column. Git status shows two related migrations were *deleted* (`add_owner_username_to_properties` and `rename_owner_username_to_owner_sub`), suggesting this column was reverted. If `owner_sub` does not exist on main, the ERD is inaccurate. - **`monthly_price` on services**: The doc's SERVICE entity shows `decimal monthly_price`. The actual `services` table only has `name` and timestamps -- no price column. - **`crew_members` table**: The doc's CREW_MEMBER entity implies this table exists. The model file `app/models/crew_member.rb` does exist with validations, but `db/schema.rb` has no `crew_members` table. Same stale-checkout caveat applies. - **`property_comments` table**: The doc's PROPERTY_COMMENT entity implies this table exists. Neither the model file nor the schema table exist on this checkout. **Recommendation:** Either (a) rename the section to "Data Model (target)" or "Data Model (as of PR #178)" to avoid the "as built" claim if the tables are not yet on main, or (b) verify that PR #178 actually delivered all five entities shown in the ERD and update the doc to match exactly what shipped. The doc's note about `accepted` being planned for #179 is good -- apply the same honesty to the rest of the ERD. **2. Status Transitions** The doc lists 6 statuses: `requested, quoted, paid, scheduled, completed, declined`. It also shows an `accepted` state in the state diagram with a note that `accepted` is planned for #179. This is well-handled -- the note clearly delineates current vs planned. However, since I cannot verify the model's `validates :status, inclusion:` on this checkout, the 6-status claim is unverifiable from here. **3. Mermaid Diagram Syntax** All four diagrams use valid mermaid syntax: - `stateDiagram-v2` -- valid state transitions, proper `[*]` start/end markers - `erDiagram` -- valid entity definitions with PK/FK annotations, proper relationship notation (`||--o{`, `}o--o{`) - Two `flowchart TD` blocks -- valid node syntax, decision diamonds, proper arrow notation One minor note: the `erDiagram` uses the `}o--o{` (many-to-many) notation for `PROPERTY }o--o{ SERVICE : "subscribed via property_services"` which correctly represents the join table relationship. Good. **4. Cross-Reference Accuracy** Verified against the Forgejo issue list: - **#122** -- Not visible in the first 50 issues (numbered below 133). The doc says "#122 (model, DONE)". Cannot verify state from here, but #178 ("Add ServiceRequest model") references #122 by convention and is closed. - **#123** -- Not visible in the first 50 issues. The doc says "client request UI". Referenced in `docs/user-stories-auth.md` as "#123 -- Client request UI (My Property view)". Consistent. - **#176** -- Exists, open: "Property detail page: Projects section (depends on #122)". Doc says "admin property projects display". Accurate. - **#179** -- Exists, open: "Admin project workflow: create, bid, accept, invoice on property page". Doc says "admin project workflow". Accurate. - **#180** -- Exists, open: "Property comments: timestamped discussion for all roles". Doc says "property comments". Accurate. - **#125** (mentioned in Stripe section) -- Not visible in first 50 issues. Referenced in `docs/user-stories-auth.md` as "#125 -- Stripe integration (payment links + webhook)". Consistent. **5. Role Access Matrix** The matrix has 5 columns: Client, Member, Lead, Admin. Compared against `docs/user-stories-auth.md` role permission matrix: - "Schedule work" shows Lead + Admin. The user-stories doc says leads can "Manage own daily list" but does not explicitly say leads can schedule service request work. This could be intentional evolution or a discrepancy -- worth confirming. - "Mark completed" shows Lead + Admin. Same consideration as scheduling. - "Create a request (admin on behalf of client)" is Admin-only. Reasonable and consistent with the admin-initiated flow section. - Missing `super_admin` column. The user-stories doc includes Super Admin as a separate column. The service-requests doc collapses it, which is fine for a focused doc but should be noted. The matrix omits Member/Lead from "View own property's requests" (showing `--`). If leads and members can also be clients of the business they work for (as stated in `docs/user-stories-auth.md`), they would view requests through their client role, not their crew role. The `--` is technically correct (they don't have *member/lead-specific* access) but could confuse readers. ### BLOCKERS None. This is a documentation-only PR -- no code changes, no security implications, no test requirements. ### NITS 1. **"as built" label is premature** -- The ERD section title "Data Model (as built)" implies these tables are on main right now. If PR #178 landed the `service_requests` and `crew_members` tables, this is fine. But `property_comments` (ticket #180) is open and not built yet. The ERD shows `PROPERTY_COMMENT` as if it exists. Consider labeling it "Data Model (current + planned)" or adding a note to the PROPERTY_COMMENT entity indicating it is planned (#180), similar to how `accepted` status is labeled as planned for #179. 2. **`monthly_price` on SERVICE** -- The actual `services` table has no `monthly_price` column. If this is planned, label it. If it was added by a recent PR, verify. 3. **`owner_sub` on PROPERTY** -- Same concern. If this column was reverted (the deleted migrations suggest it was), remove it from the ERD or label it as planned. 4. **Super Admin omitted from role matrix** -- Minor, but the user-stories doc distinguishes Super Admin. A footnote like "Super Admin has all Admin permissions" would close the gap. 5. **Stripe section references #125** -- The doc's header "Related tickets" at the top references #122, #123, #176, #179, #180 but not #125. Since the Stripe section explicitly depends on #125, consider adding it to the top-level references for completeness. ### SOP COMPLIANCE - [x] Branch named after issue -- `service-request-docs` is descriptive; parent issue is #181. Strictly, the convention is `{issue-number}-{kebab-case}` (e.g., `181-service-request-docs`). The branch omits the issue number prefix. - [x] PR body follows template -- Has Summary, Changes, Test Plan, Review Checklist, Related Notes sections. Uses `Closes #181`. - [ ] Related references plan slug -- No plan slug provided (caller confirmed "No plan slug"). Not blocking for a docs PR. - [x] No secrets committed -- Documentation only, no credentials. - [x] No unnecessary file changes -- Single file, scoped to the issue. - [x] Commit messages are descriptive -- PR title follows `docs:` convention. ### PROCESS OBSERVATIONS - **Documentation-first approach is strong.** Writing the architecture doc before building #123, #176, #179, #180 gives agents a clear reference. This reduces rework risk. - **"As built" vs "as planned" distinction matters.** Agents reading this doc will assume the ERD entities exist in the schema. If `property_comments` or `monthly_price` do not exist yet, an agent could write code referencing nonexistent tables. Label planned items clearly. - **Branch naming convention drift.** The branch is `service-request-docs` instead of `181-service-request-docs`. Minor, but consistency helps when correlating branches to issues in CI logs. ### VERDICT: APPROVED The doc is well-structured, the mermaid syntax is valid, cross-references are accurate for the tickets that are verifiable, and the role matrix is reasonable. The "as built" labeling concern is a nit, not a blocker -- the doc explicitly notes which features are planned vs current in the status lifecycle section, and should apply the same discipline to the ERD. No code changes means no test or security concerns.

ldraney added 1 commit 2026-06-08 05:11:08 +00:00

fix: label ERD as current + planned, annotate PropertyComment ... 0c7fda9729

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney referenced this pull request 2026-06-10 01:49:23 +00:00

Property detail page: Projects section (depends on #122) #176

ldraney referenced this pull request 2026-06-10 01:49:27 +00:00

Client request UI: My Property view with package and request form #123

ldraney added 1 commit 2026-06-10 01:49:56 +00:00

fix: update role access — any role can create requests ... e0bcadcb9d

Members/leads can submit requests on behalf of clients (e.g., client
asks crew on-site). Only admin sets bids, only client accepts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney merged commit 8877822fca into main 2026-06-11 03:12:07 +00:00

ldraney deleted branch service-request-docs 2026-06-11 03:12:07 +00:00

ldraney referenced this pull request from a commit 2026-06-11 03:12:08 +00:00

docs: service request architecture with mermaid diagrams (#182)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ldraney/landscaping-assistant!182

No description provided.