Skip to content

Conversation

@n7studios
Copy link
Contributor

Summary

Adds a method for bulk tagging subscribers.

Testing

  • testTagSubscribers: Test that tag_subscribers() returns the expected data.
  • testTagSubscribersWithInvalidTagID : Test that tag_subscribers() throws a ClientException when an invalid tag ID is specified.
  • testTagSubscribersWithInvalidSubscriberID : Test that tag_subscribers() throws a ClientException when an invalid subscriber ID is specified.

Checklist

@n7studios n7studios added this to the 2.2.1 milestone Sep 30, 2025
@n7studios n7studios self-assigned this Sep 30, 2025
@n7studios n7studios changed the base branch from fix-tests to master October 3, 2025 08:30
@n7studios n7studios requested review from a team, corydhmiller and noelherrick and removed request for a team October 8, 2025 05:28
@n7studios n7studios marked this pull request as ready for review October 8, 2025 05:28

/**
* Test that tag_subscribers() throws a ClientException when an invalid
* tag ID is specified, as this is only supported using OAuth.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment makes it seem like tagging with an invalid is possible with OAuth. Can we drop or modify the last clause? Same goes for the comments on the following functions in this file. I'm also wondering if these test are necessary since we're testing the specific behavior in ConvertKitAPITest.php

Copy link
Contributor Author

@n7studios n7studios Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can't be removed, as ConvertKitAPIKeyTest and ConvertKitAPIOAuthTest both extend ConvertKitAPITest, which is where most of the tests are contained (and therefore run) - otherwise, there'd be duplicate tests in two classes.

I've marked these methods to be skipped in ConvertKitAPIKeyTest, given testCreateTags checks for the ClientException.

Other redundant tests skipped in this PR.

@n7studios n7studios requested a review from noelherrick October 10, 2025 01:51
Copy link

@noelherrick noelherrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know on the inheritance constraints - after your explanation, it makes sense that you'd need to implement all parent methods.

@n7studios n7studios merged commit a0dde7d into master Oct 11, 2025
0 of 5 checks passed
@n7studios n7studios deleted the add-tag-subscribers-method branch December 5, 2025 06:55
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.

3 participants