Skip to content

Convert some Wagtail hooks into signals#455

Merged
RealOrangeOne merged 13 commits intomainfrom
chore/convert-hooks-to-signals
Jan 19, 2026
Merged

Convert some Wagtail hooks into signals#455
RealOrangeOne merged 13 commits intomainfrom
chore/convert-hooks-to-signals

Conversation

@RealOrangeOne
Copy link
Copy Markdown
Contributor

What is the context of this PR?

As mentioned in the Wagtail docs (see note at top of page), signals are only intended for modifying functionality in the Wagtail Admin (eg registering URLs), rather than firing on events. For events, signals are much preferred, as hooks only fire in the admin, rather than everywhere.

There should be no functionality change, since these triggers will still fire during the admin. This just means that during other paths, they'll fire, leading to the correct behaviour in other codepaths (eg #445 ).

How to review

Confirm that no hooks were dropped, only moved to the appropriate signal.

Follow-up Actions

This lets the signals be sent when running non-wagtail admin codepaths.
@RealOrangeOne RealOrangeOne requested a review from a team as a code owner January 7, 2026 10:19
Comment thread cms/articles/signal_handlers.py Outdated
…may have already created an instance.

This happens because instances are created automatically when the TopicPage is created
Copy link
Copy Markdown
Contributor

@MaciekBaron MaciekBaron left a comment

Choose a reason for hiding this comment

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

Three tests have regressed - the first two can be solved by running the hooks on the newly created pages (so that the appropriate translated pages get created), see my comment.

The third test might be due to additional log entries being created in your solution.

Comment thread cms/articles/signal_handlers.py
Comment thread cms/methodology/signal_handlers.py
@MaciekBaron MaciekBaron added the Enhancement Enhance the functionality of existing features. label Jan 9, 2026
Copy link
Copy Markdown
Contributor

@sanjeevz3009 sanjeevz3009 left a comment

Choose a reason for hiding this comment

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

Re-reviewed. LGTM!

@RealOrangeOne RealOrangeOne merged commit 05cd0f4 into main Jan 19, 2026
11 checks passed
@RealOrangeOne RealOrangeOne deleted the chore/convert-hooks-to-signals branch January 19, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Enhance the functionality of existing features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants