Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions app/controllers/api/v1/chats_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
class Api::V1::ChatsController < ApplicationController
def index
room = Room.find_by(key: params[:room_key])
if room.blank?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI
こんな感じで毎回チェックするの面倒なので

ApiCommonにこんなの追加して

module ApiCommon
  included do
    rescue_from ActiveRecord::RecordNotFound do |_e|
      render json: { error: t("404 error") }, status: 404
    end
  end
end
class Api::V1::ChatsController < ApplicationController
  def index
    room = Room.find(key: params[:room_key])
    ...
  end
end

って書けた方が冗長な箇所がなくなって綺麗になると思う

render json: { error: t("404 error") }, status: 404
return
end

if params[:cursor].present?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このifのチェックいらない気がする?
下のChat.find_byでnilが返るだけなので
https://github.com/cyder/SyncPod-server/pull/110/files#diff-95f5b6164012ab93315ecba597608414R17 は意味を為してない気がします

cursor = Chat.find_by(id: params[:cursor])
if cursor.blank?
render json: { error: t("404 error") }, status: 404
return
end
@chats = room.past_chats(10, cursor)
else
@chats = room.past_chats(10)
end
end
end
6 changes: 5 additions & 1 deletion app/models/chat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class Chat < ApplicationRecord
end

scope :latest_by, ->(num) do
order(created_at: :desc).limit(num)
order(id: :desc).limit(num)
end

scope :before, ->(cursor) do
where("id < ?", cursor)
end
end
8 changes: 6 additions & 2 deletions app/models/room.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ def last_played_video
videos.ended.order_by_end.last
end

def past_chats(num)
chats.latest_by(num).reverse
def past_chats(num, cursor = nil)
if cursor.blank?
chats.latest_by(num).reverse
else
chats.before(cursor).latest_by(num).reverse
end
end

def online_users
Expand Down
3 changes: 3 additions & 0 deletions app/views/api/v1/chats/index.json.jbuilder
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
json.chats @chats do |chat|
json.partial! "chats/chat", chat: chat
end
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
get :recommend
end
end
resources :chats, only: [:index]
get "joined_rooms", to: "users#joined_rooms"
get "youtube/search", to: "youtube#search"
get "youtube/video", to: "youtube#video"
Expand Down
4 changes: 2 additions & 2 deletions spec/channels/room_channel_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@
before { chat.save! }
it "expect to have broadcast with chat" do
expect { subject }.to have_broadcasted_to(target).with { |data|
expect(data).to be_json_eql(%("#{chat.message}")).at_path("data/past_chats/0/message")
expect(data).to be_json_eql(user.id).at_path("data/past_chats/0/user/id")
expect(data).to be_json_eql(%("#{chat.message}")).at_path("data/past_chats/1/message")
expect(data).to be_json_eql(user.id).at_path("data/past_chats/1/user/id")
}
end
end
Expand Down
11 changes: 11 additions & 0 deletions spec/models/room_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,17 @@
}
it { is_expected.to eq [chat2, chat3] }
end

context "with cursor" do
subject { room.past_chats 2, chat3.id }

before {
chat1.save!
chat2.save!
chat3.save!
}
it { is_expected.to eq [chat1, chat2] }
end
end

describe "#online_users" do
Expand Down
53 changes: 53 additions & 0 deletions spec/requests/api/v1/chats_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
require "rails_helper"

describe "chats" do
let(:room) { create(:room) }

describe "GET /api/v1/chats" do
let(:params) { { room_key: room.key } }
let!(:chat) { create(:user_chat, room: room) }

context "with valid params" do
it "returns a chat", :autodoc do
is_expected.to eq 200
body = response.body
expect(body).to be_json_eql(chat.id).at_path("chats/0/id")
end
end

context "without room_key" do
let(:params) { {} }

it { is_expected.to eq 404 }
end

context "with invalid room_key" do
let(:params) { { room_key: "invalid_key" } }

it { is_expected.to eq 404 }
end

context "with valid cursor" do
let(:before_chat) { create(:user_chat, room: room) }
let(:params) { { room_key: room.key, cursor: before_chat.id } }

it "returns a chat", :autodoc do
is_expected.to eq 200
body = response.body
expect(body).to be_json_eql(chat.id).at_path("chats/0/id")
end

it "dose not contain before_chat" do
is_expected.to eq 200
body = response.body
expect(body).not_to have_json_path("chats/1/id")
end
end

context "with invalid cursor" do
let(:params) { { room_key: room.key, cursor: "invalid" } }

it { is_expected.to eq 404 }
end
end
end