Skip to content

Conversation

@ecgan
Copy link
Contributor

@ecgan ecgan commented Sep 14, 2022

All Submissions:

Changes proposed in this Pull Request:

When I was doing testing and review in #34679 (review), I found that the new Marketing option checkbox does not show up when we use a build zip.

It seems like that is because the multichannel-marketing flag is set to false in core.json file.

In this PR, we set the value to true. With that, when we build a zip file and upload it to a site, the checkbox would then show up.

image

How to test the changes in this Pull Request:

  1. Run pnpm build.
  2. Run pnpm build:zip --filter=woocommerce to build a WooCommerce zip file.
  3. In your test site, install or update WooCommerce using the zip file above.
  4. Go to WooCommerce Settings > Advanced > Features page. You should see the new Marketing option checkbox, which should be disabled by default.
  5. Open another tab and go to the Marketing page: /wp-admin/admin.php?page=wc-admin&path=%2Fmarketing. You should see the normal marketing page.
  6. Check the new Marketing option checkbox and save, and then reload the Marketing page. You should see the new beta marketing page.

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 run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

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.

This is to make the new Marketing option show up in WC Settings when WC is installed or updated via build zip file.
@ecgan ecgan requested review from a team September 14, 2022 18:41
@ecgan ecgan self-assigned this Sep 14, 2022
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Sep 14, 2022
@ecgan
Copy link
Contributor Author

ecgan commented Sep 14, 2022

Hi @chihsuan , could you help to review this please? 🙏 Could you confirm that it is correct for me to set the flag to true in core.json file? (I think I had myself confused with what it is actually supposed to do.)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 14, 2022

Test Results Summary

Commit SHA: d3feb6c

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests11800201200m 41s
E2E Tests186001018713m 30s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@chihsuan
Copy link
Member

Hi @chihsuan , could you help to review this please? 🙏 Could you confirm that it is correct for me to set the flag to true in core.json file? (I think I had myself confused with what it is actually supposed to do.)

@ecgan Yea, it's correct to set the flag true to show it up on the setting page.

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

LGTM! 🚢

@ecgan ecgan merged commit b73b2ff into trunk Sep 15, 2022
@ecgan ecgan deleted the feature/update-multichannel-marketing-flag branch September 15, 2022 06:59
@github-actions github-actions bot added this to the 7.0.0 milestone Sep 15, 2022
@github-actions
Copy link
Contributor

Hi @ecgan, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants