easy-digital-downloads icon indicating copy to clipboard operation
easy-digital-downloads copied to clipboard

EDD Heartbeat actions may run outside of the Dashboard

Open robincornett opened this issue 4 years ago • 8 comments

Bug Report

Expected behavior

Currently, the EDD_Heartbeat functionality exists to update the Sales Summary dashboard widget in real time. It should run only on the dashboard screen.

Actual behavior

The footer_js method in the EDD_Heartbeat class checks the global $pagenow to determine if the script should load, and loads it if the $pagenow is index.php. However, there may be pages in the admin which will meet that criteria other than the Dashboard, such as the EDD 3.0 migration screen ( /wp-admin/index.php?page=edd-upgrades&edd-upgrade=v30_migration). On that screen, the EDD heartbeat script is running and this error is being logged as long as it's pinging:

PHP Notice: The edd_report_views hook is <strong>deprecated</strong> since Easy Digital Downloads version 3.0! Use the edd_reports_get_reports hook instead.

Steps to reproduce the behavior

  1. In the EDD_Heartbeat class, add an error_log call inside the heartbeat_received method, inside the $data['edd_heartbeat]` condition.
  2. Visit an admin screen that's not the dashboard but does return true for index.php.
  3. Stay on the screen for several minutes.
  4. Check the error logs.

Information (if a specific version is affected):

PHP Version: 7.3.5 EDD Version (or branch): 2.9.x, 3.0 WordPress Version: 5.4.1

Any other relevant information: I don't know what other pages might return index.php for the $pagenow variable, but I believe this could be corrected by changing that check to to be more specific by using get_current_screen and making sure that the screen ID is dashboard.

robincornett avatar May 14 '20 18:05 robincornett

On that screen, the EDD heartbeat script is running and this error is being logged as long as it's pinging:

PHP Notice: The edd_report_views hook is <strong>deprecated</strong> since Easy Digital Downloads version 3.0! Use the edd_reports_get_reports hook instead.

This has been driving me nuts. It's because of the use of the stats class for the dashboard widget, which of course 100% makes sense, but it's also loading any legacy reports that have been added via the old edd_report_views hook (Software Licensing, Recurring, etc.). The loading of the legacy reports is what triggers the notice. Maybe we could unhook those or something while we're on the dashboard because they're not actually needed there.

ashleyfae avatar May 15 '20 07:05 ashleyfae

I was thinking more about the notice--should that be made a separate issue maybe?

I can update the screen check, at least, but should it be for 2.9 or 3.0?

robincornett avatar May 15 '20 12:05 robincornett

@robincornett Let's go 3.0. The behavior has likely been the same since the introduction of Heartbeat so it doesn't need an immediate fix.

spencerfinnell avatar May 15 '20 13:05 spencerfinnell

If it's in 2.9 as well then maybe we wait for 3.0.5 in light of Chris's comment:

At this point that is a question I would like us to ask on every bug. Is it a regression or still broken. That will change how we assess it for the 3.0 release.

If this is an issue in 3.0 as well then that classifies as still broken.

ashleyfae avatar May 15 '20 14:05 ashleyfae

@ashleyfae I think the PHP notices being thrown on unrelated pages would be classified as a 3.0 introduction.

Loading on every page is still broken. Additional PHP notices on every page is a 3.0 introduction.

spencerfinnell avatar May 15 '20 14:05 spencerfinnell

@ashleyfae I think the PHP notices being thrown on unrelated pages would be classified as a 3.0 introduction.

Loading on every page is still broken. Additional PHP notices on every page is a 3.0 introduction.

That's true, so I think if we can fix it like this:

Maybe we could unhook those or something while we're on the dashboard because they're not actually needed there. https://github.com/easydigitaldownloads/easy-digital-downloads/issues/7904#issuecomment-629071095

then that would address the excessive notice we've introduced.

ashleyfae avatar May 15 '20 14:05 ashleyfae

Could be as simple as adding this remove_all_filters( 'edd_report_views' ); here: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/admin/dashboard-widgets.php#L62

ashleyfae avatar May 15 '20 15:05 ashleyfae

Created a new issue specifically to target the excessive notice: https://github.com/easydigitaldownloads/easy-digital-downloads/issues/7914

ashleyfae avatar May 15 '20 15:05 ashleyfae