ユーザアイコンにroleを表す色枠が出ていないバグの修正

[hirano-vm4]

Issue

• #7516

概要

日報一覧ページ/reportsのミニアイコンなど、一部のユーザアイコンでroleを表す色枠が表示されていないバグを修正するIsuseです。

下記の該当ページroleが表示されているかの確認をお願いします！

変更確認方法

1. bug/fix-missing-role-border-on-iconsをローカルに取り込む
2. 管理者・アドバイザー・メンターいずれかでログイン
3. http://localhost:3000/reports?page=4 でコメントユーザーのroleを表す色枠が表示されているかを確認
4. http://localhost:3000/products でコメントユーザーのroleを表す色枠が表示されているかを確認
5. http://localhost:3000/external_entries で投稿者のroleを表す色枠が表示されているかを確認

※一般ユーザーは枠線はありません ※同じユーザーが複数回コメントをしてもアイコンの表示は1回のみ

Screenshot

変更前

---

/reportsのミニアイコン

---

productsのミニアイコン

---

/external_entriesの普通のアイコン

---

変更後

---

/reportsのミニアイコン

---

productsのミニアイコン

---

/external_entriesの普通のアイコン

---

[hirano-vm4]

@dowdiness

レビュー依頼

お疲れ様です🙏 お忙しい中、申し訳ありませんがレビューをお願いすることは可能でしょうか🙆？

急ぎではありませんので、無理ないタイミングで全く問題ありません！ ご検討のほど、よろしくお願いいたします🙇‍♂️

[hirano-vm4]

@dowdiness

お疲れ様です🙏 念の為の確認になります〜！

お忙しい中、申し訳ありませんがレビューをお願いすることは可能でしょうか🙆？

急ぎではありませんので、無理ないタイミングで全く問題ありません！ ご検討のほど、よろしくお願いいたします🙇‍♂️

追記

お忙しそうなので今回は別の方に依頼したいと思います！またお願いいたします〜

[hirano-vm4]

@kyokucho1989

お疲れ様です🙏 お忙しい中、申し訳ありませんがレビューをお願いすることは可能でしょうか🙆？

急ぎではありませんので、無理ないタイミングで全く問題ありません！(1~2週間後とかでもOKです)

ご検討のほど、よろしくお願いいたします🙇‍♂️

[kyokucho1989]

@hirano-vm4 了解しました！ 一週間くらいかかるかと思いますー

ひとまずコードをざっくり読んで、週末に不明点など質問するかと思います。

[kyokucho1989]

@hirano-vm4 お疲れ様です。まだコードの確認中ですが、コミットメッセージについて、修正が必要かもしれません。

「コメントユーザーアイコンの重複を削除」 https://github.com/fjordllc/bootcamp/commit/699553c4dbe804774cce96a0d7c03673bab1753b

これは、「本Issue以前からアイコンが重複していた」というわけではありませんよね？ 今回の試行錯誤の結果、アイコンが重複するようになったので修正した、ということですよね。

あとから見返すと分かりづらくなってしまいそうなので、コミットをまとめるとよいかと思います。

[kyokucho1989]

https://github.com/fjordllc/bootcamp/commit/1f7e5e8250ee1fedbbb61a37d53550c7751b2765

このコミットから最新のコミットまでをひとまとめにするとよいかと思いますー

[hirano-vm4]

@kyokucho1989

ありがとうございます！おっしゃる通り、このIssueで作業の過程で発生した確認の必要のない修正まで細かくコミットしてしまっていたのでコミットをまとめました🙏

確認しにくい状態になっていて、お手数おかけしました！引き続きよろしくお願いします🙇‍♂️

[hirano-vm4]

@kyokucho1989

ありがとうございました🙏分かりにくい部分を確認いただき、ありがとうございました！引き続きよろしくお願いします！

@komagata

レビュー依頼

チームメンバーによるレビューが終わりました！レビューをお願いします🙏

コードで確認したい点

現状、バグの修正は終了していますが、 app/views/api/reports/index.json.jbuilderから_user_icons.json.jbuilderにreportを渡すとActiveDecoratorが適用され、report.commentsを渡すとprimary_roleを取得できない部分がありました。

これはActiveDecoratorによるもので、関連モデルを別ファイルに渡すと自動デコレートされないことが原因です。

今回はデコレートされたreportを渡すことで対応しましたが、それにともなってこのコミットにあるように複数の.jbuilderも同様に修正しました。

しかし、明示的に以下のようにdecorateすると以下のファイルの一箇所だけの修正だけで実現できそうです。

app/views/api/comments/_user_icons.json.jbuilder

  json.comments do
    json.array! comments.commented_users do |user|
      user.extend(UserDecorator::Role)

      json.user_icon user.avatar_url
      json.user_id user.id
      json.primary_role user.primary_role
    end
  end

上記のアプローチと、現状の変更のどちらがFBCには適しているのか、判断ができなかったのでご意見いただければと思います。

個人的には上記のように明示的にdecorateすると変更するコードの記述量が少ないので良いと考えていますが、他のファイルを見る限り同じような記述はないので統一した方が良いという考えもあるような気がしています。

この点についていかがでしょうか？

[komagata]

@hirano-vm4

上記のアプローチと、現状の変更のどちらがFBCには適しているのか、判断ができなかったのでご意見いただければと思います。

extendが大丈夫なのかちょっと怖い感じがしました。

https://www.satoryu.com/blog/2021/08/01/how-to-use-active_decorator-for-ruby-object.html

partial使ったときにdecorateされるのでjbuilder内でpartialを使うのはどうでしょうか？

[hirano-vm4]

@komagata

extendが大丈夫なのかちょっと怖い感じがしました。

https://www.satoryu.com/blog/2021/08/01/how-to-use-active_decorator-for-ruby-object.html

partial使ったときにdecorateされるのでjbuilder内でpartialを使うのはどうでしょうか？

ありがとうございます！extend を使用すると、オブジェクトの振る舞いがその場その場で変更されるためにコードを追うのも難しくなるのと、意図しない挙動に繋がる恐れがあるので避けたいと思います🙏

参考URLもありがとうざいます！

現在の実装のままでいきたいと思いますがいかがでしょうか？

app/views/api/reports/index.json.jbuilder

• api/comments/user_iconsに渡した先でuser_roleを使える現在の実装

json.reports @reports do |report|
  json.partial! "api/reports/report", report: report
  json.partial! "api/reports/checks", checks: report.checks
  json.partial! "api/comments/user_icons", report: report # コントローラーのインスタンス変数の時点でデコレートされた@reportの要素自身を渡す
  json.user do
    json.partial! "api/users/user", user: report.user
  end
end

このようにするとapi/comments/user_iconsで、デコレートされたreportからcommented_usersと辿ってuser_roleを利用できる。

コントローラー（ActionController）で（暗黙的なものも含めて）呼ばれるrender

• デコレートされない実装

json.reports @reports do |report|
  json.partial! "api/reports/report", report: report
  json.partial! "api/reports/checks", checks: report.checks
  json.partial! "api/comments/user_icons", comments: report.comments # reportから関連オブジェクト`comments`を渡す
  json.user do
    json.partial! "api/users/user", user: report.user
  end
end

上記のように、report.commentsという具体的な関連オブジェクトを直接partialに渡すと、この場合、ActiveDecoratorは自動的にデコレーターを適用しますが。こちらの記述にあるように、render partialの時点でcommentsを渡すと、渡したクラスの文脈からCommentsDecoratorを探してしまうため、user_roleは使えない。

• 他の方法

json.partial! "api/comments/user_icons", comments: report.comments, commented_users: report.commented_users.uniq

このように渡す時点でuserオブジェクトを指定すれば上記のようにUserDecoratoeを自動で適用してくれるので、問題なくuser_roleは使用できました。

これであれば、シンプルにインスタンス変数の時点でデコレートされたものを渡す現在の実装でも良いかな？と個人的には感じましたがいかがでしょうか？

[komagata]

@hirano-vm4 良いと思います～！

[hirano-vm4]

@komagata

ありがとうございます！現在のコードで大丈夫そうであればマージしていただき、ステージング環境にデプロイされたことを確認次第、動作確認に入ります🙆

[hirano-vm4]

@komagata
お忙しいところ、恐れ入ります！こちらいかがでしょうか？

お手隙の際に確認をお願いいたします！