fix(mailchimp): fatal error with invalid API keys

[dkoo]

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:

Fixes a fatal error when trying to fetch audience/list data using an invalid Mailchimp API key. The fatal was happening because our Mailchimp integration's API response validation logic only considered status codes of 400 or 404 to be errors, but an invalid API key error is returned with a status code of 401, and unhandled WP errors are returned with status codes of 500. So this PR will handle any response with a status code of 400 or greater as an error.

How to test the changes in this Pull Request:

1. On trunk, connect your site to Mailchimp using a valid API key.
2. In the Mailchimp dashboard, revoke that API key.
3. Using WP CLI, trigger the cron job to refresh the Mailchimp cache (or just wait >20 minutes): wp cron event run newspack_nl_mailchimp_refresh_cache
4. Visit Newspack > Engagement and Newsletters > Settings admin pages and observe fatal errors in each.
5. Check out this branch and refresh both admin pages, and confirm no fatal error and a more helpful error message shown in both places:

6. Confirm that you can update the API key to a new valid key and that saving persists the key and restores the connection to the ESP.

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?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1208644983311128

[adekbadek]

I was not able to complete the testing steps until I've emptied the cache (newspack_nl_mailchimp_cache_lists option). With the wrong key, the list retrieval doesn't update the lists and fails silently, keeping the cached lists.

[dkoo]

I was not able to complete the testing steps until I've emptied the cache (newspack_nl_mailchimp_cache_lists option). With the wrong key, the list retrieval doesn't update the lists and fails silently, keeping the cached lists.

@adekbadek aha, you're correct. 41db9b6 should fix this by returning a proper error in this scenario. So now, if you run wp cron event run newspack_nl_mailchimp_refresh_cache or wait 20 minutes for the cache to refresh, the cached data will no longer be returned if we fail to refresh the cache, and the error message should appear in the UIs.

[leogermani]

I was not able to complete the testing steps until I've emptied the cache (newspack_nl_mailchimp_cache_lists option). With the wrong key, the list retrieval doesn't update the lists and fails silently, keeping the cached lists.

@adekbadek aha, you're correct. 41db9b6 should fix this by returning a proper error in this scenario. So now, if you run wp cron event run newspack_nl_mailchimp_refresh_cache or wait 20 minutes for the cache to refresh, the cached data will no longer be returned if we fail to refresh the cache, and the error message should appear in the UIs.

Wait! That's by design!

From the docs:

If the cache refresh fails, we will store the error in a separate option, and will only surface it to the user after 20 minutes.
In every admin page we will display a generic Warning message, telling the user to go to Newsletters > Settings to see the errors.
In Newsletters > Settings we will output the errors details.

Maybe what we can do is clear the cache if the API key changes

[codecov]

Codecov Report

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

Project coverage is 20.37%. Comparing base (44c1b12) to head (bb82561).
Report is 1 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...ass-newspack-newsletters-mailchimp-cached-data.php	0.00%	6 Missing ⚠️
includes/class-newspack-newsletters-settings.php	0.00%	2 Missing ⚠️
...mailchimp/class-newspack-newsletters-mailchimp.php	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #1690      +/-   ##
============================================
- Coverage     20.38%   20.37%   -0.02%     
- Complexity     2664     2666       +2     
============================================
  Files            48       48              
  Lines         10619    10626       +7     
============================================
  Hits           2165     2165              
- Misses         8454     8461       +7     

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

[dkoo]

Thanks for clarifying, @leogermani! I'll spin up another commit with that change