Don't check allowed sections or default off tags for affiliate links in frontend #27561
Conversation
emma-imber
changed the title
| } else { | ||
| supportedSections.contains(section) && !tagPaths.exists(path => defaultOffTags.contains(path)) | ||
| } | ||
| } else false | ||
| } else false |
There was a problem hiding this comment.
This makes it much clearer to understand, just confirming my understanding that the logic now defaults to not showing affiliate links if showAffiliateLinks is not defined?
To echo Dina the great dev time talk really helped understand the flow of affiliate links 👍
There was a problem hiding this comment.
Yep that's correct. In the cases that we want to show affiliate links, the value showAffiliateLinks should be defined. Defaulting to false is safer and shouldn't cause issues where we do want to see affiliate links
There was a problem hiding this comment.
Great so unless we get showAffiliateLinks from composer (the new source of truth) we will not show them Im assuming 👍
| supportedSections = Configuration.affiliateLinks.affiliateLinkSections, | ||
| defaultOffTags = Configuration.affiliateLinks.defaultOffTags, |
There was a problem hiding this comment.
Just confirming my understanding, uses similar logic as below to determine whether to add affiliate links?
Both functions have the same name shouldAddAffiliateLinks but serve slightly different purpose?
This one checking 2 paragraphs and the length of the second; and the function in HtmlCleaner being more towards checking the tags associated with the content?
There was a problem hiding this comment.
This logic is in Composer so the duplication here isn't necessary
| } else { | ||
| supportedSections.contains(section) && !tagPaths.exists(path => defaultOffTags.contains(path)) | ||
| } | ||
| } else false | ||
| } else false |
There was a problem hiding this comment.
Great so unless we get showAffiliateLinks from composer (the new source of truth) we will not show them Im assuming 👍
emma-imber
force-pushed
the
ei/tidy-affiliates-sections-logic
branch
from
c6efb93
to
d10baec
Compare
What does this change?
This change removes the check for supported sections and default off tags for articles where showAffiliateLinks is not defined. We are moving to treat Composer as the source of truth for affiliate links, so if showAffiliateLinks is not defined for an article, we will default to not showing affiliate links. If affiliate links should be shown on an article, showAffiliateLinks should be defined.
This change has meant we can simplify the logic and function in frontend, and I have updated and removed some tests to reflect the logic change. I've also added comments to document the tests, as they were pretty opaque. The diff isn't super helpful for the test file, might be easier to read without the diff highlighting