Skip to content

Make post topic page child creation safer#456

Closed
RealOrangeOne wants to merge 2 commits intomainfrom
fix/make-post-topic-page-child-creation-safer
Closed

Make post topic page child creation safer#456
RealOrangeOne wants to merge 2 commits intomainfrom
fix/make-post-topic-page-child-creation-safer

Conversation

@RealOrangeOne
Copy link
Copy Markdown
Contributor

What is the context of this PR?

Sometimes, the child pages already exist (for a variety of reasons). This modifies the hook to only create the page if one doesn't already exist.

How to review

Note that this is built on #455, so best review once that has merged.

Follow-up Actions

RealOrangeOne and others added 2 commits January 7, 2026 10:14
This lets the signals be sent when running non-wagtail admin codepaths.
Comment on lines +62 to +64
# If the page already exists, do nothing
if ArticlesIndexPage.objects.child_of(instance).exists():
return
Copy link
Copy Markdown
Contributor

@MebinAbraham MebinAbraham Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you include a scenario where this is true? If it was just created, how can it already exist? Is it meant to cover edge cases like multiple requests at the same time, or if the transaction rolled back? (I suspect the signal is independent of the transcation?) It is worth expanding the comment to avoid questions in the future about what this is intended to protect against.

@RealOrangeOne
Copy link
Copy Markdown
Contributor Author

On reflection, I think this might in fact be a problem with the deletion script in #445. It deletes the parent pages whilst leaving their child pages in tact. I'll do some more digging, but it's likely this can be closed.

@RealOrangeOne RealOrangeOne added Bug Fix Something isn't working do not merge labels Jan 7, 2026
@zerolab zerolab closed this Jan 7, 2026
@zerolab zerolab deleted the fix/make-post-topic-page-child-creation-safer branch January 7, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Fix Something isn't working do not merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants