fix: allow for context-specific reader-facing error messsages

[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:

Builds on filters added in #1745 by allowing reader-facing error messages to be altered depending on the context of the original error thrown by the ESP's API.

This PR adds a special error message for the Mailchimp error that occurs when a contact attempts to resubscribe to a list they've previously unsubscribed from. In all other cases, the error shown will be a generic error: Sorry, an error has occurred. Please try again later or contact us for support.

Also standardizes the error code used across all ESP's when add_contact results in an error to: newspack_{esp}_api_error

How to test the changes in this Pull Request:

set up a contact for the compliance state

1. Check out this branch.
2. With Mailchimp enabled, sign up as a reader with a real email address for a test newsletter list/group/tag via any means on the site.
3. Send a live newsletter campaign to the list/group/tag that the reader will receive.
4. Once you receive the email in the reader's inbox, click the "Unsubscribe" link at the bottom and confirm.
5. In a new session, attempt to resubscribe to the same newsletter lists using the same reader email.
6. Confirm the following error is shown:

7. Temporarily force an error to occur in the first line of the Mailchimp class validate() method:

	public function validate( $result, $preferred_error = null, $payload = [] ) {
+		$result['status'] = 400;
+		$result['title']  = 'Test error';
+		$result['detail'] = 'Test error message';
		$default_error = $this->get_reader_error_message( $payload, $result );

8. In a new session, subscribe to a newsletter list with a fresh email address and confirm you get the generic error message:

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?

[codecov]

Codecov Report

Attention: Patch coverage is 22.85714% with 27 lines in your changes missing coverage. Please review.

Project coverage is 23.35%. Comparing base (69ed769) to head (4acbef7).
Report is 42 commits behind head on release.

Files with missing lines	Patch %	Lines
...mailchimp/class-newspack-newsletters-mailchimp.php	43.75%	9 Missing ⚠️
includes/class-newspack-newsletters-contacts.php	11.11%	8 Missing ⚠️
...rs/class-newspack-newsletters-service-provider.php	0.00%	4 Missing ⚠️
...or/class-newspack-newsletters-campaign-monitor.php	0.00%	2 Missing ⚠️
...ct/class-newspack-newsletters-constant-contact.php	0.00%	2 Missing ⚠️
...ign/class-newspack-newsletters-active-campaign.php	0.00%	1 Missing ⚠️
...lass-newspack-newsletters-constant-contact-sdk.php	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             release    #1754      +/-   ##
=============================================
+ Coverage      23.29%   23.35%   +0.05%     
- Complexity      2710     2714       +4     
=============================================
  Files             49       49              
  Lines          10754    10778      +24     
=============================================
+ Hits            2505     2517      +12     
- Misses          8249     8261      +12     

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

[leogermani]

this logging repeats in every esp. Is it possible to move this up in the abstraction and automatically do it for all of them?

Also, wdyt of adding all errors into the same bucket and add the provider as a piece of data in the params. It's what we do with all the other logs

[dkoo]

4acbef7 moves the reader-facing error handling and logging to the higher-level Newspack_Newsletters_Contacts::upsert(). It simplifies things a bit by repurposing the existing log with the newspack_esp_sync_upsert_contact error code to log the error message from the ESP's API instead of the reader-facing error message, so there's now no longer a separate log for each. There's probably no reason to log the reader-facing error message for debugging purposes.

The testing instructions remain the same, but you can also verify that the logged output shows the original error message from the API instead of the reader-facing error message: