Skip to content

RhythmOne Adapter: dedupe frameImp and interpretResponse flows#14571

Open
patmmccann wants to merge 1 commit intomasterfrom
codex/extract-common-functions-into-library-sunlom
Open

RhythmOne Adapter: dedupe frameImp and interpretResponse flows#14571
patmmccann wants to merge 1 commit intomasterfrom
codex/extract-common-functions-into-library-sunlom

Conversation

@patmmccann
Copy link
Collaborator

@patmmccann patmmccann commented Mar 9, 2026

Motivation

  • Remove remaining duplicated impression framing and response interpretation logic shared between the Marsmedia and RhythmOne adapters by centralizing common flows into a shared library to improve maintainability.

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):

  • libraries/rhythmoneMarsUtils/index.js (+2 errors)

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16a4fd2760

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const impObj = {
id: bidRequestData.adUnitCode,
secure: isSecure,
ext: frameExt(bidRequestData)

Choose a reason for hiding this comment

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

P2 Badge Validate media before invoking frameExt

buildImpressionList now calls frameExt before verifying that a bid produced a valid banner/video impression, which changes behavior for malformed bids that should be skipped. In the Marsmedia path, frameExt performs banner-size processing (getMinSize(processedSizes)) and can throw on invalid mediaTypes.banner.sizes shapes; previously frameExt was only reached after frameBanner/frameVideo succeeded, so these bids were dropped instead of crashing request building. This can cause the adapter request flow to fail for bad publisher size configs that were previously tolerated.

Useful? React with 👍 / 👎.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 22873766541

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 86 of 88 (97.73%) changed or added relevant lines in 3 files are covered.
  • 46 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+33.2%) to 96.331%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/rhythmoneMarsUtils/index.js 73 74 98.65%
modules/marsmediaBidAdapter.js 7 8 87.5%
Files with Coverage Reduction New Missed Lines %
src/adloader.js 9 76.92%
src/utils.js 37 90.51%
Totals Coverage Status
Change from base Build #3: 33.2%
Covered Lines: 217469
Relevant Lines: 225753

💛 - Coveralls

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.

2 participants