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

action cable example with nested comments #1

Open
balakirevs opened this issue Jun 16, 2018 · 41 comments
Open

action cable example with nested comments #1

balakirevs opened this issue Jun 16, 2018 · 41 comments

Comments

@balakirevs
Copy link

balakirevs commented Jun 16, 2018

Hi,
Thank you for the app example, very useful.

Trying implement the same but with action cable to make it more live. It works though but the issue is doubled comments displayed & on reply incorrectly rendered i.e displayed as a comment but not a reply.
I've added the following code to your existing app:

js/channel/comments.js:

App.comments = App.cable.subscriptions.create('CommentsChannel', {
  collection: function() {
    return $("[data-channel='comments']");
  },

  connected: function() {
    return setTimeout((function(_this) {
      return function() {
        return _this.followCurrentPost();
      };
    })(this), 1000);
  },

  received: function(data) {
    if (!this.userIsCurrentUser(data.comment)) {
      return this.collection().append(data.comment);
    }
  },

  userIsCurrentUser: function(comment) {
    return $(comment).attr('data-user-id') === $('meta[name=current-user]').attr('id');
  },

  disconnected: function() {},

  followCurrentPost: function() {
    var commentableId;
    commentableId = this.collection();
    if (commentableId = this.collection().data('commentable-id')) {
      return this.perform('follow', {
        commentable_id: commentableId
      });
    } else {
      return this.perform('unfollow');
    }
  },

  installPageChangeCallback: function() {
    if (!this.installedPageChangeCallback) {
      this.installedPageChangeCallback = true;
      return $(document).on('turbolinks:load', function() {
        return App.comments.followCurrentPost();
      });
    }
  }
});

channels/application_cable/connection.rb:

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    identified_by :current_user

    def connect
      self.current_user = find_verified_user
      logger.add_tags 'ActionCable', current_user.email
    end

    protected

    def find_verified_user
      verified_user = User.find_by(id: cookies.signed['user.id'])
      if verified_user && cookies.signed['user.expires_at'] > Time.now
        verified_user
      else
        reject_unauthorized_connection
      end
    end
  end
end

channels/comments_channel.rb:

class CommentsChannel < ApplicationCable::Channel
  def follow(data)
    stop_all_streams
    stream_from "#{data['commentable_id'].to_i}:comments"
  end

  def unfollow
    stop_all_streams
  end
end

jobs/broadcasts_jobs.rb:

class BroadcastCommentJob < ApplicationJob
  queue_as :default

  def perform(comment)
    ActionCable.server.broadcast "#{comment.commentable_id}:comments",
                                 comment: render_comment(comment)
  end

  private

  def render_comment(comment)
    ApplicationController.render(partial: 'comments/comment', locals: { comment: comment } )
  end
end

comment.rb:

after_create_commit { BroadcastCommentJob.perform_now self }

views/comments/_comment.html.erb:

<li id="comment_<%= comment.id %>" class="comment" data-user-id="<%= comment.user.id %>">
  <div class="comment-content">
    <%= comment.content %>
  </div>
  <div class="comment-info">
    <small>-
      <%= comment.user.name %> &bull;
      <%= localize(comment.created_at, format: :long) %> &bull;
      <%= link_to "Edit", edit_polymorphic_path([comment.commentable, comment]), class: 'edit-comment-link', remote: true  %> &bull;
      <%= link_to "Destroy", [comment.commentable, comment], method: :delete, class: 'delete-comment-link', data:{confirm:"Are your sure?"}, remote: true %> &bull;
      <%= link_to "Reply", polymorphic_path([:reply, comment.commentable, comment]), class: 'reply-comment-link', remote: true  %>
    </small>
  </div>
  <% if comment.replies.any? %>
    <ul>
      <%= render comment.replies %>
    </ul>
  <% end %>
</li>

views/comments/_widget.html.erb:

<div id="comments-widget-of-commentable-<%= commentable.id %>" class="comments-widget">
  <h2 class="comments-header">
    Comments <small id="comments-count-of-commentable-<%= commentable.id %>"><%= commentable.comments.size %></small>
  </h2>
  <ul class="comments-list" data-channel="comments" data-commentable-id="<%= commentable.id %>">
    <%= render commentable.comments.where(parent: nil) %>
  </ul>
  <!-- comment-form -->
  <%= render "comments/form", comment: commentable.comments.build  %>
</div>

application.html.erb:

<!DOCTYPE html>
<html>
  <head>
    <title>RORLA-Blog</title>
    <meta name="viewport" content="width=device-width, user-scalable=no">
    <%= csrf_meta_tags %>
    <%= stylesheet_link_tag    'application', media: 'all', 'data-turbolinks-track': 'reload' %>
    <%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>
    <%= favicon_link_tag 'rorlab_logo.png' %>
    <% if user_signed_in? %>
      <%= tag :meta, name: 'current-user', data: { id: current_user.id } %>
    <% end %>
  </head>

  <body>
    <div class="container">
      <header class='row'>
        <%= render "shared/header" %>
      </header>
      <div id="trunk" class='row'>
        <main class='col-md-9'>
          <div id="flash_messages"><%= flash_messages(:close) %></div>
          <%= yield %>
        </main>
        <aside class='col-md-3'>
          <%= render "shared/sidebar" %>
        </aside>
      </div>
      <footer class='row'>
        <%= render 'shared/footer' %>
      </footer>
    </div>
  </body>
</html>

routes.rb:

mount ActionCable.server => '/cable'

cable.yml:

development:
  adapter: redis
  url: redis://localhost:6379/1

Gemfile:

gem 'redis'

config/initializers/warden_hooks.rb:

Warden::Manager.after_set_user do |user, auth, opts|
  scope = opts[:scope]
  auth.cookies.signed["#{scope}.id"] = user.id
  auth.cookies.signed["#{scope}.expires_at"] = 60.minutes.from_now
end

Warden::Manager.before_logout do |_user, auth, opts|
  scope = opts[:scope]
  auth.cookies.signed["#{scope}.id"] = nil
  auth.cookies.signed["#{scope}.expires_at"] = nil
end
@luciuschoi
Copy link
Owner

luciuschoi commented Jun 18, 2018 via email

@balakirevs
Copy link
Author

balakirevs commented Jun 18, 2018

Hi, thank you for reply.
i'm using pg with action cable, with sqlite3 I have not tried it.
Probably, try to switch to postgres and please add this missing code and it should work:
As i said on replies partial is double rendered...
Thank you

views/comments/_comment.html.erb -> line 1

class="comment" data-user-id="<%= comment.user.id %>"

views/comments/_widget.html.erb -> line 5

 data-channel='comments' data-commentable-id="<%= commentable.id %>">

@luciuschoi
Copy link
Owner

Oh~ It works.
But there were found two bugs: 1) when a comment is created, comment partial template is rendered twice on the sender side but once normally on the receiver side. 2) when a reply is created, comment partial template is rendered on the correct position just below the comment on the sender side but appended at the bottom of messages on the receiver side.

Is that right?

@balakirevs
Copy link
Author

Correct. Currently i have not found how to fix it.

@luciuschoi
Copy link
Owner

OK, I'll also try to find how to fix it.
If the bugs were to be fixed, your ActionCable version of nested comment module might be very useful.
Thank you.

@balakirevs
Copy link
Author

Thank you too.

@luciuschoi
Copy link
Owner

Voila~ finally, it works. Bugs were disappeared.

  1. First of all, there was a bug in create.js.erb.
    : I removed append function. This code line caused the comment created to be rendered twice
  2. Secondly, when its reply is created for a comment, it was appended into whole collection for comments. So, I added custom data attribute data-channel='comment-<%=comment.id %>' to _comment.html.erb 1st line. And I updated collection and received callback function in comments.js.

But this is only for creating commens or replies but not for editing and destroying. Maybe, this is also possible on adding codes.

Good luck!

@luciuschoi
Copy link
Owner

Oh, I've also updated balakirevs branch.

@balakirevs
Copy link
Author

Nice! Thank you!

@luciuschoi
Copy link
Owner

I'm sorry that I've updated your markdown codes in your comments without your permission for pretty viewing.

@balakirevs
Copy link
Author

:) it became much nicer.

@balakirevs
Copy link
Author

balakirevs commented Jun 19, 2018

I think we should add also that user can edit destroy his own comments not others, as well as if parent comment has replies we should not update or destroy it any more...
And it would be also nice to provide some notification system using also action cable to notify user that smb created post or made some comments.

If you do not mind i will provide it a bit later or what do you think?

@luciuschoi
Copy link
Owner

Oh~ What a wonderful idea!
Thank you so much.

@luciuschoi
Copy link
Owner

As you mentioned, it was so easy to disable to update or destroy a comment or a reply if it had already replies. In _comment.html.erb file, if clause was given to edit and destroy links.

<% if comment.replies.size.zero? %>
  <%= link_to "Edit", edit_polymorphic_path([comment.commentable, comment]), class: 'edit-comment-link', remote: true  %> &bull;
  <%= link_to "Destroy", [comment.commentable, comment], method: :delete, class: 'delete-comment-link', data:{confirm:"Are your sure?"}, remote: true %> &bull;
<% end %>

@luciuschoi
Copy link
Owner

A redundant code was found:

commentableId = this.collection();
if (commentableId = this.collection().data('commentable-id') { ···· }

Finally, codes are as follows:

  followCurrentPost: function () {
    var commentableId;
    commentableId = this.collection().data('commentable-id');
    if (commentableId) {
      return this.perform('follow', {
        commentable_id: commentableId
      });
    } else {
      return this.perform('unfollow');
    }
  },

Is that right?

@balakirevs
Copy link
Author

Ah yes, genius as for disabling the links!
may be we should also put smth like this in the create.js.erb in order not to refresh the page after reply to not see the links:

$edit_link = $comment.find('a.edit-comment-link')[0]
$delete_link = $comment.find('a.delete-comment-link')[0]
$delete_link.remove();
$edit_link.remove();

@balakirevs
Copy link
Author

sure! sorry for the redundancy.

@luciuschoi
Copy link
Owner

Yes, we should do.
I'll add your suggestion to the create.js.erb file.

@luciuschoi
Copy link
Owner

Yeap!!!
I've just added codes for removing edit and destroy link after creating its reply, which was mentioned by @balakirevs. Yes, it works well. So I've updated balakirevs branch.

@balakirevs
Copy link
Author

balakirevs commented Jun 20, 2018

Thanks @luciuschoi!
finishing comments topic lets add that user can edit and destroy his own comments, not others.

Gemfile:

gem 'cancancan', '~> 2.0'

rails g cancan:ability

in the generated models/ability.rb:

class Ability
  include CanCan::Ability

  def initialize(user)
    user ||= User.new
    can [:edit, :update, :destroy], Comment, user_id: user.id
    can [ :read, :show, :create, :reply], Comment
  end
end

after before_action in controllers/comments_controller.rb:

load_and_authorize_resource :comment, param_method: :comment_params

comments/_comment.html.erb:

<span class='dot-bullet'><%= link_to "Edit", edit_polymorphic_path([comment.commentable, comment]), class: 'edit-comment-link', remote: true if can? :update, comment %></span>
        <span class='dot-bullet'><%= link_to "Destroy", [comment.commentable, comment], method: :delete, class: 'delete-comment-link', data:{confirm:"Are your sure?"}, remote: true if can? :destroy, comment %></span>

now user can edit destroy only his own comments not others.

To do fix: currently we need to refresh page in order to see edit destroy links of user own comment.

just came accros here with the problem of warden proxy bug due to devise (ActionView::Template::Error (Devise could not find the Warden::Proxy instance on your request environment)
heartcombo/devise#4271
or more details on it
https://evilmartians.com/chronicles/new-feature-in-rails-5-render-views-outside-of-actions

so we need to do some workaround:

jobs/brodcast_comment_job.rb:

class BroadcastCommentJob < ApplicationJob
  before_perform :wardenize
  queue_as :default

  def perform(comment)
    ActionCable.server.broadcast "#{comment.commentable_id}:comments",
                                 comment: render_comment(comment), parent_id: comment&.parent&.id
  end

  private

  def render_comment(comment)
    @job_renderer.render(partial: 'comments/comment', locals: { comment: comment })
  end

  def wardenize
    @job_renderer = ::ApplicationController.renderer.new
    renderer_env = @job_renderer.instance_eval { @env }
    warden = ::Warden::Proxy.new(renderer_env, ::Warden::Manager.new(Rails.application))
    renderer_env['warden'] = warden
  end
end

models/comment.rb:

remove

 after_create_commit { BroadcastCommentJob.perform_now self }

in controllers/comments_controller.rb:

if @comment.save
        BroadcastCommentJob.perform_now @comment

@balakirevs
Copy link
Author

sure, notification to be continued...

@balakirevs
Copy link
Author

balakirevs commented Jun 20, 2018

Notification (after creating a comment - notifying users who participating in current post discussion (comments)):

Gemfile:

gem 'activity_notification'
gem 'js_cookie_rails'   // we need it to get current user cookie id for action cable

app/assets/javascripts/application.js:

//= require js.cookie

create devise controller app/controllers/users/sessions_controller.rb

class Users::SessionsController < Devise::SessionsController

  def create
    super
    cookies[:current_user_id] = current_user.id
  end
end

generating activity_notification necessary files:

$ bin/rails generate activity_notification:install
$ bin/rails generate activity_notification:migration
$ bin/rake db:migrate
$ bin/rails generate activity_notification:views -v notifications
$ bin/rails generate activity_notification:views users

if you have problems with migration just add [5.1] at the end of the CreateActivityNotificationTables migration file:
before

class CreateActivityNotificationTables < ActiveRecord::Migration

after

class CreateActivityNotificationTables < ActiveRecord::Migration[5.1]

add in models/user.rb:

acts_as_target email: :email, batch_email_allowed: :confirmed_at

add in models/post.rb:

has_many :commented_users, through: :comments, source: :user

add in models/comment.rb:

belongs_to :post, optional: true
...

acts_as_notifiable :users,
                     targets: ->(comment, key) {
                       ([comment.commentable.user] + comment.commentable.commented_users.to_a - [comment.user]).uniq
                     },
                     notifiable_path: :post_notifiable_path,
                     notifier: :user,
                     group: :commentable,
                     tracked: { only: [:create] }

  def post_notifiable_path
    post_path(post)
  end

config/routes.rb:

devise_for :users, controllers:{ sessions: 'users/sessions' }
  notify_to :users, with_devise: :users

views/shared/_header.html.erb:

<div id="notifications_in_header">
 <%= render_notifications_of current_user, index_content: :unopened_with_attributes %>
</div>

in newly generated by gem folder 'activity_notification' create a file:
views/activity_notification/_socket_notifications.html.erb

<%= link_to notification.notifier.name + ' commented on your post', post_path(notification.group) %>

create directory comment and _create.html.erb file views/activity_notification/notifications/users/comment/_create.html.erb

<%= link_to notification.target.name + ' commented on your post', post_path(notification.group) %>

controllers/comments_controller.rb:

 def create
    @comment = @commentable.comments.new(comment_params)
    @comment.user = current_user
    respond_to do |format|
      if @comment.save
        BroadcastCommentJob.perform_now @comment

        ActivityNotification::Notification.notify :users, @comment, key: "comment.create", notifier: @comment.user, group: @commentable
        notification_targets(@comment, "comment.create" ).each do |target_user|
          BroadcastNotificationJob.perform_later target_user, @comment
        end
        format.js
      else
        format.html { render :back, notice: "Comment was not created." }
        format.json { render json: @comment.errors }
        format.js
      end
    end
  end

private

  def notification_targets comment, key
    ([comment.commentable.user] + comment.commentable.commented_users.to_a - [comment.user]).uniq
  end

create a job for notification

jobs/broadcast_notification_job.rb:

class BroadcastNotificationJob < ApplicationJob
  queue_as :default

  def perform(user, comment)
    ActionCable.server.broadcast "notification_channel_#{user.id}",
                                 notifications: render_notifications_for(user),
                                 message: user.name + ' commented on your post',
                                 link: 'posts/' + comment.commentable.id.to_s
  end

  private
  def render_notifications_for user
    ApplicationController.renderer.render partial: "activity_notification/socket_notifications",
                                          locals: {user: user}
  end
end

finally lets add action cable to have live notifications:

js/channel/notifications.js:

App.notifications = App.cable.subscriptions.create(
  {channel: 'NotificationChannel', user_id: Cookies.get('current_user_id')},
  {
    connected: function () {},
    disconnected: function () {},
    received: function (data) {
      console.log('new notification');
      $('#notifications_in_header').empty().html(data.notifications);
      $('.dropdown_notification').dropdown();
    }
  });

channels/notifications_channel.rb:

class NotificationsChannel < ApplicationCable::Channel
  def subscribed
    stop_all_streams
    stream_from "notification_channel_#{params[:user_id]}"
  end

  def unsubscribed
    stop_all_streams
  end
end

views/layouts/applicationhtml.erb:
add action cable meta tag in head

<%= action_cable_meta_tag %>

Now you should receive notifications of newly arrived comments.

P.S. we can further experiment with it by doing other custom solutions or by using other gems like public_activity which is almost the same.

Otherwise there are plenty of good screencasts of notifications:
https://gorails.com/search?utf8=%E2%9C%93&q=notifications

Thanks!

@luciuschoi
Copy link
Owner

comments/_comment.html.erb :

<span class='dot-bullet'><%= link_to "Edit", edit_polymorphic_path([comment.commentable, comment]), class: 'edit-comment-link', remote: true if can? :update, comment %></span>
<span class='dot-bullet'><%= link_to "Destroy", [comment.commentable, comment], method: :delete, class: 'delete-comment-link', data:{confirm:"Are your sure?"}, remote: true if can? :destroy, comment %></span>

This is NOT well-functioning. That is, when broadcasting after creating replies, their accessibility is already setup and so if condition is not functioning in _comment.html.erb partial.

Do you understand what I mean? It is so difficult to explain this context in detail with my poor English.

@balakirevs
Copy link
Author

balakirevs commented Jun 20, 2018

OK, may be that was the problem.
We need to find a solution for it.
Thanks for pointing out.

@luciuschoi
Copy link
Owner

Yeap!!!
I have solved it. But a bug exists: a local variable(broadcasted) cannot be access in partial template.
I absolutely don't know why.
I have already pushed to balakirevs branch.
Would you find out the cause? @balakirevs

@balakirevs
Copy link
Author

balakirevs commented Jun 21, 2018

Thanks ! Will try to figure out...

@balakirevs
Copy link
Author

balakirevs commented Jun 21, 2018

i think i found, in view/comments/_comment.html.erb try this:

<%= render comment.replies, broadcasted: false %>

i.e you should pass smth in comment.replies

@balakirevs
Copy link
Author

balakirevs commented Jun 21, 2018

for the better performance i think we should replace in _widget.html.erb this

<%= render partial: 'comments/comment', collection: commentable.comments.where(parent: nil), locals: { broadcasted: true } %>

by this

<%= render commentable.comments.includes(:user, :replies, :commentable).where(parent: nil).reverse, broadcasted: true  %>

i put reverse here as when i had lots of comments some of them appeared at the top... otherwise we can research this issue

@luciuschoi
Copy link
Owner

luciuschoi commented Jun 22, 2018

@klashe : "uninitialized constant ActivityNotification::Target::Association" error was already known.
Can you try this link: simukappu/activity_notification#52

@luciuschoi
Copy link
Owner

luciuschoi commented Jun 22, 2018

At the commit number ad51927a95cc2963c04531357215b0eb669fb69c, N+1 query and weird (actually, not weird, lol) not-recognized partial local variable problems were solved with help of @balakirevs. It's great. Finally, _commit_broadcasted.html.erb file was needed no more and deleted completely. Thanks. Your notification suggestion is not yet applied on balakrevs branch. Until now, it works well like we intended. I am ready to try @balakirevs' notification codes.

@luciuschoi
Copy link
Owner

Updated setup.sh and Vagrantfile to work well.

@luciuschoi
Copy link
Owner

luciuschoi commented Jun 23, 2018

Now, when you edit and destroy comments or replies already created, also they will be broadcasted to all subscribers. Commit number: 90bf1c48555a3aa1fa107509f415b051b8fd7117

@balakirevs
Copy link
Author

You even went further :) Nice job!

@balakirevs
Copy link
Author

there is a small bug, when creating a comment a form completely disappears, needs page to be refreshed....

@balakirevs
Copy link
Author

could you check please, in create.js.erb on line 20 remove this

$comment_form.remove();

and put it somewhere after line 22

<% if @comment.parent.present? %>
...
$restore_link = $comment.find('a.delete-comment-link')[0];
$reply_link = $comment.find('a.reply-comment-link')[0];
$reply_link.href = $restore_link.href + "/reply";

$comment_form.remove();

@luciuschoi
Copy link
Owner

luciuschoi commented Jun 25, 2018

Oh, I'm so sorry to be late.
Over the last weekend, I was sick with common cold with general ache.
0ae268d2ddc95a3bcb445c608be1ffb3a8d07af0

@balakirevs
Copy link
Author

Now you feel better?

@luciuschoi
Copy link
Owner

@balakirevs : Where is thenotification object in notification.notifier.name from?

: views/activity_notification/_socket_notifications.html.erb

<%= link_to notification.notifier.name + ' commented on your post', post_path(notification.group) %>

Rendering error on performing BroadcastNotificationJob occurs.

[ActiveJob] [BroadcastNotificationJob] [f62a11e6-8ac4-4205-bfea-39bc35ec3d2d] Performing BroadcastNotificationJob (Job ID: f62a11e6-8ac4-4205-bfea-39bc35ec3d2d) from Async(default) with arguments: #<GlobalID:0x00007fa8dae3ebc8 @uri=#<URI::GID gid://vagrant/User/2>>, #<GlobalID:0x00007fa8dae3d778 @uri=#<URI::GID gid://vagrant/Comment/467>>
[ActiveJob] [BroadcastNotificationJob] [f62a11e6-8ac4-4205-bfea-39bc35ec3d2d]   Rendered activity_notification/_socket_notifications.html.erb (78.1ms)
[ActiveJob] [BroadcastNotificationJob] [f62a11e6-8ac4-4205-bfea-39bc35ec3d2d] Error performing BroadcastNotificationJob (Job ID: f62a11e6-8ac4-4205-bfea-39bc35ec3d2d) from Async(default) in 118.23ms: ActionView::Template::Error (undefined local variable or method `notification' for #<#<Class:0x00007fa8db12b980>:0x00007fa8dae37058>

@balakirevs
Copy link
Author

could you try to put this :
<%= render_notifications_of user, index_content: :unopened_with_attributes %>

into views/activity_notification/_socket_notifications.html.erb

And i will check a bit later why i put this :
<%= link_to notification.notifier.name + ' commented on your post', post_path(notification.group) %>
probably i mistyped

@luciuschoi
Copy link
Owner

On this time, the error was disappeared but the number of notifications is not updated in real time.
Additionally, I think the user name of notification message should be one who wrote the comment.
Would you check it up?

@balakirevs
Copy link
Author

with this change i have notifications in real time...
could you show me the code ?

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

No branches or pull requests

2 participants