Skip to content

Search: Fixes an undefined index when filters were disabled via filter#8381

Merged
oskosk merged 1 commit intomasterfrom
fix/search-filters-widget-undefined-index
Dec 20, 2017
Merged

Search: Fixes an undefined index when filters were disabled via filter#8381
oskosk merged 1 commit intomasterfrom
fix/search-filters-widget-undefined-index

Conversation

@ebinnion
Copy link
Copy Markdown
Contributor

If a site had disabled search filters via the jetpack_search_disable_widget_filters filter, then when a widget was saved, it caused an undefined index notice:

PHP Notice:  Undefined index: filter_type in .../jetpack/modules/search/class.jetpack-search-widget-filters.php on line 152

I didn't catch this before because I was testing on widgets where I had previously configured a filter.

To fix this, I added a condition to not check $new_instance['filter_type'] if widget wasn't supposed to use filters.

To test:

  • On a site with a professional plan
  • Checkout Jetpack master
  • Set add_filter( 'jetpack_search_disable_widget_filters', '__return_true' );
  • Add a new Jetpack search filters widget
  • Check error log for undefined index notice
  • Checkout this branch
  • Add a new Jetpack search filters widget
  • Ensure there is no notice

@ebinnion ebinnion added [Feature] Search For all things related to Search [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended labels Dec 18, 2017
@ebinnion ebinnion added this to the 5.7 milestone Dec 18, 2017
@ebinnion ebinnion self-assigned this Dec 18, 2017
@ebinnion ebinnion requested a review from a team as a code owner December 18, 2017 22:57
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Dec 20, 2017
@oskosk oskosk merged commit 9f2d474 into master Dec 20, 2017
@oskosk oskosk deleted the fix/search-filters-widget-undefined-index branch December 20, 2017 14:18
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Search For all things related to Search

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants