[wp-trac] [WordPress Trac] #42254: Duplicate news entries in Events & News dashboard widget

[WordPress Trac]

#42254: Duplicate news entries in Events & News dashboard widget
-------------------------------------------------+-------------------------
 Reporter:  iandunn                              |       Owner:
     Type:  defect (bug)                         |      Status:  new
 Priority:  low                                  |   Milestone:  Future
Component:  Administration                       |  Release
 Severity:  minor                                |     Version:  4.8
 Keywords:  good-first-bug has-patch has-unit-   |  Resolution:
  tests                                          |     Focuses:
-------------------------------------------------+-------------------------
Changes (by birgire):

 * keywords:  good-first-bug has-patch => good-first-bug has-patch has-unit-
     tests


Comment:

 @Iceable Here's a first pass for adding tests.

 In [attachment:42254.3.diff]

 - added a test for {{{wp_widget_rss_get_entries()}}}.
 - added a test for {{{wp_widget_rss_output()}}}.
 - added a test for {{{wp_dashboard_primary_output()}}} to make sure there
 are no duplicated entries.
 - used local test feed (xml) files. Trimmed the Planet feed down to 10
 items.
 - changed {{{wp_widget_rss_get_entries()}}} to support the {{{'items'}}}
 argument.
 - changed {{{wp_dashboard_primary_output()}}} to check for an empty
 entries array.
 - changed {{{wp_widget_rss_output()}}} to check for an empty entries
 array.
 - changed {{{__()}}} to {{{esc_html__()}}} in one place, as recommended by
 phpcs.
 - fixed some indices in {{{wp_widget_rss_get_entries()}}}.

 There are no tests for these dashboard functions, so I created a new test
 class for the {{{dashboard.php}}} include.

 There also seems to be no tests for the core {{{fetch_feed()}}} function,
 so I tried to find a way to let it use local xml files, instead of running
 external feed requests.

 The {{{fetch_feed()}}} is a wrapper for a {{{SimplePie}}} feed object.
 Instead of trying to mock the {{{SimplePie}}} class, I used the
 {{{wp_feed_options}}} hook to let the object use raw data, via the
 {{{SimplePie::set_raw_data()}}} method.

 See e.g. http://simplepie.org/api/class-SimplePie.html

 The {{{SimplePie::set_feed_url()}}} method overrides any raw data and
 {{{fetch_feed()}}} calls it with  {{{$feed->set_feed_url( $url )}}}. Note
 that we can't use {{{$feed->set_feed_url( null )}}}, because it will set
 the feed url as {{{http://?#}}}, thus overriding the raw data. So the
 workaround here was to directly null the public {{{feed_url}}} property
 with {{{$feed->feed_url = null}}}. Then I was able to load the local test
 xml files via:

 {{{
 $feed->set_raw_data( file_get_contents( DIR_TESTDATA . '/feeds/news/' .
 $xml_files[ $url ] ) );

 }}}

 where we disable caching with {{{$feed->enable_cache( false );}}}

 I considered adding the xml files into a method, to get rid of
 {{{file_get_contents()}}}, but this way made the test class much more
 readable, not having bunch of XML data in there.

 I also made some adjustments to {{{wp_widget_rss_get_entries()}}}, so it
 supports the {{{items}}} argument, instead of returning all feed entries
 (they could be many).

 Instead I added a margin of 5 entries (just an example) within
 {{{wp_dashboard_primary_output()}}}, to be able to handle possible
 duplicated entries.

 I trimmed the Planet feed test file to include 10 items. (might clean it
 better for non-ascii?)

 The Planet test feed (first 5 items):

 {{{
 Dev Blog: 2017 WordPress Survey and WordCamp US
 https://wordpress.org/news/2017/10/2017-wordpress-survey-and-wordcamp-us/

 WPTavern: WordPress 4.9 Will Support Shortcodes and Embedded Media in the
 Text Widget
 https://wptavern.com/wordpress-4-9-will-support-shortcodes-and-embedded-
 media-in-the-text-widget

 WPTavern: WPWeekly Episode 292 Recap of WooConf and CaboPress
 https://wptavern.com/wpweekly-episode-292-recap-of-wooconf-and-cabopress

 WPTavern: Goodnight Firebug
 https://wptavern.com/goodnight-firebug

 WPTavern: WordPress 4.9 Beta 4 Removes Try Gutenberg Call to Action
 https://wptavern.com/wordpress-4-9-beta-4-removes-try-gutenberg-call-to-
 action

 }}}

 The News test feed (first 5 items):

 {{{
 2017 WordPress Survey and WordCamp US
 https://wordpress.org/news/2017/10/2017-wordpress-survey-and-wordcamp-us/

 WordPress 4.9 Beta 4
 https://wordpress.org/news/2017/10/wordpress-4-9-beta-4/

 WordPress 4.9 Beta 3
 https://wordpress.org/news/2017/10/wordpress-4-9-beta-3/

 WordPress 4.9 Beta 2
 https://wordpress.org/news/2017/10/wordpress-4-9-beta-2/

 WordPress 4.9 Beta 1
 https://wordpress.org/news/2017/10/wordpress-4-9-beta-1/

 }}}

 We note that the first item is (link) duplicated, i.e. contained in both
 feeds.

 We want to avoid displaying duplicated items in
 {{{wp_dashboard_primary_output()}}}.

 So we expected output of {{{wp_dashboard_primary_output()}}} for (1 News
 item + 3 Planet items), is to contain:

 {{{
 2017 WordPress Survey and WordCamp US
 https://wordpress.org/news/2017/10/2017-wordpress-survey-and-wordcamp-us/

 WPTavern: WordPress 4.9 Will Support Shortcodes and Embedded Media in the
 Text Widget
 https://wptavern.com/wordpress-4-9-will-support-shortcodes-and-embedded-
 media-in-the-text-widget

 WPTavern: WPWeekly Episode 292 Recap of WooConf and CaboPress
 https://wptavern.com/wpweekly-episode-292-recap-of-wooconf-and-cabopress

 WPTavern: Goodnight Firebug
 https://wptavern.com/goodnight-firebug

 }}}

 instead of:

 {{{
 2017 WordPress Survey and WordCamp US
 https://wordpress.org/news/2017/10/2017-wordpress-survey-and-wordcamp-us/

 Dev Blog: 2017 WordPress Survey and WordCamp US
 https://wordpress.org/news/2017/10/2017-wordpress-survey-and-wordcamp-us/

 WPTavern: WordPress 4.9 Will Support Shortcodes and Embedded Media in the
 Text Widget
 https://wptavern.com/wordpress-4-9-will-support-shortcodes-and-embedded-
 media-in-the-text-widget

 WPTavern: WPWeekly Episode 292 Recap of WooConf and CaboPress
 https://wptavern.com/wpweekly-episode-292-recap-of-wooconf-and-cabopress

 }}}

 ----

 The SimplePie also supports fetching multiple feeds, thus also
 {{{fetch_feed()}}}, but I'm not yet convinced that this would be the way
 to go here.

 There's a link in the inline docs for
 [https://developer.wordpress.org/reference/functions/fetch_feed/
 fetch_feed()] for:

 http://simplepie.org/wiki/faq/typical_multifeed_gotchas

 ps: this link is not parsed correctly in the DevHub, I've reported it
 recently [https://meta.trac.wordpress.org/ticket/3212 here] on Meta trac:

--
Ticket URL: <https://core.trac.wordpress.org/ticket/42254#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform