-
Notifications
You must be signed in to change notification settings - Fork 3
ページング付きのchat rest apiを用意した #110
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
base: development
Are you sure you want to change the base?
Conversation
c2b24c1 to
f1fbf63
Compare
Codecov Report
@@ Coverage Diff @@
## development #110 +/- ##
===============================================
+ Coverage 97.59% 97.68% +0.09%
===============================================
Files 33 34 +1
Lines 415 432 +17
===============================================
+ Hits 405 422 +17
Misses 10 10
Continue to review full report at Codecov.
|
| class Api::V1::ChatsController < ApplicationController | ||
| def index | ||
| room = Room.find_by(key: params[:room_key]) | ||
| if room.blank? |
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.
FYI
こんな感じで毎回チェックするの面倒なので
ApiCommonにこんなの追加して
module ApiCommon
included do
rescue_from ActiveRecord::RecordNotFound do |_e|
render json: { error: t("404 error") }, status: 404
end
end
endclass Api::V1::ChatsController < ApplicationController
def index
room = Room.find(key: params[:room_key])
...
end
endって書けた方が冗長な箇所がなくなって綺麗になると思う
| return | ||
| end | ||
|
|
||
| if params[:cursor].present? |
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.
このifのチェックいらない気がする?
下のChat.find_byでnilが返るだけなので
https://github.com/cyder/SyncPod-server/pull/110/files#diff-95f5b6164012ab93315ecba597608414R17 は意味を為してない気がします
/api/v1/chatsにroom_keyと前回取得した最後のchat id(cursor)を指定することで、それより前の(cursor自体は含まない)過去チャットが取得できる。cursorを指定しないときは最新のチャットを取得する
検討事項