Skip to content
Merged
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
24 changes: 22 additions & 2 deletions direct_cli/commands/ads.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ def ads():
@click.option("--fetch-all", is_flag=True, help="Fetch all pages")
@click.option("--format", "output_format", default="json", help="Output format")
@click.option("--output", help="Output file")
@click.option("--fields", help="Comma-separated field names")
@click.option("--fields", help="Comma-separated top-level field names")
@click.option(
"--text-ad-fields", help="Comma-separated TextAd field names (e.g. Title,Text,Href)"
)
@click.option("--dry-run", is_flag=True, help="Show request without sending")
@click.pass_context
def get(
ctx,
Expand All @@ -37,6 +41,8 @@ def get(
output_format,
output,
fields,
text_ad_fields,
dry_run,
):
"""Get ads"""
try:
Expand All @@ -52,6 +58,12 @@ def get(
else ["Id", "CampaignId", "AdGroupId", "Status", "State", "Type"]
)

text_ad_field_names = (
text_ad_fields.split(",")
if text_ad_fields
else ["Title", "Title2", "Text", "Href"]
)

criteria = {}
if ids:
criteria["Ids"] = parse_ids(ids)
Expand All @@ -62,13 +74,21 @@ def get(
if status:
criteria["Statuses"] = [status]

params = {"SelectionCriteria": criteria, "FieldNames": field_names}
params = {
"SelectionCriteria": criteria,
"FieldNames": field_names,
"TextAdFieldNames": text_ad_field_names,
}

if limit:
params["Page"] = {"Limit": limit}

body = {"method": "get", "params": params}

if dry_run:
format_output(body, "json", None)
return
Comment on lines +88 to +90

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--dry-run is checked after create_client(...). If the user runs direct ads get --dry-run without YANDEX_DIRECT_TOKEN, create_client() will raise (token required) and the dry-run path won't work. Move client creation (and any token resolution) to after the dry_run early-return so dry-run can run without credentials, consistent with ads add/update.

Copilot uses AI. Check for mistakes.

result = client.ads().post(data=body)

if fetch_all:
Expand Down
2 changes: 1 addition & 1 deletion direct_cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def parse_date(date_str: str) -> str:
"ClientInfo",
],
"adgroups": ["Id", "Name", "CampaignId", "Status", "Type", "RegionIds"],
"ads": ["Id", "CampaignId", "AdGroupId", "Status", "State", "Type"],
"ads": ["Id", "CampaignId", "AdGroupId", "Status", "State", "Type", "TextAd"],

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COMMON_FIELDS["ads"] now includes "TextAd", but ads.get (and the new dry-run tests) treat FieldNames as top-level ad fields and do not include TextAd. Keeping "TextAd" here makes the default fields inconsistent and risks sending an invalid FieldNames value if get_default_fields("ads") is used later. Consider removing "TextAd" from COMMON_FIELDS["ads"] and keeping type-specific fields only in TextAdFieldNames.

Suggested change
"ads": ["Id", "CampaignId", "AdGroupId", "Status", "State", "Type", "TextAd"],
"ads": ["Id", "CampaignId", "AdGroupId", "Status", "State", "Type"],

Copilot uses AI. Check for mistakes.
"keywords": [
"Id",
"Keyword",
Expand Down
44 changes: 44 additions & 0 deletions tests/test_dry_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,50 @@ def test_ads_update_extra_json_merges_into_payload():
assert ad["TextAd"] == {"Title": "Updated"}


def test_ads_get_default_fieldnames():
"""Default FieldNames includes basic top-level fields, plus TextAdFieldNames."""
body = _dry_run("ads", "get", "--campaign-ids", "12345")
assert body["method"] == "get"
assert body["params"]["FieldNames"] == [
"Id",
"CampaignId",
"AdGroupId",
"Status",
"State",
"Type",
]
assert body["params"]["TextAdFieldNames"] == ["Title", "Title2", "Text", "Href"]


def test_ads_get_with_fields_overrides_defaults():
"""--fields and --text-ad-fields override the defaults."""
body = _dry_run(
"ads",
"get",
"--campaign-ids",
"12345",
"--fields",
"Id,State",
"--text-ad-fields",
"Title",
)
assert body["params"]["FieldNames"] == ["Id", "State"]
assert body["params"]["TextAdFieldNames"] == ["Title"]


def test_ads_get_with_ids_and_status():
"""Multiple selection criteria are combined correctly."""
body = _dry_run(
"ads", "get", "--ids", "1,2,3", "--status", "ACCEPTED", "--limit", "10"
)
assert body["params"]["SelectionCriteria"] == {
"Ids": [1, 2, 3],
"Statuses": ["ACCEPTED"],
}
assert body["params"]["Page"] == {"Limit": 10}
assert "TextAdFieldNames" in body["params"]


# ----------------------------------------------------------------------
# adgroups
# ----------------------------------------------------------------------
Expand Down
29 changes: 29 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,35 @@ def test_get_ads(self):
)
assert_success(result, "ads get")

def test_get_ads_returns_textad(self):
"""TEXT_AD ads must include TextAd with Title and Text."""
if not self.campaign_id:
self.skipTest("No campaigns found in account")
result = invoke_get(
"ads",
"get",
"--campaign-ids",
str(self.campaign_id),
"--limit",
"50",
"--format",
"json",
)
assert_success(result, "ads get (TextAd check)")
data = json.loads(result.output)
text_ads = [ad for ad in data if ad.get("Type") == "TEXT_AD"]
if not text_ads:
self.skipTest("No TEXT_AD ads found in first 50 results")
for ad in text_ads:
self.assertIn(
"TextAd",
ad,
f"TEXT_AD {ad['Id']} missing TextAd — "
"TextAdFieldNames may not be sent",
)
self.assertIn("Title", ad["TextAd"])
self.assertIn("Text", ad["TextAd"])


@pytest.mark.integration
@skip_if_no_token
Expand Down
Loading