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

Fix AdPods parsing behavior with resolveAll to false #487

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

ZacharieTFR
Copy link
Contributor

Description

Previously, when a VAST contained an adPod with standalone ads, if a standalone ad appeared first in the VAST order, it would be parsed first, causing the adPod to not be resolved. As a result, the Player couldn't be aware that the VAST contained an adPod. According to IAB specifications, the adPod should take priority and play before any standalone ads. This PR addresses the issue by ensuring that the adPod is resolved first, followed by the standalone ads, when getNextAds is called.

Issue

#360

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

@ZacharieTFR ZacharieTFR self-assigned this Jan 24, 2025
@dm-ad-sdk dm-ad-sdk bot requested a review from clarissedmn January 27, 2025 08:54
@clementFrancon clementFrancon self-requested a review January 27, 2025 09:10
@@ -161,7 +158,7 @@ describe('VASTClient', () => {
expect(res.ads).toHaveLength(3);
});

it('resolves only next ad if requested', async () => {
fit('resolves only next ad if requested', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can create a ticket to add an ESLint rule to prevent fit from being pushed to production?

Suggested change
fit('resolves only next ad if requested', async () => {
it('resolves only next ad if requested', async () => {

@clarissedmn
Copy link
Contributor

After removing fit from the tests, it looks like two tests are failing

@ZacharieTFR ZacharieTFR merged commit 22b9de8 into master Feb 13, 2025
2 checks passed
@ZacharieTFR ZacharieTFR deleted the ad-pod-resolving branch February 13, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants