-
Notifications
You must be signed in to change notification settings - Fork 9
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
CU-bcqgq6 Feature/design chat area #9
Conversation
2ac32c0
to
7b074b4
Compare
3740068
to
10b96b6
Compare
bdb6d5c
to
9442fcd
Compare
Task linked: CU-bcqgq6 Basic Chat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! One change I'd make is noted in the comments.
@@ -1,7 +1,7 @@ | |||
# frozen_string_literal: true | |||
|
|||
class ChatMessagesController < ApplicationController | |||
before_action :messages_params | |||
before_action :messages_params, only: [:create] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ActionCable.server.broadcast( | ||
"live_video_#{message.video_id}", | ||
{text: message.body, user_name: current_user.full_name, video_id: message.video_id}, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
module ChatHelper | ||
def current_user_id | ||
user_signed_in? ? current_user.id : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like we should return something falsey here, right? Maybe even false
itself?
user_name: user.username.capitalize, | ||
}, | ||
) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 much better place for this
No description provided.