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

New bid adapter: Wipes #5051

Merged
merged 12 commits into from
Apr 8, 2020
Merged

New bid adapter: Wipes #5051

merged 12 commits into from
Apr 8, 2020

Conversation

Tosh39
Copy link
Contributor

@Tosh39 Tosh39 commented Mar 31, 2020

Type of change

  • New bidder adapter

Description of change

  • test parameters for validating bids
{
  bidder: 'wipes',
  params: {
    asid: 'XXXXXXXXXX'
  }
}
  • contact email of the adapter’s maintainer

contact@3-shake.com

  • official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

@bretg bretg changed the title Feature/add wipes New bid adapter: Wipes Mar 31, 2020
@bretg bretg requested a review from musikele March 31, 2020 16:05
Copy link
Contributor

@musikele musikele left a comment

Choose a reason for hiding this comment

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

Hello @Tosh39 ,
I have some questions for this bidder adapter.

  1. is it possible to get a test asid in order to test it with a valid response?
  2. Can you paste also a test configuration to test it? What are the right values for mediaType ?

Answers are not mandatory but they may help to test the bidder with a real example.

@Tosh39
Copy link
Contributor Author

Tosh39 commented Apr 3, 2020

@musikele
Thank you for checking my code.

I updated test parameter to make test easier.

@musikele
Copy link
Contributor

musikele commented Apr 3, 2020

Hello @Tosh39 ,
I still see a couple of issues.
For example, you adjusted the example data this way:

bids: [{
         bidder: 'wipes',
         params: {
           asid: 'dWyPondh2EGB_bNlrVjzIXRZO9F0k1dpo0I8ZvQ',
           mediaTypes: {
              video: {
                 context: 'outstream'
              }
           },
         }
       }]

but in my tests, to get bids, I had to do this:

mediaTypes: {
                video: {
                    context: 'outstream'
                }
            },
            bids: [{
                bidder: 'wipes',
                params: {
                    asid: 'dWyPondh2EGB_bNlrVjzIXRZO9F0k1dpo0I8ZvQ',
                }
            }]

Anyway, even if I get bids, I then see this error:

ERROR: Invalid bid from wipes. Ignoring bid: Video bid does not have required vastUrl or renderer property

I tried to add a renderer to the ad unit, without success.

Am I missing something?

(Also - the response requestId is different from the request. I know this is only a fake response but I had to tweak prebid code in order to validate it.)

@Tosh39
Copy link
Contributor Author

Tosh39 commented Apr 6, 2020

@musikele
Thank you for reviewing my code. I misunderstood about mediaType and our tool doesn't return video info, only html tag, so I changed mediaType to banner from video.
It's corrected now.

Could you check again please?

Copy link
Contributor

@musikele musikele left a comment

Choose a reason for hiding this comment

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

fix problems in interpretResponse

modules/wipesBidAdapter.js Outdated Show resolved Hide resolved
@Tosh39 Tosh39 requested a review from musikele April 6, 2020 14:16
Copy link
Contributor

@musikele musikele left a comment

Choose a reason for hiding this comment

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

In my tests I couldn't make an ad to show with integrationExamples/gpt/helloWorld.html test case. I see the bid is being requested, and that prebid chooses it, but I don't see it showing. It may be that I have to be in Japan (I've tried via a VPN) or with a specific referrer url, with no luck.

In hello_world.html here's what I've set to adUnits, row 18:

var adUnits = [{
            code: 'div-gpt-ad-1460505748561-0',
            mediaTypes: {
                banner: {
                    sizes: [[160, 300]],
                }
            },
            // Replace this object to test a new Adapter!
            bids: [{
                bidder: 'wipes',
                params: {
                    asid: 'dWyPondh2EGB_bNlrVjzIXRZO9F0k1dpo0I8ZvQ'
                }
            }]
        }];

If you think this is "normal" and that I should not see any ad, we can move this forward and approve the PR.

modules/wipesBidAdapter.js Outdated Show resolved Hide resolved
@Tosh39
Copy link
Contributor Author

Tosh39 commented Apr 8, 2020

@musikele
I guess that's because of network problem, so could you move forward please?

And I am sorry about adapter code. It's fixed.

@musikele musikele merged commit 5fab5e1 into prebid:master Apr 8, 2020
rjvelicaria pushed a commit to openx/Prebid.js that referenced this pull request Apr 9, 2020
* add wipes

* done update notes

* URL変更

* update test parameter

* fix mediaType to make it work

* fix spec error

* fix comment

* fix param names

* update fix

* erase unused code

Co-authored-by: 嘉数貴明 <yaki180sx@gmail.com>
Co-authored-by: yaki(kkz) <yaki_180sx@hotmail.com>
redaguermas added a commit to redaguermas/Prebid.js that referenced this pull request Apr 16, 2020
* 'master' of https://github.com/prebid/Prebid.js: (102 commits)
  Marsmedia - Add vastXml and fix id response (prebid#5067)
  PubMatic adapter to support image sync (prebid#5104)
  minor consentManagement fix (prebid#5050)
  fix circle ci failing tests (prebid#5113)
  Add Relaido Adapter (prebid#5101)
  Add new bid adapter for ConnectAd (prebid#4806)
  change payload (prebid#5105)
  Utils updates (prebid#5092)
  Read OpenRTB app objects if set in config + bug fix for when ad units are reloaded (prebid#5086)
  Criteo : added first party data mapping to bidder request (prebid#4954)
  updateAdGenerationManual (prebid#5032)
  New bid adapter: Wipes (prebid#5051)
  Prebid manager analytics utm tags (prebid#4998)
  CRITEO RTUS Integration with Yieldmo Prebid (prebid#5075)
  isSafariBrowser update  (prebid#5077)
  Support min &max duration for onevideo (prebid#5079)
  increment pre version
  Prebid 3.15.0 release
  prebid#5011 Fix to set Secure attribute on cookie when SameSite=none (prebid#5064)
  Prebid adapter for windtalker (prebid#5040)
  ...
@Tosh39 Tosh39 mentioned this pull request Jun 4, 2020
1 task
iggyfisk pushed a commit to happypancake/Prebid.js that referenced this pull request Jun 22, 2020
* add wipes

* done update notes

* URL変更

* update test parameter

* fix mediaType to make it work

* fix spec error

* fix comment

* fix param names

* update fix

* erase unused code

Co-authored-by: 嘉数貴明 <yaki180sx@gmail.com>
Co-authored-by: yaki(kkz) <yaki_180sx@hotmail.com>
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