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

Array Merge Errors #124

Closed

Conversation

koconder
Copy link
Contributor

Possible fix suppressing errors for #121

koconder and others added 2 commits June 20, 2019 10:44
Sync Changes from Upstream Origin
Fix for duracelltomi#121 - Unable to replace nor find an issue, there is built in error checking and use of global gtm4wp_def_user_notices_dismisses for fallback. Possible PHP version issue? Supressing.
@duracelltomi
Copy link
Owner

Thanks for the addition!

I am not sure whether suppressing the error would actually fix this as there must be a reason for the wrong variable type.

According to the doc, get_user_meta() will return the value of the meta_value field or an empty string if the value does not exists yet. This empty value is checked just after the get_user_meta() call. If empty value is returned, a default array value is passed to $gtm4wp_user_notices_dismisses. I wonder if there is any case when this fallback variable is not yet initialized while gtm4wp_show_warning() is executing?

My second guess is whether there might be a state when unserialize() is not returning an array type result? I do not see yet how this could happen besides directly manipulating DB fields but perhaps if ( false === $gtm4wp_user_notices_dismisses ) { after unserialize() could be changed to a type check rather then checking the boolean value false.

What do you think?

@koconder
Copy link
Contributor Author

@duracelltomi i might leave this with you then, outside of my understanding of the original code

@duracelltomi
Copy link
Owner

No problem, I will take care of this

@koconder koconder deleted the feature/array-merge-errors branch November 25, 2019 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants