Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

695 error handling if add ad_group/keyword creation failed while being created in the create_ad_group_with_ad_and_keywords #709

Merged
136 changes: 98 additions & 38 deletions captn/captn_agents/backend/tools/_campaign_creation_team_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from pydantic import BaseModel, Field, StringConstraints
from typing_extensions import Annotated

from google_ads.model import RemoveResource

from ....google_ads.client import check_for_client_approval, google_ads_create_update
from ..toolboxes import Toolbox
from ._functions import (
Expand Down Expand Up @@ -96,7 +98,7 @@ class AdGroupWithAdAndKeywords(BaseModel):


def _get_resource_id_from_response(response: str) -> str:
return response.split("/")[-1].replace(".", "")
return response.split("/")[-1].split("~")[-1].replace(".", "").strip()


def _create_ad_group(
Expand Down Expand Up @@ -178,6 +180,7 @@ def _create_ad_group_keywords(
],
ad_group_with_ad_and_keywords: AdGroupWithAdAndKeywords,
ad_group_id: str,
list_of_resources: List[RemoveResource],
) -> Union[Dict[str, Any], str]:
response = ""
for keyword in ad_group_with_ad_and_keywords.keywords:
Expand All @@ -189,10 +192,101 @@ def _create_ad_group_keywords(
ad_group_id=ad_group_id,
customer_id=ad_group_with_ad_and_keywords.customer_id,
)
keyword_id = _get_resource_id_from_response(keyword_response) # type: ignore[arg-type]
list_of_resources.append(
RemoveResource(
customer_id=ad_group_with_ad_and_keywords.customer_id,
parent_id=ad_group_id,
resource_id=keyword_id,
resource_type="ad_group_criterion",
)
)
response += f"Keyword '{keyword.keyword_text}': {keyword_response}\n"
return response


def _remove_resources(
user_id: int, conv_id: int, list_of_resources: List[RemoveResource]
) -> None:
for remove_resource in list_of_resources[::-1]:
remove_response = google_ads_create_update(
user_id=user_id,
conv_id=conv_id,
recommended_modifications_and_answer_list=[],
ad=remove_resource,
endpoint="/remove-google-ads-resource",
already_checked_clients_approval=True,
)
print(remove_response)
print("Resources removed successfully")


def _create_ad_group_with_ad_and_keywords(
ad_group_with_ad_and_keywords: Annotated[
AdGroupWithAdAndKeywords, "An ad group with an ad and a list of keywords"
],
context: Context,
) -> Union[Dict[str, Any], str]:
list_of_resources = []
try:
ad_group_response = _create_ad_group(
user_id=context.user_id,
conv_id=context.conv_id,
recommended_modifications_and_answer_list=context.recommended_modifications_and_answer_list,
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
)
if isinstance(ad_group_response, dict):
return ad_group_response
response = f"Ad group '{ad_group_with_ad_and_keywords.ad_group.name}': {ad_group_response}\n"
ad_group_id = _get_resource_id_from_response(ad_group_response)
list_of_resources.append(
RemoveResource(
customer_id=ad_group_with_ad_and_keywords.customer_id,
resource_id=ad_group_id,
resource_type="ad_group",
)
)

ad_group_ad_response = _create_ad_group_ad(
user_id=context.user_id,
conv_id=context.conv_id,
recommended_modifications_and_answer_list=context.recommended_modifications_and_answer_list,
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
ad_group_id=ad_group_id,
)
ad_group_ad_id = _get_resource_id_from_response(ad_group_ad_response) # type: ignore[arg-type]
list_of_resources.append(
RemoveResource(
customer_id=ad_group_with_ad_and_keywords.customer_id,
parent_id=ad_group_id,
resource_id=ad_group_ad_id,
resource_type="ad",
)
)

response += f"Ad group ad with final url - '{ad_group_with_ad_and_keywords.ad_group_ad.final_url}': {ad_group_ad_response}\n"

ad_group_keywords_response = _create_ad_group_keywords(
user_id=context.user_id,
conv_id=context.conv_id,
recommended_modifications_and_answer_list=context.recommended_modifications_and_answer_list,
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
ad_group_id=ad_group_id,
list_of_resources=list_of_resources,
)
response += ad_group_keywords_response # type: ignore
context.changes_made += f"\n{response}"
except Exception as e:
_remove_resources(
user_id=context.user_id,
conv_id=context.conv_id,
list_of_resources=list_of_resources,
)
raise e

return response


def create_campaign_creation_team_toolbox(
user_id: int,
conv_id: int,
Expand Down Expand Up @@ -230,55 +324,21 @@ def create_ad_group_with_ad_and_keywords(
"""
# TODO: If any of the creation fails, we should rollback the changes
# and return the error message (delete ad_group, other resources will be deleted automatically)

user_id = context.user_id
conv_id = context.conv_id
recommended_modifications_and_answer_list = (
context.recommended_modifications_and_answer_list
)

modification_function_parameters = {}
modification_function_parameters["ad_group_with_ad_and_keywords"] = (
ad_group_with_ad_and_keywords.model_dump()
)
error_msg = check_for_client_approval(
modification_function_parameters=modification_function_parameters,
recommended_modifications_and_answer_list=recommended_modifications_and_answer_list,
recommended_modifications_and_answer_list=context.recommended_modifications_and_answer_list,
)
if error_msg:
raise ValueError(error_msg)

ad_group_response = _create_ad_group(
user_id=user_id,
conv_id=conv_id,
recommended_modifications_and_answer_list=recommended_modifications_and_answer_list,
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
)
if isinstance(ad_group_response, dict):
return ad_group_response
response = f"Ad group '{ad_group_with_ad_and_keywords.ad_group.name}': {ad_group_response}\n"
ad_group_id = _get_resource_id_from_response(ad_group_response)

ad_group_ad_response = _create_ad_group_ad(
user_id=user_id,
conv_id=conv_id,
recommended_modifications_and_answer_list=recommended_modifications_and_answer_list,
return _create_ad_group_with_ad_and_keywords(
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
ad_group_id=ad_group_id,
context=context,
)
response += f"Ad group ad with final url - '{ad_group_with_ad_and_keywords.ad_group_ad.final_url}': {ad_group_ad_response}\n"

ad_group_keywords_response = _create_ad_group_keywords(
user_id=user_id,
conv_id=conv_id,
recommended_modifications_and_answer_list=recommended_modifications_and_answer_list,
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
ad_group_id=ad_group_id,
)
response += ad_group_keywords_response # type: ignore
context.changes_made += f"\n{response}"

return response

add_shared_functions(toolbox)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
AdGroupCriterionForCreation,
AdGroupForCreation,
AdGroupWithAdAndKeywords,
_get_resource_id_from_response,
_remove_resources,
create_campaign_creation_team_toolbox,
)
from captn.captn_agents.backend.tools._functions import Context
from captn.google_ads.client import clean_nones
from google_ads.model import RemoveResource

from .helpers import check_llm_config_descriptions, check_llm_config_total_tools

Expand Down Expand Up @@ -52,7 +55,38 @@ def test_llm_config(self) -> None:
},
)

def test_create_ad_group_with_ad_and_keywords(self) -> None:
@pytest.mark.parametrize(
"response",
[
"Created customers/1212/adGroups/3434.",
"Created customers/7119828439/adGroupCriteria/1212~3434.",
],
)
def test_get_resource_id_from_response(self, response) -> None:
resource_id = _get_resource_id_from_response(response)
assert resource_id == "3434", resource_id

@pytest.mark.parametrize(
"side_effect",
[
[
"Created customers/1212/adGroups/3434.",
"Created customers/1212/adGroupAds/3434~5656.",
"Created customers/1212/adGroupCriteria/3434~7878.",
"Created customers/1212/adGroupCriteria/3434~8989.",
],
[
"Created customers/1212/adGroups/3434.",
"Created customers/1212/adGroupAds/3434~5656.",
"Created customers/1212/adGroupCriteria/3434~7878.",
ValueError("Error: 400: The keyword text 'keyword1' is invalid."),
"Removed customers/1212/adGroupCriteria/3434~7878.",
"Removed customers/1212/adGroupAds/3434~5656.",
"Removed customers/1212/adGroups/3434.",
],
],
)
def test_create_ad_group_with_ad_and_keywords(self, side_effect) -> None:
ad_group_ad = AdGroupAdForCreation(
final_url="https://www.example.com",
headlines=["headline1", "headline2", "headline3"],
Expand Down Expand Up @@ -86,17 +120,16 @@ def test_create_ad_group_with_ad_and_keywords(self) -> None:
keywords=[keyword1, keyword2],
)

with unittest.mock.patch(
"captn.captn_agents.backend.tools._campaign_creation_team_tools.google_ads_create_update"
) as mock_google_ads_create_update:
side_effect = [
"Created customers/1212/adGroups/3434.",
"Created customers/1212/adGroupAds/3434~5656.",
"Created customers/1212/adGroupCriteria/3434~7878.",
"Created customers/1212/adGroupCriteria/3434~8989.",
]
mock_google_ads_create_update.side_effect = side_effect

with (
unittest.mock.patch(
"captn.captn_agents.backend.tools._campaign_creation_team_tools.google_ads_create_update",
side_effect=side_effect,
) as mock_google_ads_create_update,
unittest.mock.patch(
"captn.captn_agents.backend.tools._campaign_creation_team_tools._remove_resources",
wraps=_remove_resources,
) as mock_remove_resources,
):
modification_function_params = clean_nones(
{
"ad_group_with_ad_and_keywords": ad_group_with_ad_and_keywords.model_dump()
Expand All @@ -114,23 +147,54 @@ def test_create_ad_group_with_ad_and_keywords(self) -> None:
"create_ad_group_with_ad_and_keywords"
)

response = create_ad_group_with_ad_and_keywords(
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
context=context,
)

expected_response = f"""Ad group '{ad_group.name}': {side_effect[0]}
if isinstance(side_effect[3], ValueError):
with pytest.raises(ValueError):
create_ad_group_with_ad_and_keywords(
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
context=context,
)

# 4 create calls (the last one failed) + 3 remove calls
assert mock_google_ads_create_update.call_count == 7
mock_remove_resources.assert_called_once_with(
user_id=1,
conv_id=1,
list_of_resources=[
RemoveResource(
customer_id="1111",
resource_id="3434",
resource_type="ad_group",
),
RemoveResource(
customer_id="1111",
parent_id="3434",
resource_id="5656",
resource_type="ad",
),
RemoveResource(
customer_id="1111",
parent_id="3434",
resource_id="7878",
resource_type="ad_group_criterion",
),
],
)

else:
response = create_ad_group_with_ad_and_keywords(
ad_group_with_ad_and_keywords=ad_group_with_ad_and_keywords,
context=context,
)

expected_response = f"""Ad group '{ad_group.name}': {side_effect[0]}
Ad group ad with final url - '{ad_group_ad.final_url}': {side_effect[1]}
Keyword '{keyword1.keyword_text}': {side_effect[2]}
Keyword '{keyword2.keyword_text}': {side_effect[3]}
"""

print()
print(f"{response=}")
print(f"{expected_response=}")

assert mock_google_ads_create_update.call_count == 4
assert response == expected_response, response
assert mock_google_ads_create_update.call_count == 4
assert response == expected_response, response
mock_remove_resources.assert_not_called()


class TestContext:
Expand Down
Loading