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

Somo prebid 3.0 updates #4595

Merged
merged 5 commits into from Dec 19, 2019
Merged

Somo prebid 3.0 updates #4595

merged 5 commits into from Dec 19, 2019

Conversation

travisbeale
Copy link
Contributor

@travisbeale travisbeale commented Dec 13, 2019

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

Adding back Somo Audience Prebid adapter with 3.0 compliance.

  • test parameters for validating bids
{
  bidder: 'somo',
          mediaTypes: {
            banner: {
              sizes: [[300, 250]]
            }
          },
          params: {
            placementId: 'test'
          }
  }
}

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

Other information

@travisbeale
Copy link
Contributor Author

Failing test does not appear to be due to our code.

@jsnellbaker
Copy link
Collaborator

Hi @travisbeale

It seems that some of the errors in the browserstack results look to be tied to the somoBidAdapter tests (specifically in IE11). They appear to be failing when the tests are trying to validate the value in the ortbRequest.site.domain field.

I think this potentially may be due to the slightly malformed mock URLs used in the bidderRequest.refererInfo objects used in the tests. Below is a copy of one:

const bidderRequest = {
          refererInfo: {
            referer: 'https:/www.test.com/page?var=val',
            canonicalUrl: 'https:/www.test.com/page'
          }
        }

These URLs only have 1 / instead of 2. I think IE11 may be handling this type of URL differently than the other modern browsers. Can you try to set the extra / character to see if those IE11 tests pass? If that's not the trick, can you dig a bit deeper to see what the issue may be?

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

The rest of the files/updates appear to be fine. Just need to address the test failures in IE11 (see comment above).

@travisbeale
Copy link
Contributor Author

travisbeale commented Dec 18, 2019

@jsnellbaker Fixed the test, thanks for catching that.

@jsnellbaker
Copy link
Collaborator

@travisbeale It seems that the error are still happening per the latest browserstack test (here). Could you check around some more to see what may be the issue in IE11?

@travisbeale
Copy link
Contributor Author

@jsnellbaker The CircleCI tests are now passing for our bid adapter in IE11. The remaining CircleCI test failures do not appear to be in our code.

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

@travisbeale
Thanks for making the additional updates. I ran another set of the browserstack tests locally and they passed successfully.

@jsnellbaker jsnellbaker merged commit 271d634 into prebid:master Dec 19, 2019
tadam75 pushed a commit to smartadserver/Prebid.js that referenced this pull request Jan 9, 2020
* Re-add Somo bid adapter with v3 compliance

* Somo: fixed malformed url in test

* Use an alternative method of getting the domain from a url when the URL API isn't supported (IE11)

* Somo: fixed indent error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants