Story 2375: Wagtail Integration for create a post - #2526
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughV3 submissions now create Wagtail drafts with optional images and library tags, save revisions, and start workflows. Library options are dynamic, image limits are 5 MB, link summaries use safer extraction, draft navigation changes, and hourly scheduling publishes eligible pages. ChangesV3 Post Creation
Scheduled Page Publishing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant V3CreateForm
participant V3AllTypesCreateView
participant PostIndexPage
participant PostPage
participant ContentTag
participant Workflow
V3CreateForm->>V3AllTypesCreateView: submit selected post type and fields
V3AllTypesCreateView->>PostIndexPage: resolve singleton index page
V3AllTypesCreateView->>PostPage: create draft with block content
V3AllTypesCreateView->>ContentTag: attach related library tags
V3AllTypesCreateView->>PostPage: save revision
V3AllTypesCreateView->>Workflow: start workflow
sequenceDiagram
participant CeleryBeat
participant publish_scheduled_pages
participant publish_scheduled
CeleryBeat->>publish_scheduled_pages: trigger hourly task
publish_scheduled_pages->>publish_scheduled: call management command
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
news/views.py (1)
524-529: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMap post types directly to block names.
The block instance is never used, triggering RUF059, while Ruff also requires the mutable class mapping to be declared as
ClassVar.Proposed cleanup
+from typing import ClassVar - _POST_BLOCK_MAP: dict[str, tuple[str, Block]] = { - "blog": BLOG_BLOCK, - "news": NEWS_BLOCK, - "link": LINK_BLOCK, - "video": VIDEO_BLOCK, + _POST_BLOCK_MAP: ClassVar[dict[str, str]] = { + "blog": BLOG_BLOCK[0], + "news": NEWS_BLOCK[0], + "link": LINK_BLOCK[0], + "video": VIDEO_BLOCK[0], } ... - block_config = self._POST_BLOCK_MAP.get(post_type, None) + block_name = self._POST_BLOCK_MAP.get(post_type) ... - if block_config is None or form_class is None: + if block_name is None or form_class is None: ... - block_name, block_class = block_configAlso applies to: 552-563
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@news/views.py` around lines 524 - 529, Update the post-type mappings in the relevant view class to map each type directly to its block-name string instead of storing unused block instances, and annotate the mutable class-level mappings with ClassVar. Apply the same change to both _POST_BLOCK_MAP and the additional mapping around the referenced section, preserving all existing post-type keys and block names.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@news/views.py`:
- Around line 580-582: Update the view flow around PostIndexPage.objects.first()
to explicitly handle a missing index_page before calling
index_page.add_child(...). Ensure fresh or misconfigured sites receive the
intended safe response or initialization behavior instead of dereferencing None,
while preserving the existing path when a PostIndexPage exists.
- Around line 596-603: Update the image handling in the form submission flow to
avoid using Image.objects.get_or_create with title=image.name, which can
associate an unrelated existing image. Create a fresh Wagtail Image for each
upload, or use an explicit file-content-based deduplication strategy if reuse is
required, then assign the resulting image to page.image.
- Around line 604-627: Update the tag assignment in the flow around
index_page.add_child and page.tags.add so TaggableManager receives the
ContentTag instance directly, or explicitly unpack the collection if retaining
plural handling; preserve the existing conditional behavior and ensure tagging
succeeds after page creation.
- Around line 586-588: In the page update flow, replace the assignment to the
read-only cached property page.publish_at with an assignment to page.go_live_at
using the cleaned publish_at value. Ensure this occurs before save_revision() so
the requested publish time is persisted on PostPage.
---
Nitpick comments:
In `@news/views.py`:
- Around line 524-529: Update the post-type mappings in the relevant view class
to map each type directly to its block-name string instead of storing unused
block instances, and annotate the mutable class-level mappings with ClassVar.
Apply the same change to both _POST_BLOCK_MAP and the additional mapping around
the referenced section, preserving all existing post-type keys and block names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e130d60d-0c42-4ee0-911e-df1366e9b5e7
📒 Files selected for processing (2)
news/views.pypages/blocks.py
julhoang
left a comment
julhoang
left a comment
There was a problem hiding this comment.
Hi Jeremy! I think we're missing the image reduction task from the workflow. Would you mind adding it?
Accepted formats: PNG and JPEG. Maximum upload file size: 5MB. Any uploaded file larger 1MB should be reduced to <1MB before saving to S3, the reduction function is out of scope. Note: Please update the image limit in the copy from 1MB to 5MB
| @@ -392,11 +397,13 @@ def _v3_create_context(): | |||
| ], | |||
| "related_libraries_options": [ | |||
| ("", "Select"), | |||
There was a problem hiding this comment.
Should we remove this ("", "Select") option as well?
There was a problem hiding this comment.
I'd like to maintain the Select option, or otherwise have a "clear selection" option, just in case the post is actually unrelated to any library and one is selected in error.
There was a problem hiding this comment.
Also, I've gone ahead and added the image scaling, thanks for the catch!
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Heya! Awesome to see such a small PR to integrate with the Wagtail workflow :D
It's all looking good, I just wanted one clarification: how exactly are scheduled publishes gonna be switched to published? Apparently there's a wagtail command called publish_scheduled that must be executed to do that, and there's no place in code where that happens.
Claude suggested something like:
# This in news/tasks.py
from celery import shared_task
from django.core.management import call_command
@shared_task
def publish_scheduled_pages():
"""Publish Wagtail pages whose approved go_live_at has passed
(and unpublish pages past their expire_at)."""
call_command("publish_scheduled")
# Then this in config/celery.py inside setup_periodic_tasks:
# Publish approved Wagtail pages whose go_live_at has passed.
# Executes every 10 minutes.
sender.add_periodic_task(
crontab(minute="*/10"),
app.signature("news.tasks.publish_scheduled_pages"),
) Does that look accurate?
Good call! I've added this task, although I'm scheduling it for once an hour instead of every ten minutes, per Wagtails recommendation: https://docs.wagtail.org/en/stable/reference/management_commands.html#publish-scheduled |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/celery.py`:
- Around line 141-145: Update the periodic task registration using crontab in
the sender.add_periodic_task call to explicitly set minute=0 while retaining the
hourly hour="*/1" schedule, so publish_scheduled_pages runs once per hour.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44c04487-d05e-4afc-80ca-c18050d25a01
📒 Files selected for processing (3)
config/celery.pynews/tasks.pynews/views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- news/views.py
julhoang
left a comment
julhoang
left a comment
There was a problem hiding this comment.
Hi @jlchilders11 , I just have 2 small additional requests and an optional suggestion down below – everything else looks great and works great! 🙏
1/ Would you mind updating the image size restriction (both UI and BE) to 5MB, as per the ticket requirement? I think the image reduction should still trigger for any images larger than 1MB as-is, but from a UX perspective we'll allow users to upload up to 5MB for their convenience.
2/ This suggestion is slightly out of scope and optional:
I noticed the Wagtail implementation ticket for the set_summary_for_link_page function is missing the hardening methods that block SSRF to internal/private hosts and re-validate redirects – the ones Javier already set up in the Entry-type set_summary_for_link_entry with safe_get + extract_article. I'm wondering if we might be able to reuse those two functions in set_summary_for_link_page as well?
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@news/models.py`:
- Line 115: Update the EntryForm.image validators to remove or replace the 1 MB
max_file_size_validator so form validation accepts uploads up to the model’s 5
MB limit, while retaining image_validator and
downscale_image_file_size_validator.
In `@news/tests/test_models.py`:
- Line 107: Update the relevant image-size test fixtures to include a valid 5 MB
or intermediate 1–5 MB payload, retain the existing invalid payload at 5 MB plus
one byte, and revise the nearby “just over 1MB” comment to accurately describe
the boundary being tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc7e1dc4-0c4e-4997-8ddd-a433d359e9d9
📒 Files selected for processing (3)
news/forms.pynews/models.pynews/tests/test_models.py
🚧 Files skipped from review as they are similar to previous changes (1)
- news/forms.py
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Heya, sorry for the vague question, but any ideas of what might be causing this error?
I created a post, approved it as a moderator then changed the Celery task schedule to run it in the next minute, then I got this error.
Manually approving and publishing a post or clicking "schedule to publish" also yields the same errors.
celery-worker-1 | 2026-07-22 13:49:11 [ERROR] index.py:185 - Exception raised while adding <PostPage: Something in the news post title> into the 'default' search backend
celery-worker-1 | Traceback (most recent call last):
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 105, in _execute
celery-worker-1 | return self.cursor.execute(sql, params)
celery-worker-1 | ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
celery-worker-1 | psycopg2.errors.NotNullViolation: null value in column "title_text" of relation "wagtailsearch_indexentry" violates not-null constraint
celery-worker-1 | DETAIL: Failing row contains (411, 128, 1, 113, 'in':2 'news':4 'post':5 'something':1 'the':3 'title':6, 'news':4B 'post':5B 'someth':1B 'titl':6B, , null, null).
celery-worker-1 |
celery-worker-1 |
celery-worker-1 | The above exception was the direct cause of the following exception:
celery-worker-1 |
celery-worker-1 | Traceback (most recent call last):
celery-worker-1 | File "/venv/lib/python3.13/site-packages/modelsearch/index.py", line 182, in insert_or_update_object
celery-worker-1 | backend.add(indexed_instance)
celery-worker-1 | ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/modelsearch/backends/base.py", line 510, in add
celery-worker-1 | self.get_index_for_object(obj).add_item(obj)
celery-worker-1 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/modelsearch/backends/base.py", line 441, in add_item
celery-worker-1 | self.add_items(obj._meta.model, [obj])
celery-worker-1 | ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/modelsearch/backends/database/postgres/postgres.py", line 294, in add_items
celery-worker-1 | cursor.execute(
celery-worker-1 | ~~~~~~~~~~~~~~^
celery-worker-1 | f"""
celery-worker-1 | ^^^^
celery-worker-1 | ...<8 lines>...
celery-worker-1 | data_params,
celery-worker-1 | ^^^^^^^^^^^^
celery-worker-1 | )
celery-worker-1 | ^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 122, in execute
celery-worker-1 | return super().execute(sql, params)
celery-worker-1 | ~~~~~~~~~~~~~~~^^^^^^^^^^^^^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 79, in execute
celery-worker-1 | return self._execute_with_wrappers(
celery-worker-1 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
celery-worker-1 | sql, params, many=False, executor=self._execute
celery-worker-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
celery-worker-1 | )
celery-worker-1 | ^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 92, in _execute_with_wrappers
celery-worker-1 | return executor(sql, params, many, context)
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 100, in _execute
celery-worker-1 | with self.db.wrap_database_errors:
celery-worker-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/utils.py", line 94, in __exit__
celery-worker-1 | raise dj_exc_value.with_traceback(traceback) from exc_value
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 105, in _execute
celery-worker-1 | return self.cursor.execute(sql, params)
celery-worker-1 | ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
celery-worker-1 | django.db.utils.IntegrityError: null value in column "title_text" of relation "wagtailsearch_indexentry" violates not-null constraint
celery-worker-1 | DETAIL: Failing row contains (411, 128, 1, 113, 'in':2 'news':4 'post':5 'something':1 'the':3 'title':6, 'news':4B 'post':5B 'someth':1B 'titl':6B, , null, null).
celery-worker-1 |
celery-worker-1 | 2026-07-22 13:49:11 [ERROR] signals.py:65 - Task id=IlN92VNdFatTLBCjgXyeDkhqFWONdadJ path=modelsearch.tasks.insert_or_update_object_task state=FAILED
celery-worker-1 | Traceback (most recent call last):
celery-worker-1 | File "/venv/lib/python3.13/site-packages/django/db/backends/utils.py", line 105, in _execute
celery-worker-1 | return self.cursor.execute(sql, params)
celery-worker-1 | ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
celery-worker-1 | psycopg2.errors.NotNullViolation: null value in column "title_text" of relation "wagtailsearch_indexentry" violates not-null constraint
celery-worker-1 | DETAIL: Failing row contains (411, 128, 1, 113, 'in':2 'news':4 'post':5 'something':1 'the':3 'title':6, 'news':4B 'post':5B 'someth':1B 'titl':6B, , null, null).
Huh, not seen that one before. It seems that there is a required column in the search_index. Without more info, my guess would be that your database is in a strange state due to running and then removing migrations while switching branches. I would recommend resetting to prod, and seeing if the issue persists. |
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Hey my environment is still not working properly. Thought I could leave this small comment while fixing things here.
julhoang
left a comment
julhoang
left a comment
There was a problem hiding this comment.
Hi @jlchilders11 , upon double-checking the ticket, I think we're missing the multi-selection support for related_library tags. Would you mind adding that?
|
@henryajisegiri Can you confirm whether the libraries dropdown is intended to be multi select? While the use of the work "libraries" implies that it should, I believe the design in other places (such as the news index) implies that a post may only have one related library? |
|
Hey @jlchilders11, the intention is for a multi-select. You're correct on what the current design implies and we will fix that next week. |
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Everything else is looking pretty good, I'm ready to approve when this and Julia's comments are addressed!
| try: | ||
| logger.info(f"Fetching content from {external_url=} for entry.{pk=}") | ||
| response = requests.get(external_url, timeout=10) | ||
| response = safe_get(external_url, timeout=10) |
There was a problem hiding this comment.
Nice addition!!
Only one thing comes to mind now: safe_get raises UnsafeURLError as the validation failure, so this error will bubble up to the Celery task if it happens.
I think adding a new catch statement like this is enough:
except UnsafeURLError:
logger.warning(f"Refusing to fetch unsafe {external_url=} for {pk=}")
returnThere was a problem hiding this comment.
Good catch, updated
|
@julhoang @henryajisegiri The related libraries selection is now a multi select, and the backend now support selecting multiple related libraries as tags. Thanks for the insight/clarification! |
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Re-approving, all good there, thanks!
julhoang
left a comment
julhoang
left a comment
There was a problem hiding this comment.
Everything works great, awesome work and thanks for all the updates @jlchilders11 !! 🙌
…mage downsizing to form submit
…use safe article functions for summary
… draft on page type chosen, ensure 5mb limit on uploaded images
64ac8c2 to
4cce2b7
Compare
6 participants
Issue: #2375
Summary & Context
Implements backend wagtail integration with the V3 create a post page. When creating a post, new pages are saved as revisions and submitted for moderation.
Changes
Entrymodels with the creation ofPostPagemodels on the V3 create a post pagePlease list any potential risks or areas that need extra attention during review/testing
Notes: The moderation queue will be upgraded in another ticket in order to only send pages to human moderation if they fail automated moderation. Additionally, we currently only bother running the summary and video thumbnail tasks if the page is set to live. This means that moderators will not see the description and thumbnail before the page is live.
Testing notes:
In order to see the moderation queue in action, the tester may want to set themselves as a moderation. This can be done through the cms at
Settings > User > Username, going to the group tab, checking the box next to moderator, and saving the model. This will cause them to get alerts when pages are sent to moderation as well as being able to see the moderation queue on the home page of the cms.Self-review Checklist
Summary by CodeRabbit