Skip to content

Conversation

@jerclarke
Copy link
Contributor

@jerclarke jerclarke commented Mar 8, 2019

  • affects notification_status_change()
  • Moves the definition of $old_status_friendly_name (as well as $new_status_friendly_name) to above the complex else if block to ensure that it is always defined.
  • Fixes a recent change to this code, which moved the definition of $old_status_friendly_name to places where it didn’t effect all of the else if clauses:

071791d#diff-59ee072dc425b09b6d748cb84634734b

  • This triggered a PHP Warning: PHP Notice: Undefined variable: old_status_friendly_name because the variable is used right after in all cases.
  • This warning also reflected that the string would be missing in many cases.
  • I tried to test this with all the possible status transition but might have missed some. Certainly the common ones of new->draft->pending->publish->trash are all working as expected now.

- affects `notification_status_change()`
- Moves the definition of $old_status_friendly_name (as well as $new_status_friendly_name) to above the complex `else if` block to ensure that it is always defined.
- Fixes a recent change to this code, which moved the definition of $old_status_friendly_name to places where it didn’t effect all of the `else if` clauses:

Automattic@071791d#diff-59ee072dc425b09b6d748cb84634734b

- This triggered a PHP Warning: PHP Notice:  Undefined variable: old_status_friendly_name
- This warning also reflected that the string would be missing in many cases.
- I tried to test this with all the possible status transition but might have missed some. Certainly the common ones of new->draft->pending->publish->trash are all working as expected now.
jerclarke referenced this pull request Mar 8, 2019
This prevents duplicate conditional statements and sets a value for $old_status_friendly_name so that New or Auto-Draft that had no previous value will have something valid.
@cojennin
Copy link
Contributor

cojennin commented Nov 25, 2019

I'd like to merge this in, but it reintroduces a problem mentioned here.

One way it triggers: If you programmatically call wp_insert_post then the $old_status will be new and get_post_status_object will return null, and you'll get an exception here.

One thing I can think of is adding null check around $old_status_post_obj and $new_status_post_obj and make $old_status_friendly_name and $new_status_friendly_name default to something, maybe an empty string?

Open to suggestions. => New Status doesn't seem that odd if we're missing the old status, but Old Status => might be confusing if we're missing the new status

@mjangda
Copy link
Member

mjangda commented Dec 13, 2019

Continued in #560

@mjangda mjangda closed this Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants