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

Prevent PHP Warning when Statistics is null #2400

Merged
merged 2 commits into from
May 14, 2024

Conversation

puntope
Copy link
Contributor

@puntope puntope commented May 13, 2024

Changes proposed in this Pull Request:

Closes part of #2398

This PR prevents a PHP warning when statistics is null as we try to access ['statistics']['active`] in some parts of the code without checking if it's set.

Detailed test instructions:

  1. Enable debug logging to see PHP warnings
  2. Clear statistics cache
  3. Trigger the scheduled action to run notification ( wc_gla_cron_daily_notes )
  4. Confirm that the PHP warning is not generated and note is not created

Notice that some conditions are required for this Note to be triggered. For testing this I recommend commenting off some checks under AbstractSetupCampaign::should_be_added or be sure you pass all those checks.

So we can focus on the problematic method $this->merchant_center_service->has_at_least_one_synced_product()

Additional details:

Changelog entry

Fix - Prevent PHP Warning when Statistics is null

@puntope puntope self-assigned this May 13, 2024
@puntope puntope marked this pull request as ready for review May 13, 2024 10:19
@puntope puntope requested a review from a team May 13, 2024 10:19
@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels May 13, 2024
Copy link
Member

@ianlin ianlin left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it, I no longer see the error message after running the job when statistics doesn't exist. Just one question, in the PR description you mentioned:

Closes part of #2398

Would it still be follow ups, or it already completely solved the issue #2398?

@puntope puntope merged commit 977311b into develop May 14, 2024
10 checks passed
@puntope puntope deleted the fix/warning-null-statistics branch May 14, 2024 09:45
@jorgemd24 jorgemd24 mentioned this pull request May 29, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants