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

Feat/mailchimp related tweaks #3381

Merged
merged 5 commits into from
Sep 3, 2024
Merged

Conversation

adekbadek
Copy link
Member

@adekbadek adekbadek commented Aug 30, 2024

All Submissions:

Changes proposed in this Pull Request:

Two tweaks related to Mailchimp handling:

  1. Removes the mailchimp-for-woocommerce plugin installation prompt from the Engagement Wizard
  2. Adds an error notice if the ESP sync list (for Mailchimp and ActiveCampaign) is set to "None":
image

How to test the changes in this Pull Request:

  1. Set Mailchimp as the ESP
  2. Go to Engagement wizard -> Reader Activation tab -> Advanced Settings, and toggle on the "Sync contacts to ESP" option
  3. Observe that if the Audience ID is set to "None", an error notice with the appropriate message appears
  4. Try to save the settings – observe a browser alert displayed, and that the settings won't be saved if the ESP Sync is enabled and the Audience ID is set to "None"
  5. Go to Newsletters tab in the same wizard, observe there's no "WooCommerce integration" section at the bottom

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 ran tests with your changes locally?

@adekbadek adekbadek added the [Status] Needs Review The issue or pull request needs to be reviewed label Aug 30, 2024
@adekbadek adekbadek requested a review from a team as a code owner August 30, 2024 09:54
@adekbadek adekbadek force-pushed the feat/mailchimp-related-tweaks branch from 4cd2052 to d5dc9de Compare August 30, 2024 10:00
@leogermani
Copy link
Contributor

Observe that if the Audience ID is set to "None", an error notice with the appropriate message appears

Didn't work for me.

Also, I don't think this is specific to Mailchimp. It also applies to Active Campaign

@adekbadek
Copy link
Member Author

Didn't work for me.

Hm, what about trying to save the settings with "None" selected?

Also, I don't think this is specific to Mailchimp. It also applies to Active Campaign

Added the same for ActiveCampaign in 6a75d78.

Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Now it's working 🤷

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Sep 2, 2024
@adekbadek adekbadek merged commit ac68b67 into trunk Sep 3, 2024
8 checks passed
@adekbadek adekbadek deleted the feat/mailchimp-related-tweaks branch September 3, 2024 06:25
matticbot pushed a commit that referenced this pull request Sep 20, 2024
# [5.4.0-alpha.6](v5.4.0-alpha.5...v5.4.0-alpha.6) (2024-09-20)

### Bug Fixes

* change the current product criteria for sync ([#3416](#3416)) ([28a84bc](28a84bc))
* **esp-sync:** sync Connected Account field ([#3414](#3414)) ([61c02bc](61c02bc))
* hide My Account links if not relevant ([#3394](#3394)) ([0d039d8](0d039d8))
* **phpcs:** specify path in custom ruleset ref ([#3384](#3384)) ([b143e74](b143e74))
* prevent PHP notice while checking my-account page ([#3435](#3435)) ([146a26f](146a26f))
* **ras-sync:** deprecate redundant Signup_Page meta field ([#3439](#3439)) ([61d6de8](61d6de8))
* **reader-registration-block:** fix initial newsletter checkbox state ([1890efe](1890efe))
* replace `newspack_image_credits_placeholder` default value ([#3433](#3433)) ([c754fc2](c754fc2))
* **sync:** method name for `membership_saved` handler ([#3399](#3399)) ([2c0bf26](2c0bf26))
* **sync:** place esp sync admin features behind a constant ([#3438](#3438)) ([20a0970](20a0970))
* **sync:** remove localized number format ([#3434](#3434)) ([2243a5d](2243a5d))

### Features

* add a new action to when a ras setting is updated ([#3357](#3357)) ([35d3492](35d3492))
* **ga4:** detect gate interaction blocks ([#3408](#3408)) ([e14913c](e14913c))
* media kit page handling ([#3358](#3358)) ([4454850](4454850))
* **ras:** sync class ([#3362](#3362)) ([88acbee](88acbee))
* **ras:** unify ESP connector strategy for data events ([#3360](#3360)) ([7080864](7080864))
* **reader-activation:** ESP-related tweaks ([#3381](#3381)) ([ac68b67](ac68b67))
* remove Woo Membersip sync fields ([#3411](#3411)) ([28052e8](28052e8))
* **sync:** add ESP sync notice to RAS wizard ([#3400](#3400)) ([f9acd56](f9acd56))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.4.0-alpha.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants