Skip to content

Search: Fix search plan detection#15156

Merged
jsnmoon merged 13 commits intoadd/search-planfrom
fix/search-plan-detection
Mar 29, 2020
Merged

Search: Fix search plan detection#15156
jsnmoon merged 13 commits intoadd/search-planfrom
fix/search-plan-detection

Conversation

@jsnmoon
Copy link
Copy Markdown
Contributor

@jsnmoon jsnmoon commented Mar 27, 2020

Changes proposed in this Pull Request:

  • Check for Search product eligibility by checking the site's purchases instead of its plan.
  • Removes Instant Search feature gate for Jetpacks' Plan page.
  • Known issue: Enabling the Jetpack Search module without a Professional Plan is currently not possible.

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

  • No.

Testing instructions:

  • Apply this change to your Jetpack installation.
  • Using a site without a Jetpack Search purchase, navigate to the following pages below. Ensure that each page works as expected (e.g. see upsells for Jetpack Search as necessary).
    1. The dashboard (/wp-admin/admin.php?page=jetpack#/dashboard)
    2. My Plan page (/wp-admin/admin.php?page=jetpack#/my-plan)
    3. Plans page (/wp-admin/admin.php?page=jetpack#/plans)
    4. Performance settings page (/wp-admin/admin.php?page=jetpack#/performance).
  • Purchase a Jetpack Search product to your site and try the four pages listed above again. Ensure that each page works as expected.

Proposed changelog entry for your changes:

  • None

@jsnmoon jsnmoon added Bug When a feature is broken and / or not performing as intended Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Search For all things related to Search Plans [Feature] Backups [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Mar 27, 2020
@jsnmoon jsnmoon requested review from a team, AnnaMag, gibrown and keoshi March 27, 2020 22:13
@jsnmoon jsnmoon self-assigned this Mar 27, 2020
jsnmoon
jsnmoon commented Mar 27, 2020
View reviewed changes
_inc/lib/class.core-rest-api-endpoints.php
'type' => 'boolean',
'default' => 0,
'validate_callback' => __CLASS__ . '::validate_boolean',
'jp_group' => 'search',
Copy link
Copy Markdown
Contributor Author

@jsnmoon jsnmoon Mar 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Would deleting this jp_group value prevent module de/activations from mutating this option value? cc @AnnaMag @gibrown

Copy link
Copy Markdown
Member

@gibrown gibrown Mar 28, 2020

Choose a reason for hiding this comment

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

I think I added this because someone suggested it at some point. I was thinking it had more to do with documentation and grouping of the options in the api, but I am unsure.

@gibrown
Copy link
Copy Markdown
Member

gibrown commented Mar 28, 2020

This seems to break the settings page when the site is on a free plan: wp-admin/admin.php?page=jetpack#/performance

gibrown
gibrown reviewed Mar 28, 2020
View reviewed changes
gibrown
gibrown requested changes Mar 28, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

In addition to the performance settings tab breaking, when I toggle back and forth from my-plans to plans when i have a free plan and a search plan then the app crashes.

Maybe this has to do with me not having some options already set, or that they are already set?

@gibrown gibrown requested a review from mdbitz March 28, 2020 01:52
@jsnmoon
Copy link
Copy Markdown
Contributor Author

jsnmoon commented Mar 28, 2020

This seems to break the settings page when the site is on a free plan: wp-admin/admin.php?page=jetpack#/performance @gibrown

✅ This was caused by a typo in one of the import statements.

when I toggle back and forth from my-plans to plans [...] the app crashes. @gibrown

✅ This was caused by an incorrect usage of React's useEffects function.

Both issues have been fixed!

@gibrown gibrown mentioned this pull request Mar 28, 2020
Closed
gibrown
gibrown reviewed Mar 28, 2020
View reviewed changes
gibrown
gibrown reviewed Mar 28, 2020
View reviewed changes
gibrown
gibrown approved these changes Mar 29, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

I think this is all working now... Code looks good.

@jsnmoon jsnmoon merged commit f324618 into add/search-plan Mar 29, 2020
@jsnmoon jsnmoon deleted the fix/search-plan-detection branch March 29, 2020 01:37
@jsnmoon jsnmoon removed [Status] Needs Changelog [Status] Needs Team Review Obsolete. Use Needs Review instead. Bug When a feature is broken and / or not performing as intended labels Mar 29, 2020
gibrown added a commit that referenced this pull request Mar 30, 2020
@gibrown
Search: Add search plan to dashboard (#15153)
a5ec8c3 
* Search: Add Search plan to wp-admin Plans/My-Plans pages (#15011)
* Add Instant Search feature gate
* Add Jetpack Search to the Plans page
* Instant Search: Add support in glance and performance sections (#15043)
* Search: Add instant search auto config (#15026)
* Significant refactoring of Plans pages, especially for Backups
* Update the copy for search module. (#15123)
* Search: Add search plan to my-plans page (#15095)
* Search: Add pricing and tier information to Plans page (#15125)
* Search: Fix search plan detection (#15156)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gibrown gibrown gibrown approved these changes

@keoshi keoshi Awaiting requested review from keoshi

@AnnaMag AnnaMag Awaiting requested review from AnnaMag

@mdbitz mdbitz Awaiting requested review from mdbitz

Assignees

@jsnmoon jsnmoon

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Backups [Feature] Search For all things related to Search Plans Touches WP.com Files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jsnmoon @gibrown @matticbot