fix: bugs from Newsletters editor data refactor #1671
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
This PR fixes several bugs in the Newsletters editor data refactor done in #1658 that became more apparent now that we've been able to test with various publisher accounts and real data.
How to test the changes in this Pull Request:
Handling for large contact counts in Send List summary (9d95c3c)
In the send list summaries for selected lists/sublists, we use
Number.toLocaleString()
to show large numbers in a localized string format for human readability, e.g.10,000 contacts
instead of10000 contacts
. However, we're erroneously using the%d
placeholder in the summary string, which expects a true number instead of a stringified number, so these large numbers aren't getting populated as intended. This PR fixes it by using%s
instead.trunk
, using Mailchimp, temporarily change this line to hardcode a very large number such as1000000
for thecount
value%d
instead of the contact count number, and a JS console error[sprintf] expecting number but found string
:1,000,000 contacts
Handling for Mailchimp accounts with multiple audiences (60ee0f3 and f59cece)
When fetching sublist info in the editor, we retain all previously fetched sublists in the Redux store and add newly fetched sublists to the store—this way, on subsequent searches or sidebar component mounts, we don't have to go all the way to the REST API again to rebuild this info on the store. However, for all ESPs, sublists are unique to a top-level list, so if you select a new top-level list and then interact with the sublist field, we shouldn't expect to see info for sublists that were previously fetched for the old list. This PR ensures that when selecting a top-level list, we empty out all existing sublist info in the store and post meta. It also avoids an infinite loop if you select an audience that has no tags/groups/segments at all.
trunk
, create or edit a newsletter and select an audience. Confirm that the sublist suggestions shown in the sublist autocomplete field match the selected audience.Handling for Mailchimp accounts with many sublist entities (more than 10 per audience) (35d1aba)
Unique to Mailchimp, when setting the sublist for a campaign via their API, the shape of the payload we pass depends on the type of sublist we're setting (tag, segment, or group). Since the sublist is saved to the post only as an ID, we need to fetch info about the sublist to know what the payload should look like, so we use
$provider->get_send_lists()
to fetch the sublist's info by ID. However, this will only look for cached sublist info, and if we've previously only fetched a limited number of sublists to the cache (as might happen if you load up the editor after the previous cache has been invalidated, and we prefetch only 10 segments in the first fetch), the info we're looking for might not yet be in the cache. This PR fixes this scenario by ensuring we fetch info for up to 1000 sublists from the Mailchimp API when we go to build the sync payload.trunk
, create or edit a newsletter and select an audience.PHP Warning: Undefined array key 0 in /srv/htdocs/wp-content/plugins/newspack-newsletters/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php on line 1054
Overzealous overwriting by layout campaign defaults (19ce5c8)
When setting a newsletter post to a custom newsletter layout with saved sender and send-to info (either on new post creation or by resetting the layout via the Layout sidebar), we expect the defaults stored in the layout to overwrite whatever sender/send-to meta the post has already. However, after the layout has been set, we don't want the layout defaults to overwrite anything anymore. This PR fixes things by ensuring that layout defaults are only taken into account immediately after selecting a new layout, and then cleared from the post upon the next manual save.
trunk
, using any ESP, create a new layout from a post with saved sender and send-to info.Migrating legacy meta for AC (9aff88c)
We went through a lot of iterations of newsletter data schema and handling of "legacy" meta fields. Along the way it looks like this handling broke for ActiveCampaign lists and sublists. This PR fixes things for AC and also ensures that send list/sublist properties in the Redux store are always named the same thing across all ESPs.
trunk
and refresh this newsletter post in the editor. Observe that the send list and segment are not populated.Other information: