Skip to content

JITM: Correct the namespace and class in the add_filter() calls.#13720

Merged
kraftbj merged 1 commit intomasterfrom
fix/jitm_add_filter_ns_class
Oct 16, 2019
Merged

JITM: Correct the namespace and class in the add_filter() calls.#13720
kraftbj merged 1 commit intomasterfrom
fix/jitm_add_filter_ns_class

Conversation

@kbrown9
Copy link
Copy Markdown
Member

@kbrown9 kbrown9 commented Oct 11, 2019

The add_filter() calls in the get_message() method are using an old class name, Jetpack_JITM. When a JITM that uses one of these filters is displayed, the JITM's CTA does not work properly and the following PHP warning is generated:

PHP Warning: call_user_func_array() expects parameter 1 to be a valid callback, class 'Jetpack_JITM'

Changes proposed in this Pull Request:

  • Fix the add_filter() calls by adding the correct namespace and class name.
  • Fix various PHPCS comment errors in the get_messages() method.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

This fixes a bug in an existing feature.

Testing instructions:

  1. Install and activate Jetpack and WooCommerce. Do not install WooCommerce Services. Enable debug mode and debug logging.
  2. Connect Jetpack.
  3. On wp-admin, navigate to WooCommerce -> Settings and set the Country / State to United States (any state).
  4. Refresh the WooCommerce -> Settings page.
  5. Dismiss the JITMs (which are located above the tabs) until you see one with the button text "Install WooCommerce Services". Do not dismiss this JITM.
  6. Check debug.log - you should see the PHP warning for call_user_func_array().
  7. Apply this patch.
  8. Refresh WooCommerce ->Settings. The "Install WooCommerce Services" JITM should still be displayed at the top of the page.
  9. Check debug.log. The PHP warning for call_user_func_array() should not be generated.

Proposed changelog entry for your changes:

  • TBD

@kbrown9 kbrown9 requested a review from a team October 11, 2019 00:07
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Oct 11, 2019
@kbrown9 kbrown9 added [Status] In Progress [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. labels Oct 11, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Oct 11, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against d101760

@kbrown9 kbrown9 self-assigned this Oct 11, 2019
@kbrown9 kbrown9 added [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended and removed [Status] In Progress labels Oct 14, 2019
@jeherve jeherve added this to the 7.9 milestone Oct 15, 2019
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 15, 2019
PHPCS comment errors in the 'get_messages()' method.
@kbrown9 kbrown9 force-pushed the fix/jitm_add_filter_ns_class branch from f9a8d16 to d101760 Compare October 16, 2019 17:40
@kbrown9 kbrown9 added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 16, 2019
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@kraftbj kraftbj merged commit 970c62b into master Oct 16, 2019
@kraftbj kraftbj deleted the fix/jitm_add_filter_ns_class branch October 16, 2019 20:46
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Oct 16, 2019
@jeherve jeherve removed [Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Status] Needs Changelog labels Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants