Skip to content

fix(masters): add --tracking-params for UTMInput field (#761) - #762

Merged
axisrow merged 4 commits into
mainfrom
fix/761-landing-url-utm-field
Aug 5, 2026
Merged

fix(masters): add --tracking-params for UTMInput field (#761)#762
axisrow merged 4 commits into
mainfrom
fix/761-landing-url-utm-field

Conversation

@axisrow

@axisrow axisrow commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • --landing-url behaves as before (masters update: редактирование/удаление UTM-меток посадочной страницы #757) — writes the whole "Ссылка на продвигаемую страницу" field (LinkInput) as a single value.
  • New --tracking-params flag writes to the separate, dedicated "UTM-метки и параметры URL" field (CampaignLinkEditorLite.UTMInput, under the "Дополнительные параметры" spoiler) — independent of --landing-url.
  • Corrects an earlier, backwards conclusion documented in the code: UTMInput being normally empty was mistaken for evidence it's an unused "extra params" helper, when live behavior confirms it's exactly where a UTM template belongs — templates have just historically been typed into the main URL field out of habit.
  • Adds an early ARCHIVED-campaign guard in update_master: an ARCHIVED campaign's edit page renders no save control at all, previously surfacing as an opaque "neither button appeared" timeout regardless of which field was being updated.

Live verification

Verified against a real Yandex Direct account: writing the full URL including a ?utm_source=... query string into LinkInput is rejected by Yandex, while writing the bare UTM query string into the separate UTMInput field is accepted and persists correctly. --tracking-params set and clear both verified via direct masters update end-to-end (including _verify_saved reload+re-read), then the test campaign was restored to its original state.

Test plan

  • Full offline suite: 3217 passed, 23 skipped
  • masters update --help documents --tracking-params
  • Live: set --tracking-params, verified via reload; cleared with empty string, verified via reload; campaign restored to original state

Closes #761

🤖 Generated with Claude Code

axisrow and others added 4 commits August 5, 2026 10:14
--landing-url writes the whole "Ссылка на продвигаемую страницу" field
(LinkInput) as before. UTMInput ("UTM-метки и параметры URL", under the
"Дополнительные параметры" spoiler) is a separate, dedicated UTM
query-string field — confirmed live it was empty only because nobody had
used it, not because it's an unused "extra params" helper as previously
assumed. A new --tracking-params flag writes directly to it, independent
of --landing-url.

Also adds an early ARCHIVED-campaign guard in update_master: an ARCHIVED
campaign's edit page renders no save control at all, which previously
surfaced as an opaque "neither button appeared" timeout regardless of
which field was being updated.

Live-verified against a real Yandex Direct account: split write to
UTMInput accepted where writing the full URL+query into LinkInput was
previously rejected.

Closes #761
…1_utm_split

CI 'quality' job failed on ruff F541 (3 occurrences) in the newly added
recon script — plain string literals had no interpolated values.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex adversarial review (cycle-review round 1) found the new campaign-wide
ARCHIVED guard in update_master read the landing-URL Clear button's
disabled state BEFORE clicking/focusing the field. The branch's own
scripts/recon_761_utm_split.py documents live that this button is disabled
whenever the field is unfocused, independent of ARCHIVED status — so the
guard could misclassify any ordinary campaign as ARCHIVED and block every
masters update (including unrelated --weekly-budget etc. calls).

Fix: _is_landing_url_archived clicks/focuses the URL field first, then
polls the Clear button briefly for it to settle, before treating it as an
ARCHIVED signal — mirrors the click-then-check order recon_761_utm_split.py
already uses. Reused by both the new update_master guard and the
pre-existing _set_landing_url check (same underlying issue, narrower blast
radius before this PR).

Also fixes the recon script's own --mutate rollback (Codex finding #2):
restore no longer skips writing UTMInput back when the original value was
empty (previously left the test query permanently installed), wraps
mutate+restore in try/finally so an exception can't skip rollback, and
refuses to mutate when either baseline field couldn't be conclusively read.

New regression test: test_does_not_misclassify_unfocused_non_archived_campaign_as_archived
(confirmed red against the pre-fix code, green after).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex adversarial review round 2 (cycle-review) found the round-1 fix
itself unsound: clicking the URL field before reading the Clear button's
disabled state distinguishes 'unfocused' from 'ARCHIVED', but the button
is ALSO legitimately disabled on an ordinary, non-ARCHIVED campaign
whenever the landing URL is currently empty (nothing to clear) — so the
click-then-poll proxy could still misclassify a normal campaign (e.g. one
mid --landing-url '' workflow) as ARCHIVED.

Removes the Clear-button-based ARCHIVED inference entirely, from both the
update_master guard (already removed in this commit) and _set_landing_url
itself (same underlying issue, pre-dating this PR from #757 — the cold
disabled-check without a prior click never uniquely meant ARCHIVED either).

Both call sites now rely on the write actually failing where it already
reliably does for an ARCHIVED campaign:
- _wait_for_draft_status already times out on an ARCHIVED campaign (no
  terminal save control renders for one) — its error message is upgraded
  with a best-effort ARCHIVED hint via the existing _read_status_text,
  never used as a pre-mutation gate.
- _set_landing_url now just attempts the write via
  _set_contenteditable_field and lets its own 'could not clear the field'
  error surface, similarly upgraded with the same best-effort hint.

Neither hint can misfire into blocking an unrelated update, since neither
gates anything before the underlying operation was already going to fail
on its own.

Updated tests accordingly: removed the now-redundant weekly-budget-only
ARCHIVED-guard test, replaced the Clear-button-disabled fixture modeling
with a read-only-field (ignores Backspace) model, and added focused
regression tests on _wait_for_draft_status directly (fast, no browser
round-trip) for both the ARCHIVED-hint and inconclusive-status paths.

Full offline suite: 3220 passed, 23 skipped. ruff/mypy clean.

Follow-up: a third Codex review round is still pending for this change
(not yet run) — flagging for whoever picks this back up, per user request
to stop here for now.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@axisrow

axisrow commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

Статус cycle-review (остановлено по запросу — не хватило времени на полный цикл)

Цикл 1 (round 1): Codex нашёл 2 critical finding'а — исправлены и запушены (e65ecfe):

  1. Новый ARCHIVED-guard в update_master мог ложно блокировать любой masters update на обычной кампании (проверял disabled Clear-кнопку до фокуса поля).
  2. scripts/recon_761_utm_split.py --mutate мог необратимо испортить UTM-данные реальной кампании при rollback.

Цикл 1 round 2: Codex нашёл ещё один critical finding в моём же фиксе — click-then-poll подход тоже ненадёжен (Clear-кнопка легитимно disabled и на обычной кампании с пустым URL). Исправлено (20d378d): полностью убрана ARCHIVED-инференция по Clear-кнопке; вместо неё используется уже существующий надёжный timeout _wait_for_draft_status + best-effort подсказка через _read_status_text (не gate, только для текста ошибки).

Полный офлайн-набор тестов зелёный (3220 passed), ruff/mypy чисты.

Follow-up (не сделано): 3-й раунд Codex-review на финальный HEAD 20d378d ещё не запущен. Перед мержем стоит прогнать ещё один раунд review, чтобы убедиться, что round-2 фикс не содержит новых проблем.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

masters: --landing-url writes UTM into the wrong field — UTMInput is the intended UTM field, not an 'extra params' helper

1 participant