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

Add filter for recommended WPCOM themes #1324

Merged
merged 4 commits into from
Oct 24, 2023

Conversation

crunnells
Copy link
Contributor

@crunnells crunnells commented Oct 17, 2023

Changes proposed in this Pull Request:

Adds a filter to change the recommended themes for Woo Express sites on WPCOM.

Closes #1316.

How to test the changes in this Pull Request:

  1. Follow the instructions to setup a Woo Express site to test CYS: pdibGW-2pG-p2
  2. Update the wc-calypso-bridge plugin with this branch.
  3. Visit My Home in wp-admin and click "Customize Your Store"
  4. The Intro page will show 4 themes; confirm that the site URL is appended to the theme links, eg: https://wordpress.com/theme/tazza/yourblog.com
  5. Activate one of the recommended themes (Tsubaki, Tazza, Amulet, or Zaino) via the Themes showcase
  6. Re-visit the "Customize Your Store" page and confirm that active theme is marked as such:
Screen Shot 2023-10-17 at 2 57 39 PM

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?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@crunnells crunnells requested a review from a team October 17, 2023 23:25
@crunnells crunnells self-assigned this Oct 17, 2023
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Size Change: 0 B

Total Size: 196 kB

ℹ️ View Unchanged
Filename Size
./build/53.js 1.05 kB
./build/index.css 883 B
./build/index.js 121 kB
./build/marketing.js 58 kB
./build/payment-gateway-suggestions.css 1.25 kB
./build/payment-gateway-suggestions.js 6.45 kB
./build/plugins.js 3.92 kB
./build/style-index.css 2 kB
./build/style-marketing.css 805 B

compressed-size-action

Copy link
Contributor

@moon0326 moon0326 left a comment

Choose a reason for hiding this comment

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

Thank you for working on it 👍

LGTM and tested well with an ECOM plan. @chihsuan Do we also need to use this filter in trial mode?

@chihsuan
Copy link
Member

LGTM and tested well with an ECOM plan. @chihsuan Do we also need to use this filter in trial mode?

Yes. 🙂

Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @crunnells !

@moon0326
Copy link
Contributor

moon0326 commented Oct 19, 2023

@crunnells As confirmed by @chihsuan, we also need to run the hook for free trials. What do you think about creating a new class (e.g., class-wc-calypso-bridge-cys-themes.php) to customize the list for both trial and econ plans? You can include the new class in https://github.com/Automattic/wc-calypso-bridge/blob/master/class-wc-calypso-bridge.php#L100. You can check for free trial plan with wc_calypso_bridge_is_ecommerce_trial_plan function.

@adrianduffell Do you think we have a different (or possibly better) approach?

@crunnells
Copy link
Contributor Author

As confirmed by @chihsuan, we also need to run the hook for free trials. What do you think about creating a new class (e.g., class-wc-calypso-bridge-cys-themes.php) to customize the list for both trial and econ plans?

Do we restrict which themes free trial accounts can use and activate?

@moon0326
Copy link
Contributor

As confirmed by @chihsuan, we also need to run the hook for free trials. What do you think about creating a new class (e.g., class-wc-calypso-bridge-cys-themes.php) to customize the list for both trial and econ plans?

Do we restrict which themes free trial accounts can use and activate?

I don't think we need to restrict the themes. @chihsuan @adrianduffell Could you confirm please?

@moon0326
Copy link
Contributor

@crunnells As confirmed by @chihsuan, we also need to run the hook for free trials. What do you think about creating a new class (e.g., class-wc-calypso-bridge-cys-themes.php) to customize the list for both trial and econ plans? You can include the new class in https://github.com/Automattic/wc-calypso-bridge/blob/master/class-wc-calypso-bridge.php#L100. You can check for free trial plan with wc_calypso_bridge_is_ecommerce_trial_plan function.

@adrianduffell Do you think we have a different (or possibly better) approach?

This is actually not correct.

if ( ! wc_calypso_bridge_has_ecommerce_features() ) {
checked for both econ and econ trial plans.

Sorry for the false alarm 🙏

Copy link
Contributor

@moon0326 moon0326 left a comment

Choose a reason for hiding this comment

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

LGTM and tested well 👍 🚀

@chihsuan
Copy link
Member

I don't think we need to restrict the themes. @chihsuan @adrianduffell Could you confirm please?

Thanks for the ping. We don't need to restrict the themes! 🙂

@crunnells crunnells merged commit 63feb4b into master Oct 24, 2023
5 checks passed
@crunnells crunnells deleted the add/1316-filter-recommended-themes branch October 24, 2023 02:26
ilyasfoo added a commit that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Customize Your Store] Filter recommended themes
4 participants