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

fix: wizards - update type check conditional for custom_logo #3442

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

jaredrethman
Copy link
Collaborator

@jaredrethman jaredrethman commented Sep 20, 2024

All Submissions:

Changes proposed in this Pull Request:

Changes introduced in #3433 produced a bug relating to how custom_logo and newspack_footer_logo theme mods are saved.

PHP Fatal error: Uncaught TypeError: Cannot access offset of type string on string

This PR fixes this issue by correctly testing incoming values by the correct type.

How to test the changes in this Pull Request:

  1. Checkout this branch i.e. git checkout origin/fix/wizard-custom-logo-save
  2. Navigate to /wp-admin/admin.php?page=newspack-site-design-wizard#/settings
  3. Scroll to Header and Footer sections. Test the following:
    • You're able to add/update Logos and changes persist between page refreshes.
    • You're able to remove Logos (saving without error) and changes persist between page refreshes.

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?

@jaredrethman jaredrethman marked this pull request as ready for review September 20, 2024 18:57
@jaredrethman jaredrethman requested a review from a team as a code owner September 20, 2024 18:57
@jaredrethman jaredrethman added the [Status] Needs Review The issue or pull request needs to be reviewed label Sep 20, 2024
@@ -520,7 +520,7 @@ public function api_update_theme_with_mods( $request ) {
continue;
}

if ( null !== $value && in_array( $key, $this->media_theme_mods ) ) {
if ( '' !== $value && in_array( $key, $this->media_theme_mods ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if $value needs to be an array there, what about a more explicit check like ! empty( $value['id'] )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I have updated with your recommendation 22c742a

@leogermani leogermani merged commit 125f756 into trunk Sep 20, 2024
8 checks passed
@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 Sep 20, 2024
@leogermani leogermani deleted the fix/wizard-custom-logo-save branch September 20, 2024 19:39
matticbot pushed a commit that referenced this pull request Sep 20, 2024
# [5.4.0-alpha.7](v5.4.0-alpha.6...v5.4.0-alpha.7) (2024-09-20)

### Bug Fixes

* wizards - update type check conditional for `custom_logo` ([#3442](#3442)) ([125f756](125f756))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.4.0-alpha.7 🎉

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
Labels
released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants