fix(advertising): gate updateCampaign on account approval — close the suspended-account spend leg (#11364) - #11619
Merged
Conversation
… suspended-account spend leg (#11364) #11516 gated createCampaign and startCampaign on account.status === "active" but left updateCampaign ungated: a suspended (or still-pending) account could PATCH a campaign budget increase, which deducts credits and pushes the change live to the ad platform — the exact spend the approval workflow exists to stop. Add the same fail-closed gate after the account lookup in updateCampaign (mirrors the createCampaign/startCampaign wording), and extend the #11364 suite with updateCampaign-blocked tests across pending/suspended/disconnected. Verification: ad-account-approval.test.ts 15 pass / 0 fail with the gate; reverting the source change alone fails exactly the 3 new tests (proves the tests pin the hole). cloud/shared tsgo --noEmit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
6 tasks
This was referenced Jul 3, 2026
lalalune
pushed a commit
that referenced
this pull request
Jul 3, 2026
…e — un-red develop after #11619 gate #11619 gated updateCampaign on account.status === "active" but did not update the ad-campaign-credit-reconciliation mocks, which stub adAccountsRepository.findById without a status field. Since that merge, 7 of the suite's 13 tests fail on develop with 'Ad account is not active (status: undefined)' before ever reaching the credit logic under test. Mark both beforeEach account mocks active so the suite exercises hold reconciliation again; the gate itself stays covered by ad-account-approval.test.ts (15 tests). Before: 6 pass / 7 fail. After: 13 pass / 0 fail (48/0 across the four ad suites). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lalalune
added a commit
that referenced
this pull request
Jul 3, 2026
…e — un-red develop after #11619 gate (#11715) #11619 gated updateCampaign on account.status === "active" but did not update the ad-campaign-credit-reconciliation mocks, which stub adAccountsRepository.findById without a status field. Since that merge, 7 of the suite's 13 tests fail on develop with 'Ad account is not active (status: undefined)' before ever reaching the credit logic under test. Mark both beforeEach account mocks active so the suite exercises hold reconciliation again; the gate itself stays covered by ad-account-approval.test.ts (15 tests). Before: 6 pass / 7 fail. After: 13 pass / 0 fail (48/0 across the four ad suites). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lalalune
pushed a commit
that referenced
this pull request
Jul 3, 2026
…ampaign approval gate The updateCampaign persistence test mocked adAccountsRepository.findById without a status field. After develop merged #11619 (updateCampaign now throws unless account.status === "active"), the merged tree fails this test with 'Ad account is not active (status: undefined)'. Mark the mock account active so the test exercises the metadata merge, not the gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 3, 2026
Closed
lalalune
pushed a commit
that referenced
this pull request
Jul 3, 2026
Adds `linkedin` to AdPlatform across schemas, DB typing, credit markup, the provider registry, and app-promotion validation, plus a real LinkedIn Marketing API provider (versioned REST gateway): - adAccounts search finder for account discovery/validation - campaign group + paused campaign creation with objective, budget, geo-targeting (urn:li:geo pass-through, worldwide default, loud failure on free-text locations), and #11621 bid-control mapping to costType/optimizationTargetType per the documented combinations - Rest.li PARTIAL_UPDATE for update/pause/activate and PENDING_DELETION deletes - Images/Videos API media upload (initializeUpload -> PUT -> finalizeUpload) owned by the account's organization reference - inline dark-post creative creation (creatives?action=createInline) - adAnalytics analytics-finder metrics mapping - OAuth2 refresh_token grant support Unit tests use fixtures lifted from the Microsoft Learn LinkedIn Marketing API reference pages and drive advertisingService with the real provider to prove the #11619 approval gate, #11621 bid metadata, and the fail-closed refund path apply to LinkedIn automatically. A credential-gated linkedin.real.test.ts live lane loud-skips without LINKEDIN_ADS_ACCESS_TOKEN. Closes #11663. Refs #11361. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
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
1 participant
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.
Refs #11364. Follow-up closing the residual the #11516 verification flagged.
Hole
#11516 (merged) gated
createCampaign+startCampaignonaccount.status === "active"— butupdateCampaignstayed ungated. Exploit: account approved → campaign created → account suspended for ToS →PATCH /api/v1/advertising/campaigns/:idwith abudgetAmountincrease still deducts credits and pushes the raise live to the ad platform. A suspended account could keep increasing spend.Fix
One fail-closed gate in
updateCampaignafter the account lookup (packages/cloud/shared/src/lib/services/advertising/index.ts), mirroring the createCampaign/startCampaign gates.UpdateCampaignInputis name/budget/dates/targeting only — pausing/stopping a campaign goes through separate methods — so the blanket gate blocks no legitimate wind-down action for suspended accounts.Verification
ad-account-approval.test.ts: 15 pass / 0 fail (3 new: updateCampaign blocked for pending/suspended/disconnected).packages/cloud/sharedtsgo --noEmit: clean.🤖 Generated with Claude Code