【定期勉強会】 5/15 Railsポートフォリオレビュー会
スパルタコースでは毎週何かしらのイベントを開催しています。
今回もまた「Railsポートフォリオレビュー会」です。
週に一人くらいまでなら無償でレビューできると思うのでお気軽にお声がけください。
動画
勤怠管理システム ポートフォリオレビュー会 | TechEssentials
雑なメモですが、こんなフィードバックをしました。
5/15 ポートフォリオレビュー会
githubのリポジトリ
https://github.com/KawaiAkihiro/seven-rule-beta
オンラインエディタで確認できるやつ
https://github1s.com/KawaiAkihiro/seven-rule-beta
サイトのURL
だいそん
- 誰かに使ってもらいましたか?
- httpsになってない
- 店名(日本語)でログインさせるのが違和感あり
- 新規登録時、店名と従業員番号を入力させるのは違和感あり。普通店舗側のアカウントを最初に作って、そのユーザーが従業員に対して招待リンクを送る、とかかな。
- ルーティングが全くRESTfulになっていない https://github.com/KawaiAkihiro/seven-rule-beta/blob/master/config/routes.rb
- memo.txtとは? https://github.com/KawaiAkihiro/seven-rule-beta/blob/master/memo.txt
- https://github.com/KawaiAkihiro/seven-rule-beta/blob/master/Gemfile#L45 この辺は
group
の上の方にまとめた方が見やすいかも? rspec-rails
とかtestでしか使わないものはtestグループの中でインストールした方がいいんじゃない?- カラム名が大文字は違和感 https://github.com/KawaiAkihiro/seven-rule-beta/blob/0fa49bf74e0090afecd4de4b9f7dc29a74991bef/db/schema.rb#L18
- https://github.com/KawaiAkihiro/seven-rule-beta/blob/0fa49bf74e0090afecd4de4b9f7dc29a74991bef/db/schema.rb#L75 null: falseなど
- https://github.com/KawaiAkihiro/seven-rule-beta/blob/0fa49bf74e0090afecd4de4b9f7dc29a74991bef/app/controllers/individual_shifts_controller.rb#L22
みけた
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使うと無料でできますよ!
- https://www.kagoya.jp/howto/webhomepage/lets-encrypt/
- AWSだとAmazon Certificate Managerで対応できそう
- https://aws.amazon.com/jp/certificate-manager/
- rubyの場合はインデントは2つの方が一般的らしいです
- ここは危険な香りがしましたが、エスケープ処理ってされますか?
# 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 ですかね?
- Storeモデルを作成し、MaterモデルとStaffモデルを別に作成
-
命名規則について
- Staffモデルのカラムにstaff_nameが入っているので@staff.staff_nameとなるので
- @staff.nameのようになってもいいのかなと思います。
- start_h や finish_h の h って何か気になります。
- Staffモデルのカラムにstaff_nameが入っているので@staff.staff_nameとなるので
-
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以降は、2つ目の条件がいらない気がします!
こんな感じで毎週勉強会をやってます。
スパルタコースはサポートの質が落ちないように少数のコミュニティとして活動しています。
実際に未経験からRailsの業務委託で仕事をしている人もいるくらいなのでちゃんと学習すればスキルはつきます。
普段はSlackでやりとりしてますのでリンクを貼っておきますね。
Slackコミュニティへの招待リンク
ではでは。