7e-1a: QA nits cleanup — redundant assignment + test helper hygiene #111

New issue

Closed

opened 2026-03-08 07:06:22 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-03-08 07:06:22 +00:00

Contributor

Lineage

plan-2026-02-26-tf-modularize-postgres → Phase 7 → Phase 7e → Phase 7e-1a

Repo

forgejo_admin/pal-e-docs

User Story

As a developer reading the note sync code,
I want the create_note flow to clearly show that html_content comes from the compiler,
So that I don't mistakenly think raw input is persisted.

Context

QA review of PR #110 (7e-1) found 3 non-blocking nits. This issue addresses them.

File Targets

Files to modify:

src/pal_e_docs/routes/notes.py — change html_content=body.html_content to html_content="" in create_note() Note constructor (line ~273). parse_and_store_blocks() overwrites it with compiled output anyway.
tests/test_note_block_sync.py — improve _count_blocks(), _get_blocks(), _get_compiled_page() helpers:
1. Consider using the test client's DB session instead of opening separate TestingSessionLocal() sessions
2. If separate sessions are kept, add a comment explaining the commit-ordering dependency
3. For _get_compiled_page(), either eagerly load needed attributes or add a comment noting the detached object limitation

Acceptance Criteria

create_note constructor uses html_content="" (not raw input)
Test helpers either use shared session or document the separate-session pattern
All 503+ tests still pass
No behavior changes — purely readability/hygiene

Test Expectations

Run command: pytest tests/ -v

Constraints

No behavior changes. Readability only.

Checklist

PR opened
Tests pass
No unrelated changes

PR #110 — the QA review that identified these nits

### Lineage `plan-2026-02-26-tf-modularize-postgres` → Phase 7 → Phase 7e → Phase 7e-1a ### Repo `forgejo_admin/pal-e-docs` ### User Story As a developer reading the note sync code, I want the create_note flow to clearly show that html_content comes from the compiler, So that I don't mistakenly think raw input is persisted. ### Context QA review of PR #110 (7e-1) found 3 non-blocking nits. This issue addresses them. ### File Targets Files to modify: - `src/pal_e_docs/routes/notes.py` — change `html_content=body.html_content` to `html_content=""` in `create_note()` Note constructor (line ~273). `parse_and_store_blocks()` overwrites it with compiled output anyway. - `tests/test_note_block_sync.py` — improve `_count_blocks()`, `_get_blocks()`, `_get_compiled_page()` helpers: 1. Consider using the test client's DB session instead of opening separate `TestingSessionLocal()` sessions 2. If separate sessions are kept, add a comment explaining the commit-ordering dependency 3. For `_get_compiled_page()`, either eagerly load needed attributes or add a comment noting the detached object limitation ### Acceptance Criteria - [ ] `create_note` constructor uses `html_content=""` (not raw input) - [ ] Test helpers either use shared session or document the separate-session pattern - [ ] All 503+ tests still pass - [ ] No behavior changes — purely readability/hygiene ### Test Expectations - Run command: `pytest tests/ -v` ### Constraints - No behavior changes. Readability only. ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No unrelated changes ### Related - PR #110 — the QA review that identified these nits