feat(conversion-action): CRUD tools (split 2/4 of #34)#51
Open
illia-sapryga wants to merge 1 commit into
Open
Conversation
Split (b) of kLOsk#34, rebuilt fresh on main — create/update/remove conversion actions only (uploads are split c). Registers only its own three tools. - current_config() resolution; the three ops wired into the _execute_plan dispatch, matching main's lazy-import style. - Preserves the immutable/invalid-field handling the maintainer flagged as correct: include_in_conversions_metric is immutable on create; the view-through window is AD_CALL-only; auto-managed types are warned. - Fix: a positive default_value with always_use_default_value=False no longer silently flips the flag to True. It now emits a preview warning and leaves the flag exactly as the caller set it (fallback vs override is the caller's decision, not a silent override). The GAQL apostrophe-escaping fix lands in split (c), where the only by-name conversion-action lookup (the upload resolvers) actually lives. Claude-Session: https://claude.ai/code/session_01FKcPiYrtXWtg4b7dnW4o5s
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.
Split 2/4 of #34 — conversion-action CRUD
Second of the four-way split, rebuilt fresh on
main, registering only its own three tools (no uploads — those are split (c) — no assets/RSA).Tools
draft_create_conversion_actiondraft_update_conversion_actiondraft_remove_conversion_actionWrappers resolve config via
current_config(); impls takeconfigfirst; the three ops are wired into the_execute_plandispatch with lazy imports (matching main's style). Newsrc/adloop/ads/conversion_actions.pyholds only the CRUD functions — split (c) will add the upload functions to it.Your fix:
default_value>0no longer silently flips the flagA positive
default_valuepaired withalways_use_default_value=Falseused to silently force the flag toTrue— turning a legal "tag value with fallback" config into "always override." It now emits a preview warning and leaves the flag exactly as the caller set it:Applied to both create and update. Tests assert the warning fires and the flag is not force-set;
default_value=0/ explicitTrueproduce no warning.Preserved (your "reviewed clean" items)
include_in_conversions_metricis immutable on create — the create op leaves the proto field unset (regression test assertsnot HasField(...)); a test confirms it IS mutable on update.view_through_lookback_windowis settable only forAD_CALL.SMART_CAMPAIGN_*,GOOGLE_HOSTED) are warned.On your GAQL-escaping note
The apostrophe-doubling bug is in the by-name conversion-action lookup, which only exists in the upload resolvers — so the backslash-escaping fix lands in split (c) alongside that code, rather than as a dead helper here. Flagging so you know it isn't dropped.
Tests & hygiene
Example Co), no real client data.Next: (c) conversion uploads (folds #43 + the GAQL fix), then (d) RSA update (folds #42).
https://claude.ai/code/session_01FKcPiYrtXWtg4b7dnW4o5s