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.

Sign up for GitHub

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

Jump to bottom

Intentiq id value change #5746

Merged
merged 4 commits into from
Sep 16, 2020
Merged

Intentiq id value change #5746

merged 4 commits into from
Sep 16, 2020

Conversation

yuvalgg
Copy link
Contributor

@yuvalgg yuvalgg commented Sep 15, 2020

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

IntentIQ id will use plain id string instead of decoding a JSON value.

  • test parameters for validating bids
{
  bidder: '<bidder name>',
  params: {
    // ...
  }
}

Be sure to test the integration with your adserver using the Hello World sample page.

  • contact email of the adapter’s maintainer
  • official adapter submission

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

Other information

smenzer
smenzer reviewed Sep 15, 2020
View reviewed changes
Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

question about handling legacy values

modules/intentIqIdSystem.js Outdated
return (value && typeof value['ctrid'] === 'string') ? { 'intentIqId': value['ctrid'] } : undefined;
},
/**
* performs action to obtain id and return a value in the callback's response argument
* @function
* @param {SubmoduleParams} [configParams]
* @returns {IdResponse|undefined}
*/
getId(configParams) {
if (!configParams || typeof configParams.partner !== 'number') {
utils.logError('User ID - intentIqId submodule requires a valid partner to be defined');
return;
}

// use protocol relative urls for http or https
const url = `https://api.intentiq.com/profiles_engine/ProfilesEngineServlet?at=39&mi=10&dpi=${configParams.partner}&pt=17&dpn=1`;
const resp = function (callback) {
const callbacks = {
success: response => {
let responseObj;
if (response) {
try {
responseObj = JSON.parse(response);
} catch (error) {
utils.logError(error);
}
}
callback(responseObj);
},
error: error => {
utils.logError(`${MODULE_NAME}: ID fetch encountered an error`, error);
callback();
}
};
ajax(url, callbacks, undefined, {method: 'GET', withCredentials: true});
};
return {callback: resp};
}
};

submodule('userId', intentIqIdSubmodule);
/**
* This module adds IntentIqId to the User ID module
* The {@link module:modules/userId} module is required
* @module modules/intentIqIdSystem
* @requires module:modules/userId
*/

import * as utils from '../src/utils.js'
import {ajax} from '../src/ajax.js';
import {submodule} from '../src/hook.js'

const MODULE_NAME = 'intentIqId';

/** @type {Submodule} */
export const intentIqIdSubmodule = {
/**
* used to link submodule with config
* @type {string}
*/
name: MODULE_NAME,
/**
* decode the stored id value for passing to bid requests
* @function
* @param {{string}} value
* @returns {{intentIqId:string}}
*/
decode(value) {
return (value != undefined && value != '') ? { 'intentIqId': value } : undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

will this properly handle legacy values that are already stored in cache? it looks like for all users that hit a page with this new code, until a getId call is made again, you will lose the old value of the ID, which seems like it could be bad. Is that how this would work and is that intentional?

@yuvalgg
Copy link
Contributor Author

yuvalgg commented Sep 15, 2020 via email

@yuvalgg
Copy link
Contributor Author

yuvalgg commented Sep 15, 2020 via email

@smenzer
Copy link
Collaborator

smenzer commented Sep 15, 2020

Yes, we are currently only testing Prebid with one partner so there isn't much data to lose, even though my intention was that the cache would get overwritten fast. Is there any change to be made to make the getId call happen more often until all the cache values are changed to the new one? Besides, It is my understanding that we previously saved only the id string into the cache and not the HTTP response so using a plain string id shouldn't be any different. *Can't comment on the PR itself mate

Ok understood, as long as it's intended. If the publisher sets the storage.refreshInSeconds configuration option, they can force calls to getId more quickly.

@smenzer
Copy link
Collaborator

smenzer commented Sep 15, 2020

once you get the CI failures fixed, this LGTM to be merged

@yuvalgg
Copy link
Contributor Author

yuvalgg commented Sep 15, 2020 via email

@smenzer smenzer self-assigned this Sep 15, 2020
@yuvalgg
Copy link
Contributor Author

yuvalgg commented Sep 16, 2020 via email

smenzer
smenzer approved these changes Sep 16, 2020
View reviewed changes
Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

LGTM

@smenzer smenzer merged commit 04ea603 into prebid:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smenzer smenzer smenzer approved these changes

Assignees

@smenzer smenzer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yuvalgg @smenzer