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

[IOS-5813]Fix nurl not triggered issue #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

YueVungle
Copy link

Before loading an ad, the MAX Adapter will check if the ad is cached. If the ad is cached, the MAX Adapter will not call the load ad API. This will cause the issue that the nurl is not be triggered. This issue happens in both bidding and waterfall ads.
nurl must be triggered after calling load ad API explicitly.

IOS-5813

* Address Banner and MREC ad display issue.

This commit will update the show ad for Banner/Mrec to be
called after the ad finishes downloading the assets.
Also updates the finish display ad to be called on destroy.

IOS-5559
IOS-5560
@YueVungle
Copy link
Author

YueVungle commented Feb 21, 2023

About how to test this issue:

For non-bidding ads:
Step1: I used the mocked config response(in the attachment) to make INTERSTITIAL_1-0553904 as auto cached.
Step2: I changed the INTERSTITIAL_1-0553904 placement ad response(in the attachment) to update the notification part for the nurl.

"notification": ["https://events.ads.vungle.com/api/v5/load_ad?ext=EeDenlZPg8uojfjhhgiLc7daqVmJIBGeFwOmYc70thFVla7AFRkJbap-Zc1dPmp32kbFzqc-xCDE5MCoreoz78uSiRFnsNTid9ouqpjdwgcL3mbCG9xzJOwS98QWs_STlkuGs4rV--TUu2wj0Ydi0_mOzVA96Ro7kiia"],

From Charles logs nurl_not_triggered_before_fix.chls and nurl_triggered_after_fix.chls, we can see that https://events.ads.vungle.com/api/v5/load_ad?... is triggered after adding this change(nurl must be triggered after calling load ad explicitly).

config-max-test.zip
INTERSTITIAL_1-0553904.zip
nurl_not_triggered_before_fix.chls.zip
nurl_triggered_after_fix.chls.zip

For Bidding ads:
I received sleeping code for all the bidding placements. After I used the mocked ad response, the MAX Adapter level was not called. So @ManojBudumuru-vungle and other team members might help to verify the bidding ad case in the US time zone.
Thanks a lot. :)

Copy link

@crbinsf crbinsf left a comment

Choose a reason for hiding this comment

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

Changes look good - still having issues with cocoapods and getting max repos set up for testing, so I'll hold on approval and let others confirm change works as expected.

@ManojBudumuru-vungle
Copy link

@YueVungle the fix u did works only for water fall. the issue is with sdk.

  1. For bidding with MaxCache = 1 SDK triggers nurl on the seq-download - wrong behaviour
  2. SDK skips nurl for waterfall autocache.
    For the above issues i have created a pr in 6.x branch. u can test it with your changes changes. https://github.com/Vungle/ios-sdk-for-real/pull/2156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants