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

ad param to setTargetingForAst() #1357

Merged
merged 5 commits into from
Jun 25, 2019
Merged

Conversation

sumit116
Copy link
Contributor

@sumit116 sumit116 commented Jun 19, 2019

Issue:
Currently when someone calls pbjs.setTargetingForAst(), it automatically pulls all the adUnits. You cannot specifically ask to set the targeting for certain adUnits, like we can for the pbjs.setTargetingForGPTAsync(). We need to add the ability to choose specific adUnits to the setTargetingForAst() call.

Solution Added:
Allow user to pass an array of adUnitCodes to the setTargetingForAst() function so that it only provides the targeting data for those listed adUnits.
Maintain support that calling setTargetingForAst() without any values pulls the targeting data for all adUnits.

@bretg bretg requested review from jeanstemp and removed request for bretg June 20, 2019 01:02
@bretg
Copy link
Contributor

bretg commented Jun 20, 2019

@jeanstemp - I think the wording here is a little awkward... can you smith it a bit?

{: .table .table-bordered .table-striped }
| Param | Scope | Type | Description |
| --- | --- | --- | -- |
| [adUnits] | Optional | `array` | an array of adUnitCodes to set targeting for. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually an array of strings containing adUnits.code values, right? If so it should look more like http://prebid.org/dev-docs/publisher-api-reference.html#module_pbjs.removeAdUnit

Type should be array of strings. I'd change the description to be a little clearer: "Codes of the adUnits for which targeting is being set. Omitting this parameter will set targeting on all adUnits."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jeanstemp for your feedback. I have updated type as String or Array of strings as the function can take a single adUnitCode string also as an argument similar to module_pbjs.removeAdUnit.

@bretg bretg merged commit 2a05fb8 into master Jun 25, 2019
smenzer added a commit to smenzer-forks/prebid.github.io that referenced this pull request Jun 26, 2019
* 'master' of github.com:prebid/prebid.github.io: (28 commits)
  adding mediavine as pub member (prebid#1371)
  SafeReach Adapter (prebid#1237)
  Add docs for /info/bidders/all endpoint (prebid#1297)
  Updated documentation for video adapter support. (prebid#1358)
  ID5 ID user id module docs (prebid#1363)
  Add resize function to iOS integration (prebid#1352)
  adding COPPA support docs, feature table (prebid#1367)
  added digitrust to rubicon (prebid#1369)
  Digitrust docs (prebid#1298)
  Added bidglass dev-doc (prebid#1328)
  Add staq analytics to downloads page (prebid#1364)
  Update PubMatic information about mutli-size ad units and Prebid Server (prebid#1362)
  ad param to setTargetingForAst() (prebid#1357)
  removing jsdelivr example for load-cookie (prebid#1368)
  fixed link
  Pbjs releases update (prebid#1360)
  Adding Scaleable Analytics to download/overview markdown files (prebid#1322)
  removing references to ozoneData (prebid#1356)
  Update undertone.md (prebid#1355)
  London event blog (prebid#1359)
  ...
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.

4 participants