search

googleads / google-ads-php

Google Ads API Client Library for PHP
https://developers.google.com/google-ads/api/docs/client-libs/php
Apache License 2.0
288 stars 260 forks source link

ResourceNames::forTopicConstant is removed in v14 #946

Closed minaTfn closed 11 months ago

minaTfn commented 1 year ago

Hi there, I recently upgraded my google ads API PHP package from v13 to v14.1. Right after that I got this error while creating a ResponsiveSearchAd. Call to undefined method Google\Ads\GoogleAds\Util\V14\ResourceNames::forTopicConstant()

The error is because I’m using this piece of code:

new TopicInfo([
                'topic_constant' => Google\Ads\GoogleAds\Util\V14\ResourceNames::forTopicConstant($topic_id),
            ])

The reason for the error is that forTopicConstant method was removed from version 14.

Since it was a breaking change for me, my questions are: why isn’t this listed in the v13 to v14 changes documentation? How can I prevent such a problem in the future?

Thanks in advance for the response.

fiboknacky commented 1 year ago

What's your error message?

minaTfn commented 1 year ago

What's your error message?

Call to undefined method Google\Ads\GoogleAds\Util\V14\ResourceNames::forTopicConstant()

fiboknacky commented 1 year ago

What client library version are you using?

minaTfn commented 1 year ago

What client library version are you using?

"googleads/google-ads-php": "20.1.0"

minaTfn commented 1 year ago

What client library version are you using?

"googleads/google-ads-php": "20.1.0"

fiboknacky commented 1 year ago

Hello,

Since it was a breaking change for me, my questions are: why isn’t this listed in the v13 to v14 changes documentation? How can I prevent such a problem in the future?

The API actually doesn't change anything about the topic_constant, so there is no unintended breaking change here. Upon checking, it seems the ResourceNames utility was wrongly generated by our automation tool. I'm investigating the tool and will release a patch once I find a solution.

Sorry for your inconvenience.

fiboknacky commented 12 months ago

@minaTfn

By the way, could you share a bit more details at what step did you need to use forTopicConstant to create a responsive search ad?

minaTfn commented 11 months ago

@minaTfn

By the way, could you share a bit more details at what step did you need to use forTopicConstant to create a responsive search ad?

I was using ResourceNames::forTopicConstant to get the Topic Constant resource name to create topic criteria for adgroup. AdGroupCriterion -> TopicInfo -> topic_constant

fiboknacky commented 11 months ago

Thanks a lot. I found that the definition of TopicInfo in the proto definition don't contain the annotation needed for generating the name templates, used in the ResourceNames util. It should have been like this:

optional string topic_constant = 1 [(google.api.resource_reference).type = "[googleads.googleapis.com/TopicConstant](http://googleads.googleapis.com/TopicConstant)"]

This may be due to the fact that some messages (like TopicInfo) were added a long time ago before we adopted the recent styles.

For now, I'll work on restoring the method for this resource. In the long term, I'll coordinate with the Google Ads API team to properly add the annotation.

fiboknacky commented 11 months ago

Fixed in https://github.com/googleads/google-ads-php/releases/tag/v21.0.1.