# 12/25? 26? JINさんのポートフォリオレビュー
### GitHubのリポジトリ
フロントエンド: https://github.com/higakijin/Vote-App
バックエンド: https://github.com/higakijin/VoteApp-Api
### コードリーディング用
https://github.dev/higakijin/Vote-App
https://github.dev/higakijin/VoteApp-Api
### サイトのURL
Heroku:https://higakijin-vote-app.herokuapp.com/
(一応API:https://vote-app-api.herokuapp.com/ ※開いても何も表示されません)
### ER図
<img src="https://camo.githubusercontent.com/49d22f3eabc4d151dea65bbf1020af880cb1dda2b5b1fb8083096fc146b01f77/68747470733a2f2f692e6779617a6f2e636f6d2f36336432316238623331633832666438643966636162353739353637303537332e706e67">
# レビュー
参考までに過去のHackMDを貼っておきます。
https://hackmd.io/s/S10wrrNQF
## サービス面
### だいそん
#### GOOD
- 案外複雑なこと(vote後に非同期的に画面の一部を切り替えたり)をサラッと実装してて良い
- SSR
#### TO BE IMPROVED
- LIKEボタンのオン, オフがわかりづらい
- LIKEしたあと並び順が変わってわかりづらかった
### みけた
#### GOOD
- シンプルですが、ポイントを絞って、目立つ不具合なく動くアプリを作ったのはすごいと思います!
- レスポンシブ対応できている!
- 非同期処理により、操作感がよいアプリに仕上がっていると思います!
- アイコンをうまく使って、シンプルなデザインにすることに成功していると思います!
#### TO BE IMPROVED
- YESとNOの左右の配置が、個人的には少し気になります笑
- ログインしていませんの表示場所は工夫してあげると良いかも

### getumei
#### GOOD
- デザイン崩れしておらず、配色のセンスもいいので、良いのではないかと思いました。
#### TO BE IMPROVED
- ユーザーの投稿一覧ページに至るリンクが分かりにくかったです。
### Kei
#### GOOD
- デザイン性もよく直感的に操作できるアプリだと思います!
#### TO BE IMPROVED
- 投稿の詳細ページに「投稿一覧へのリンク」が有ると親切かなと思います(ロゴとは別に)。
### Todaka
#### Good・その他感想
- 2-3週間で作成したとチラッとお聞きしたので、アプリを作り上げる&ここまで作り込むのはすごいなと思いました。
- Yes/Noを投票するアプリなので、一般的なアンケートのような質問が登録されると、後から閲覧・投票するユーザーにとっては読みづらいと感じました。パン派かご飯派か?など。yesとnoの値を設定できると読みやすさは上がりそうですけど、アプリの主旨からずれますし入力を面倒くさがるユーザーも多そうなので、割り切りが必要ですね。
- フロントとバックエンドを分けて作られたアプリを見るのは初だったので、良い刺激になりました。理解できてないことが多いですが、、、
- 他の方も触れてますが、各ボタンやリンクをもう少し工夫すると更にユーザーフレンドリーになるかと思います。
- 個人的にはPCで見たとき、ログイン画面のユーザー登録用リンクが目立たないことと、投稿一覧(家のロゴ)がなんのボタンか迷ったこと、が気になりました。
[](https://gyazo.com/703e472c772c2f5bcbb8caed54966884)
## 技術面
### だいそん
### みけた
- Nuxtに挑戦したのは素直にすごいです
- どうやって勉強したんですか?
- https://www.techpit.jp/courses/195
- schema.rbについて
- comment_likes, post_likes, votesの複合キーにユニーク制約がもれていそう
- usersのuidは何使っている? https://github.com/lynndylanhurley/devise_token_auth
- commentsのlike_countやis_agreeは必ずしもなくてもよさそう
- commentに紐づくcomment_likesの数を数えてもよさそう
- is_agreeは何に使っている?
- postsのagree_countやdisagree_count、like_countは必ずしもなくてもよさそう
- postに紐づくvotesの数を数えたり、 postに紐づくlikeの数を数えてもよい
- commentsやpostsに対して、一定の制約をかけた方がよさそう
- 少なくとも、アプリケーションレベルでvalidationは実装するべき
- 例えば、text型のカラムには長すぎる文章は保存できないようにしたほうがいいかも
- postgresだと長さ制限がないらしく、1GBまで保存できてしまうらしい
- [PostgreSQL の文字列型についてまとめてみた \| SIOS Tech\. Lab](https://tech-lab.sios.jp/archives/8640)
- バックエンドエンジニアを目指す場合、SQLも勉強しましょう!
- もう1段階ステップアップします!
- いや、フロントエンドであっても必要かな。。。
[データベーステーブル設計の基礎の基礎〜エンティティの抽出・定義から正規化まで \- エンジニアHub|若手Webエンジニアのキャリアを考える!](https://employment.en-japan.com/engineerhub/entry/2018/06/22/110000)
[【DB設計入門\|ER図\|MySQL】コンビニレシートから学ぶ!データモデリング手法 \| Wedding Park CREATORS Blog](https://engineers.weddingpark.co.jp/mysql-database-design/)
[はじめてのテーブル設計・データベース設計【わかりやすい解説 \+ 身近なテーマでレッスン】 \| Udemy](https://www.udemy.com/course/hajimete-ronrisekkei/)
[3時間で学ぶ SQL ・データベース 超入門【丁寧な解説\+演習問題で SQL データ抽出の基本が身につく】標準 SQL \| Udemy](https://www.udemy.com/course/sql-begginer/)
- PostsController
- indexアクション: likesもeager_loadした方がよさそう
- create, unpublish, destroyアクションは、他の人に悪さをされない?
- unpulishアクションだけれども、やっていることはpublishなので誤解されそう
- createアクションはリファクタできそう
```rb
def create
post = Post.eager_load(:user, :votes, :comments).find(params[:post][:id])
if vote_by_current_user = current_user.votes.find_by(post_id: post.id)
# 既に投票を行っていた場合
create_vote(post) if vote_by_current_user.is_agree != params[:post][:is_agree]
vote_by_current_user.destroy
current_user.comments.where(post_id: post.id).destroy_all # 意見が変わるのでコメントも消す
else
# 初めての投票の場合
create_vote(post)
end
post.update(agree_count: post.votes.where(is_agree: true).length,
disagree_count: post.votes.where(is_agree: false).length
)
render json: @vote, status: 200
end
private
def create_vote(post)
@vote = Vote.create(is_agree: params[:post][:is_agree], user_id: current_user.id, post_id: post.id)
end
end
```
```rb
def create
post = Post.eager_load(:user, :votes, :comments).find(params[:post][:id])
vote_finished_user = current_user.votes.find_by(post_id: post.id)
# 送信パラメータとDBの値の整合性確認(例: 賛成と投票済みなのに、賛成に変更できない)
return unless [‘true’, ‘false’].includes?(params[:post][:is_agree])
return if vote_finished_user.is_agree == params[:post][:is_agree]
# 既に投票を行っていた場合
if vote_finished_user
ActiveRecord::Base.transaction do
create_vote(post)
vote_finished_user.destroy!
# 意見が変わるのでコメントも消す
current_user.comments.where(post_id: post.id).destroy_all!
end
else
# 初めての投票の場合
create_vote(post)
end
# agreeとdisagreeの数を更新
post.update!(
agree_count: post.votes.where(is_agree: true).length,
disagree_count: post.votes.where(is_agree: false).length
)
render json: @vote, status: 200
end
private
def create_vote(post)
@vote = Vote.create!(is_agree: params[:post][:is_agree], user_id: current_user.id, post_id: post.id)
end
end
```
### getumei
- コードを見させていただきました。APIでアプリケーションを作るとこんな感じになるんですね。勉強になりました。自分の周りのエンジニア転職希望者でAPI + SPAみたいなモダンなポートフォリオ用意した人を存じないので、いい差別化になるのではないかと感じました。
### junnosuke
ポートフォリオ作成お疲れ様です。初心者ながら簡単ではありますがコメントをさせてもらいます。
- N+1問題などに対して対応しているのでいいと思いました。参考にさせてもらいます。
- フロントエンドとバックエンドが別れているのはどのようなメリットがあるのでしょうか。
### yu-ki
あんまりフロント詳しくないんで、バックエンド部分だけでレビューしました。フロント自分も勉強しないと...
#### GOOD
- フロントにNuxtを採用していてすごい!見た目もTailwindCSSで書いていてかなり綺麗です!
#### TO BE IMPROVED
##### Gemfile
- dev,testがsqliteでproductionがpostgresだが、db特有のエラーとかが起きないように全ての環境で同じdbを使った方がいいのかなと思いました。
##### db/schema.rb
comment_likesテーブル
- user_id,comment_idセットでユニーク制約をかけた方がいいかも。そうしないと一人のユーザーが同じコメントに複数回できてしまうから。
post_likesテーブル
- user_id, post_idも上に同じく
commentsテーブル
- likes_countなくてもいいねの数は表せそう。
postsテーブル
- agree_count, disagree_countは、自分だったらenumでagree,disagreeを表すかも。
間違ってるかもしれないですけど、個人的にcount系はカラムで持たない方が良さそうだと感じました。
##### controllers
post_likes_controller, comment_likes_controller
- create, destroyアクション内で最後updateしてたのが気になった(自分の知識・経験不足なだけでいいのかもしれない)
users_controller
- checkCurrentUserアクションはrubyだとスネークケースで書いた方がいい。check_current_user
- showアクション内で、変数名がp,u,vとかよりもpost,user,voteとかの方がいいかも。user_arrayの変数も、変数の中に入ってるのは、配列じゃないから変えた方がいいかも。
- showアクション内のjson返してる部分はactive_model_serializerを使えばすっきりしそう。(他のjsonを返すところも同じ。)
自分的にリファクタしてみました。今のshowアクションと同じjsonを返します。
```rb
app/serializers/user_serializer.rb
class UserSerializer < ActiveModel::Serializer
attributes :id, :name, :uid, :introduction
has_many :posts
end
app/serializers/post_serializer.rb
class PostSerializer < ActiveModel::Serializer
attributes :id, :topic, :agree_count, :likes_count, :user_id, :is_published
has_many :votes
has_many :post_likes
end
app/serializers/vote_serializer.rb
class VoteSerializer < ActiveModel::Serializer
attributes :id, :is_agree, :uid
def uid
object.user.email
end
end
app/serializers/post_like_serializer.rb
class PostLikeSerializer < ActiveModel::Serializer
attributes :id, :uid
def uid
object.user.email
end
end
users_controller.rb
def show
user = User.eager_load(:posts).find(params[:id])
render json: user, include: { posts: %i[votes post_likes] }, status: 200
end
```
参考までに
https://qiita.com/qsona/items/f9d58976c561b8331922
##### routes.rb
- votes,commentsとかにresourceを使ってるのはなんでだろう?
### Kei
- ランキングは何を基準にソートしているのでしょうか?(知識不足の為コードではわからなかったのですいません、、、)
### Todaka
##### controllers
- pやvの変数が分かりづらいので、英単語をそのまま書いた方が良さそう。
- postコントローラ内など、ハッシュに入れる工程が何度か出てきますけど、バックエンドをAPIにしている都合でしょうか?
- コードの量が多く、一部のコードが重複してそうなので、共通化できたら良さそう。post_arryなど。(どうするかは思い付いていないのでごめんなさい)
- createやupdateなど、エラー制御が足りてない箇所がありそうです。