Skip to content

Conversation

jeremybmerrill
Copy link
Collaborator

enhancement to do an ok job of displaying post2020 ads in the archive

jeremybmerrill and others added 30 commits July 2, 2020 13:17
I chose to filter them out in sendPosts as sendPosts was already
filtering for posts that were already in the archive.

The other place you might think to put the filter was in `applyFilters`
as invoked in `feedScanner.js`. However, this place is not correct,
because `applyFilters` doesn't mean what you think it does. It does not
mean "applying filters". It actually means "apply info on posts that
pass this filter". What kind of "info"? Something like
applyAdTargetingInfo and applyAdId (going off the top of my head).

So...applyFilters wasn't actually doing any filtering at all. Very
confusing. It should be renamed.
mallochine and others added 13 commits August 30, 2020 12:46
the number of posts will be shortened by some filter, when that is not
true at all. The actual behavior is that ad info is applied to the
posts.

The only 'filtering' going on is that the ad info is only applied to
certain posts, not all posts.

An example of a filter is "canShareSponsored". If not true, the ad info
is not applied. (this last bit is off the top of my head)
coreDataGrab exists to find the ad ID and token pertaining to an element by
recursively iterating through its keys -- and those of any
elements/arrays/objects that are values -- to find anything that seems like
an ad ID or client token. Previously, this would analyze the `return` key
which is something like the parent element. This meant that it would traverse
up the tree and back down to siblings, which led to mistakes.
Fix: a custom parser to extract the ad copy and images. Then write our
own html to display the contents.
the image.

Currently it's broken in that the pre and post image text appear as one
paragraph.

Now they are properly separated.
@online-pol-ads online-pol-ads deleted the feature/post2020-archive-display branch September 3, 2021 20:04
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.

2 participants