Automattic / Automattic/edit-flow

Consider switching save_post_subscriptions from transition_post_status to save_post hook

Open
#886 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.