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

PubxAi Analytics Adapter : code cleanup and additional data collection #11425

Merged
merged 41 commits into from
Jul 4, 2024

Conversation

pnhegde
Copy link
Contributor

@pnhegde pnhegde commented Apr 30, 2024

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

Code cleanup and refactoring, renaming of fields, and additional data collection as part of analytics.

@pnhegde pnhegde changed the title PubxAi analytics adapter updates WIP: PubxAi analytics adapter updates Apr 30, 2024
@pnhegde pnhegde changed the title WIP: PubxAi analytics adapter updates PubxAi analytics adapter updates Apr 30, 2024
@ChrisHuie ChrisHuie changed the title PubxAi analytics adapter updates PubxAi Analytics Adapter : code cleanup and additional data collection Apr 30, 2024
@pnhegde
Copy link
Contributor Author

pnhegde commented May 20, 2024

@ChrisHuie All the tests have passed already. Anything else needed from our end? Thanks.

switching from sessionStorage to localStorage
Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

  • modules/pubxaiAnalyticsAdapter.js has 12 duplicated lines with modules/terceptAnalyticsAdapter.js
  • modules/pubxaiAnalyticsAdapter.js has 11 duplicated lines with modules/roxotAnalyticsAdapter.js
  • modules/mediakeysBidAdapter.js has 18 duplicated lines with modules/pubxaiAnalyticsAdapter.js

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

1 similar comment
Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

  • modules/pubxaiAnalyticsAdapter.js has 12 duplicated lines with modules/terceptAnalyticsAdapter.js
  • modules/pubxaiAnalyticsAdapter.js has 11 duplicated lines with modules/roxotAnalyticsAdapter.js
  • modules/mediakeysBidAdapter.js has 18 duplicated lines with modules/pubxaiAnalyticsAdapter.js

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

@pnhegde
Copy link
Contributor Author

pnhegde commented Jun 12, 2024

@ncolletti @patmmccann I'm not sure what to do to avoid duplication error here. I don't think we can import code from other analytics adapters or if we can move them to a common library.

Also, we have fixed the storage issue but sendBeacon is kept as it is instead of switching to fetch-keepalive due to a firefox compatibility issue. Let me know if this works or anything else we need to do.

Thanks!

@patmmccann
Copy link
Collaborator

patmmccann commented Jun 13, 2024

Please move into a common library, eg this appears to be a direct copy, not sure of which direction, but both modules could easily import from a common place in the libraries folder:

"format": "javascript",
"lines": 18,
"fragment": "function getOS() {\n if (navigator.userAgent.indexOf('Android') != -1) return 'Android';\n if (navigator.userAgent.indexOf('like Mac') != -1) return 'iOS';\n if (navigator.userAgent.indexOf('Win') != -1) return 'Windows';\n if (navigator.userAgent.indexOf('Mac') != -1) return 'Macintosh';\n if (navigator.userAgent.indexOf('Linux') != -1) return 'Linux';\n if (navigator.appVersion.indexOf('X11') != -1) return 'Unix';\n return 'Others';\n}\n\n/**\n * Returns floor from priceFloors module or MediaKey default value.\n \n * @param {} bid a Prebid.js bid (request) object\n * @param {string} mediaType the mediaType or the wildcard ''\n * @param {string|Array} size the size array or the wildcard ''\n * @returns {number|boolean}\n */",

@patmmccann patmmccann assigned patmmccann and unassigned ncolletti Jun 17, 2024
@patmmccann patmmccann removed the request for review from ncolletti June 17, 2024 12:26
Nathan Oliver and others added 2 commits June 17, 2024 23:43
modifying functions to avoid prebid duplication checker
@patmmccann patmmccann merged commit 0110b3c into prebid:master Jul 4, 2024
5 checks passed
DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
prebid#11425)

* PTOW-2; updates to the pubx analytics adapter

* PTOW-2 review actions

* PTOW-2 Review actions

* PTOW-2 updating pubx.ai analytics version

* remove empty line

* linting changes

* PTOW-2; updates to the pubx analytics adapter

* PTOW-2 review actions

* PTOW-2 Review actions

* PTOW-2 updating pubx.ai analytics version

* PTOW-2 resolving conflicts

* PTOW-2-fix-linting-errors

* PTOW-2 fixing tests

* fixing bugs, modifying blob behaviour, addressing browser compatibility

* add source field

* switching from sessionStorage to localStorage

* fixing tests

* modifying functions to avoid prebid duplication checker

* implementing enums

* moving user agent code to libraries

* updated return types

* switching to macro substitution for prebid version

* adding centralised sendBeacon wrapper

* 'fixing' tests

---------

Co-authored-by: Nathan Oliver <[email protected]>
Co-authored-by: tej656 <[email protected]>
Co-authored-by: Tej <[email protected]>
Co-authored-by: nathan-pubx <[email protected]>
mefjush pushed a commit to adhese/Prebid.js that referenced this pull request Jul 19, 2024
prebid#11425)

* PTOW-2; updates to the pubx analytics adapter

* PTOW-2 review actions

* PTOW-2 Review actions

* PTOW-2 updating pubx.ai analytics version

* remove empty line

* linting changes

* PTOW-2; updates to the pubx analytics adapter

* PTOW-2 review actions

* PTOW-2 Review actions

* PTOW-2 updating pubx.ai analytics version

* PTOW-2 resolving conflicts

* PTOW-2-fix-linting-errors

* PTOW-2 fixing tests

* fixing bugs, modifying blob behaviour, addressing browser compatibility

* add source field

* switching from sessionStorage to localStorage

* fixing tests

* modifying functions to avoid prebid duplication checker

* implementing enums

* moving user agent code to libraries

* updated return types

* switching to macro substitution for prebid version

* adding centralised sendBeacon wrapper

* 'fixing' tests

---------

Co-authored-by: Nathan Oliver <[email protected]>
Co-authored-by: tej656 <[email protected]>
Co-authored-by: Tej <[email protected]>
Co-authored-by: nathan-pubx <[email protected]>
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

7 participants