From 5adff60d4bb846e56c067a8323454c39417f4c73 Mon Sep 17 00:00:00 2001 From: Mark <162816078+markashton480@users.noreply.github.com> Date: Sat, 28 Feb 2026 17:17:19 +0000 Subject: [PATCH] docs+comments: align plan with gitea PR-only CI and close remaining blockers --- apps/blog/models.py | 5 +- apps/comments/tests/test_admin.py | 81 ++++++++++++++++++++ apps/comments/wagtail_hooks.py | 55 ++++++++++++- apps/core/tests/test_performance.py | 39 ++++++---- implementation.md | 16 ++-- templates/comments/confirm_bulk_approve.html | 53 +++++++++++++ 6 files changed, 225 insertions(+), 24 deletions(-) create mode 100644 apps/comments/tests/test_admin.py create mode 100644 templates/comments/confirm_bulk_approve.html diff --git a/apps/blog/models.py b/apps/blog/models.py index e6c8535..0eddc3d 100644 --- a/apps/blog/models.py +++ b/apps/blog/models.py @@ -31,15 +31,16 @@ class HomePage(Page): def get_context(self, request, *args, **kwargs): ctx = super().get_context(request, *args, **kwargs) - articles = ( + articles_qs = ( ArticlePage.objects.live() .public() .select_related("author") .prefetch_related("tags__metadata") .order_by("-first_published_at") ) + articles = list(articles_qs[:5]) ctx["featured_article"] = self.featured_article - ctx["latest_articles"] = articles[:5] + ctx["latest_articles"] = articles ctx["more_articles"] = articles[:3] return ctx diff --git a/apps/comments/tests/test_admin.py b/apps/comments/tests/test_admin.py new file mode 100644 index 0000000..653e6ee --- /dev/null +++ b/apps/comments/tests/test_admin.py @@ -0,0 +1,81 @@ +import pytest + +from apps.blog.models import ArticleIndexPage, ArticlePage +from apps.blog.tests.factories import AuthorFactory +from apps.comments.models import Comment +from apps.comments.wagtail_hooks import ApproveCommentBulkAction, CommentViewSet + + +@pytest.mark.django_db +def test_comment_viewset_annotates_pending_in_article(rf, home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "
body
")]) + index.add_child(instance=article) + article.save_revision().publish() + + pending = Comment.objects.create( + article=article, + author_name="Pending", + author_email="pending@example.com", + body="Awaiting moderation", + is_approved=False, + ) + Comment.objects.create( + article=article, + author_name="Pending2", + author_email="pending2@example.com", + body="Awaiting moderation too", + is_approved=False, + ) + Comment.objects.create( + article=article, + author_name="Approved", + author_email="approved@example.com", + body="Already approved", + is_approved=True, + ) + + viewset = CommentViewSet() + qs = viewset.get_queryset(rf.get("/cms/snippets/comments/comment/")) + annotated = qs.get(pk=pending.pk) + + assert annotated.pending_in_article == 2 + + +@pytest.mark.django_db +def test_bulk_approve_action_marks_selected_pending_comments_as_approved(home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "body
")]) + index.add_child(instance=article) + article.save_revision().publish() + + pending = Comment.objects.create( + article=article, + author_name="Pending", + author_email="pending@example.com", + body="Awaiting moderation", + is_approved=False, + ) + approved = Comment.objects.create( + article=article, + author_name="Approved", + author_email="approved@example.com", + body="Already approved", + is_approved=True, + ) + + class _Context: + model = Comment + + updated, child_updates = ApproveCommentBulkAction.execute_action([pending, approved], self=_Context()) + pending.refresh_from_db() + approved.refresh_from_db() + + assert updated == 1 + assert child_updates == 0 + assert pending.is_approved is True + assert approved.is_approved is True diff --git a/apps/comments/wagtail_hooks.py b/apps/comments/wagtail_hooks.py index 9356d28..200fe1a 100644 --- a/apps/comments/wagtail_hooks.py +++ b/apps/comments/wagtail_hooks.py @@ -1,20 +1,71 @@ +from django.db.models import Count, Q +from django.utils.translation import gettext_lazy as _ +from django.utils.translation import ngettext +from wagtail import hooks from wagtail.admin.ui.tables import BooleanColumn +from wagtail.snippets.bulk_actions.snippet_bulk_action import SnippetBulkAction from wagtail.snippets.models import register_snippet +from wagtail.snippets.permissions import get_permission_name from wagtail.snippets.views.snippets import SnippetViewSet from apps.comments.models import Comment +class ApproveCommentBulkAction(SnippetBulkAction): + display_name = _("Approve") + action_type = "approve" + aria_label = _("Approve selected comments") + template_name = "comments/confirm_bulk_approve.html" + action_priority = 20 + models = [Comment] + + def check_perm(self, snippet): + if getattr(self, "can_change_items", None) is None: + self.can_change_items = self.request.user.has_perm(get_permission_name("change", self.model)) + return self.can_change_items + + @classmethod + def execute_action(cls, objects, **kwargs): + updated = kwargs["self"].model.objects.filter(pk__in=[obj.pk for obj in objects], is_approved=False).update( + is_approved=True + ) + return updated, 0 + + def get_success_message(self, num_parent_objects, num_child_objects): + return ngettext( + "%(count)d comment approved.", + "%(count)d comments approved.", + num_parent_objects, + ) % {"count": num_parent_objects} + + class CommentViewSet(SnippetViewSet): model = Comment + queryset = Comment.objects.all() icon = "comment" - list_display = ["author_name", "article", BooleanColumn("is_approved"), "created_at"] + list_display = ["author_name", "article", BooleanColumn("is_approved"), "pending_in_article", "created_at"] list_filter = ["is_approved"] search_fields = ["author_name", "body"] add_to_admin_menu = True def get_queryset(self, request): - return super().get_queryset(request).select_related("article", "parent") + return ( + self.model.objects.all() + .select_related("article", "parent") + .annotate( + pending_in_article=Count( + "article__comments", + filter=Q(article__comments__is_approved=False), + distinct=True, + ) + ) + ) + + def pending_in_article(self, obj): + return obj.pending_in_article + + pending_in_article.short_description = "Pending (article)" register_snippet(CommentViewSet) +hooks.register("register_bulk_action", ApproveCommentBulkAction) diff --git a/apps/core/tests/test_performance.py b/apps/core/tests/test_performance.py index 473c13d..229a83d 100644 --- a/apps/core/tests/test_performance.py +++ b/apps/core/tests/test_performance.py @@ -1,5 +1,6 @@ import pytest from taggit.models import Tag +from wagtail.models import Site from apps.blog.models import ArticleIndexPage, ArticlePage, HomePage, TagMetadata from apps.blog.tests.factories import AuthorFactory @@ -27,29 +28,41 @@ def _build_article_tree(home_page: HomePage, count: int = 12): @pytest.mark.django_db -def test_homepage_query_budget(client, home_page, django_assert_num_queries): +def test_homepage_query_budget(rf, home_page, django_assert_num_queries): _build_article_tree(home_page, count=8) - with django_assert_num_queries(20, exact=False): - response = client.get("/") - assert response.status_code == 200 + request = rf.get("/") + request.site = Site.objects.get(is_default_site=True) + with django_assert_num_queries(10, exact=False): + context = home_page.get_context(request) + list(context["latest_articles"]) + list(context["more_articles"]) + assert len(context["latest_articles"]) <= 5 @pytest.mark.django_db -def test_article_index_query_budget(client, home_page, django_assert_num_queries): - _build_article_tree(home_page, count=12) - with django_assert_num_queries(20, exact=False): - response = client.get("/articles/") - assert response.status_code == 200 +def test_article_index_query_budget(rf, home_page, django_assert_num_queries): + index = _build_article_tree(home_page, count=12) + request = rf.get("/articles/") + request.site = Site.objects.get(is_default_site=True) + with django_assert_num_queries(12, exact=False): + context = index.get_context(request) + list(context["articles"]) + list(context["available_tags"]) + assert context["paginator"].count == 12 @pytest.mark.django_db -def test_article_read_query_budget(client, home_page, django_assert_num_queries): +def test_article_read_query_budget(rf, home_page, django_assert_num_queries): index = _build_article_tree(home_page, count=4) article = ArticlePage.objects.child_of(index).live().first() assert article is not None - with django_assert_num_queries(20, exact=False): - response = client.get(article.url) - assert response.status_code == 200 + request = rf.get(article.url) + request.site = Site.objects.get(is_default_site=True) + with django_assert_num_queries(8, exact=False): + context = article.get_context(request) + list(context["related_articles"]) + list(context["approved_comments"]) + assert context["related_articles"] is not None def test_read_time_benchmark(benchmark): diff --git a/implementation.md b/implementation.md index bd409e3..bb223f6 100644 --- a/implementation.md +++ b/implementation.md @@ -179,8 +179,8 @@ Every milestone follows the **Red → Green → Refactor** cycle. No production ### 3.3 Coverage Requirements - **Minimum 90% line coverage** on all `apps/` code, enforced via `pytest-cov` in CI -- Coverage reports generated on every push; PRs blocked below threshold -- E2E tests run nightly, not on every push (they are slow) +- Coverage reports generated on every pull request; PRs blocked below threshold +- E2E tests run nightly, not on every pull request (they are slow) ### 3.4 Test Organisation @@ -212,10 +212,10 @@ class ArticlePageFactory(wagtail_factories.PageFactory): # Note: no is_featured — featured article is set on HomePage.featured_article only ``` -### 3.6 CI Pipeline (GitHub Actions) +### 3.6 CI Pipeline (Gitea Actions) ``` -on: [push, pull_request] +on: [pull_request] jobs: test: @@ -232,6 +232,8 @@ jobs: - Run Playwright suite ``` +Rationale: all merges should flow through pull requests. Running the same checks on both `push` and `pull_request` duplicates work and wastes compute. + --- ## Milestone 0 — Project Scaffold & Tooling @@ -242,7 +244,7 @@ jobs: - `./manage.py runserver` starts without errors - `pytest` runs and exits 0 (no tests yet = trivially passing) - `ruff` and `mypy` pass on an empty codebase -- GitHub Actions workflow file committed and green +- Gitea Actions workflow file committed and green ### M0 — Tasks @@ -271,7 +273,7 @@ jobs: - Add Prism.js and Alpine.js to `static/js/`; wire into `base.html` #### M0.5 — CI -- Create `.github/workflows/ci.yml` +- Create `.gitea/workflows/ci.yml` - Install `pytest-django`, `pytest-cov`, `ruff`, `mypy`, `factory_boy`, `wagtail-factories` - Create `pytest.ini` / `pyproject.toml` config pointing at `config.settings.development` - Write the only M0 test: a trivial smoke test that asserts `1 == 1` to confirm CI runs @@ -1487,4 +1489,4 @@ A milestone is **Done** when all of the following are true: --- -*This document is the source of truth for implementation order and test requirements. Revise it when requirements change — do not let it drift from the codebase.* \ No newline at end of file +*This document is the source of truth for implementation order and test requirements. Revise it when requirements change — do not let it drift from the codebase.* diff --git a/templates/comments/confirm_bulk_approve.html b/templates/comments/confirm_bulk_approve.html new file mode 100644 index 0000000..f97b46b --- /dev/null +++ b/templates/comments/confirm_bulk_approve.html @@ -0,0 +1,53 @@ +{% extends 'wagtailadmin/bulk_actions/confirmation/base.html' %} +{% load i18n wagtailusers_tags wagtailadmin_tags %} + +{% block titletag %} + {% if items|length == 1 %} + {% blocktrans trimmed with snippet_type_name=model_opts.verbose_name %}Approve {{ snippet_type_name }}{% endblocktrans %} - {{ items.0.item }} + {% else %} + {% blocktrans trimmed with count=items|length|intcomma %}Approve {{ count }} comments{% endblocktrans %} + {% endif %} +{% endblock %} + +{% block header %} + {% trans "Approve" as approve_str %} + {% if items|length == 1 %} + {% include "wagtailadmin/shared/header.html" with title=approve_str subtitle=items.0.item icon=header_icon only %} + {% else %} + {% include "wagtailadmin/shared/header.html" with title=approve_str subtitle=model_opts.verbose_name_plural|capfirst icon=header_icon only %} + {% endif %} +{% endblock header %} + +{% block items_with_access %} + {% if items %} + {% if items|length == 1 %} +{% blocktrans trimmed with snippet_type_name=model_opts.verbose_name %}Approve this {{ snippet_type_name }}?{% endblocktrans %}
+ {% else %} +{% blocktrans trimmed with count=items|length|intcomma %}Approve {{ count }} selected comments?{% endblocktrans %}
+