Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions MIGRATION_v3_to_v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,40 @@ if package.status == PackageStatus.active: ...
if media_buy.status == MediaBuyStatus.active: ...
```

### `MediaBuyStatus.pending_activation` → split

The single `pending_activation` enum value was split into two
distinct states based on cause. The codemod flags every reference; the
correct replacement is per-call-site.

| Cause | Replacement |
| --- | --- |
| Buy is scheduled and waiting for its start time | `MediaBuyStatus.pending_start` |
| Buy is waiting on creative approval / asset processing | `MediaBuyStatus.pending_creatives` |

**Before (v3.x):**
```python
if media_buy.status == MediaBuyStatus.pending_activation:
notify_trafficker(media_buy)
```

**After (v4.0):**
```python
if media_buy.status in (
MediaBuyStatus.pending_start,
MediaBuyStatus.pending_creatives,
):
notify_trafficker(media_buy)
```

When the original branch only fired for one cause, narrow to the right
state (e.g. only `pending_creatives` for creative-review notifications).
A blanket replacement to either single value is almost always wrong —
the spec split was driven by adopters needing distinct behaviour for
the two cases. The wire enum still accepts `pending` as a legacy alias
for `pending_start` (per the schema description), so existing buyer
clients reading older payloads keep working without code changes.

### `ResolvedBrand.brand_manifest` field removed

`RegistryClient.lookup_brand()` returns a `ResolvedBrand` whose
Expand Down
24 changes: 20 additions & 4 deletions src/adcp/migrate/v3_to_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,23 @@

# Attribute accesses that moved / were removed. Flagged not rewritten
# because context determines the right replacement.
REMOVED_ATTRIBUTE_ACCESSES: dict[str, str] = {
".brand_manifest": ("ResolvedBrand.brand_manifest removed — use .brand instead"),
#
# Each entry is keyed on the dotted attribute including the leading dot
# (the regex below requires it for word-boundary safety) and maps to a
# ``(hint, migration_anchor)`` tuple. ``migration_anchor`` may be
# ``None`` for entries without a dedicated migration-guide section.
REMOVED_ATTRIBUTE_ACCESSES: dict[str, tuple[str, str | None]] = {
".brand_manifest": (
"ResolvedBrand.brand_manifest removed — use .brand instead",
None,
),
".pending_activation": (
"MediaBuyStatus.pending_activation was split into "
"`pending_start` (schedule-future buys) and `pending_creatives` "
"(buys waiting on creative approval). The correct replacement "
"is per-call-site — review each usage manually.",
"mediabuystatuspending_activation--split",
),
}


Expand Down Expand Up @@ -386,7 +401,7 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str |
# Removed attribute accesses (.brand_manifest etc.). Regex with
# trailing word boundary prevents false-positives on
# ``.brand_manifest_v2``, ``.brand_manifest_override``, etc.
for attr, hint in REMOVED_ATTRIBUTE_ACCESSES.items():
for attr, (attr_hint, attr_anchor) in REMOVED_ATTRIBUTE_ACCESSES.items():
for match in _REMOVED_ATTRIBUTE_PATTERNS[attr].finditer(line):
findings.append(
Finding(
Expand All @@ -395,7 +410,8 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str |
line=lineno,
column=match.start() + 1,
before=attr,
hint=hint,
hint=attr_hint,
migration_anchor=attr_anchor,
)
)

Expand Down
43 changes: 43 additions & 0 deletions tests/test_migrate_v3_to_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,49 @@ def test_flags_removed_attribute_accesses(tmp_path: Path) -> None:
assert attr[0].before == ".brand_manifest"


def test_flags_pending_activation_split(tmp_path: Path) -> None:
"""``MediaBuyStatus.pending_activation`` was split into
``pending_start`` and ``pending_creatives``. Flag every reference
with the per-call-site decision hint and the migration anchor."""
_write(
tmp_path,
"code.py",
"if media_buy.status == MediaBuyStatus.pending_activation:\n" " notify(media_buy)\n",
)

report = v3_to_v4.run(tmp_path, apply_changes=False)

attr = [
f
for f in report.flagged
if f.kind == "flag_attribute" and f.before == ".pending_activation"
]
assert len(attr) == 1
assert "pending_start" in (attr[0].hint or "")
assert "pending_creatives" in (attr[0].hint or "")
assert attr[0].migration_anchor == "mediabuystatuspending_activation--split"


def test_pending_activation_word_boundary_no_false_positive(tmp_path: Path) -> None:
"""``.pending_activation_v2`` / ``.pending_activation_count`` are
adopter extensions that share the prefix; the trailing word
boundary keeps them out of the flagged set."""
_write(
tmp_path,
"code.py",
"x = stats.pending_activation_count\n" "y = obj.pending_activation_v2 = True\n",
)

report = v3_to_v4.run(tmp_path, apply_changes=False)

flagged = [
f
for f in report.flagged
if f.kind == "flag_attribute" and f.before == ".pending_activation"
]
assert flagged == [], f"false-positive on pending_activation_* suffixes: {flagged}"


def test_brand_manifest_word_boundary_no_false_positive(tmp_path: Path) -> None:
"""``.brand_manifest_v2`` / ``.brand_manifest_override`` are
seller-specific extensions that happen to share a prefix. They
Expand Down
Loading