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

Comment Likes Does Not Work #5113

Closed
sashadev-sky opened this issue Mar 16, 2019 · 12 comments
Closed

Comment Likes Does Not Work #5113

sashadev-sky opened this issue Mar 16, 2019 · 12 comments
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed JavaScript

Comments

@sashadev-sky
Copy link
Member

Find a comment on any page on the PL website and you will find that clicking its like button results in a POST request to show a 'noty notification' (a green popup to the right saying 'Reacted'! or 'Unreacted!') and does not send any request to actually "like" the comment or update the comment's like button UI to show it has been liked:

Here is an example page:
https://publiclab.org/questions/OrionAllgaier/03-13-2019/questions-from-the-university-of-wisconsin-eau-claire

The code needs to be added to implement the like functionality for a comment. Please reference the PR #4852 to find some files that may be useful to you in implementing this and to note some bugs we experienced while adding like functionality elsewhere and how it was solved.

  • If my branch is not merged yet I would consider creating a new branch that pull it in and working on top of that (comment here if you need help)
  • You will want to make sure any comment likes are debounced
  • Feel free to ignore 'noty notifications' as the PR I have referenced here removes this feature for comment likes and should be merged soon
  • You might need to check your updates on the "unstable" branch because the "button mashing" bug does not present itself in development localhost environment ever. (comment here if you need help)

Thank you!!

@sashadev-sky
Copy link
Member Author

@jywarren Do you think this is descriptive enough for someone to take on this issue with everything in mind from the last one I did or can I improve it?

@sashadev-sky sashadev-sky added bug the issue is regarding one of our programs which faces problems when a certain task is executed JavaScript labels Mar 16, 2019
@jywarren
Copy link
Member

jywarren commented Mar 17, 2019 via email

@CleverFool77
Copy link
Member

@sashadev-sky @jywarren Is this issue available. ?

@sashadev-sky
Copy link
Member Author

@CleverFool77 hey it is! All yours! You have enough information to get started but I’ll post some more things that might help you shortly

@sashadev-sky
Copy link
Member Author

sashadev-sky commented Mar 22, 2019

(This was specifically for liking notes)

javascript driving the like button functionality: https://github.com/publiclab/plots2/blob/master/app/assets/javascripts/like.js

  • initially used promises here to slow down the rate requests can come in if the user mashes the button, then @jywarren suggested I debounce it, so I added that in the erb file to wrap the event listener in:
    // triggers liking and unliking
    <% if current_user %>
    $('#like-button-<%= node.id %>').on('click', debounce(<% if node.liked_by(current_user.uid) %>clickliked<% else %>clicknotliked<% end %>, 200, true));

So now there is both but implement it however you would like! I definitely think my code could be improved on if you have any ideas for the implementation for comments.

@rkpattnaik780
Copy link
Contributor

@sashadev-sky I am looking forward to work on this issue. What if we just increase the size of icons representing the expressions the current user has already reacted? We might need some extra data from back-end as well, I am not sure if the back-end is already checking whether current user has reacted to a given comment.

@stale stale bot added the stale label Oct 7, 2020
@publiclab publiclab deleted a comment from stale bot Oct 8, 2020
@stale stale bot removed the stale label Oct 8, 2020
@jywarren jywarren added this to the Comment editor milestone Oct 13, 2020
@jywarren
Copy link
Member

Just adding some context here, i believe the reactions system was built over the previous comment liking system, and we had hoped that the 👍 reaction would replace "liking" -- in fact, i think i recall that previously made "likes" were converted to 👍 retroactively!

I wonder also if the interface "like" buttons @sashadev-sky had seen are now gone too?

Does that mean that this issues is mostly complete? Is there more we could do to clarify that 👍 == 'like'?

@noi5e
Copy link
Contributor

noi5e commented Jan 24, 2021

@jywarren Doing some research on this.

Liking Notes

The code in like.js seems to apply to liking research notes (not liking comments on research notes).

For example this function here does some eventHandling on #like-button-1234

function changeLikeStatus(node_id, method) {
$('#like-button-' + node_id).off();
$.getJSON("/likes/node/" + node_id + method)
.then(function(resp) {
updateLikeCount(parseInt(resp), node_id);
renderLikeStar(parseInt(resp), node_id);
})
.then(function(resp) {
let method1 = method === "/delete" ? clicknotliked : clickliked
$('#like-button-' + node_id).on('click', method1);
});
}

Which is an ID that appears here:

<span data-toggle="tooltip" data-placement="top" rel="tooltip" title="Liked by <%= node.likers.size %> people" class="btn btn-outline-secondary btn-circle btn-like" node-id="<%= node.id %>" id="like-button-<%= node.id %>">
<% if !current_user %>
<a id="open-login-like" data-hashparams="like" data-toggle="modal" data-target="#loginModal">
<span id="like-star-<%= node.id %>" class="ff fa fa-star"></span>
</a>
<% else %>
<span id="like-star-<%= node.id %>" class="ff fa fa-star<% if !node.liked_by(current_user.uid) %>-o<% end %>"></span>
<% end %>
</span>

That corresponds to this star button below, which is for liking a research note in general:
Screen Shot 2021-01-23 at 7 01 16 PM

Current Video of Liking Notes in Action:
https://user-images.githubusercontent.com/4361605/105620297-49e05d00-5db0-11eb-9d82-c8a3690804f7.mov

Find a comment on any page on the PL website and you will find that clicking its like button results in a POST request to show a 'noty notification' (a green popup to the right saying 'Reacted'! or 'Unreacted!') and does not send any request to actually "like" the comment or update the comment's like button UI to show it has been liked

Current Status of Liking Notes:

  • clicking on star button DOES send a POST request to server to update note's like status—I tested this by liking the note and refreshing the page to see if the star was filled in
  • star button's UI is updated for both liking and disliking the note
  • 'Noty Notification' doesn't seem to appear for either liking or unliking a note, which would be nice.

The code needs to be added to implement the like functionality for a comment. Please reference the PR #4852 to find some files that may be useful to you in implementing this and to note some bugs we experienced while adding like functionality elsewhere and how it was solved.

It seems like this PR #4852 was merged right around the time this issue was created. It seems like that PR actually solves this issue? Not sure what is left over, I'd have to sift through that issue to see.

@noi5e
Copy link
Contributor

noi5e commented Jan 24, 2021

Just browsed #4852, it seems like that PR was fixing a problem with likes via debouncing (liking a note was triggering a million likes and dislikes, GIF here in this note)

I think this issue can be closed... It really seems like the problem's been solved since it was created? Liking notes AND liking comments work as expected, they send POST requests to the server, the models are updated etc.

The only thing that is missing in both cases which would be nice is a Noty Notification appearing for 'Note Liked!' and 'Comment Liked!' I can make an issue in both cases.

@noi5e
Copy link
Contributor

noi5e commented Jan 24, 2021

@jywarren Last note, in reference to this comment:

Just adding some context here, i believe the reactions system was built over the previous comment liking system, and we had hoped that the 👍 reaction would replace "liking" -- in fact, i think i recall that previously made "likes" were converted to 👍 retroactively!

I think that was done in 2018 in this PR #2869, so that was already in place by the time this issue was made.

Can you review and close this issue if you think that's appropriate?

@noi5e
Copy link
Contributor

noi5e commented Jan 24, 2021

I just did a little more browsing to make absolutely certain that things are working like they should be. I still think they are! Just leaving a trail here so that others can follow.

Clicking on the star button to like a note calls the following (event attached to #like-button-1234):

$('#like-button-<%= node.id %>').on('click', debounce(<% if node.liked_by(current_user.uid) %>clickliked<% else %>clicknotliked<% end %>, 200, true));

clickliked is defined here in like.js:

function clickliked() {
var node_id = $(this).attr('node-id');
changeLikeStatus(node_id, "/delete");
}

changeLikeStatus is what actually sends a request to the server:

function changeLikeStatus(node_id, method) {
$('#like-button-' + node_id).off();
$.getJSON("/likes/node/" + node_id + method)
.then(function(resp) {
updateLikeCount(parseInt(resp), node_id);
renderLikeStar(parseInt(resp), node_id);
})
.then(function(resp) {
let method1 = method === "/delete" ? clicknotliked : clickliked
$('#like-button-' + node_id).on('click', method1);
});
}

The route `/likes/node/123/create' is defined here:

def create
render json: Node.like(params[:id], current_user)
end

Node.like is defined here:

plots2/app/models/node.rb

Lines 1093 to 1125 in 38e37eb

def self.like(nid, user)
# scope like variable outside the transaction
like = nil
count = nil
ActiveRecord::Base.transaction do
# Create the entry if it isn't already created.
like = NodeSelection.where(user_id: user.uid,
nid: nid).first_or_create
like.liking = true
node = Node.find(nid)
if node.type == 'note' && !UserTag.exists?(node.uid, 'notify-likes-direct:false')
SubscriptionMailer.notify_note_liked(node, like.user).deliver_now
end
if node.uid != user.id && UserTag.where(uid: user.id, value: ['notifications:all', 'notifications:like']).any?
notification = Hash.new
notification[:title] = "New Like on your research note"
notification[:path] = node.path
option = {
body: "#{user.name} just liked your note #{node.title}",
icon: "https://publiclab.org/logo.png"
}
notification[:option] = option
User.send_browser_notification [user.id], notification
end
count = 1
node.toggle_like(like.user)
# Save the changes.
node.save!
like.save!
end
count
end

The DB is updated here to show that user liked the node:

plots2/app/models/node.rb

Lines 1079 to 1091 in 38e37eb

def toggle_like(user)
node_likes = NodeSelection.where(nid: id, liking: true)
.joins(:user)
.references(:rusers)
.where(liking: true)
.where('rusers.status': 1)
.size
self.cached_likes = if is_liked_by(user)
node_likes - 1
else
node_likes + 1
end
end

@jywarren (or anyone else that can review this), can you double-check the above to see that all the parts are working as expected? I think they are, AFAIK.

If everything looks okay, I recommend we close this issue, and open a new one: "Display Notification When User Likes Research Note"

@jywarren
Copy link
Member

This all looks right, and esp the part about:

I think that was done in 2018 in this PR #2869, so that was already in place by the time this issue was made.

as that is specific to liking individual comments, not just notes. Sounds good to me and THANK YOU SO MUCH for digging through this so thoroughly!!!

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 JavaScript
Projects
None yet
Development

No branches or pull requests

5 participants