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

MobianRtdModule: Add more signals from API endpoint to first-party data #11999

Conversation

ehb-mtk
Copy link
Contributor

@ehb-mtk ehb-mtk commented Jul 18, 2024

Type of change

  • Feature

Description of change

In addition to adding Mobian's determination of brand safety to the first party data, we now also add its
determination of sentiment, emotion, and garm risk categories.

/**
* This module adds the Mobian RTD provider to the real time data module
* The {@link module:modules/realTimeData} module is required
* @module modules/anonymisedRtdProvider
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you from idward / anonymised? do you mean to import their type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no -- that's embarrassing. I've had trouble getting the build to include the real time data module without including it explicitly, and hoped that typing would help. Will remove...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add yourself to .submodules.js

Copy link
Collaborator

Choose a reason for hiding this comment

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

"medianetRtdProvider",

Copy link
Collaborator

@patmmccann patmmccann left a comment

Choose a reason for hiding this comment

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

Submodules file reference is missing

@ehb-mtk
Copy link
Contributor Author

ehb-mtk commented Jul 19, 2024

(Thanks -- fixed!! In the last commit, we've fixed another bug where the browser interprets the API response as a string, in which case we need to call JSONParse. We're doing this conservatively in case it's browser-dependent. Let me know if you'd prefer that we make this a separate PR.)

@@ -24,6 +22,21 @@ export const mobianBrandSafetySubmodule = {
function init() {
return true;
}

function safeJSONParse(input) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you import this?

export function safeJSONParse(data) {
and if that is problematic, could you pr the util instead of your adpter with the extra logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll just import this, I haven't got any reason to believe that in fact the API response ever came in pre-parsed. It'll be a bit cause I need to fix the tests.

@patmmccann patmmccann self-assigned this Jul 19, 2024
@patmmccann patmmccann merged commit 7110bc7 into prebid:master Jul 19, 2024
6 checks passed
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.

None yet

2 participants