Automattic / Automattic/edit-flow
Consider switching save_post_subscriptions from transition_post_status to save_post hook
- Dominant language
- PHP
- Stars
- 366
- Forks
- 137
- Avg merge
- 2h 11m
- Merged PRs (30d)
- 6
Description
## Context
Raised by @dd32 in https://github.com/Automattic/edit-flow/pull/883#issuecomment-3736453948:
> I think it might be better if that was hooked onto the [save_post](https://developer.wordpress.org/reference/hooks/save_post/) action instead of the transition.
Currently, `save_post_subscriptions()` is hooked to `transition_post_status` at priority 0, with `notification_status_change()` on the same hook at priority 10. This ensures subscriptions are saved before notifications are sent.
## Current architecture
```php
add_action( 'transition_post_status', [ $this, 'save_post_subscriptions' ], 0, 3 );
add_action( 'transition_post_status', [ $this, 'notification_status_change' ], 10, 3 );
```
## Considerations
### Why `save_post` might be preferable
- `save_post` is the canonical hook for saving post meta alongside a post
- Clearer semantic intent ("saving data when post saves")
- Less likely to interfere with status-transition-specific logic
### Why this could be a breaking change
1. **Hook signature differs**: `transition_post_status` provides `($new_status, $old_status, $post)` while `save_post` provides `($post_id, $post, $update)`. The notification logic relies on comparing old vs new status.
2. **Third-party timing assumptions**: Developers may have hooked into `transition_post_status` at priorities 1-9 expecting Edit Flow's subscription data to already be available.
3. **Notification timing shifts**: Moving both functions to `save_post` means notifications fire later in the post-save lifecycle.
4. **Status detection changes**: Would need an alternative approach to detect the previous status (e.g., tracking via static variable, post meta, or hooking earlier to capture it).
### If we proceed
Both `save_post_subscriptions` and `notification_status_change` would likely need to move together to maintain their ordering relationship. The old/new status detection would need refactoring.
## Recommendation
This should be treated as a potentially breaking change and scheduled for a minor or major version bump with appropriate changelog documentation. Alternatively, maintain backwards compatibility by keeping the existing hooks but deprecating them in favour of the new approach.
Contributor guide
Research direction
Start by reviewing the save_post_subscriptions and notification_status_change entry points and their transition_post_status registrations at priorities 0 and 10. Compare the save_post and transition_post_status signatures, status-detection needs, ordering, and compatibility concerns; done requires a decided migration or compatibility plan with appropriate version and changelog treatment.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100