benlk commented 5 years ago

Changes

This PR merges the last couple year's work from the WordPress Mailchimp Tools repo into this plugin. Resolves https://github.com/INN/link-roundups/issues/136

Notable change from https://github.com/INN/wordpress-mailchimp-tools/commits/master:

Improved PHP 7 compat via https://github.com/INN/wordpress-mailchimp-tools/pull/4
Update to version 3 of the WordPress Mailchimp API in https://github.com/INN/wordpress-mailchimp-tools/pull/3

Also:

Updates install docs, resolves https://github.com/INN/link-roundups/issues/145
Wraps the Source: label in the Saved Links List widget in a span.source-label, resolves https://github.com/INN/link-roundups/issues/144
Adds proper HTML5 input types for widget forms, resolves https://github.com/INN/link-roundups/issues/143
Updates release.sh and adds maintainer docs, resolves https://github.com/INN/link-roundups/issues/140
Bumps Tested up to: to 5.2 and adds a minimum PHP version, resolves https://github.com/INN/link-roundups/issues/134
Updates the local clone of WP_List_Table, fixing compatibility, and updating docs for that process, resolves https://github.com/INN/link-roundups/issues/128 and resolves https://github.com/INN/link-roundups/issues/118

Testing

Can these steps still be followed? https://github.com/INN/link-roundups/blob/master/docs/mailchimp.md

Why

To address several PHP errors and notices in the WordPress MailChimp Tools.

To support the new version of the MailChimp API, introduced in 2017: https://mailchi.mp/f2bbb62b3016/were-shutting-down-api-11-12-and-13

benlk commented 5 years ago

@kaylima assigning this to you for several tasks:

Install the plugin from this zip (this is the v0.5-rc1 tag created today): wp-release.zip
Can the instructions at https://github.com/INN/link-roundups/blob/master/docs/mailchimp.md be followed with a testing Mailchimp account?

benlk commented 5 years ago

The WordPress Mailchimp API submodule https://bitbucket.org/mailchimp/mailchimp-api-php hasn't been updated since 2015.

The Mailchimp API settings block seen in https://github.com/INN/link-roundups/blob/136-update-wordpress-mailchimp-tools/docs/mailchimp.md isn't appearing, in my specific install, at this time. This appears to be because my install lacks the entire vendor/ directory of components, which was present in wp-release.zip above. I don't know where or how that came from, and git submodule update --init --recursive is failing to create at least one submodule that should be in the vendor/ directory.

Next steps:

[x] get a reproducible build
[x] test signup flow described in https://github.com/INN/link-roundups/blob/master/docs/mailchimp.md

benlk commented 5 years ago

Correction: the MailChimp settings page is no longer in Link Roundups > Options; it's in Settings > MailChimp Settings. This is true of both the pure-git-clone branch and the wp-release.zip install above.

Edited 2019-05-02 to add: This, too, was caused by https://github.com/INN/link-roundups/commit/675a1631f85cb056ca183206749e1cf60b9bc498's removal of a lot of code into the INN/wordpress-mailchimp-tools module

benlk commented 5 years ago

After enabling the MailChimp API key in the energynews.test site settings, the site begins timing out when trying to reach https://energynews.test/wp-admin/edit.php?post_type=roundup&page=roundup_mailchimp_settings

benlk commented 5 years ago

The timing-out page doesn't come from this plugin directly; it's generated by the WordPress MailChimp Tools:

https://github.com/INN/wordpress-mailchimp-tools/blob/04f246d463053693bcbd355fb23ec3533a7d27bb/inc/admin/class-mailchimp-post-type-settings.php#L21

I'm guessing this is because the API query to get the list of campaigns, which runs on every fetch of the page, is timing out:

https://github.com/INN/wordpress-mailchimp-tools/blob/04f246d463053693bcbd355fb23ec3533a7d27bb/inc/admin/class-mailchimp-post-type-settings.php#L43-L50

benlk commented 5 years ago

The Mailchimp Tools Settings Page only displays 10 templates; the account I'm testing with has more than 10.

[x] increase that beyond 10
[x] sort that list alphabetically/ASCIIbetically

benlk commented 5 years ago

[x] increase that 10 count in the same spot in the post editor

benlk commented 5 years ago

Draft created? Yes. Send a drafted campaign? Yes. Campaign works as described? No.

benlk commented 5 years ago

So it looks like 675a1631f85cb056ca183206749e1cf60b9bc498 removed this bit by removing the entire inc/lroundups/mailchimp-admin.php:

function _lroundups_render_mailchimp_template( $source, $post ) {
    $author = get_user_by( 'id', $post->post_author );

    $tags = array(
        '*|ROUNDUPLINKS|*'      => apply_filters( 'the_content', $post->post_content ),
        '*|ROUNDUPTITLE|*'      => $post->post_title,
        '*|ROUNDUPAUTHOR|*'     => $author->display_name,
        '*|ROUNDUPDATE|*'       => get_the_date( '',$post->ID ),
        '*|ROUNDUPPERMALINK|*'  => get_post_permalink( $post->ID )
    );

    foreach ( $tags as $tag => $value )
        $source = str_replace( $tag, $value, $source );

    return $source;
}

That commit just outright removed the file, and the file doesn't appear to have been replaced. Nor was the functionality in there.

That was release v0.4.

I remember us having bug reports about it, but I can't find an issue for it.

benlk commented 5 years ago

In https://secure.helpscout.net/conversation/573233977/1940?folderId=1259183 on May 30, 2018:

I wanted to make sure that MailChimp merge tags worked, and they do in the single-column templates we were experimenting with (Sherry made one and I made an additional one to troubleshoot). Neither will pull in the link roundup tags. I tried in caps and in all lowercase in case it was a syntax error on my part. None of the other link roundup tags were working for me either (*|ROUNDUPAUTHOR|*, etc.). Do you have any idea of where we went wrong?

https://github.com/INN/link-roundups/issues/146

benlk commented 5 years ago

146 should be fixable by re-adding that removed function.

What's not clear there yet is:

if anything else should be re-added.
where in the filesystem this should go.

benlk commented 5 years ago

The removed function was applied here:

        /** 
         * Replace the template tags in the MailChimp post  
         */ 
        $html = _lroundups_render_mailchimp_template( $template_info['source'], $post );    
        $campaign_content = array(  
            'html' => $html, // the content!    
            'text' => '' // Leave blank for the auto-generated text content 
        );

The analogous functionality in the current WP MC Tools is:

https://github.com/INN/wordpress-mailchimp-tools/blob/0ff5674b85b8604895a1022c9d0c7f83cf3a0c10/inc/admin/class-mailchimp-campaign-editor.php#L234-L242

But there's no way to configure the parameters that are set on that email. So:

[x] the WordPress Mailchimp Tools will need to be modified to allow filtering parameters
[x] this will need a filter on the parameters to set the right parameter, using the template parameter: https://developer.mailchimp.com/documentation/mailchimp/reference/campaigns/content/#%20
[x] the docs in here will need to be modified to talk about mc:edit="" areas, which will be filled with the information from the parameters https://templates.mailchimp.com/getting-started/template-language/

benlk commented 5 years ago

It is currently creating the email, but entirely replacing the content of the email with the *|ROUNDUPLINKS|* content rather than filling the template areas. This should be investigated.

Right now it's using a drag-and-drop template with a block that contains the following:

This should be the roundup title:
<section mc:edit="ROUNDUPTITLE">This should be replaced with the roundup title</section>
This should be the roundup author:

<section mc:edit="ROUNDUPAUTHOR">This should be replaced with the roundup author</section>
This should be the roundup date:

<section mc:edit="ROUNDUPDATE">This should be replaced with the roundup date</section>
This should be the roundup links:

<section mc:edit="ROUNDUPLINKS">This should be replaced with the roundup links</section>
This should be the roundup permalink:

<section mc:edit="ROUNDUPPERMALINK">This should be replaced with the roundup permalink</section>

<hr />
<h1 style="text-align: center;">*|ROUNDUPTITLE|*</h1>

<p><em>*|ROUNDUPAUTHOR|* &mdash; *|ROUNDUPDATE|*</em></p>
*|ROUNDUPLINKS|*

<p>Originally posted at *|ROUNDUPPERMALINK|*</p>

benlk commented 5 years ago

It's not possible to get the whole default content of a template via the API.

The closest thing is https://developer.mailchimp.com/documentation/mailchimp/reference/templates/default-content/, which returns the default content of the various editable areas defined in the template, but not the whole body content of the campaign. Therefore, the approach taken in the previous version of the WordPress MailChimp Tools to set the $html parameter to the post content will not work for the Link Roundup Plugin's purposes.

The filter introduced in https://github.com/INN/wordpress-mailchimp-tools/commit/4e768f7661a2fe8fc2785140e8313280eb230c3f will have to be used to set the HTML parameter on the campaign content, if the user of the WMT package desires to have that set.

Because the whole content cannot be accessed, the idea of filtering that content for the various merge tags in this plugin to do a search and replace for this plugin's desired content output is no longer valid.

[ ] remove the *|ROUNDPLINKS|* and other such tags
[ ] replace the docs for that with docs for setting up editable content areas
[x] remove the default assertion of $html in https://github.com/INN/wordpress-mailchimp-tools/blob/6271fd78bce655f4dfd8c685cf4f024d0c8ee923/inc/admin/class-mailchimp-campaign-editor.php#L233-L241
[x] discuss with @INN/tech-team whether this is a "breaking" change deserving of bumping the plugin version to 1.0 -> https://github.com/INN/link-roundups/issues/148

benlk commented 5 years ago

It worked. https://us1.admin.mailchimp.com/reports/show?id=2907321

benlk commented 5 years ago

Consolidating earlier checklists:

[x] remove the *|ROUNDPLINKS|* and other such tags
[x] replace the docs for that with docs for setting up editable content areas https://mailchimp.com/help/create-editable-content-areas-with-mailchimps-template-language/
[x] the docs in here will need to be modified to talk about mc:edit="" areas, which will be filled with the information from the parameters https://templates.mailchimp.com/getting-started/template-language/
[x] ~update this plugin's example email templates to use those areas~ This plugin does not have example templates.
[x] ~Replace "List" with "Audience" in docs and WMT: https://mailchimp.com/help/create-audience/~ Nope, it's still called a List in the API: https://developer.mailchimp.com/documentation/mailchimp/reference/lists/
[x] get the WMT audience box to show the subgroups targeted (Will require parsing the interest ID)

benlk commented 5 years ago

[x] get the WMT audience box to allow un-targeting a targeted subgroup.

benlk commented 5 years ago

🎉

Next step: Someone please test that this works for them.

INN / link-roundups

Update INN/wordpress-mailchimp-tools submodule, and diverse other updates #139

Changes

Testing

Why

146 should be fixable by re-adding that removed function.