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

Update gambid aliases #3410

Merged
merged 13 commits into from
Jan 10, 2019
Merged

Update gambid aliases #3410

merged 13 commits into from
Jan 10, 2019

Conversation

sa1omon
Copy link
Contributor

@sa1omon sa1omon commented Dec 30, 2018

Type of change

  • Feature

Description of change

Added two new aliases for Gamoshi adapter: gambid, cleanmedia. Added unit tests.

@harpere harpere self-requested a review January 2, 2019 21:55
@harpere harpere self-assigned this Jan 2, 2019
const rtbEndpoint = `${baseEndpoint}/r/${params.supplyPartnerId}/bidr?rformat=open_rtb&reqformat=rtb_json&bidder=prebid` + (params.query ? '&' + params.query : '');
let url = config.getConfig('pageUrl');
Copy link
Collaborator

Choose a reason for hiding this comment

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

config.getConfig('pageUrl') will never be used because if it exists it will trigger the else block below where it's reset to utils.getTopWindowUrl()
maybe:
let url = config.getConfig('pageUrl') || bidderRequest && bidderRequest.refererInfo && bidderRequest.refererInfo.referer || utils.getTopWindowUrl();

Copy link
Collaborator

Choose a reason for hiding this comment

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

which means there must not be a test verifying the expected behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harpere fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sa1omon I don't see any new commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harpere please check it again now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sa1omon - Sorry, logically that should work, but lint doesn't like it. CircleCI error: Unexpected mix of '||' and '&&' no-mixed-operators.

@harpere
Copy link
Collaborator

harpere commented Jan 3, 2019

Also, the docs (which show up here) should be updated.

@sa1omon
Copy link
Contributor Author

sa1omon commented Jan 6, 2019

Also, the docs (which show up here) should be updated.

@harpere Can you please give me more details about the updates that I need to do?

@harpere
Copy link
Collaborator

harpere commented Jan 7, 2019

Also, the docs (which show up here) should be updated.

@harpere Can you please give me more details about the updates that I need to do?

I figured you'd want to document all 3 bidder codes gamoshi, gambid, and cleanmedia.

@sa1omon
Copy link
Contributor Author

sa1omon commented Jan 7, 2019

Also, the docs (which show up here) should be updated.

@harpere Can you please give me more details about the updates that I need to do?

I figured you'd want to document all 3 bidder codes gamoshi, gambid, and cleanmedia.

@harpere I have created this pull request: Add cleanmedia bidder code #1078, in the prebid.github.io repository to update the docs. thanks

const rtbEndpoint = `${baseEndpoint}/r/${params.supplyPartnerId}/bidr?rformat=open_rtb&reqformat=rtb_json&bidder=prebid` + (params.query ? '&' + params.query : '');
let url = config.getConfig('pageUrl') || bidderRequest &&
bidderRequest.refererInfo && bidderRequest.refererInfo.referer ||
utils.getTopWindowUrl();
Copy link
Collaborator

Choose a reason for hiding this comment

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

logically this works, but apparently lint doesn't like it. CircleCI error: Unexpected mix of '||' and '&&' no-mixed-operators.

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe
let referer = bidderRequest && bidderRequest.refererInfo && bidderRequest.refererInfo.referer
let url = config.getConfig('pageUrl') || referer || utils.getTopWindowUrl();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harpere I have uploaded a fix. thanks

@harpere harpere removed the needs docs label Jan 7, 2019
@bretg
Copy link
Collaborator

bretg commented Jan 8, 2019

Docs PR prebid/prebid.github.io#1078

@sa1omon
Copy link
Contributor Author

sa1omon commented Jan 9, 2019

Docs PR prebid/prebid.github.io#1078

@bretg Did you see my answer for your question on that PR? thanks.

@harpere harpere added LGTM and removed needs review labels Jan 10, 2019
@harpere harpere merged commit 7904b4f into prebid:master Jan 10, 2019
amuraco pushed a commit to amuraco/Prebid.js that referenced this pull request Jan 28, 2019
* Update the Gamoshi Gambid adapter to simply the Gamoshi adapter

This is a minor branding change to highlight the Gamoshi brand name. The
biddercode "gambid" is still retained in the adapter aliases array.

* update whitelabel

* Add  test that checks support for outstream renderers

* Update testing and page ref

* Modify bidder's url resolving

* Fix lint issue with bidder's url resolving
ghost pushed a commit to devunrulymedia/Prebid.js that referenced this pull request Jan 30, 2019
* Update the Gamoshi Gambid adapter to simply the Gamoshi adapter

This is a minor branding change to highlight the Gamoshi brand name. The
biddercode "gambid" is still retained in the adapter aliases array.

* update whitelabel

* Add  test that checks support for outstream renderers

* Update testing and page ref

* Modify bidder's url resolving

* Fix lint issue with bidder's url resolving
@sa1omon sa1omon deleted the update-gambid-aliases branch February 4, 2019 09:22
pedrolopezmrf pushed a commit to Marfeel/Prebid.js that referenced this pull request Mar 18, 2019
* Update the Gamoshi Gambid adapter to simply the Gamoshi adapter

This is a minor branding change to highlight the Gamoshi brand name. The
biddercode "gambid" is still retained in the adapter aliases array.

* update whitelabel

* Add  test that checks support for outstream renderers

* Update testing and page ref

* Modify bidder's url resolving

* Fix lint issue with bidder's url resolving
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants