refactor(DCT-266): migrate remaining client methods to ExecuteBuilder - #486
Open
script-this wants to merge 5 commits into
Open
refactor(DCT-266): migrate remaining client methods to ExecuteBuilder#486script-this wants to merge 5 commits into
script-this wants to merge 5 commits into
Conversation
script-this
marked this pull request as draft
August 14, 2026 15:12
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
… methods Add PutRequest to ExecuteBuilder for UpdateCollection's PUT request, and migrate GetFilterSets, GetFilterSet, CreateFilterSet, GetSurveys, GetSurvey, CreateSurvey, DeleteSurvey, GetSurveyResponses, GetSurveyResponse, CreateSurveyResponse, DeleteSurveyResponse, DeleteAllSurveyResponses, GetSurveyResponseSummary, UpdateCollection, GetMessages, SendMessage, GetUnreadMessages, BulkSendMessage, SendGroupMessage, CreateBonusPayments and PayBonusPayments onto the builder. 65 of 90 client methods now use ExecuteBuilder. Remaining: AI Task Builder and credential pool methods.
…ilder Migrate GetAITaskBuilderBatch, GetAITaskBuilderBatchStatus, GetAITaskBuilderBatches, GetAITaskBuilderResponses, GetAITaskBuilderTasks, GetAITaskBuilderTaskGroups, InitiateBatchExport, GetBatchExportStatus and SyncAITaskBuilderBatch onto the builder. UpdateAITaskBuilderBatch needed more care: it does errors.As(err, *UnrecognizedAPIError) directly on the raw error from Client.Execute to special-case INVALID_BATCH_ITEMS validation errors, which only worked because that error wasn't wrapped before the check. ExecuteBuilder.Execute wrapped errors with %s, which would have silently broken that type assertion (formatBatchErrorBody's existing unit tests wouldn't have caught it, since they test the formatter directly, not the errors.As wiring). Added TestUpdateAITaskBuilderBatch to characterize the current error-branching behaviour end-to-end first, then fixed ExecuteBuilder.Execute to wrap with %w instead of %s (identical Error() text, but preserves the chain for errors.Is/errors.As) before migrating the method — verified against the same characterization tests. 74 of 90 client methods now use ExecuteBuilder.
…ool methods Migrate GetAITaskBuilderDataset, GetAITaskBuilderDatasetStatus, GetAITaskBuilderDatasetUploadURL, GetAITaskBuilderDatasetImportStatus, GetAITaskBuilderBatchSyncStatus, CreateAITaskBuilderInstructions, SetupAITaskBuilderBatch, CreateAITaskBuilderDataset, CreateAITaskBuilderCollection, CreateCredentialPool, UpdateCredentialPool and ListCredentialPools onto ExecuteBuilder. CreateAITaskBuilderBatch had the same errors.As(*UnrecognizedAPIError) pattern as UpdateAITaskBuilderBatch (see previous commit) — added TestCreateAITaskBuilderBatch to characterize it first, then migrated using the same approach now that ExecuteBuilder.Execute preserves the error chain. Also caught and migrated GetAITaskBuilderBatchSyncStatus, which was missed in the earlier AI Task Builder batch. 88 of 90 Client methods now use ExecuteBuilder — every method that makes an HTTP call. The remaining 2 (userAgent, Execute itself) don't call Execute and have nothing to migrate. This completes DCT-266.
Same simplification as the base branch: GetFilterSet, GetSurvey, GetSurveyResponse, GetSurveyResponseSummary, GetBatchExportStatus, GetAITaskBuilderBatchSyncStatus, GetAITaskBuilderDatasetImportStatus and ListCredentialPools were reinventing ExecuteBuilder.Get(url, &response) via its constituent calls instead of using it directly.
script-this
force-pushed
the
refactor/dct-266-execute-builder-migration-part2
branch
from
August 14, 2026 15:20
b9d317a to
d1732f3
Compare
Base automatically changed from
refactor/dct-266-execute-builder-migration
to
main
August 14, 2026 15:59
script-this
marked this pull request as ready for review
August 14, 2026 16:01
script-this
added
team-dct
customer-facing
dct-ready-for-review
cemprolific
approved these changes
Aug 14, 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
3 participants
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
Second and final PR for the ExecuteBuilder migration (DCT-266). Stacked on #485 — migrates the remaining methods so every one of the 88
Clientmethods that makes an HTTP call now usesExecuteBuilder(up from 44/90 in #485; the other 2 of 90,userAgentandExecuteitself, don't callExecuteand have nothing to migrate).Migrated
Filter sets, surveys,
UpdateCollection, messages, bonus payments, and the full AI Task Builder + credential pool surface (batches, datasets, instructions, setup, sync, export). AddedPutRequesttoExecuteBuilderforUpdateCollection's PUT request.The tricky part: typed error inspection
UpdateAITaskBuilderBatchandCreateAITaskBuilderBatchboth doerrors.As(err, *UnrecognizedAPIError)directly on the raw error fromClient.Executeto special-caseINVALID_BATCH_ITEMSvalidation errors into a readable message. This only worked because that error wasn't wrapped before the check —ExecuteBuilder.Executewrapped errors with%s, which would have silently destroyed the type and broken that formatting.formatBatchErrorBody's existing unit tests wouldn't have caught this, since they test the formatter directly, not theerrors.Aswiring around it.Before touching either method, added
TestUpdateAITaskBuilderBatchandTestCreateAITaskBuilderBatchto characterize the full error-branching behaviour end-to-end (4xx +INVALID_BATCH_ITEMS, non-4xx-but-wrong-status +INVALID_BATCH_ITEMS, a recognized error shape that should not get special formatting, and the success path). Then fixedExecuteBuilder.Executeto wrap with%winstead of%s— identicalError()text, but preserves the chain forerrors.Is/errors.As— and migrated both methods, verified against the same tests.Testing
New
TestUpdateAITaskBuilderBatch/TestCreateAITaskBuilderBatchcharacterization tests, full suite (make test),make lint, andcontract_test's spec-matching test all pass.