feat: add html_content to BoardOut schema #271

Merged
forgejo_admin merged 1 commit from 270-board-html-content into main 2026-04-13 00:13:27 +00:00
Contributor

Summary

Add html_content field to BoardOut so the frontend can render board note content (User Stories, Architecture, Acceptance Criteria) above kanban columns.

Changes

  • src/pal_e_docs/schemas.py — Added html_content: str | None = None to BoardOut
  • src/pal_e_docs/routes/boards.py — Populated html_content from board_note.html_content in _board_to_out()

Test Plan

  • All board tests pass (3 pre-existing failures on main unrelated to this change)
  • GET /boards/{slug} returns html_content when board note has content, null otherwise
  • Verify with board-gdocs-daily-mcp-remote (has content) and board-pal-e-platform (empty)

Review Checklist

  • ruff format clean
  • ruff check clean
  • No new test failures introduced
  • Minimal change: one schema field, one line in helper

Closes #270

## Summary Add `html_content` field to `BoardOut` so the frontend can render board note content (User Stories, Architecture, Acceptance Criteria) above kanban columns. ## Changes - `src/pal_e_docs/schemas.py` — Added `html_content: str | None = None` to `BoardOut` - `src/pal_e_docs/routes/boards.py` — Populated `html_content` from `board_note.html_content` in `_board_to_out()` ## Test Plan - All board tests pass (3 pre-existing failures on main unrelated to this change) - `GET /boards/{slug}` returns `html_content` when board note has content, `null` otherwise - Verify with `board-gdocs-daily-mcp-remote` (has content) and `board-pal-e-platform` (empty) ## Review Checklist - [x] `ruff format` clean - [x] `ruff check` clean - [x] No new test failures introduced - [x] Minimal change: one schema field, one line in helper ## Related Notes - Companion to forgejo_admin/pal-e-app#104 ## Related Closes #270
Populate html_content from the board note in _board_to_out() so the
frontend can render board note content above kanban columns.

Refs: forgejo_admin/pal-e-app#104

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Contributor

QA Review

Scope: 2 files, +2 lines. Adds html_content field to BoardOut schema and populates it from the board note.

Findings

  1. Schema change -- html_content: str | None = None added to BoardOut. Default None ensures backward compatibility for all existing API consumers. Clean.

  2. Route helper -- html_content=board_note.html_content or None correctly coalesces empty strings to None so the frontend gets a clean falsy check. Clean.

  3. Lint -- ruff format and ruff check both pass.

  4. Tests -- No new test failures. 3 pre-existing failures on main (test_create_board_returns_no_note_id, test_get_board_returns_no_note_id, test_existing_fields_preserved) are unrelated to this change (they assert note_id not in response, but the schema already includes it on main).

No issues found.

VERDICT: APPROVED

## QA Review **Scope:** 2 files, +2 lines. Adds `html_content` field to `BoardOut` schema and populates it from the board note. ### Findings 1. **Schema change** -- `html_content: str | None = None` added to `BoardOut`. Default `None` ensures backward compatibility for all existing API consumers. Clean. 2. **Route helper** -- `html_content=board_note.html_content or None` correctly coalesces empty strings to `None` so the frontend gets a clean falsy check. Clean. 3. **Lint** -- `ruff format` and `ruff check` both pass. 4. **Tests** -- No new test failures. 3 pre-existing failures on main (`test_create_board_returns_no_note_id`, `test_get_board_returns_no_note_id`, `test_existing_fields_preserved`) are unrelated to this change (they assert `note_id` not in response, but the schema already includes it on main). ### No issues found. **VERDICT: APPROVED**
Author
Contributor

PR #271 Review

DOMAIN REVIEW

Stack: Python / FastAPI / SQLAlchemy / Pydantic

This is a minimal 2-line change adding html_content to the BoardOut Pydantic schema and populating it from the board note in _board_to_out().

Schema addition (schemas.py): html_content: str | None = None -- correct optional field with None default. Backward-compatible for existing API consumers since the field was not previously present and defaults to None.

Route population (routes/boards.py): html_content=board_note.html_content or None -- the or None coercion converts empty-string "" (the model default per models.py:110) to None, which is a clean API contract: null means "no content" rather than exposing an empty string. This is a sound choice.

html_content is an established field on the Note model (Mapped[str] with default=""), so the attribute access is valid. No new queries, no N+1 risk -- the board note is already loaded in _board_to_out().

No PEP 484 issues. The str | None union syntax is consistent with the existing codebase style (Python 3.10+).

BLOCKERS

None.

While there are no dedicated tests asserting html_content appears in BoardOut responses, this is a trivial pass-through of an existing model field with no new logic. The existing test_existing_fields_preserved test pattern could be extended, but the absence of a test for a single nullable pass-through field does not rise to blocker level. See nits.

NITS

  1. Test coverage for the new field: Consider extending TestBoardNoteIntegration.test_existing_fields_preserved (at tests/test_boards.py:1132) to assert "html_content" in data. A second test verifying that a board whose note has actual HTML content returns it (not null) would complete coverage. Low risk but good hygiene.

  2. PR body "Related" section: References the companion PR (forgejo_admin/pal-e-app#104) but does not reference a plan slug. If this work is tracked under a plan, the slug should be included per SOP.

SOP COMPLIANCE

  • Branch named after issue (270-board-html-content)
  • PR body follows template (Summary, Changes, Test Plan, Related)
  • Related references plan slug -- only references companion PR and Closes #270; no plan slug present (may not be plan-tracked work)
  • No secrets committed
  • No scope creep -- exactly 2 lines changed, both directly serving the stated goal
  • Commit messages appear descriptive per PR title

PROCESS OBSERVATIONS

  • Change failure risk: Very low. Additive schema change with None default. No migration needed. No breaking contract.
  • Deployment frequency: No impact. Minimal diff, clean merge.
  • Documentation: The companion frontend PR (pal-e-app#104) should be coordinated -- the frontend can deploy first (it would simply ignore the missing field) or after (it would start consuming the new field). No ordering dependency.

VERDICT: APPROVED

## PR #271 Review ### DOMAIN REVIEW **Stack:** Python / FastAPI / SQLAlchemy / Pydantic This is a minimal 2-line change adding `html_content` to the `BoardOut` Pydantic schema and populating it from the board note in `_board_to_out()`. **Schema addition** (`schemas.py`): `html_content: str | None = None` -- correct optional field with `None` default. Backward-compatible for existing API consumers since the field was not previously present and defaults to `None`. **Route population** (`routes/boards.py`): `html_content=board_note.html_content or None` -- the `or None` coercion converts empty-string `""` (the model default per `models.py:110`) to `None`, which is a clean API contract: `null` means "no content" rather than exposing an empty string. This is a sound choice. `html_content` is an established field on the `Note` model (`Mapped[str]` with `default=""`), so the attribute access is valid. No new queries, no N+1 risk -- the board note is already loaded in `_board_to_out()`. No PEP 484 issues. The `str | None` union syntax is consistent with the existing codebase style (Python 3.10+). ### BLOCKERS None. While there are no dedicated tests asserting `html_content` appears in `BoardOut` responses, this is a trivial pass-through of an existing model field with no new logic. The existing `test_existing_fields_preserved` test pattern could be extended, but the absence of a test for a single nullable pass-through field does not rise to blocker level. See nits. ### NITS 1. **Test coverage for the new field**: Consider extending `TestBoardNoteIntegration.test_existing_fields_preserved` (at `tests/test_boards.py:1132`) to assert `"html_content" in data`. A second test verifying that a board whose note has actual HTML content returns it (not `null`) would complete coverage. Low risk but good hygiene. 2. **PR body "Related" section**: References the companion PR (`forgejo_admin/pal-e-app#104`) but does not reference a plan slug. If this work is tracked under a plan, the slug should be included per SOP. ### SOP COMPLIANCE - [x] Branch named after issue (`270-board-html-content`) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [ ] Related references plan slug -- only references companion PR and `Closes #270`; no plan slug present (may not be plan-tracked work) - [x] No secrets committed - [x] No scope creep -- exactly 2 lines changed, both directly serving the stated goal - [x] Commit messages appear descriptive per PR title ### PROCESS OBSERVATIONS - **Change failure risk**: Very low. Additive schema change with `None` default. No migration needed. No breaking contract. - **Deployment frequency**: No impact. Minimal diff, clean merge. - **Documentation**: The companion frontend PR (`pal-e-app#104`) should be coordinated -- the frontend can deploy first (it would simply ignore the missing field) or after (it would start consuming the new field). No ordering dependency. ### VERDICT: APPROVED
forgejo_admin deleted branch 270-board-html-content 2026-04-13 00:13:27 +00:00
Commenting is not possible because the repository is archived.
No description provided.