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

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

Open
MirandaEcho opened this issue Jun 26, 2020 · 20 comments
Assignees

Comments

@MirandaEcho
Copy link
Collaborator

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
Copy link
Collaborator

benlk commented Jun 26, 2020

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
Copy link
Collaborator

benlk commented Jun 26, 2020

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
Copy link
Collaborator

benlk commented Jun 26, 2020

A quick review of the terms assigned to this post:

  • tag "police brutality" has one post, which is the initial post
  • tag "police reform" has one post, which is the initial post
  • tag "protests" has three posts, which the initial post and two of the three related posts
  • tag "protest" has 8 posts, of which 4254 is the post in that tag previous to this post
  • category "Social Justice" has 124 posts

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
Copy link
Collaborator

benlk commented Jun 26, 2020

So this is what's actually happening:

Given tags and counts

  • c -> 4
  • a -> 3
  • f -> 2
  • e -> 1

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
Copy link
Collaborator

benlk commented Jun 26, 2020

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 Largo_Related::popularity_sort sorts in wrong order WPBuddy/largo#1868, tag a new Largo prerelease, and put SFPP there. This will require testing SFPP on the new Largo version.

@MirandaEcho
Copy link
Collaborator Author

Final tag merge should resolve this - will check back.

@MirandaEcho
Copy link
Collaborator Author

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

@MirandaEcho
Copy link
Collaborator Author

Ah, looks like #126 will need to happen first.

@benlk
Copy link
Collaborator

benlk commented Jun 29, 2020

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.

@benlk benlk self-assigned this Jun 29, 2020
@MirandaEcho
Copy link
Collaborator Author

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
Copy link

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

original post:
categories:

  • “Civic” Podcast
  • HIV & AIDS
  • LGBTQ
  • Opinion
  • Social Justice

tags:

  • none

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

  • Opinion
  • Public Safety
  • Transportation

tags:

  • bicyclists
  • cars
  • coronavirus
  • coronavirus pandemic
  • COVID-19
  • pandemic
  • public space
  • social distancing
  • street closures
  • street-closing
  • walkers

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

  • Health
  • HIV & AIDS
  • News
  • Technology

tags:

  • coronavirus
  • coronavirus pandemic
  • COVID-19

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

  • Health
  • HIV & AIDS
  • LGBTQ

tags:

  • none

@benlk
Copy link
Collaborator

benlk commented Jun 30, 2020

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
Copy link

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

@MirandaEcho
Copy link
Collaborator Author

This is another Largo bug, correct?

@joshdarby
Copy link

@MirandaEcho Yes, the same one as before.

@MirandaEcho
Copy link
Collaborator Author

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

@benlk
Copy link
Collaborator

benlk commented Jun 30, 2020

It's a low-use category.

@joshdarby
Copy link

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

@benlk
Copy link
Collaborator

benlk commented Jun 30, 2020

The issue in WPBuddy/largo#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
Copy link
Collaborator Author

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

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

No branches or pull requests

3 participants