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

Fix browsiImpression callback. #8

Merged
merged 5 commits into from
Nov 8, 2021
Merged

Conversation

from20020516
Copy link

@from20020516 from20020516 commented Nov 4, 2021

  • browsi仕様変更に伴う修正
  • テストケース追加
  • リファクタ

@from20020516 from20020516 self-assigned this Nov 4, 2021
Comment on lines 333 to +339
window.addEventListener('browsiImpression', (data) => {
const auction = find(Object.values(cache.auctions), auction => auction.adUnitCodes.includes(data.detail.adUnit.code))
const adUnitCode = Object.entries(getAdUnitMap())
.reduce((prev, [code, path]) => {
return data.detail.adUnit === path && isBrowsiDivId(code)
? code
: prev
}, '')
Copy link
Author

Choose a reason for hiding this comment

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

data.detail.adUnit に adUnitPath が渡ってくるようになったため adUnitCodeに変換する

Copy link

Choose a reason for hiding this comment

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

なるほど、ここが以前と仕様が違った部分の対応

@@ -308,7 +306,7 @@ const sendMessage = (auctionId) => {
status,
adId,
adUrl,
adUnitCode,
adUnitCode: getAdUnitCodeBeforeReplication(slots, adUnitCode),
Copy link
Author

Choose a reason for hiding this comment

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

  • bids[].adUnitCode にbrowsi由来の枠code browsi_ が含まれるとき、それを元の枠code div-gpt- に置き換える
  • bids[].bid.adUnitCode はそのままの枠code browsi_ が残る

Copy link

Choose a reason for hiding this comment

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

DATA STRAP側との兼ね合いで元の枠codeの方が都合がいい感じです?

Copy link
Author

Choose a reason for hiding this comment

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

ですです。

Comment on lines 279 to 283
adUnits = adUnits.map(adUnit => ({
...adUnit,
analytics: adUnit.analytics ?? find(pbjs.adUnits, _adUnit => _adUnit.code === getAdUnitCodeBeforeReplication(slots, adUnit.code)).analytics,
bids: undefined
}))
Copy link
Author

Choose a reason for hiding this comment

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

Object構造を修正

@saxsir
Copy link

saxsir commented Nov 5, 2021

@from20020516 手元で回したテストの結果を貼っていただけると〜

@from20020516
Copy link
Author

@saxsir #6 で触ってたtestcase修正もこちらにまとめました:

  • OK: gulp test --nolint
  • OK: gulp test --nolint --file ./test/spec/modules/fluctAnalyticsAdapter_spec.js

Comment on lines -211 to +215
}, pbjs.getConfig().bidderTimeout || 3000);
}, config.getConfig('bidderTimeout') ?? 3000);
Copy link
Author

Choose a reason for hiding this comment

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

  • config.getConfig: pbjs.getConfig と等価、testcase実行の都合で変更
  • ??: 0を有効にする

Comment on lines +469 to +470
events.emit(AUCTION_END, MOCK.AUCTION_END)
expect(server.requests.length).to.equal(1)
Copy link
Author

Choose a reason for hiding this comment

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

request(sendMessage)を1回実行している

events.emit(AUCTION_INIT, MOCK.AUCTION_INIT)
events.emit(AUCTION_END, MOCK.AUCTION_END)
expect(server.requests.length).to.equal(1)
expect(JSON.parse(server.requests[0].requestBody).auctionId).to.equal(MOCK.AUCTION_END.auctionId)
Copy link
Author

@from20020516 from20020516 Nov 5, 2021

Choose a reason for hiding this comment

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

sendMessageしたrequest bodyを検証

/**
* @returns {string|undefined}
*/
const getSiteKey = () => find(config.getConfig('realTimeData.dataProviders') ?? [], provider => provider.name === 'browsi')?.params.siteKey
Copy link
Author

@from20020516 from20020516 Nov 5, 2021

Choose a reason for hiding this comment

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

browsi関係のケースをケアするためテストに以下のように記述すると、test/spec/modules/realTimeDataModule_spec.js が落ちる...

TODO: 工夫が必要そう

config.setConfig({
  realTimeData: {
    'dataProviders': [
      {
        'name': 'browsi',
        'params': {
          'keyName': 'browsiViewability',
          'pubKey': 'PUBKEY',
          'siteKey': 'SITEKEY',
          'url': 'URL'
        }
      }
    ]
  }
})


adUnits = adUnits.map(adUnit => ({
...adUnit,
analytics: adUnit.analytics ?? find(Object.values(cache.auctions).flatMap(auction => auction.adUnits), _adUnit => _adUnit.code === getAdUnitCodeBeforeReplication(slots, adUnit.code)).analytics,
Copy link
Author

@from20020516 from20020516 Nov 5, 2021

Choose a reason for hiding this comment

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

getConfig の変更とセットで window.pbjsFluct 由来の変数使用を廃止

  • pbjs.adUnits を cacheしている各auctionsが持つadUnitsに変更

Copy link

@toshi17 toshi17 Nov 8, 2021

Choose a reason for hiding this comment

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

ajiyoshi json由来のanalyticsが存在する場合はそれを使用し、存在しない場合にはadUnitPath経由で共通の枠を探して、そのanalytics情報を使用する

Copy link

@toshi17 toshi17 left a comment

Choose a reason for hiding this comment

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

手元でテストが通っているならLGTMです〜

@from20020516 from20020516 merged commit 8b37962 into bid-strap Nov 8, 2021
@from20020516 from20020516 deleted the browsi-impression branch November 8, 2021 04:24
toshi17 pushed a commit that referenced this pull request Mar 17, 2022
* feature: add Hubvisor richmedia adapter

* feature(hubvisor-bid-adapter): fix lint error

* feature: Replay support for Hubvisor richmedia adapter

* feature: do not need size 1800x1000 for skin

* feature: rename hbvRichmediaAdapter to bigRichmediaAdapter (#7)

* feature: add tests and documentation (#8)

* Richmedia adapter : rename files (#9)

Co-authored-by: Julie <[email protected]>
Co-authored-by: JulieLorin <[email protected]>
saxsir pushed a commit that referenced this pull request Nov 10, 2022
…prebid#9158)

* collect EIDs for bid request

* add ad slot positioning to payload

* RPO-2012: Update local storage name-spacing for c_uid (#8)

* Updates c_uid namespacing to be more specific for concert

* fixes unit tests

* remove console.log

* RPO-2012: Add check for shared id (#9)

* Adds check for sharedId

* Updates cookie name

* remove trailing comma

Co-authored-by: antoin <[email protected]>
Co-authored-by: Antoin <[email protected]>
saito-hir pushed a commit that referenced this pull request Jan 31, 2023
* adding ccpa support for emx_digital adapter

* emx_digital ccpa compliance: lint fix

* emx 3.0 compliance update

* fix outstream renderer issue, update test spec

* refactor formatVideoResponse function to use core-js/find

* Add support for schain forwarding

* Resolved issue with Schain object location

* prebid 5.0 floor module and advertiserDomain support

* liveramp idl and uid2.0 support for prebid

* gpid support

* remove utils ext

* remove empty line

* remove trailing spaces

* move gpid test module

* move gpid test module

* removing trailing spaces from unit test

* remove comments from unit test

* Include us_privacy string in redirects (#8)

* include us_privacy string in redirects

* added test cases for us_privacy and gdpr

* added test cases for  gdpr without usp

* updated test case when no privacy strings and fixed package-lock.json

* revert package-lock.json

Co-authored-by: EMXDigital <[email protected]>

* kick off ci tests

Co-authored-by: Nick Colletti <[email protected]>
Co-authored-by: Nick Colletti <[email protected]>
Co-authored-by: Kiyoshi Hara <[email protected]>
Co-authored-by: Dan Bogdan <[email protected]>
Co-authored-by: Jherez Taylor <[email protected]>
Co-authored-by: EMXDigital <[email protected]>
Co-authored-by: Rakesh Balakrishnan <[email protected]>
Co-authored-by: Kevin <[email protected]>
Co-authored-by: Chris Huie <[email protected]>
toshi17 pushed a commit that referenced this pull request Jul 3, 2023
…ync (prebid#9700)

* collect EIDs for bid request

* add ad slot positioning to payload

* RPO-2012: Update local storage name-spacing for c_uid (#8)

* Updates c_uid namespacing to be more specific for concert

* fixes unit tests

* remove console.log

* RPO-2012: Add check for shared id (#9)

* Adds check for sharedId

* Updates cookie name

* remove trailing comma

* [RPO-3152] Enable Support for GPP Consent (#12)

* Adds gpp consent integration to concert bid adapter

* Update tests to check for gpp consent string param

* removes user sync endpoint and tests

* updates comment

* cleans up consentAllowsPpid function

* comment fix

* rename variables for clarity

* fixes conditional logic for consent allows function (#13)

---------

Co-authored-by: antoin <[email protected]>
Co-authored-by: Antoin <[email protected]>
toshi17 pushed a commit that referenced this pull request Jul 3, 2023
…populate imp-level `ext.tid` (prebid#9726)

* RTBHouse Bid Adapter: add global vendor list id

* structured user agent - browsers.brands

* fix lint errors

* Added sda into rtbhouse adapter

* spreading ortb2: user & site props

* examples reverted

* init version

* using mergedeep

* removed wrong imp array augm.; slot imp augm. with addtl check

* [SUA] merging ortb2.device into request

* fledge auctionConfig adapted to our bid response structure

* new bidder response structure for fledge

* make sure bidderRequest has proper flag turned on

* fledge endpoint hardcoded; code cleanups

* remove obsolete function

* obsolete function removed

* [RTB House] Process FLEDGE request/response (#4)

* [SDA & SUA] refactor using mergedeep

* [FLEDGE] fledge auctionConfig adapted to our bid response structure

* [FLEDGE] new bidder response structure for fledge

* [FLEDGE] make sure bidderRequest has proper flag turned on

* [FLEDGE] fledge endpoint hardcoded; code cleanups

* [FLEDGE] remove obsolete functions

* fixed lint errors

* fledge test suites; adapter: delete imp.ext.ae when no fledge (#5)

* RTBHouse Bid Adapter: use auctionId for source.tid

* RTBHouse bid adapter: fixed source.tid tests

* Imp level transaction id + mapSource fix

* lint: removed obsolete whitespaces

* RTBHouse Bid Adapter: change `source.tid` to contain `auctionId` and populate imp-level `ext.tid` (#8)

* RTBHouse Bid Adapter: use auctionId for source.tid

* Imp level transaction id + mapSource fix

* lint: removed obsolete whitespaces

---------

Co-authored-by: Leandro Otani <[email protected]>
Co-authored-by: rtbh-lotani <[email protected]>
Co-authored-by: Tomasz Swirski <[email protected]>
toshi17 pushed a commit that referenced this pull request Jul 3, 2023
)

* collect EIDs for bid request

* add ad slot positioning to payload

* RPO-2012: Update local storage name-spacing for c_uid (#8)

* Updates c_uid namespacing to be more specific for concert

* fixes unit tests

* remove console.log

* RPO-2012: Add check for shared id (#9)

* Adds check for sharedId

* Updates cookie name

* remove trailing comma

* [RPO-3152] Enable Support for GPP Consent (#12)

* Adds gpp consent integration to concert bid adapter

* Update tests to check for gpp consent string param

* removes user sync endpoint and tests

* updates comment

* cleans up consentAllowsPpid function

* comment fix

* rename variables for clarity

* fixes conditional logic for consent allows function (#13)

* [RPO-3262] Update getUid function to check for pubcid and sharedid (#14)

* Update getUid function to check for pubcid and sharedid

* updates adapter version

---------

Co-authored-by: antoin <[email protected]>
Co-authored-by: Antoin <[email protected]>
yowcow pushed a commit that referenced this pull request Aug 23, 2023
…ebid#10356)

* collect EIDs for bid request

* add ad slot positioning to payload

* RPO-2012: Update local storage name-spacing for c_uid (#8)

* Updates c_uid namespacing to be more specific for concert

* fixes unit tests

* remove console.log

* RPO-2012: Add check for shared id (#9)

* Adds check for sharedId

* Updates cookie name

* remove trailing comma

* [RPO-3152] Enable Support for GPP Consent (#12)

* Adds gpp consent integration to concert bid adapter

* Update tests to check for gpp consent string param

* removes user sync endpoint and tests

* updates comment

* cleans up consentAllowsPpid function

* comment fix

* rename variables for clarity

* fixes conditional logic for consent allows function (#13)

* [RPO-3262] Update getUid function to check for pubcid and sharedid (#14)

* Update getUid function to check for pubcid and sharedid

* updates adapter version

* [RPO-3405] Add browserLanguage to request meta object

---------

Co-authored-by: antoin <[email protected]>
Co-authored-by: Antoin <[email protected]>
Co-authored-by: Brett Bloxom <[email protected]>
himu62 pushed a commit that referenced this pull request Oct 16, 2023
* collect EIDs for bid request

* add ad slot positioning to payload

* RPO-2012: Update local storage name-spacing for c_uid (#8)

* Updates c_uid namespacing to be more specific for concert

* fixes unit tests

* remove console.log

* RPO-2012: Add check for shared id (#9)

* Adds check for sharedId

* Updates cookie name

* remove trailing comma

* [RPO-3152] Enable Support for GPP Consent (#12)

* Adds gpp consent integration to concert bid adapter

* Update tests to check for gpp consent string param

* removes user sync endpoint and tests

* updates comment

* cleans up consentAllowsPpid function

* comment fix

* rename variables for clarity

* fixes conditional logic for consent allows function (#13)

* [RPO-3262] Update getUid function to check for pubcid and sharedid (#14)

* Update getUid function to check for pubcid and sharedid

* updates adapter version

* [RPO-3405] Add browserLanguage to request meta object

* ConcertBidAdapter: Add TDID (#20)

* Add tdid to meta object

* Fix null handling and add tests

---------

Co-authored-by: antoin <[email protected]>
Co-authored-by: Antoin <[email protected]>
Co-authored-by: Brett Bloxom <[email protected]>
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.

3 participants