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.

Sign up for GitHub

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

Jump to bottom

fix(emails): account for false order value in email hook #3590

Merged
merged 1 commit into from
Dec 2, 2024
Merged

fix(emails): account for false order value in email hook #3590

merged 1 commit into from
Dec 2, 2024

Conversation

chickenn00dle
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Looks like $order can be false in some cases:

An error of type E_ERROR was caused in line 271 of the file /srv/htdocs/wp-content/plugins/newspack-plugin/includes/reader-revenue/woocommerce/class-woocommerce-connection.php. Error message: Uncaught Error: Call to a member function get_id() on null in /srv/htdocs/wp-content/plugins/newspack-plugin/includes/reader-revenue/woocommerce/class-woocommerce-connection.php:271

This PR fixes this by checking the value of $order BEFORE calling get_id

How to test the changes in this Pull Request:

  1. Go through the checkout flow for a donation product and for a non-donation product and monitor logs.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle chickenn00dle added [Status] Needs Review The issue or pull request needs to be reviewed ras-acc testing labels Dec 2, 2024
@chickenn00dle chickenn00dle requested a review from a team as a code owner December 2, 2024 20:12
laurelfulford
laurelfulford approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@laurelfulford laurelfulford left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 I ran a couple transactions (donation and regular), and didn't see anything off in the logs!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Dec 2, 2024
@chickenn00dle chickenn00dle merged commit a2e4042 into trunk Dec 2, 2024
10 checks passed
@chickenn00dle chickenn00dle deleted the fix/handle-false-order-object branch December 2, 2024 20:49
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 2, 2024

Hey @chickenn00dle, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request Dec 2, 2024
@semantic-release-bot
chore(release): 5.9.0-alpha.2 [skip ci]
c557e37 
# [5.9.0-alpha.2](v5.9.0-alpha.1...v5.9.0-alpha.2) (2024-12-02)

### Bug Fixes

* **emails:** account for false order value ([#3590](#3590)) ([a2e4042](a2e4042))
* **ras-acc:** correct My Account custom font sizing clash ([#3588](#3588)) ([080f1ce](080f1ce))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.9.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Dec 9, 2024
@semantic-release-bot
chore(release): 5.9.0 [skip ci]
460e716 
# [5.9.0](v5.8.2...v5.9.0) (2024-12-09)

### Bug Fixes

* **emails:** account for false order value ([#3590](#3590)) ([a2e4042](a2e4042))
* **ras-acc:** correct My Account custom font sizing clash ([#3588](#3588)) ([080f1ce](080f1ce))
* **ras-acc:** make helper text size more specific ([#3584](#3584)) ([5bcc688](5bcc688))
* **ras-acc:** re-add recaptcha to the WooCommerce checkout ([#3605](#3605)) ([07f46b3](07f46b3))
* undefined var ([#3585](#3585)) ([00d8bc7](00d8bc7))
* **woocommerce-emails:** use the default email payload if there are no donation products ([#3545](#3545)) ([60c21f3](60c21f3))

### Features

* mark perfmatters as a required plugin ([#3578](#3578)) ([f20291c](f20291c))
* **ras-acc:** add reader account creation and login improvements ([#3582](#3582)) ([b66de08](b66de08))
* **reader-data:** add a CLI command to align reader membership data ([#3548](#3548)) ([8e49bf0](8e49bf0))
* **subscriptions:** add setting to reattempt payment after final retry ([#3560](#3560)) ([553c3ac](553c3ac))
* **woocommerce:** remove internal metadata from REST API response ([6b659a6](6b659a6))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurelfulford laurelfulford laurelfulford approved these changes

Assignees
No one assigned
Labels
ras-acc testing released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chickenn00dle @matticbot @laurelfulford