Feature/#58comment delete api #70
Conversation
| def destroy | ||
| memo = Memo.find(params[:memo_id]) | ||
| comment = memo.comments.find(params[:id]) | ||
| comment.destroy | ||
| head :no_content | ||
| end |
There was a problem hiding this comment.
IMO/Idiomatic
destroyがfalseになった時には、
削除に失敗したメッセージと共に、400を返すようにしましょうか！
There was a problem hiding this comment.
承知しました！
ありがとうございます！
こちらfalseの時に422を返すように実装したのですが、テストはどのように記載したら良いのでしょうか？
allow_any_instance_ofメソッドでスタブにしてdestroy呼び出したとき、Falseに返すようにと思ったのですが、調べると指定した全てのインスタンスに対して行われ多くのバグを生むので非推奨と出てきまして・・・
Rspecで異常系の処理を意図的に発生させテスト実施させる方法、教えていただきたいです！
There was a problem hiding this comment.
allow_any_instance_ofメソッドでスタブにしてdestroy呼び出したとき、Falseに返すようにと思ったのですが、調べると指定した全てのインスタンスに対して行われ多くのバグを生むので非推奨と出てきまして・・・
前提として、allow_any_instance_ofを利用したとしても一つのテストケースに閉じるので、
allow_any_instance_ofでも良いですが、以下のようにすることができると思います。
（ちょっと動くか試してないので、修正が必要かも？）
before do
find_by = comment
allow(comment).to receive(:destroy).and_return(false)
allow(Comment).to receive(:find_by).and_return(find_by)
end| end | ||
| end | ||
|
|
||
| context 'コメントが存在しない場合' do |
There was a problem hiding this comment.
IMO
memoが存在しない場合もテストしたいですね！
また、コメントが存在しない場合は、以下のようなリクエストが適切かなと思います！
delete "/memos/#{memo.id}/comments/0", as: :json
| memo = Memo.find(params[:memo_id]) | ||
| comment = memo.comments.find(params[:id]) |
There was a problem hiding this comment.
Performant/Simple
以下のように書くとクエリの発行数が2回から1回になって少しだけパフォーマンスがいいと思います。
| memo = Memo.find(params[:memo_id]) | |
| comment = memo.comments.find(params[:id]) | |
| comment = \ | |
| Comment.find_by(id: params[:id], memo_id: params[:memo_id]) |
対応するissue
対応内容