Skip to content

Bug/merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page #110

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.

Sign up for GitHub

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

Jump to bottom
Merged
vikrampm1 merged 1 commit into from
Jan 28, 2025
Merged

Bug/merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page #110

vikrampm1 merged 1 commit into from
Jan 28, 2025

Conversation

@MaxwellGarceau
Copy link
Collaborator

@MaxwellGarceau MaxwellGarceau commented Jan 3, 2025 •
edited by vikrampm1
Loading

Description of the Change

In order to avoid duplicating information, please see #105 for a description of the original bug.

non-visible-merge-fields-can-still-be-set-in-the-mailchimp-wp-admin.mov

Solution

Add another conditional check to display the include checkboxes only when the merge field is public.

Closes #105

How to test the Change

This video in the "Description of the Change" demonstrating the original bug can be used as a walkthrough. However, instead of the bug you should see the fix.

  1. Uncheck the "Visible?" column in a sampling of merge fields in the test user Mailchimp account
  2. On the Mailchimp WP settings page click "Update List" to refresh the data from Mailchimp
  3. In the "Merge Fields Included" section, you should only see checkboxes for the merge fields that have the "Visible?" column checked in the Mailchimp account
  4. (optional) Navigate to the front end to ensure that the non visible merge fields do not display.

Changelog Entry

Fixed - Fixed an issue that was allowing a user to select merge fields that were not selected as visible in the Mailchimp account.

Credits

Props @MaxwellGarceau

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly. No documentation to update
  • I have added tests to cover my change. Small fix, no test needed
  • All new and existing tests pass.

@github-actions github-actions bot added this to the 1.7.0 milestone Jan 3, 2025
@MaxwellGarceau MaxwellGarceau changed the title Add public as requirement to show include box on merge field Bugfix: Merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page Jan 3, 2025
@MaxwellGarceau MaxwellGarceau marked this pull request as ready for review January 3, 2025 01:50
@MaxwellGarceau MaxwellGarceau requested a review from dkotter January 3, 2025 01:50
@github-actions github-actions bot added the needs:code-review This requires code review. label Jan 3, 2025
@MaxwellGarceau MaxwellGarceau changed the title Bugfix: Merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page Bug/Merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page Jan 3, 2025
@MaxwellGarceau MaxwellGarceau changed the title Bug/Merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page Bug/merge fields not marked as "Visible?" in the user's Mailchimp account will still display as includable in the WP Mailchimp settings page Jan 3, 2025
@qasumitbagthariya
Copy link
Collaborator

qasumitbagthariya commented Jan 18, 2025

QA Update ✅

I have verified this PR in the bug/non-visible-merge-fields-will-display-in-settings-menu-as-options-in-the-include-column branch which has been fixed and is functioning as intended.

I tested the following on this branch:

  1. In the "Merge Fields Included" section, only see checkboxes for the merge fields that have the "Visible?" column checked in the Mailchimp account
  2. Navigate to the front end to ensure that the non visible merge fields do not display. ✅
Screen.Recording.2025-01-18.at.3.29.05.PM.mov

Testing Environment

  • WordPress: 6.7.1
  • Theme: Twenty Twenty-Four 1.3
  • WooCommerce - 9.5.1
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS 15.2
  • Branch: bug/non-visible-merge-fields-will-display-in-settings-menu-as-options-in-the-include-column

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for code review (Woo)
Testing Document status:
Cases related to this Issue/PR are added to the Critical Flow Wiki pages:

  • Yes
  • Not Required/Applicable for this PR

@qasumitbagthariya
Copy link
Collaborator

qasumitbagthariya commented Jan 28, 2025

Regression / Smoke Test Report ✅

Tested with the smoke-testing branch, it works as expected, similar to the fix-specific branch.

Testing Environment

  • WordPress: 6.7.1
  • Theme: Twenty Twenty-Four 1.3
  • WooCommerce - 9.5.1
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS 15.2
  • Branch: smoke-testing

@vikrampm1 vikrampm1 modified the milestones: 1.7.0, 1.6.3 Jan 28, 2025
@vikrampm1 vikrampm1 merged commit 59b1a97 into develop Jan 28, 2025
16 checks passed
@vikrampm1 vikrampm1 mentioned this pull request Jan 28, 2025
Closed
27 tasks
@iamdharmesh iamdharmesh mentioned this pull request Mar 20, 2025
Closed
1 task
@dkotter dkotter deleted the bug/non-visible-merge-fields-will-display-in-settings-menu-as-options-in-the-include-column branch May 8, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkotter dkotter dkotter approved these changes

@jeffpaul jeffpaul jeffpaul approved these changes

@qasumitbagthariya qasumitbagthariya qasumitbagthariya approved these changes

Assignees

@MaxwellGarceau MaxwellGarceau

Labels

needs:code-review This requires code review.

Projects

None yet

Milestone

1.6.3

Development

Successfully merging this pull request may close these issues.

Non visible merge fields in a Mailchimp account will still display in the WP Mailchimp settings menu under the "Include?" column

6 participants

@MaxwellGarceau @qasumitbagthariya @dkotter @jeffpaul @vikrampm1