fix: address review feedback from #170#171
Conversation
- Delete placeholder test files for removed resources instead of stubs - Reject storefront persona in Scope3Client constructor at runtime - Remove index signatures from CreateCampaignInput/UpdateCampaignInput - Add response validation to campaigns create/update methods - Type autoSelectProducts, getMediaBuyStatus, getProducts responses using shapes from OpenAPI spec
| salesAgentId: string; | ||
| groupId: string; | ||
| groupName: string; | ||
| cpm?: number; |
There was a problem hiding this comment.
this type is missing fields from AutoSelectProductsResponse in buyer.ts. selectedProducts items need budget: number (required) and pricingOptionId?: string. top level is also missing budgetContext: { campaignBudget, totalAllocated, remainingBudget, currency }, productCount: number, and previouslySelectedCount?: number
| budget?: number; | ||
| pricingOptionId?: string; | ||
| pricingModel?: string; | ||
| } |
There was a problem hiding this comment.
missing fields vs CampaignProductEntry in the schema: selectedAt: string (required), searchContext?: { id: string; brief: string }, and mediaBuys: Array<{ MediaBuyId: string; Status: string; name: string }> (also required)
| | 'ACTIVE' | ||
| | 'PAUSED' | ||
| | 'COMPLETED' | ||
| | 'CANCELED'; |
There was a problem hiding this comment.
FAILED, REJECTED, and ARCHIVED are in the MediaBuyStatus schema but missing here
| Status: string; | ||
| startTime?: string; | ||
| endTime?: string; | ||
| }>; |
There was a problem hiding this comment.
no corresponding exported schema for this in buyer.ts - looks hand-rolled. if it's based on the campaign response's mediaBuys field, it should reuse that shape rather than being a standalone interface
| ): Promise<ApiResponse<Record<string, unknown>>> { | ||
| return this.adapter.request<ApiResponse<Record<string, unknown>>>( | ||
| data?: { | ||
| refine?: Array<Record<string, unknown>>; |
There was a problem hiding this comment.
RefinementItem is already defined in the types - using it here would give callers actual type safety instead of Record<string, unknown>
- AutoSelectProductsResult: add budget (required), pricingOptionId, budgetContext, productCount, previouslySelectedCount - CampaignProductEntry: add selectedAt (required), searchContext, mediaBuys (required) - MediaBuyStatusValue: add FAILED, REJECTED, ARCHIVED - CampaignMediaBuyStatus: match GetAdcpStatusOutput schema shape (campaign_id, media_buys with full status fields, agents_queried, errors) - Add RefinementItem discriminated union type, use in autoSelectProducts
2 participants
Summary
Addresses retroactive review comments from #170:
describe('removed')stubs were noise with temporal references)storefrontpersona inScope3Clientconstructor at runtime — prevents silent misconfiguration[key: string]: unknownindex signatures fromCreateCampaignInputandUpdateCampaignInputto restore type safetycampaignSchemas.responsetocreateandupdatemethods (matchingget)autoSelectProducts,getMediaBuyStatus,getProductsreturn values using shapes from the OpenAPI spec instead ofRecord<string, unknown>Test plan
tsc --noEmitpasses