Conversation
|
|
||
| speak:(message) -> | ||
| @perform 'speak', message: message | ||
| to_id =$('input:hidden[name="to_id"]').val() |
| speak:(message) -> | ||
| @perform 'speak', message: message | ||
| to_id =$('input:hidden[name="to_id"]').val() | ||
| @perform 'speak', {message: message,to_id:to_id} |
|
|
||
| def speak(data) | ||
| Message.create! content: data['message'], user_id: current_user.id | ||
| user=User.find_by(id: data['to_id']) |
There was a problem hiding this comment.
idで検索するならfind(data[‘to_id’])の方が良いかな、という気がします。
find_byだと、対象のレコードがない時にnilを返すので、この先でnull参照して落ちます。
findの場合、RecordNotFoundを発生します。
|
|
||
| def message_room_id(first_user, second_user) | ||
| first_id = first_user.id.to_i | ||
| second_id = second_user.id.to_i |
There was a problem hiding this comment.
19-20行で=位置を揃えると可読性が上がります。
first_id = first_user.id.to_i
second_id = second_user.id.to_i
| @messages = Message.all | ||
| @user=User.find_by(id: params[:id]) | ||
| @room_id = message_room_id(current_user, @user) | ||
| @messages = Message.where(room_id: @room_id) |
There was a problem hiding this comment.
@user後の=前後にはスペースをあけましょう
また、ここの3行でも=位置を揃えましょう
There was a problem hiding this comment.
view側の実装を見ていないのでなんとも言えないですが、
room_idをview側で使うことがないのではないかな？ 🤔 と思ったので、ローカル変数でいい気がしてきました。
あと、
messageは時系列順に並んで欲しい気がするので、orderを明示的に指定してあげたほうがいいと思います！
多分よく使うと思うので、
継承元のApplicationRecordでscopeを定義しておくと幸せなのかな、と思いました！
| <input type="hidden" name="to_id" value="<%= @user.id %>"> | ||
| <% if current_user.id==@user.id %> | ||
| <label>It's your space. Write down whatever you like here! :</label><br> | ||
| <% else%> |
| <% if current_user.id==@user.id %> | ||
| <label>It's your space. Write down whatever you like here! :</label><br> | ||
| <% else%> | ||
| <label>Send messages to <%= @user.username%>:</label><br> |
| <div class="users-index-item"> | ||
| <div class="user-right"> | ||
| <p><%= link_to user.username,root_url%></p> | ||
| <p><%= link_to user.username,"/rooms/#{user.id}"%></p> |
| t.integer "user_id" | ||
| t.integer "from_id" | ||
| t.integer "to_id" | ||
| t.string "room_id" |
| # by default. You can change it below and use your own secret key. | ||
| # config.secret_key = '36225be56b61efa00ddd192276401308319f57986469c4f528653eda4f44aed253fc887a07711360b0b3da4e641427cf3e02bd71350b62c10b83407f6acd7d1d' | ||
|
|
||
| config.secret_key = '6ff05a2a5b47194687fe711bcec8a4dfed7f7350c2ab7842cb7cfe8ce0bf52b259b2b72f7635341c80f6437df9fe39b0e5f75a4bbce788bc249a5b6ce1de2f38' |
There was a problem hiding this comment.
このようにAuthに関連するキーなどはgitにコミットしないようにしましょう
環境変数にして、環境変数ファイルごとgitignoreすることをおすすめします
| "#{first_user.id}-#{second_user.id}" | ||
| else | ||
| "#{second_user.id}-#{first_user.id}" | ||
| end |
|
|
||
| def show | ||
| @messages = Message.all | ||
| @user=User.find_by(id: params[:id]) |
| @messages = Message.where(room_id: @room_id) | ||
| end | ||
|
|
||
| def message_room_id(first_user, second_user) |
There was a problem hiding this comment.
app/channels/room_channels.rbでも同じメソッドがあるので、dryにしましょう！
| has_many :messages,dependent: :destroy | ||
|
|
||
| # Validations | ||
| validates :email, presence: true |
There was a problem hiding this comment.
たぶんdeviseの認証かと思いますが、validationがザルなのでちゃんとしてあげた方が良いですね！
|
|
||
| # Validations | ||
| validates :email, presence: true | ||
| validates :username, presence: true |
There was a problem hiding this comment.
あと、ここもlengthくらいは入れた方がいいと思います！
厳密には文字コードの問題もあるので、ハイフンとかチルダもいい感じにするカスタムバリデーションを定義すると良いです！
|
|
||
| devise_for :user | ||
| get 'rooms/show'=> 'rooms#show' | ||
| get 'rooms/:id'=> 'rooms#show' |
There was a problem hiding this comment.
resource :room, only: %w(show)
の方がRails！って感じです。
3 participants
既にmasterに全てマージされている為、masterブランチのコードレビューをお願いします。(merge先のブランチはかなり前のやつで関係ないです）
アプリ概要
主な機能：
メール認証によるユーザー登録機能
ログイン/ログアウト機能
登録情報の変更機能
１対１でのチャット機能（メイン）
参考にしたサイト：
Deviseの導入：https://qiita.com/cigalecigales/items/f4274088f20832252374
ActionCableの導入：https://qiita.com/jnchito/items/aec75fab42804287d71b
１対１チャット機能の実装：https://qiita.com/YN6127yn/items/7ddd966141cca195b4da
DB設計：https://railsguides.jp/association_basics.html