refactor(campaigns): extract shared helpers to _campaigns_base.py (#602, step 1)#612
Merged
Conversation
…, step 1) First step of an incremental split of the 7 209-line campaigns.py monolith (issue #602). Moves all campaign-type-agnostic code into a new sibling module direct_cli/commands/_campaigns_base.py: - constants (CAMPAIGNS_GET_CRITERIA_LIMITS, YES_NO, ATTRIBUTION_MODELS, ...) - typed-flag validators (_validate_max_length, _validate_sms_time) - payload builders (_build_notification, _build_time_targeting, _build_relevant_keywords, _build_dynamic_placement_types, _build_frequency_cap, _build_package_bidding_strategy, _build_smart_package_bidding_strategy, _priority_goals_update_items, _route_cpa_flag) - reusable TextCampaign strategy click.option groups (_TEXT_*_STRATEGY_OPTIONS and their *_UPDATE variants) + _apply_options campaigns.py re-imports every name, so the CLI surface, every flag and every --dry-run payload is byte-for-byte identical (offline tier 2507 passed, test_wsdl_parity_gate + test_api_coverage green). Per-campaign-type logic (text/unified/dynamic/smart/mobile_app/cpm_banner) follows in subsequent PRs. Closes part of #602. Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Jul 22, 2026
axisrow
added a commit
that referenced
this pull request
Jul 23, 2026
…nner.py (#613) Step 2 of the incremental campaigns.py split (epic #602, follows shared-helpers extraction in PR #612). Moves the CPM_BANNER_CAMPAIGN add/update subtype-block composition out of the inline `elif campaign_type_norm == "CPM_BANNER_CAMPAIGN":` branches into a new sibling module direct_cli/commands/_campaigns_cpm_banner.py (build_add_block / build_update_block). Both commands now snapshot their CLI parameters once via `p = dict(locals())` and delegate; the per-type builder pulls only the CPM-relevant flags from `p`. CLI surface, every flag and every --dry-run payload is byte-for-byte identical (28 test_campaigns_*cpm* fixtures green, offline tier 2507 passed, WSDL parity + API coverage green). Smallest campaign type, used to validate the per-type extraction mechanics before the larger types (mobile_app, smart, dynamic, unified, text). Part of #602. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary
First step of an incremental split of the 7 209-line
campaigns.pymonolith (issue #602). This PR is Step 1 only — extracting shared helpers while keeping the CLI surface byte-for-byte identical. Per-campaign-type logic (text,unified,dynamic,smart,mobile_app,cpm_banner) follows in subsequent PRs.Following the architecture decision to split incrementally rather than swallow the whole file in one PR (each campaign type one at a time, verified green after each).
What moved
All campaign-type-agnostic code moved to a new sibling module
direct_cli/commands/_campaigns_base.py:CAMPAIGNS_GET_CRITERIA_LIMITS,YES_NO,ATTRIBUTION_MODELS,RELEVANT_KEYWORDS_MODES,VIDEO_TARGETS,CLIENT_INFO_MAX_LENGTH,BLOCKED_IPS_MAX_ITEMS,EXCLUDED_SITES_MAX_ITEMS,EXCLUDED_SITE_MAX_LENGTH,TIME_TARGETING_SCHEDULE_MAX_ITEMS,NEGATIVE_KEYWORD_SHARED_SET_IDS_MAX_ITEMS,HH_MM_RE,SMS_EVENTS_validate_max_length,_validate_sms_time,_upper_yes_no,_time_targeting_schedule_option,_parse_csv_option,_deprecated_campaigns_structured_option_build_notification,_build_time_targeting,_build_relevant_keywords,_build_dynamic_placement_types,_build_frequency_cap,_build_package_bidding_strategy,_build_smart_package_bidding_strategy,_priority_goals_update_items,_route_cpa_flag_array_of_string_option,_array_of_integer_optionclick.optiongroups —_TEXT_SEARCH_STRATEGY_OPTIONS,_TEXT_NETWORK_STRATEGY_OPTIONSand their*_UPDATEvariants, plus the_apply_optionshelper and the four_text_*_strategy_options[_update]decoratorscampaigns.pyre-imports every name it still references, so nothing in the command surface changed.CLI surface
No breaking changes. Every command (
get/add/update/lifecycle) and every flag is unchanged. This is a pure internal relocation — no legacy aliases, strict WSDL parity preserved.Tests run
pytest(full offline tier): 2507 passed, 8 skipped (identical to baseline)pytest tests/test_wsdl_parity_gate.py tests/test_api_coverage.py: 263 passed, 6 skipped (green)pytest tests/test_dry_run.py: 1395 passedpytest tests/test_comprehensive.py: 21 passedblack direct_cli tests— my files clean (the 16 reformatted files are pre-existing_vendor/test files, unchanged by this PR)flake8 direct_cli/commands/campaigns.py direct_cli/commands/_campaigns_base.py— clean (only pre-existing E501 long-line warnings that also exist onmain)Risks / follow-ups
COMMAND_WSDL_MAPintests/test_wsdl_parity_gate.pywas not touched (no new mutating commands added).Part of #602.
🤖 Generated with Claude Code