久しぶりにポートフォリオレビュー会を行いました。

動画

【定期勉強会】12/25 JINさんのポートフォリオレビュー

Image from Gyazo

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にチャレンジしている(SPAだと動的OGPがしんどい)

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で見たとき、ログイン画面のユーザー登録用リンクが目立たないことと、投稿一覧(家のロゴ)がなんのボタンか迷ったこと、が気になりました。
      Image from Gyazo

技術面

だいそん

PRに記載しました
https://github.com/higakijin/VoteApp-Api/pull/15/files

みけた

  • Nuxtに挑戦したのは素直にすごいです

  • 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型のカラムには長すぎる文章は保存できないようにしたほうがいいかも
  • バックエンドエンジニアを目指す場合、SQLも勉強しましょう!

    • もう1段階ステップアップします!
    • いや、フロントエンドであっても必要かな。。。

データベーステーブル設計の基礎の基礎〜エンティティの抽出・定義から正規化まで - エンジニアHub|若手Webエンジニアのキャリアを考える!

【DB設計入門|ER図|MySQL】コンビニレシートから学ぶ!データモデリング手法 | Wedding Park CREATORS Blog

はじめてのテーブル設計・データベース設計【わかりやすい解説 + 身近なテーマでレッスン】 | Udemy

3時間で学ぶ SQL ・データベース 超入門【丁寧な解説+演習問題で SQL データ抽出の基本が身につく】標準 SQL | Udemy

  • PostsController
    • indexアクション: likesもeager_loadした方がよさそう
    • create, unpublish, destroyアクションは、他の人に悪さをされない?
    • unpulishアクションだけれども、やっていることはpublishなので誤解されそう
    • createアクションはリファクタできそう
  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
  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を返します。

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など、エラー制御が足りてない箇所がありそうです。

こんな形でメンティーさん同士でフィードバックをしながら切磋琢磨していってます。
今回は久々の投稿になってしまいましたがコミュニティ内ではしばしば開催しています。