Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined index warnings in Latest Comments & Latest Posts #12149

Merged
merged 1 commit into from
Nov 21, 2018

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Nov 21, 2018

Fixes #12119.

In #12003, prepare_attributes_for_render was changed so that the $attributes array that is passed to render_callback contains the same keys as what is passed to the block on the front-end. That is, it omits keys that are not set.

This caused some 'Undefined index' notices in the Latest Comments and Latest Posts blocks. We need to be using isset() when using attributes that do not have a default value defined.

Testing

  1. Ensure that PHP is set to report notices, e.g. using error_reporting( E_ALL );
  2. Create a post
  3. Insert a Latest Comments block
  4. Insert a Latest Posts block
  5. Preview the post
  6. There should be no PHP notice messages

In 53c975b, `prepare_attributes_for_render` was changed so that
the `$attributes` array that is passed to `render_callback` contains the
same keys as what is passed to the block on the front-end.

This caused some 'Undefined index' notices in the Latest Comments and
Latest Posts blocks. We need to be using `isset()` when using attributes
that do not have a default value defined.
@noisysocks noisysocks added [Type] Bug An existing feature does not function as intended [Block] Latest Posts Affects the Latest Posts Block [Block] Latest Comments Affects the Latest Comments Block labels Nov 21, 2018
@noisysocks noisysocks requested a review from pento November 21, 2018 03:29
Copy link
Member

@pento pento left a comment

Choose a reason for hiding this comment

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

Looks good, let's go with it.

@noisysocks noisysocks merged commit 114c443 into master Nov 21, 2018
@noisysocks noisysocks deleted the fix/undefined-attribute-warnings branch November 21, 2018 04:11
@gziolo gziolo added this to the 4.6 milestone Nov 21, 2018
@gziolo
Copy link
Member

gziolo commented Nov 21, 2018

This caused some 'Undefined index' notices in the Latest Comments and Latest Posts blocks. We need to be using isset() when using attributes that do not have a default value defined.

Shouldn't it always be the case in PHP to do isset() check? 😅
At some point we will make attributes filterable so it might become only worse.

@youknowriad youknowriad modified the milestones: 4.6, 4.5.1 Nov 21, 2018
@noisysocks
Copy link
Member Author

If the attribute definition has a 'default' value then it's safe to not do an isset() in the render_callback because prepare_attributes_for_render will take care of ensuring that that attribute is set.

The point about filters is interesting... I'm not sure about that. I'd say that it should be the responsibility of the developer who is adding the filter to ensure that they're not unsetting variables that are used by Core code 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Comments Affects the Latest Comments Block [Block] Latest Posts Affects the Latest Posts Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants