fix: parse legacy layout defaults #1663

dkoo · 2024-09-20T18:07:46Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR parses legacy newsletterData metadata on preexisting custom newsletter layouts to migrate sender and send-to settings to the newer, leaner, cross-ESP-compatible campaign_defaults meta field for layout CPT posts. This should make #1658 NOT a breaking change, as publishers' preexisting custom layouts will be able to seamlessly their data.

How to test the changes in this Pull Request:

On trunk or release, create a new newsletter, set sender email + name and list/sublist options, then save as a new custom layout.
Check out this branch, then create a new newsletter using the custom layout from step 1. Confirm that the Sender Name/Email and Send To options are prepopulated from the layout settings.
Repeat for Mailchimp, ActiveCampaign, and ~~Constant Contact~~ (Campaign Monitor is not handled as there is no preexisting legacy data to handle for this ESP). EDIT: also, Constant Contact does not seem to work as creating a new campaign always seems to set the top-level list by default, but this is also the case in trunk, so I think this is non-blocking for now.

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

WFM with Mailchimp, but not with AC – the list was saved, but the sender not:

…faults

dkoo · 2024-10-09T20:06:02Z

@adekbadek good catch, I had a mistake with the senderName meta field name for the AC method, plus the legacy newsletterData has a different shape for AC vs. the other ESPs. c567728 should fix both issues.

codecov · 2024-10-09T20:12:48Z

Codecov Report

Attention: Patch coverage is 0% with 113 lines in your changes missing coverage. Please review.

Project coverage is 20.89%. Comparing base (2e7497e) to head (c567728).
Report is 26 commits behind head on epic/update-editor-uis.

Files with missing lines	Patch %	Lines
includes/class-newspack-newsletters-layouts.php	0.00%	42 Missing ⚠️
...mailchimp/class-newspack-newsletters-mailchimp.php	0.00%	31 Missing ⚠️
...ct/class-newspack-newsletters-constant-contact.php	0.00%	22 Missing ⚠️
...ign/class-newspack-newsletters-active-campaign.php	0.00%	12 Missing ⚠️
includes/class-newspack-newsletters.php	0.00%	6 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##             epic/update-editor-uis    #1663      +/-   ##
============================================================
- Coverage                     21.13%   20.89%   -0.24%     
- Complexity                     2622     2660      +38     
============================================================
  Files                            48       48              
  Lines                          9615     9757     +142     
============================================================
+ Hits                           2032     2039       +7     
- Misses                         7583     7718     +135

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adekbadek

Works as expected, but I see file changes – changelog, package.json, etc – which should not be part of this PR. Pre-approving, but these should be removed.

dkoo · 2024-10-10T16:09:02Z

Whoops, that's because I had updated epic/update-editor-uis locally but forgot to push. Should be good now, thanks!

* feat: make "Send To" UI autocomplete instead of dropdowns (#1577) * feat: autocomplete "Send To" UI for CC + AC * fix: sent post statuses for AC * feat: autocomplete send-to UI for Mailchimp * fix: support _n() contacts in AC + CC * refactor: standardize "sent" status checks for all ESPs * fix: function name for CC * fix: set list_type for audiences * fix: extra parentheses for AC * feat: show suggestions on focus * style: move external "Manage" links to button * feat: design updates for autocomplete UI * fix: send to summary * fix: contact counts in send to summary * fix: mailchimp and selected list styles * fix: remove ID from AC segments * fix: logic for contact count in send to summary * fix: parse contact counts as int to avoid "1 contacts" output * fix: translation strings for contact counts in summaries * refactor: standardize use of renderSelectedSummary functions * fix: translatable send-to summaries * fix: allow 0 to be shown * fix: grammar * Update src/service-providers/active_campaign/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * Update src/service-providers/active_campaign/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * Update src/service-providers/constant_contact/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * Update src/service-providers/constant_contact/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * Update src/service-providers/mailchimp/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * Update src/service-providers/mailchimp/ProviderSidebar.js Co-authored-by: leogermani <[email protected]> * fix: translatable strings in SendTo component’s selected item label --------- Co-authored-by: leogermani <[email protected]> * fix(editor): render notice on sync error (#1606) * fix(mailchimp): improve sync error messages (#1619) * fix(activecampaign): default message on request error (#1618) * feat: cross-ESP send-lists * refactor: move API namespace and permission callback to main class * feat: support send lists in provider classes * feat: refactor editor JS to use new standardized data * fix: ensure Newsletters plugin blocks are grouped under Newspack category (#1630) * feat: send list classes (#1629) * feat: cross-ESP send-lists * refactor: move API namespace and permission callback to main class * refactor: feedback from code review * fix: add missing get/set methods for count and parent_id * refactor: more consistent naming for set_count method * refactor: maintain exception but with WP_Error handling * docs: add more explanation about Send_Lists Co-authored-by: leogermani <[email protected]> * test: improve tests for matching methods * refactor: set values on protected $config var * docs: tweak wording of explanation --------- Co-authored-by: leogermani <[email protected]> * feat: support send lists in ESP classes (#1631) * feat: cross-ESP send-lists * refactor: move API namespace and permission callback to main class * feat: support send lists in provider classes * fix: use getter methods for Send_List classes * chore: clean up register_rest_route configs and permission callbacks * fix: improve error handling for get_send_list methods * fix: add error handling to send-lists API handler * fix: return Send_List objects as arrays from provider methods * fix: avoid unnecessary send-list API requests * chore: remove unneeded methods * fix: fatal error due to get_send_lists returning Send_List arrays instead of objects * Revert "fix: fatal error due to get_send_lists returning Send_List arrays instead of objects" This reverts commit 3ff1e65. * fix: allow get_send_list methods to return either Send_List[] or arrays * fix: correct type for supportedESPs fallback value * fix(mc): subscriber send count in pre-send modal summary * fix: formatting error * fix: ensure supported_esps is an array, not an object * fix: missed callable * fix: cc error handling * fix: better error handling for retrieve/sync request errors * refactor: unify retrieve error handling across all ESPs * refactor: throw error for missing MC creds, too * test: update expected exception message * refactor: consolidate ESP data getters into Redux store * docs: update inline docs for Redux store * fix: move SendTo to Sidebar component; avoid parallel retrieve requests * refactor: always prefetch sublists on list select * fix: account for missing selected list or sublist info * fix: default sender/send-to for saved layouts * fix: campaign_defaults meta field name * fix: don't overwrite fetched lists/sublists info on retrieve * fix: force release * feat: add link to campaign in provider (#1661) * fix: parse legacy layout defaults (#1663) * fix: handle legacy layout defaults for MC * fix: legacy layout defaults for AC + CC * fix: senderName + senderEmail for AC --------- Co-authored-by: leogermani <[email protected]> Co-authored-by: Miguel Peixe <[email protected]> Co-authored-by: Miguel Peixe <[email protected]>

dkoo added 2 commits September 19, 2024 16:46

fix: handle legacy layout defaults for MC

bb8e863

fix: legacy layout defaults for AC + CC

865cab1

dkoo added the [Status] Needs Review label Sep 20, 2024

dkoo self-assigned this Sep 20, 2024

dkoo requested a review from a team as a code owner September 20, 2024 18:07

adekbadek requested changes Oct 9, 2024

View reviewed changes

dkoo added 2 commits October 9, 2024 10:29

Merge branch 'epic/update-editor-uis' into fix/parse-legacy-layout-de…

558ff84

…faults

fix: senderName + senderEmail for AC

c567728

dkoo requested a review from adekbadek October 9, 2024 20:06

adekbadek approved these changes Oct 10, 2024

View reviewed changes

github-actions bot added [Status] Approved Ready to merge and removed [Status] Needs Review labels Oct 10, 2024

dkoo merged commit 25a9e2d into epic/update-editor-uis Oct 10, 2024
11 checks passed

dkoo deleted the fix/parse-legacy-layout-defaults branch October 10, 2024 16:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: parse legacy layout defaults #1663

fix: parse legacy layout defaults #1663

dkoo commented Sep 20, 2024

adekbadek left a comment

dkoo commented Oct 9, 2024

codecov bot commented Oct 9, 2024 •

edited

Loading

adekbadek left a comment

dkoo commented Oct 10, 2024

fix: parse legacy layout defaults #1663

fix: parse legacy layout defaults #1663

Conversation

dkoo commented Sep 20, 2024

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

adekbadek left a comment

Choose a reason for hiding this comment

dkoo commented Oct 9, 2024

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

adekbadek left a comment

Choose a reason for hiding this comment

dkoo commented Oct 10, 2024

codecov bot commented Oct 9, 2024 •

edited

Loading