From 4231f17b25ec953e0e351a8b8451ad2ef4e85d8a Mon Sep 17 00:00:00 2001 From: Xeek <6032840+x3ek@users.noreply.github.com> Date: Sat, 21 Mar 2026 08:32:37 -0500 Subject: [PATCH 1/2] fix(engine): wire notes into post and page rendering Fetch notes from NotesService in post and page routers instead of hardcoding empty list. Public notes visible to all users, private notes only to admins. Extract shared is_admin helper to dependencies.py. Closes #59 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/squishmark/dependencies.py | 13 ++++++++++++- src/squishmark/routers/pages.py | 13 ++++++++++--- src/squishmark/routers/posts.py | 28 +++++++++++----------------- tests/test_nav_pages.py | 8 ++++++-- tests/test_posts.py | 28 ++++++++++++++-------------- 5 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/squishmark/dependencies.py b/src/squishmark/dependencies.py index 18ea06d..b81a316 100644 --- a/src/squishmark/dependencies.py +++ b/src/squishmark/dependencies.py @@ -2,9 +2,20 @@ from typing import Annotated -from fastapi import Depends +from fastapi import Depends, Request from squishmark.config import Settings, get_settings # Type alias for settings dependency SettingsDep = Annotated[Settings, Depends(get_settings)] + + +def is_admin(request: Request) -> bool: + """Check if the current user is an admin without requiring auth.""" + settings = get_settings() + if settings.debug and settings.dev_skip_auth: + return True + user = request.session.get("user") if hasattr(request, "session") else None + if user is None: + return False + return user.get("login") in settings.admin_users_list diff --git a/src/squishmark/routers/pages.py b/src/squishmark/routers/pages.py index c91c7d4..fe32bb1 100644 --- a/src/squishmark/routers/pages.py +++ b/src/squishmark/routers/pages.py @@ -1,12 +1,16 @@ """Routes for static pages.""" -from fastapi import APIRouter, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import HTMLResponse +from sqlalchemy.ext.asyncio import AsyncSession +from squishmark.dependencies import is_admin from squishmark.models.content import Config +from squishmark.models.db import get_db_session from squishmark.services.content import get_all_posts, get_featured_posts from squishmark.services.github import get_github_service from squishmark.services.markdown import get_markdown_service +from squishmark.services.notes import NotesService from squishmark.services.theme import get_theme_engine router = APIRouter(tags=["pages"]) @@ -16,6 +20,7 @@ async def get_page( request: Request, slug: str, + db: AsyncSession = Depends(get_db_session), ) -> HTMLResponse: """ Get a static page by slug. @@ -46,8 +51,10 @@ async def get_page( if page.visibility == "hidden": raise HTTPException(status_code=404, detail="Page not found") - # TODO: Get notes for this page from database - notes: list = [] + # Fetch notes (private notes only visible to admins) + admin = is_admin(request) + notes_service = NotesService(db) + notes = await notes_service.get_for_path(f"/{slug}", include_private=admin) # Featured posts for template context all_posts = await get_all_posts(github_service, markdown_service) diff --git a/src/squishmark/routers/posts.py b/src/squishmark/routers/posts.py index 5aa5e4a..0531433 100644 --- a/src/squishmark/routers/posts.py +++ b/src/squishmark/routers/posts.py @@ -1,29 +1,21 @@ """Routes for blog posts.""" -from fastapi import APIRouter, HTTPException, Query, Request +from fastapi import APIRouter, Depends, HTTPException, Query, Request from fastapi.responses import HTMLResponse +from sqlalchemy.ext.asyncio import AsyncSession -from squishmark.config import get_settings +from squishmark.dependencies import is_admin from squishmark.models.content import Config, Pagination +from squishmark.models.db import get_db_session from squishmark.services.content import get_all_posts, get_featured_posts from squishmark.services.github import get_github_service from squishmark.services.markdown import get_markdown_service +from squishmark.services.notes import NotesService from squishmark.services.theme import get_theme_engine router = APIRouter(prefix="/posts", tags=["posts"]) -def _is_admin(request: Request) -> bool: - """Check if the current user is an admin without requiring auth.""" - settings = get_settings() - if settings.debug and settings.dev_skip_auth: - return True - user = request.session.get("user") if hasattr(request, "session") else None - if user is None: - return False - return user.get("login") in settings.admin_users_list - - @router.get("", response_class=HTMLResponse) async def list_posts( request: Request, @@ -40,7 +32,7 @@ async def list_posts( markdown_service = get_markdown_service(config) # Get all posts (admins can see drafts) - include_drafts = _is_admin(request) + include_drafts = is_admin(request) all_posts = await get_all_posts(github_service, markdown_service, include_drafts=include_drafts) # Paginate @@ -76,6 +68,7 @@ async def list_posts( async def get_post( request: Request, slug: str, + db: AsyncSession = Depends(get_db_session), ) -> HTMLResponse: """Get a single post by slug.""" github_service = get_github_service() @@ -88,7 +81,7 @@ async def get_post( markdown_service = get_markdown_service(config) # Get all posts and find the matching one (admins can see drafts) - include_drafts = _is_admin(request) + include_drafts = is_admin(request) all_posts = await get_all_posts(github_service, markdown_service, include_drafts=include_drafts) post = next((p for p in all_posts if p.slug == slug), None) @@ -96,8 +89,9 @@ async def get_post( if post is None: raise HTTPException(status_code=404, detail="Post not found") - # TODO: Get notes for this post from database - notes: list = [] + # Fetch notes (private notes only visible to admins) + notes_service = NotesService(db) + notes = await notes_service.get_for_path(f"/posts/{slug}", include_private=include_drafts) # Featured posts for template context featured = get_featured_posts(all_posts, config.site) diff --git a/tests/test_nav_pages.py b/tests/test_nav_pages.py index 352f8f7..dba0136 100644 --- a/tests/test_nav_pages.py +++ b/tests/test_nav_pages.py @@ -81,14 +81,16 @@ async def test_hidden_page_returns_404(self): with ( patch("squishmark.routers.pages.get_github_service") as mock_get_github, patch("squishmark.routers.pages.get_theme_engine"), + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, ): mock_github = AsyncMock() mock_get_github.return_value = mock_github mock_github.get_config.return_value = None mock_github.get_file.return_value = MagicMock(content=hidden_content) + mock_notes_cls.return_value = AsyncMock() with pytest.raises(HTTPException) as exc_info: - await get_page(mock_request, "secret") + await get_page(mock_request, "secret", db=AsyncMock()) assert exc_info.value.status_code == 404 @@ -102,18 +104,20 @@ async def test_unlisted_page_does_not_404(self): with ( patch("squishmark.routers.pages.get_github_service") as mock_get_github, patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, ): mock_github = AsyncMock() mock_get_github.return_value = mock_github mock_github.get_config.return_value = None mock_github.get_file.return_value = MagicMock(content=unlisted_content) + mock_notes_cls.return_value = AsyncMock() mock_engine = AsyncMock() mock_get_engine.return_value = mock_engine mock_engine.render_page.return_value = "Unlisted" mock_request = MagicMock() - response = await get_page(mock_request, "unlisted") + response = await get_page(mock_request, "unlisted", db=AsyncMock()) assert response.status_code == 200 diff --git a/tests/test_posts.py b/tests/test_posts.py index 91af507..3411349 100644 --- a/tests/test_posts.py +++ b/tests/test_posts.py @@ -4,7 +4,7 @@ import pytest -from squishmark.routers.posts import _is_admin +from squishmark.dependencies import is_admin from squishmark.services.content import get_all_posts @@ -17,62 +17,62 @@ def mock_request(): class TestIsAdmin: - """Tests for the _is_admin helper.""" + """Tests for the is_admin helper.""" def test_anonymous_user_is_not_admin(self, mock_request): """Anonymous visitors should not be detected as admin.""" - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = False mock_settings.return_value.dev_skip_auth = False mock_settings.return_value.admin_users_list = ["admin-user"] - assert _is_admin(mock_request) is False + assert is_admin(mock_request) is False def test_authenticated_admin_is_admin(self, mock_request): """Authenticated admin users should be detected.""" mock_request.session = {"user": {"login": "admin-user"}} - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = False mock_settings.return_value.dev_skip_auth = False mock_settings.return_value.admin_users_list = ["admin-user"] - assert _is_admin(mock_request) is True + assert is_admin(mock_request) is True def test_authenticated_non_admin_is_not_admin(self, mock_request): """Authenticated non-admin users should not be detected as admin.""" mock_request.session = {"user": {"login": "regular-user"}} - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = False mock_settings.return_value.dev_skip_auth = False mock_settings.return_value.admin_users_list = ["admin-user"] - assert _is_admin(mock_request) is False + assert is_admin(mock_request) is False def test_dev_skip_auth_is_admin(self, mock_request): """Dev mode with skip auth should be detected as admin.""" - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = True mock_settings.return_value.dev_skip_auth = True - assert _is_admin(mock_request) is True + assert is_admin(mock_request) is True def test_debug_without_skip_auth_is_not_admin(self, mock_request): """Debug mode alone (without dev_skip_auth) should not grant admin.""" - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = True mock_settings.return_value.dev_skip_auth = False mock_settings.return_value.admin_users_list = [] - assert _is_admin(mock_request) is False + assert is_admin(mock_request) is False def test_no_session_attribute(self): """Request without session attribute should not be admin.""" request = MagicMock(spec=[]) # No attributes at all - with patch("squishmark.routers.posts.get_settings") as mock_settings: + with patch("squishmark.dependencies.get_settings") as mock_settings: mock_settings.return_value.debug = False mock_settings.return_value.dev_skip_auth = False - assert _is_admin(request) is False + assert is_admin(request) is False class TestGetAllPostsDraftFiltering: From 58b150e1f2cd535ac8ec97c5f5699d4a138bab76 Mon Sep 17 00:00:00 2001 From: Xeek <6032840+x3ek@users.noreply.github.com> Date: Sat, 21 Mar 2026 08:40:25 -0500 Subject: [PATCH 2/2] test(engine): add tests for notes rendering on posts and pages Test correct path format, admin vs anonymous visibility, notes passed to templates, and empty notes rendering for both post and page routers. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_notes_rendering.py | 300 ++++++++++++++++++++++++++++++++++ 1 file changed, 300 insertions(+) create mode 100644 tests/test_notes_rendering.py diff --git a/tests/test_notes_rendering.py b/tests/test_notes_rendering.py new file mode 100644 index 0000000..0d7f92b --- /dev/null +++ b/tests/test_notes_rendering.py @@ -0,0 +1,300 @@ +"""Tests for notes rendering on posts and pages.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +POST_CONTENT = "---\ntitle: Test Post\ndate: 2026-01-01\n---\nContent" +PAGE_CONTENT = "---\ntitle: About\n---\nContent" + + +def _admin_request(): + """Create a mock request for an authenticated admin.""" + request = MagicMock() + request.session = {"user": {"login": "admin-user"}} + return request + + +def _anonymous_request(): + """Create a mock request for an anonymous visitor.""" + request = MagicMock() + request.session = {} + return request + + +class TestPostNotesRendering: + """Tests for notes being fetched and passed to templates on post pages.""" + + @pytest.mark.asyncio + async def test_notes_fetched_with_correct_path(self): + """NotesService should be called with /posts/{slug} path.""" + from squishmark.routers.posts import get_post + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.posts.get_github_service") as mock_get_github, + patch("squishmark.routers.posts.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.posts.NotesService") as mock_notes_cls, + patch("squishmark.routers.posts.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.list_directory.return_value = ["posts/2026-01-01-my-post.md"] + mock_github.get_file.return_value = MagicMock(content=POST_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_post=AsyncMock(return_value="")) + + await get_post(_anonymous_request(), "my-post", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/posts/my-post", include_private=False) + + @pytest.mark.asyncio + async def test_admin_sees_private_notes(self): + """Admin requests should fetch private notes (include_private=True).""" + from squishmark.routers.posts import get_post + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.posts.get_github_service") as mock_get_github, + patch("squishmark.routers.posts.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.posts.NotesService") as mock_notes_cls, + patch("squishmark.routers.posts.is_admin", return_value=True), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.list_directory.return_value = ["posts/2026-01-01-my-post.md"] + mock_github.get_file.return_value = MagicMock(content=POST_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_post=AsyncMock(return_value="")) + + await get_post(_admin_request(), "my-post", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/posts/my-post", include_private=True) + + @pytest.mark.asyncio + async def test_anonymous_cannot_see_private_notes(self): + """Anonymous requests should not fetch private notes.""" + from squishmark.routers.posts import get_post + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.posts.get_github_service") as mock_get_github, + patch("squishmark.routers.posts.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.posts.NotesService") as mock_notes_cls, + patch("squishmark.routers.posts.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.list_directory.return_value = ["posts/2026-01-01-my-post.md"] + mock_github.get_file.return_value = MagicMock(content=POST_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_post=AsyncMock(return_value="")) + + await get_post(_anonymous_request(), "my-post", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/posts/my-post", include_private=False) + + @pytest.mark.asyncio + async def test_notes_passed_to_template(self): + """Fetched notes should be passed to render_post.""" + from squishmark.routers.posts import get_post + + fake_notes = [MagicMock(text="Note 1"), MagicMock(text="Note 2")] + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = fake_notes + + with ( + patch("squishmark.routers.posts.get_github_service") as mock_get_github, + patch("squishmark.routers.posts.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.posts.NotesService") as mock_notes_cls, + patch("squishmark.routers.posts.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.list_directory.return_value = ["posts/2026-01-01-my-post.md"] + mock_github.get_file.return_value = MagicMock(content=POST_CONTENT) + mock_notes_cls.return_value = mock_notes + + mock_engine = AsyncMock() + mock_get_engine.return_value = mock_engine + mock_engine.render_post.return_value = "" + + await get_post(_anonymous_request(), "my-post", db=AsyncMock()) + + # Verify notes were passed as the third positional arg to render_post + call_args = mock_engine.render_post.call_args + assert call_args[0][2] == fake_notes + + @pytest.mark.asyncio + async def test_no_notes_renders_fine(self): + """Post with no notes should render without error.""" + from squishmark.routers.posts import get_post + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.posts.get_github_service") as mock_get_github, + patch("squishmark.routers.posts.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.posts.NotesService") as mock_notes_cls, + patch("squishmark.routers.posts.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.list_directory.return_value = ["posts/2026-01-01-my-post.md"] + mock_github.get_file.return_value = MagicMock(content=POST_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_post=AsyncMock(return_value="")) + + response = await get_post(_anonymous_request(), "my-post", db=AsyncMock()) + + assert response.status_code == 200 + call_args = mock_get_engine.return_value.render_post.call_args + assert call_args[0][2] == [] + + +class TestPageNotesRendering: + """Tests for notes being fetched and passed to templates on static pages.""" + + @pytest.mark.asyncio + async def test_notes_fetched_with_correct_path(self): + """NotesService should be called with /{slug} path.""" + from squishmark.routers.pages import get_page + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.pages.get_github_service") as mock_get_github, + patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, + patch("squishmark.routers.pages.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.get_file.return_value = MagicMock(content=PAGE_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_page=AsyncMock(return_value="")) + + await get_page(_anonymous_request(), "about", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/about", include_private=False) + + @pytest.mark.asyncio + async def test_admin_sees_private_notes(self): + """Admin requests should fetch private notes (include_private=True).""" + from squishmark.routers.pages import get_page + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.pages.get_github_service") as mock_get_github, + patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, + patch("squishmark.routers.pages.is_admin", return_value=True), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.get_file.return_value = MagicMock(content=PAGE_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_page=AsyncMock(return_value="")) + + await get_page(_admin_request(), "about", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/about", include_private=True) + + @pytest.mark.asyncio + async def test_anonymous_cannot_see_private_notes(self): + """Anonymous requests should not fetch private notes.""" + from squishmark.routers.pages import get_page + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.pages.get_github_service") as mock_get_github, + patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, + patch("squishmark.routers.pages.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.get_file.return_value = MagicMock(content=PAGE_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_page=AsyncMock(return_value="")) + + await get_page(_anonymous_request(), "about", db=AsyncMock()) + + mock_notes.get_for_path.assert_called_once_with("/about", include_private=False) + + @pytest.mark.asyncio + async def test_notes_passed_to_template(self): + """Fetched notes should be passed to render_page.""" + from squishmark.routers.pages import get_page + + fake_notes = [MagicMock(text="Correction")] + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = fake_notes + + with ( + patch("squishmark.routers.pages.get_github_service") as mock_get_github, + patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, + patch("squishmark.routers.pages.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.get_file.return_value = MagicMock(content=PAGE_CONTENT) + mock_notes_cls.return_value = mock_notes + + mock_engine = AsyncMock() + mock_get_engine.return_value = mock_engine + mock_engine.render_page.return_value = "" + + await get_page(_anonymous_request(), "about", db=AsyncMock()) + + call_args = mock_engine.render_page.call_args + assert call_args[0][2] == fake_notes + + @pytest.mark.asyncio + async def test_no_notes_renders_fine(self): + """Page with no notes should render without error.""" + from squishmark.routers.pages import get_page + + mock_notes = AsyncMock() + mock_notes.get_for_path.return_value = [] + + with ( + patch("squishmark.routers.pages.get_github_service") as mock_get_github, + patch("squishmark.routers.pages.get_theme_engine") as mock_get_engine, + patch("squishmark.routers.pages.NotesService") as mock_notes_cls, + patch("squishmark.routers.pages.is_admin", return_value=False), + ): + mock_github = AsyncMock() + mock_get_github.return_value = mock_github + mock_github.get_config.return_value = None + mock_github.get_file.return_value = MagicMock(content=PAGE_CONTENT) + mock_notes_cls.return_value = mock_notes + mock_get_engine.return_value = AsyncMock(render_page=AsyncMock(return_value="")) + + response = await get_page(_anonymous_request(), "about", db=AsyncMock()) + + assert response.status_code == 200 + call_args = mock_get_engine.return_value.render_page.call_args + assert call_args[0][2] == []