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

"Sort by likes" button in Agriculture and Air Quality topics does not sort correctly #8598

Open
khadija-nur opened this issue Oct 16, 2020 · 12 comments
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed

Comments

@khadija-nur
Copy link
Contributor

Sort by likes does not work in agriculture and air quality topics

  • When you go to "https://publiclab.org/tag/air-quality" or "https://publiclab.org/tag/agriculture"
    and click on sort by drop down menu then filter by likes button the posts appear randomly
    instead of being in a descending order depending on the number of likes they have

  • see below:

  • After filtering, the first post has 11 likes, the third one has 7 and the fourth has 8 likes

like-11

likes-7

likes-8

Please show us where to look

https://publiclab.org/tag/agriculture?order=likes
https://publiclab.org/tag/air-quality?order=likes

What's your PublicLab.org username?

username: knur

Browser, version, and operating system

  • Windows 10 OS
  • Chrome browser

Thank you!

@khadija-nur khadija-nur added the bug the issue is regarding one of our programs which faces problems when a certain task is executed label Oct 16, 2020
@welcome
Copy link

welcome bot commented Oct 16, 2020

Thanks for opening your first issue! This space is protected by our Code of Conduct - and we're here to help.
Please follow the issue template to help us help you 👍🎉😄
If you have screenshots or a gif to share demonstrating the issue, that's really helpful! 📸
Do join our Gitter channel for some brainstorming discussions.

@alvesitalo
Copy link
Contributor

Hey, can I work on this?

@cesswairimu
Copy link
Collaborator

Hi @alvesitalo, we would love your help. Go ahead . Thanks

@ebarry
Copy link
Member

ebarry commented Oct 19, 2020

Thank you for reporting this @khadija-nur -- this is a valuable contribution! 🤝
and thank you @@alvesitalo for putting in some work to fix this 😃

@alvesitalo
Copy link
Contributor

Hi everyone, I tested locally with some posts but I can't see any problem with sorting by likes. Have you experienced this issue? @ebarry @cesswairimu

@cesswairimu
Copy link
Collaborator

@alvesitalo yeah, it is happening in production if you go to https://publiclab.org/tag/agriculture" and select sort by likes you will see the second and third post may have more likes than the first one. I will test locally see if I can replicate

@jywarren
Copy link
Member

jywarren commented Oct 20, 2020

Hi all, thanks @khadija-nur for finding this, and @alvesitalo for looking deeper - it seems like it may be pretty intermittent, which can be really tough to debug.

I wonder if we are sorting by cached likes count versus a count "on the fly" - see the model for tracking and caching likes at:

plots2/app/models/node.rb

Lines 1039 to 1046 in 27a3839

def toggle_like(user)
nodes = NodeSelection.where(nid: id, liking: true).size
self.cached_likes = if is_liked_by(user)
nodes - 1
else
nodes + 1
end
end

The page itself is using cached_likes here:

elsif params[:order] == 'likes'
'node.cached_likes DESC'

But see how the template is using node.likers.count ? Which may cause another database query and not be very efficient.

<li><a>Total Likes: <%= node.likers.size %></a></li>

node.likers is defined here, and seems to use a different query... than the cached_likes code. Could they be drifting apart because some folks may like, but then get banned? In any case we should be able to use node.cached_likes instead in the template. That should be a pretty simple fix for now!

plots2/app/models/node.rb

Lines 256 to 264 in 27a3839

# users who like this node
def likers
node_selections
.joins(:user)
.references(:rusers)
.where(liking: true)
.where('rusers.status': 1)
.collect(&:user)
end


Notice also how we aren't filtering out spam users in the cached_likes calculation -- maybe we should do that too, just to get these better in sync?

@jywarren
Copy link
Member

I'll spin out that last idea about filtering banned users from cached_likes into its own issue. It's just a one-line change too!

@jywarren
Copy link
Member

I spun that out into a first-timers-only issue at #8639 !

I could do the same for the template change above too, if it's helpful?

@alvesitalo
Copy link
Contributor

Cool, it is. Thank you for clarifying this bug @jywarren

@jywarren
Copy link
Member

I just did that over at #8691 - thanks @alvesitalo!

@gauravahlawat81
Copy link
Member

Hey @jywarren , I was trying to reproduce this error in my local environment, but I couldn't do it. The post were all sorted correctly and I observed the same here, I wonder if this issue is solved now or are am I missing something else here ? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed
Projects
None yet
Development

No branches or pull requests

6 participants