スパルタコースでは毎週何かしらのイベントを開催しています。

今回もまた「Railsポートフォリオレビュー会」です。

週に一人くらいまでなら無償でレビューできると思うのでお気軽にお声がけください。

動画

勤怠管理システム ポートフォリオレビュー会 | TechEssentials

Image from Gyazo

雑なメモですが、こんなフィードバックをしました。

5/15 ポートフォリオレビュー会

githubのリポジトリ

https://github.com/KawaiAkihiro/seven-rule-beta

オンラインエディタで確認できるやつ
https://github1s.com/KawaiAkihiro/seven-rule-beta

サイトのURL

http://www.seven-rule.net/

だいそん

みけた

  • validate :start_finish_checkがあるせいで、masterが保存できませんでした
    • self.submits_startsやself.submits_finishがないため
# master.rb

  validate :start_finish_check

  def start_finish_check
    errors.add("日付が間違っています。正しく記入してください。") if self.submits_start >= self.submits_finish
  end
  • 申し訳ないですが、使い方が分かりづらかったです
    • シフト募集の枠が、アルバイト側から可視化できるとよいのかも
    • ただ、実際に使われているとのことだったので、きちんと機能しているのはすごい！
    • ただ、説明がしっかり書かれていたので、ひと目みた時点で直感的にわかるとより良くなると思いました
  • nginxやdockerの設定ファイルにコメントアウトで説明が書いてあり、すごいなと思いました
  • 認証を自分で作っているあたりの心意気がすごいなと思いました
  • SSLについてはLet's Encrypt使うと無料でできますよ！
  • rubyの場合はインデントは２つの方が一般的らしいです
  • ここは危険な香りがしましたが、エスケープ処理ってされますか？
# masters_controller.rb

  def shift_onoff_form
    if !current_master.shift_onoff

      render plain: render_to_string(partial: "shift_period", layout: false)
    else
      render plain: render_to_string(partial: "shift_submit_finish", layout: false)
    end
  end

にゃおき

  • stylesheetがすごい生成されてる

    • generator設定が欲しい!

  • masterモデル(店長モデル？)にstoreが入っているため拡張性が乏しいように感じました

    • Storeモデルを作成し、MaterモデルとStaffモデルを別に作成
      • Sotre -> Master & Store -> Staff の関係にすると良いと思いました
      • Storeが一人だけ店長を持つ場合は Store has_one :master ですかね？

  • 命名規則について

    • Staffモデルのカラムにstaff_nameが入っているので@staff.staff_nameとなるので
      • @staff.nameのようになってもいいのかなと思います。
    • start_h や finish_h の h って何か気になります。

  • staffs_controller のno_submit や already_submits は
    値を取得するために多くの処理を記述していてcontrollerが冗長になってしまっているため、Materモデルのクラスメソッドとして定義するべきではないかと思います.
    Mater.no_submit_staff

  • IndividualShiftの中でbackgroundcolorとか指定してるけどそんなことできるんだ！って思いました

たいしろう

機能がいっぱいあってすごいと純粋に思いました！
↓ めっちゃ同意です笑 by naoki
↑ですよね！ by taishiro
実際に使用してもらってるのもとてもすごいと思いました！自分が、一人でやっている時はここまでできなかったです！

  • DBの制約はもっと付けた方がいいと思いました.

  • null: falseとかuniqueとか

  • references型を使った方がいいと思いました。

  • 好みかもしれませんが、viewの中で完結するところは極力インスタンス変数を渡さなくても良いと思いました。ただここは、あまり気にしすぎることもないかもしれないです。

  • 非同期処理をたくさん使っていてシンプルにすごいと感じました。

class PerfectShiftsController < ApplicationController
      def index
        #このページで全てのアクションを起こす
        if logged_in? && logged_in_staff?
            @events = current_master.individual_shifts.where(Temporary: true)
        elsif logged_in_staff? && !logged_in? 
            @events = current_staff.master.individual_shifts.where(Temporary: true)
        elsif logged_in? && !logged_in_staff?
            @events = current_master.individual_shifts.where(Temporary: true)
        end
      end
end

これは、elsif以降は、２つ目の条件がいらない気がします！

こんな感じで毎週勉強会をやってます。
スパルタコースはサポートの質が落ちないように少数のコミュニティとして活動しています。
実際に未経験からRailsの業務委託で仕事をしている人もいるくらいなのでちゃんと学習すればスキルはつきます。

普段はSlackでやりとりしてますのでリンクを貼っておきますね。
Slackコミュニティへの招待リンク

ではでは。