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

Escalax Bid Adapter: initial release #12483

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

escalax
Copy link

@escalax escalax commented Nov 21, 2024

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated 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

Other information

prebid.github.io

@escalax escalax marked this pull request as draft November 21, 2024 15:10
@escalax escalax marked this pull request as ready for review November 22, 2024 09:22
const BIDDER_CODE = 'escalax';
const ESCALAX_SOURCE_ID_MACRO = '[sourceId]';
const ESCALAX_ACCOUNT_ID_MACRO = '[accountId]';
const ESCALAX_SUBDOMAIN_MACRO = '[subdomain]';
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the goal of the dynamic subdomain?

Copy link
Author

Choose a reason for hiding this comment

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

@patmmccann
We have a different subdomain for each region - us, eu and so on

@ChrisHuie ChrisHuie requested a review from JulieLorin December 6, 2024 11:20
@ChrisHuie ChrisHuie changed the title Escalax Bid Adapter: add new bid adapter Escalax Bid Adapter: initial release Dec 6, 2024
Copy link
Collaborator

@JulieLorin JulieLorin left a comment

Choose a reason for hiding this comment

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

Can you link the PR on prebid docs in this PR description ?

buildRequests: (validBidRequests, bidderRequest) => {
if (validBidRequests && validBidRequests.length === 0) return [];
const { sourceId, accountId } = validBidRequests[0].params;
const subdomain = validBidRequests[0].params.subdomain;
Copy link
Collaborator

Choose a reason for hiding this comment

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

here, you use the subdomain of the first bid for the request.
So if bids uses different subdomain, it would be ignored.
I am guessing you want the publisher to use only one value for subdomain among all bids but this forces the publisher to copy paste the same parameter everywhere.
Another solution could be to set up a parameter in the setConfig/setBidderConfig option so the publisher would only have to set the param once and there would not an issue where only the params from the first bid is used.
You can find a simple example of that with Adot bidder which setup a global publisherId :
https://github.com/prebid/Prebid.js/blob/master/modules/adotBidAdapter.md
https://github.com/prebid/Prebid.js/blob/master/modules/adotBidAdapter.js

Copy link
Author

Choose a reason for hiding this comment

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

@JulieLorin I see, that's reasonable
But wouldn't it be more difficult for the publisher to specify an additional code like in adot example with this publisherId
And what i saw in couple of adapters is region parameter, like here:

const adUnits=[
{
code:'test-div',
sizes:[[300,250]],
bids:[
{
bidder: 'lucead',
params:{
placementId: '1',
region: 'us', // optional: 'eu', 'us', 'ap'
}
}
]
}
];

or here:
| Name | Scope | Description | Example |
| :------------ | :------- | :------------------------ | :------------------- |
| `region` | optional (for prebid.js) | 3 letter country code | "USA" |
| `publisherId` | required (for prebid-server) | partner ID provided by preciso | PreTest_0001 |
| `traffic` | optional (for prebid.js) | Configures the mediaType that should be used. Values can be banner, native | "banner" |

We can agree to make a region parameter instead of subdomain and a different endpoint url for each such parameter - US, EU

Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't you detect the user region and fill in your own subdomain instead of putting this on the publisher. Here is some basic logic:

function getContinentFromTimeZone(timeZone) {
    // Validate the input timezone
    if (typeof timeZone !== 'string') {
        throw new Error('Invalid timezone: Timezone must be a string.');
    }

    // Map timezone region prefixes to continents
    const continentMap = {
        "Africa": "Africa",
        "America": "North America / South America",
        "Antarctica": "Antarctica",
        "Asia": "Asia",
        "Atlantic": "Atlantic Ocean region",
        "Australia": "Australia",
        "Europe": "Europe",
        "Indian": "Indian Ocean region",
        "Pacific": "Pacific Ocean region",
    };

    // Extract the main region from the timezone string
    const mainRegion = timeZone.split('/')[0];

    // Return the matching continent or a default message
    return continentMap[mainRegion] || "Unknown continent";
}

// Example usage:
const userTimeZone = Intl.DateTimeFormat().resolvedOptions().timeZone;
console.log(`User's timezone: ${userTimeZone}`);
console.log(`Continent: ${getContinentFromTimeZone(userTimeZone)}`);

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.

4 participants