Skip to content

Add Subscriber API functions #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 14, 2023
Merged

Add Subscriber API functions #45

merged 4 commits into from
Mar 14, 2023

Conversation

n7studios
Copy link
Contributor

Summary

Adds the following API functions to provide API coverage for Subscribers

  • update_subscriber()
  • unsubscribe()

Outputs a PHP deprecated notice if using form_unsubscribe(), because:

  • The function name is misleading; it doesn't just unsubscribe the email address from a form,
  • The argument supports an array of options; unsubscribe() provides the specific supported argument (email address)

form_unsubscribe() is not removed at this time, to ensure we don't introduce breaking changes to the SDK. The deprecation notice serves as an early warning to developers to migrate to unsubscribe() as we will remove form_unsubscribe() at a later date.

Testing

  • Tests for above functions included.

Checklist

@n7studios n7studios self-assigned this Mar 13, 2023
@n7studios n7studios added this to the 1.0.0 milestone Mar 13, 2023
@n7studios n7studios requested review from a team, noelherrick and corydhmiller and removed request for a team March 13, 2023 15:25
@n7studios n7studios marked this pull request as ready for review March 13, 2023 15:25
@noelherrick
Copy link

Looking over all these tests, I wonder if we should be throwing a custom exception and checking for that in case we replace GuzzleHttp. The error also would narrow things down if this gets used by other folks who might be using Guzzle for other HTTP requests.

@n7studios
Copy link
Contributor Author

Looking over all these tests, I wonder if we should be throwing a custom exception and checking for that in case we replace GuzzleHttp. The error also would narrow things down if this gets used by other folks who might be using Guzzle for other HTTP requests.

Can be explored in a future PR, as:

  1. Highly unlikely that we would replace Guzzle; it's one of the most popular PHP HTTP clients used
  2. Throwing our own exceptions would provide more detail, although may be seen as a breaking change, which I'm not keen on introducing to a first stable version of this SDK. At present Guzzle's ClientException is thrown, and as a developer who uses the current ConvertKit PHP SDK, I'd continue to expect the same behaviour unless expressly told otherwise.
  3. Developers are welcome to narrow down the cause of the error by inspecting the API's response e.g.:
try {
    $api->get_subscriber(12345);  
} catch (GuzzleHttp\Exception\ClientException $e) {
    $error = json_decode($e->getResponse()->getBody()->getContents());
    die($error->message); // Or however else they want to inspect the error message from the API and handle it.
}

@n7studios n7studios merged commit 9729e6d into 1.0-beta Mar 14, 2023
@n7studios n7studios deleted the subscriber-functions branch March 23, 2023 16:01
@n7studios n7studios mentioned this pull request Mar 26, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants