Merge tag-sync-PoC into main — align main with production gem#27
Merged
Conversation
…k-remove-tag-from-subscriber Gmartini/ban 8680 add bulk remove tag from subscriber
…correct-failure-data-for-bulk-tag-operations Gmartini/ban 8729 return correct failure data for bulk tag operations
roqmarcelo
approved these changes
Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bring
mainup to date with the eleven commits that have been accumulating onadin/tag-sync-PoCsince December 2024 — code that has already been running in production via Speckel's Gemfile pin. Post-merge,mainreflects the gem we actually ship.This is housekeeping, not a feature PR. The merge is a fast-forward and a no-op for production behavior.
Problem
Our
mainis frozen at June 2024 (a CI commit by Petro Podrezo). All real gem work since December 2024 has landed onadin/tag-sync-PoCvia PR #25 (BAN-8680) and PR #26 (BAN-8729) — both targeted the PoC branch directly rather thanmain. The original attempt to merge PoC → main, PR #24, was opened 2024-12-18 and closed unmerged on 2025-02-27 without comments or stated reason.Meanwhile, Speckel's Gemfile has been pinning to
branch: 'adin/tag-sync-PoC'the entire time. Production has been running the "PoC" branch for over a year.Two consequences worth fixing:
maindoes not reflect the gem code we ship. Anyone readinglib/onmainwould conclude the gem is missingDeveloperClient, bulk tag operations, and the v4 tag-update endpoint.revoke_token— the gem POSTsContent-Type: application/jsonto Kit's/oauth/revoke, which strictly requiresapplication/x-www-form-urlencoded. That fix needs to branch off amainthat's actually current.There is no upstream to sync with —
ConvertKit/convertkit-api-rubywas deleted after Kit's rebrand. This repo is the onlyconvertkit-api-rubyleft on GitHub. We are the maintainers by default.Solution
Fast-forward
maintoadin/tag-sync-PoC(commit862a9426). Theadin/tag-sync-PoCbranch is retained, not deleted on merge — Speckel's Gemfile still names it, so the branch must outlive this PR until a follow-up in Speckel repoints the gem source atmain(or at a tag cut from it).What ships, grouped by what the reviewer needs to evaluate:
DeveloperClient— API-key authenticationlib/convertkit/client.rb— newConvertKit::DeveloperClient < Clientthat authenticates with an API key instead of an OAuth access token. Some Kit endpoints can only be reached with a developer API key.lib/convertkit/connection.rb— the constructor now builds headers via adefault_headers(options)helper. Whenoptions[:api_key]is present, the connection addsX-Kit-Api-Key: <key>to every request. Bearer-token auth (options[:auth_token]) is unchanged in behavior; the only structural change is moving header construction into a private method to keep both auth modes side-by-side.New spec at
spec/lib/convertkit/developer_client_spec.rb.Tags resource —
updatemethod andcreatebugfixlib/convertkit/resources/tags.rb:Tags#update(tag_id, options = {})— PUT/tags/:id, per the v4 update-tag-name endpoint. Currently acceptsname:.Tags#create— previously returnedTagResponse.new(response), which wrapped the entire JSON envelope instead of the innertagobject. Now returnsTagResponse.new(response['tag']), matching what every other Tags method does. Spec updated to assert the new shape.Bulk tag operations
lib/convertkit/resources/tags.rbadds two methods:Tags#bulk_add_to_subscribers(taggings)— POST/bulk/tags/subscribers, returnsSubscriberBulkAddTagResponse.Tags#bulk_remove_from_subscribers(taggings)— DELETE/bulk/tags/subscribers, returnsSubscriberBulkRemoveTagResponse.Both raise
ArgumentErrorwhentaggingsisn't an Array. Backed by three new response classes plus one new subscriber shape:SubscriberBulkAddTagResponsesubscribers:(array ofTaggedSubscriberResponse) andfailures:(array ofSubscriberBulkTagFailureResponse)SubscriberBulkRemoveTagResponsefailures:only — successful removes return no per-subscriber payloadSubscriberBulkTagFailureResponse.tagging(SubscriberTagResponse) and.errorsso callers can identify which(subscriber_id, tag_id)pair failed and whyTaggedSubscriberResponse(insubscriber_response.rb)tagged_attimestamp returned by bulk-addSubscriberTagResponse(subscriber_id, tag_id)pair attached to each failurelib/convertkit.rbrequires all five new files soConvertKit::Resources::*resolves them.Registry
lib/convertkit.rb— five newrequirelines, alphabetically interleaved with existing entries. No load-order subtleties.Design Decisions
adin/tag-sync-PoCafter mergebundle install. The branch will be removed in a follow-up after Speckel's Gemfile is repointed atmain(or a tag cut from it).Blast Radius
Zero behavioral risk at merge time. Speckel already pins to SHA
862a9426d230, which is the tip ofadin/tag-sync-PoCand the same commit that becomes the new tip ofmain. The fast-forward merge produces no new code; it just makesmainreachable to that SHA.The code being aligned with
mainis in active production use by Speckel —DeveloperClient, bulk tag ops, and the v4Tags#updatemethod have all been exercised inconvert_kit_service.rbandsync_convert_kit_subscriber.rbjobs for months.Monitor nothing — there's nothing new shipping. The next PR (the
/revokeContent-Type fix) is where production-monitoring matters.Rollback Plan
Revert via GitHub's Revert button on this PR. Since
adin/tag-sync-PoCis retained, Speckel continues to pin against the unchanged branch tip — revertingmaindoes not perturb production.Dependencies
None blocking this merge. Follow-up work that depends on this PR landing:
v0.0.13) frommainafter merge.ConvertKit::OAuth#revoke_tokento send form-encoded params per RFC 7009 §2.1. Branched offmain.branch: 'adin/tag-sync-PoC'to the new tag (or tomain).adin/tag-sync-PoC.Test plan
Verifiable by agent before merging
main—git log main..adin/tag-sync-PoCshows 11 commits; reverse direction is empty.Gemfile.lockpins the gem at SHA862a9426d230, which is the current tip ofadin/tag-sync-PoCand the new tip ofmainpost-merge.ConvertKit/convertkit-api-rubyrepo exists to sync from (HTTP 404).mainin this repo.Cannot be verified before merge