[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:
-------------------------------------------------+-------------------------

Comment (by Iceable):

 @birgire
 > 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.

 I'm not convinced either: merging feeds would not allow to get the desired
 number of items from each (1 from `wordpress.org/news` and 3 from
 `planet.wordpress.org`). Or would it?

 ----

 [attachment:42254.3.diff] looks good to me, just one tiny error in `wp-
 includes/widgets.php` on line 1392:
 `$rss->get_error_message()` should be `$rss_entries->get_error_message()`
 instead:

 {{{
 if ( is_wp_error( $rss_entries ) ) {
         if ( is_admin() || current_user_can( 'manage_options' ) ) {
                 echo '<p><strong>' . __( 'RSS Error:' ) . '</strong> ' .
 $rss_entries->get_error_message() . '</p>';
         }
         return;
 }
 }}}

 The margin of 5 entries seems a bit arbitrary indeed, but now thinking
 about it:
 - It is better than my previous patch that just grabbed as many as
 available; which was of course much more than necessary.
 - The margin of 5 - when actually just 1 should be enough - allows some
 room in case the $feeds and 'items' argument come to change in the future.
 In such case I doubt we'll ever need more than 5.

 Maybe we could even go as far as to calculate the necessary offset based
 on the 'items' arguments in $feeds? (Would it be useful to go this far?)

 I only gave it a quick look so I'm not uploading another patch right now.
 I may have a bit more time for another look over the week end. Not sure
 I'll have much more to add though. :)

 To be honest I am not very experienced with unit tests yet so I don't have
 much of a feedback about these except that they look pretty fine to me.
 Just one other small thing though: I think `includes-dashboard.php` should
 be named `includesDashboard.php` to be consistent with the other files in
 `tests/phpunit/tests/admin/`

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