INN / umbrella-sfpublicpress

San Francisco Public Press
https://sfpublicpress.org/
GNU General Public License v2.0
1 stars 4 forks source link

Question: How does related stories at the bottom pull the automatic list? #111

Open MirandaEcho opened 4 years ago

MirandaEcho commented 4 years ago

Example: http://sfpublicpress.flywheelsites.com/black-lives-matter-marchers-block-traffic-in-golden-gate-bridge-protest/

The third article shown there is very old. There are newer articles that have the same category, so why aren't those showing instead?

benlk commented 4 years ago

I'm guessing it will have something to do with the innards of the Largo_Related class, but will have to take ~5m to look at the other posts to see where they intersect with this post's tags and categories

$ wp post term list 4273 category
+---------+----------------+----------------+----------+
| term_id | name           | slug           | taxonomy |
+---------+----------------+----------------+----------+
| 3928    | Social Justice | social-justice | category |
+---------+----------------+----------------+----------+
$ wp post term list 4273 post_tag
+---------+------------------+------------------+----------+
| term_id | name             | slug             | taxonomy |
+---------+------------------+------------------+----------+
| 3468    | police brutality | police-brutality | post_tag |
| 3469    | police reform    | police-reform    | post_tag |
| 324     | protest          | protest          | post_tag |
| 1619    | protests         | protests         | post_tag |
| 2705    | racism           | racism           | post_tag |
+---------+------------------+------------------+----------+
benlk commented 4 years ago

2009 has "social justice" and "protests" 4254 has "protest" and "police brutality" 4271 has "Social Justice" and "protests", and is notably the only one matching by category

But there are 120 other posts in that category! http://sfpublicpress.flywheelsites.com/wp-admin/edit.php?category_name=social-justice

benlk commented 4 years ago

A quick review of the terms assigned to this post:

When Largo_Related is generating posts from the categories and tags that are assigned to the initial post, it grabs all category and tag terms that apply to the current post, and then sorts them by popularity before looping through them to find posts to add to the "related" list.

https://github.com/INN/largo/blob/590181982d22a5444eb3c5ccca58ea8b56db12f7/inc/related-content.php#L694-L696

The function popularity_sort is a method of Largo_Related, and if its behavior here is to be believed, it's sorting the less-used terms before the more-used terms.

https://github.com/INN/largo/blob/590181982d22a5444eb3c5ccca58ea8b56db12f7/inc/related-content.php#L550-L560

As a result, no posts are grabbed from "police brutality" or "police reform", the two posts from "protests" are added, then with one more post to grab Largo_Related grabs the next-earlier post from "protest", and then because it has three posts to equal the requested count of three, it's done. It never reads the category.

But as far as I can tell, based on usort and ternary operator docs, this code is written to perform the opposite way of how it's behaving.

        return ( $a->count < $b->count ) ? -1 : 1;
benlk commented 4 years ago

So this is what's actually happening:

Given tags and counts

This usort sorting function will return e,f,a,c instead of c,a,f,e, because it orders from least to greatest.

So of course it's picking from the lower-count terms.

This is a Largo bug! 🐛

benlk commented 4 years ago

To solve this problem, pick one or both of:

  1. SFPP deletes the low-post-count tags that are included in the list of tags to be deleted
  2. We fix https://github.com/INN/largo/issues/1868, tag a new Largo prerelease, and put SFPP there. This will require testing SFPP on the new Largo version.
MirandaEcho commented 4 years ago

Final tag merge should resolve this - will check back.

MirandaEcho commented 4 years ago

@benlk now that categories are assigned, can you take another look and see if this has improved?

MirandaEcho commented 4 years ago

Ah, looks like https://github.com/INN/umbrella-sfpublicpress/issues/126 will need to happen first.

benlk commented 4 years ago

The related stories on the post http://sfpublicpress.flywheelsites.com/black-lives-matter-marchers-block-traffic-in-golden-gate-bridge-protest/ are now all drawn from the "Social Justice" category, as it is the only category shared by those posts.

MirandaEcho commented 4 years ago

http://sfpublicpress.flywheelsites.com/pandemic-and-protest-how-aids-and-lgbtq-activism-in-the-80s-informs-the-present/

Still seeing issues with older posts popping up here, even though there are newer ones in the category. @joshdarby can you take a look?

joshdarby commented 4 years ago

@MirandaEcho I'm assuming that post also has the same sort of issue that the one @benlk worked on did.

original post: categories:

tags:

S.F. Needs to Shut Down Streets During Coronavirus Pandemic categories:

tags:

AIDS Research Used to Battle COVID-19 at S.F. Lab categories:

tags:

California drugmaker's HIV prevention pill sparks public health debate categories:

tags:

benlk commented 4 years ago

Opinion has 2 posts, HIV & Aids has 9. Those are the lowest-count terms on the post, and so the related posts are drawn first from Opinion and then from HIV & AIDS.

Yes, this is the same thing.

joshdarby commented 4 years ago

@MirandaEcho Do we want to do the same fix for this article that @benlk did for the earlier one?

MirandaEcho commented 4 years ago

This is another Largo bug, correct?

joshdarby commented 4 years ago

@MirandaEcho Yes, the same one as before.

MirandaEcho commented 4 years ago

I thought the other article was fixed by removing low-used tags? Is Opinion a tag?

benlk commented 4 years ago

It's a low-use category.

joshdarby commented 4 years ago

@MirandaEcho No, it's a category. The other was fixed by removing low count terms which can be a category or tag.

benlk commented 4 years ago

The issue in https://github.com/INN/largo/issues/1868 is not tags vs categories; it's that the Largo Related Posts widget draws from the lowest-count terms assigned to the post rather than the highest-count terms.

MirandaEcho commented 4 years ago

Moving this to post-launch. They can manually update related posts for now if there's an issue like this.