Plots2: Like count error on profile page

[ViditChitkara]

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

We know that the process of creating a pull request is the biggest barrier for new contributors. This issue is for you :gift_heart:

If you have contributed before, consider leaving this one for someone new, and looking through our general help wanted issues. Thanks!
:thinking: What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

Please describe the problem

The like count on the profile page does not correspond to the number of liked notes shown on /profile/user/likes' page

Solution

https://github.com/publiclab/plots2/blob/57f0d6f97844b9ba070f7a048fc9a942c1ecf0c8/app/models/drupal_user.rb#L97-L106

Like_count method could be similar to liked_notes method. We could replace
https://github.com/publiclab/plots2/blob/57f0d6f97844b9ba070f7a048fc9a942c1ecf0c8/app/models/drupal_user.rb#L98 with something like:-
Node.includes(:node_selections).references(:node_selections).where("type = 'note' AND node_selections.liking = ? AND node_selections.user_id = ? AND node.status = 1", true, uid).count

:clipboard: Step by Step

:raising_hand_woman: Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!

:floppy_disk: Commit your changes

:twisted_rightwards_arrows: Start a Pull Request.

:checkered_flag: Done Ask in comments for a review :)

:thinking::question: Questions?

Leave a comment below!

We encourage you to link to this issue by mentioning the issue # in your pull request, so we can see if someone's already started on it. If someone seems stuck, offer them some help! Otherwise, take a look at some other issues you can help with. Thanks!
Thank you!

Your help makes Public Lab better! We deeply appreciate your helping refine and improve this site.

[visheshruparelia]

I want to take this up

[ViditChitkara]

Go ahead!!

[visheshruparelia]

I have one doubt tho, what does the 'likes' function above the function 'like_count' do in drupal_user modal? I mean if gets the list of all liked notes why won't the current code of 'like_count' work?

[ViditChitkara]

Actually like_count counts the number of NodeSelections. Whenever a Node is deleted the associated NodeSelection is not deleted which is the reason why like_count and liked_notes give different counts.

[ViditChitkara]

The tab uses liked_count method while the page showing liked notes uses the liked_notes method.

[visheshruparelia]

Yeah true, but then the 'likes' function should look like the liked_notes and like_count no?

[ViditChitkara]

It should. We need to write a migration which destroys all the NodeSelections whose associated nodes do not exist.
Then adding a dependent_destroy against :-
https://github.com/publiclab/plots2/blob/57f0d6f97844b9ba070f7a048fc9a942c1ecf0c8/app/models/node.rb#L46
could solve our problem. Does that make sense?

[visheshruparelia]

Yeah this makes more sense, then I guess the previous commit I made isn't required(If we make the required migration and adding a dependent_destroy), correct?

[ViditChitkara]

Yeah either one of them would do. The latter one would be a better option rather than changing the methods.

[visheshruparelia]

Agreed the later method is more proper than updating methods, I'll update the code acordingly and create a new PR then

[ViditChitkara]

Cool!

[visheshruparelia]

I've made the necessary changes and created a PR here, please review it

[MargaretAN9]

I am a first timer - Can I take this up ?

[aamarill]

hello @ViditChitkara and @jywarren!

It looks like @visheshruparelia has not worked on this for a while. From looking at the latest PR, it looks like it didn't pass Travis tests. Can I try to fix this? Thanks! 😄

[jywarren]

Yes please! And if you want to start from the existing PR, that'd be great.
Thank you!

On Tue, May 22, 2018, 11:25 PM Adan Amarillas [e-mail]
wrote:

hello @ViditChitkara https://github.com/ViditChitkara and @jywarren
https://github.com/jywarren!

It looks like @visheshruparelia https://github.com/visheshruparelia has
not worked on this for a while. From looking at the latest PR, it looks
like it didn't pass Travis tests. Can I try to fix this? Thanks! 😄

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/2298#issuecomment-391208510,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ21WIwOzVsAXJIsNFB-INGgYCVfyks5t1NaJgaJpZM4SBac4
.

[ViditChitkara]

That would be awesome @aamarill . Feel free to ask, if you get stuck anywhere!

[aamarill]

Thanks guys, I thought I replied to this earlier. But I am on it! 💪

[aamarill]

Hey guys in the "installation" section of the README.md you have the following:

Is that correct? The normal workflow I have seen and read in GitHub documentation is to:

1. Fork the original repo
2. Clone from your fork ->git clone https://github.com/your_username/plots2.git, not from the original repo. The original repo is synced via the upstream branch.

I would be glad to fix the README if needed 😄

[jywarren]

Hi, @aamarill that's a really good point. We'd love to have a README fix!!!

[shouri007]

Hi, I see this issue is still open but the PR seems to be merged on the master branch. Can you please let me know if I can claim this bug if it is still open? Thanks :)

[aamarill]

@shouri007 I believe this issue can be closed now. @ViditChitkara can you confirm?
Thanks!

[jywarren]

I believe that's right -- thank you both!

If you're interested, we have another issue that, although it's not quite a first-timers-only issue, is pretty straightforward. We'd be happy to support you on this:

https://github.com/publiclab/plots2/issues/2493

Thanks! 👍 🎉

[madeofhuman]

@jywarren, @gauravano this is still not working. I'd love to pick up this issue.

[madeofhuman]

@jywarren could you please re-open this issue? I just raised a PR for it. Thanks.

[jywarren]

Oh but then i just merged it! Closing again!