[MAJC-110] Bugfix: partial successful API response does not trigger fallback #13
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
MAJC-110
Problem Statement
There is a bug occurring where if the server responds with only some of the ads requested, then we do not successfully fail over to our fallback experience for the missing ads. This also resulted in
contentnot being defined causing the loading spinner to hang forever.Proposed Changes
Nothing has changed about the existing fallback logic that occurs when an API response fails or comes back completely empty. However, now during the
mapResponseToPlacementsWithContentstep where the API response is mapped to our MozAdPlacement'scontentattribute, we check to see if there are any MozAdPlacements that end up not being filled with content. If that is the case, we perform a check to see if they are able to be filled with a known fallback placement type and either fill that placement with the fallback content, or assigncontentto be{}. In either case, thedisplaystep will recognize that the placement has successfully fetched and either a) display the fallback content or b) display nothing in the casecontent: {}.Ideally, we should refactor this entire fetch file to possibly move this mapping logic into a wrapper of
fetchAdsrather than rely on the default function and types given to us by HeyAPI. This would hopefully make the package much easier to read and clean up the large amount of nested conditionals/catches needed. That said in the spirit of this being a quick bug fix, we will leave that refactor to a future ticket as we already have a larger refactor on the horizon.We have also made a slight update to the Skyscraper fallback as the width of the SVG asset was incorrect.
Verification Steps
SideBar.tsxin the example app and set the skyscraper placement to bepocket_skyscraper_1instead ofmock_pocket_skyscraper_1. This should result in nothing coming back from the staging server. Reload the app and you should see still two successful billboards render with the skyscraper falling back. You can verify the response from the API is empty for the skyscraper.