レスポンスにサムネイル情報追加 #1929
レスポンスにサムネイル情報追加 #1929
Conversation
|
ありがとうございます。 僕は |
|
このissueの目的は、UIの省エネモードでアニメーションのスタンプを表示しないために、サムネイルを代わりに表示したいことです なので、bool値で十分だと思います |
|
スタンプに対応したサムネイルを取得できる既存のメソッドを探したのですが見つけることができなかったので助言をいただきたいです． やろうとしたこと 現状 問題 手詰まり感あるので想定している実装方法などあればヒント欲しいです．．． |
|
遅くなってごめんなさい。 忙しくてちゃんとコードを読めてないのですが、データベースでサムネイルの存在を確かめるメソッドを書く必要があると思います。 docs/dbSchema にあるデータベーススキーマと、repositroryで定義されているinterfaceとそのgormの実装を参考にして、ループ回さず必要な情報を取得できるメソッドをお願いします。 |
|
返信ありがとうございます！ 教えていただいたことを参考にしながらやってみます！ |
This reverts commit e4494ee.
|
前の変更からだいぶ時間たってしまいましたがdbから一度のクエリでスタンプのサムネイル情報を取得できるようにしました． |
|
LintとTestの2つFailしてますね。一つずつ見ましょうか Lintの方は、 Testの方は、↓のチェックが失敗しているようですね。APIのレスポンスとしてJSONのArrayが返ってくることを期待したけど、実際には Line 115 in 469b8db Goでは実はsliceのゼロ値(nil)と、 make([]T, 0) などで作った長さ0のsliceは違って、JSONにエンコードするときも前者は nil ]に、後者は [] になります。これを考慮してみてくださいhttps://github.com/traPtitech/traQ/actions/runs/6402762003/job/17379996280 
|
|
Testの方の補足です |
|
エラー内容を教えていただきありがとうございます！ 修正完了したので内容についても時間があるときに見てほしいです |
There was a problem hiding this comment.
ありがとうございます！実装は大丈夫だと思います。1か所だけリファクタリングお願いします。
また、今回の変更部分のテストコードを書いてほしいです。
repositoryのStampThumbnailExists()のテストは repository/gorm/stamp_test.goに書いてください。
routerのGetStamps()のテストはrouter/v3/stamps_test.goで修正を加えてください。
他のテストの関数を参考にすると書きやすいと思います。
GetStampsの方は、レスポンスボディが変わっているので、その評価を行う部分に修正が必要です。
何かわからないことがあったら聞いてください。お願いします！
|
実装確認していただきありがとうございました！ 指摘してくださったことを踏まえ構造体の修正とテストコードの追加を行いました． ただテストコード自体はこれでいいのかよくわからなかったので確認していただけると助かります． 時間あるときに確認していただければと思います． |
|
レビューフローに割り込んでしまってすみません。 「StampThumbnailExists」の名前がちょっと気持ち悪いなぁと思って色々考えてたんですけど、その途中で「そもそもGetAllStampsWithThumbnailみたいにした方がいいのでは？」という考えが浮かびました。 すでにやっているかもしれませんが、こういう時に気にすべきは、次にこのメソッドを使って別機能を実装する人のことです。 @azbcww @ikura-hamu の2人の意見も聞きたいですが、ちょっと実装方針を再検討してもらえると嬉しいです。 |
|
@ikura-hamu @logica0419 @ikura-hamu @logica0419 |
|
@logica0419 ( @azbcww ) |
|
また時間が空いてしまったのですが修正をしたのでテストを書く前にこの実装方法で良いか一度確認してほしいです． 変更点: 懸念点: |
|
stampsキャッシュの書き換えは、今回は必要なさそうなので無くていいと思います |
|
コードの確認ありがとうございます！ 修正を終えてテストを書いているのですが実装方法で質問があります． やりたいテストは下記のものです．
このテストでサムネイルのないスタンプを作る実装方法を迷っています．
しかし，1. 2.どちらの実装方法でもmustMakeStamp内のrepo.CreateStampでサムネイルが存在している場合のみスタンプを作成しているため，サムネイルを作らずスタンプを作成する方法を書くのが大変だなと感じています． そこで，mustMakeStampでサムネイルの存在するスタンプを作成後，repo.DeleteFileMeta()でサムネイルを削除すれば結果的にサムネイルの無いスタンプを得られると考えたました．こちらの実装方法では良くないところがありますでしょうか？ 時間があるときに教えていただけると助かります． |
|
テスト追加しました． |
|
確認&指摘ありがとございます！ |
|
確認ありがとうございます！ |
|
何度も確認ありがとうございました！ |
サムネイル情報をGET /api/v3/stamps のレスポンスに載るよう変更しました．
"hasThumbnailを含める"っていうのをサムネイル情報が欲しいって解釈したんですけどあってますかね．．．？
返ってくる情報は以下のようになります．
{
"id": "68df025e-8260-4a11-bd50-f1bb32692c5a",
"name": "test",
"creatorId": "52687fda-bc08-470c-83fa-67b0fe5f644d",
"fileId": "3ec736ff-857f-420d-889e-911745c23c6b",
"isUnicode": false,
"createdAt": "2023-08-12T08:02:21.547014Z",
"updatedAt": "2023-08-12T08:02:21.547014Z",
"thumbnails": [
{
"FileID": "3ec736ff-857f-420d-889e-911745c23c6b",
"Type": 1,
"Mime": "image/png",
"Width": 128,
"Height": 128
}
]
},