bootcamp
bootcamp copied to clipboard
fjordllc
特別イベント一覧、定期イベント一覧に、直近(近日開催)のイベント一覧を表示する。
Issue
- https://github.com/fjordllc/bootcamp/issues/7162
参考にしたIssue(PR)
- https://github.com/fjordllc/bootcamp/pull/6454
概要
特別イベント一覧(/events)、定期イベント一覧(/regular_events・/regular_events?target=not_finished)にも、「近日開催のイベント」一覧を表示させました。
ダッシュボード(/)に表示されている「近日開催のイベント」一覧を参考にしていますが、デザインは異なるものを実装します。
ダッシュボード(/)の例
変更確認方法
-
feature/list_upcoming_events_on_special_and_regular_events_pagesをローカルに取り込む -
bin/rails db:resetでDBをクリーンアップする -
foreman start -f Procfile.devで起動する -
ユーザー
kimuraでログインをする -
以下の各ページの「近日開催のイベント」コンポーネントに、手順6.のイベント群が表示されていることを確認する
- 特別イベント一覧(
/events) - 全ての定期イベント一覧(
/regular_events) - 開催中の定期イベント一覧(
/regular_events?target=not_finished)
- 今日開催のイベント
- ダッシュボード表示確認用テストイベント(当日用)
- kimura専用イベント
- ダッシュボード表示確認用テスト定期イベント
- 明日開催のイベント
- ダッシュボード表示確認用テストイベント(翌日用)
- 就職関係かつ直近イベントの表示テスト用
- ダッシュボード表示確認用テスト定期イベント
- 明後日開催のイベント
- ダッシュボード表示確認用テストイベント(明後日用)
※上記以外のイベントが表示されていても問題ありません。
Screenshot
※新しい機能のため、変更後のScreenshotsのみを貼ります。
※/events・/regular_events・/regular_events?target=not_finishedの画面変更点は同じです。
※下のスクリーンショットは/eventsページのものとなります。
@machida 度々すみません。 ブラウザサイズによって、イベントが無いときの「近日開催イベント」表示の仕方が違うかもしれません。こちらお間違えなかったでしょうか👀
以下は、Event・RegularEvent共にdestroy_allをした後の画面になります。 (環境依存でしたらすみません)
横1366 縦728
横758 縦700
イベントがない場合は、後者のように「◯◯開催のイベントはありません」とウィンドウサイズに限らず、表示したいと思っていました。
お手数ですがご確認よろしくお願いいたします。
@masyuko0222 お、メインカラムが空の場合を想定してませんでした。サイドカラムはメインカラムの高さに依存があるので、メインカラムが空の場合に、サイドカラムが下まで伸びないというバグがありました。 メインカラムの高さに min-height を設定し、もし空であってもある程度高さを保ち、サイドカラムが下まで伸びるようにしておきました。
@machida お忙しいところ失礼します。
/regular_eventsページだと表示が下図のようになってしまいますので、お時間ある際に修正していただけますと幸いです。
/eventsページは問題ありません。
@machida 確認がとれましたー！お忙しいところ何度もありがとうございました！
@junohm410 ご都合よろしければこちらのレビューをお願いしたいです。 忙しかったら、遠慮なく仰ってくださいー🙇
@masyuko0222 お疲れ様です。レビュー承知いたしました👍 遅くて土日くらいまでにお戻しできるように努めます🙏 よろしくお願いいたします。
@junohm410 レビューありがとうございました！再レビューよろしくお願いいたします！
- コードは全部ご指摘通りの修正をしました。それぞれ返信しています。
- 冒頭にあった変更確認手順についても修正しました。これは自分が勘違いしていました、お手数おかけしてすみません。
また、コミットメッセージについてもありがとうございます。 リファクタリングのコミットは、「rubocopの修正」レベルの粒度で考えちゃっていました・・・。 リファクタリングにも自分の意図や意志が入ってくるので、ちゃんとメッセージを書いてあげた方がいいですね。以降気を付けます。
@junohm410 参考記事を読んだ結果、やっぱりデフォルト値はなくしました😅 シンプルな引数が一番可読性が高そうです。 度々恐縮ですが、修正をしたので再レビューいただいてもいいでしょうかm(__)m
@junohm410 何度も失礼しました！ありがとうございます！
@komagata Approveされましたので、レビューのほどお願いいたしますー。
@komagata
リファクタリングし切れてないので、レビューはまだ大丈夫です🙇♂️
１点だけgitについてのご相談で、pull --rebaseとpush --force-with-leaseをしたあとの、Github上での履歴が心配です。
現状
切り貼りしてますが、以下のようなコミットとレビューコメントの流れになっています。
前のコミット→コメント→新しいコミット、の順番です。
pull --rebaseとpush --force-with-lease後
コメント→前のコミット+新しいコミット、になってしまう気がします。 (今までやってきた感じ)
これが自分の操作が悪いのかが判断がつかず困っており、避けられるなら操作方法が知りたいです・・・！
@masyuko0222 gitの操作はコマンドだけではなく、今自分がどのブランチにいるのかが影響するので上記だけだとわからない感じです。
また、githubのコメント（コミットコメントではなく）の位置関係などは気にしなくていいと思います。
@komagata
githubのコメント（コミットコメントではなく）の位置関係などは気にしなくていいと思います。
承知しました、一番気になってた箇所はここでした！ありがとうございます！
@komagata 度々すみませんーFatControllerをリファクタリングする作業は、別ブランチを切ってもよろしいでしょうか？ また、その際のレビューもkomagataさんで大丈夫でしょうかー？
@masyuko0222 gitを使っているので好きな状態にいつでも戻せるはずですので、普通はリファクタリングにブランチは切らないです。そのままでお願いします。
@komagata お疲れ様です、レビューのほどお願いいたします。
自分が書いた部分・それ以外という分け方で考えず、コード全体としてあるべき姿に近づけるようにリファクタリングしてください
各Controllerをもっとスリムにする余地はあるのですが、修正のスコープが広くなりすぎてしまうので、一旦、本ＰＲの範囲でのレビューを頂いてもよろしいでしょうか。
その後更にリファクタリングをするべきかをご相談させて頂きたいです。
@komagata ご指摘点を修正しましたので、再レビューのほどよろしくお願いいたします。 5f27f7a52ed6d99e6f462b2167dbfdc7301963d4
また、以前、
各Controllerをもっとスリムにする余地はあるのですが、修正のスコープが広くなりすぎてしまうので、一旦、本ＰＲの範囲でのレビューを頂いてもよろしいでしょうか。 その後更にリファクタリングをするべきかをご相談させて頂きたいです。
とお話させて頂きました。
せっかくの機会なので、現在のレビューでOK貰い次第、もう少しリファクタリングをしてみてもよろしいでしょうか？ 対象はEventController、RegularEventControllerになります。
お忙しいところ恐れ入りますが、ご検討よろしくお願いします…！
@masyuko0222 レビューでOKもらってからまたリファクタリングするとなるとレビューの意味がなくなっちゃうのでリファクタリングするのであればそのままやっちゃってください。
（一般的にはリファクタリング可能なことがわかっているのにやらずにPRを提出することはない感じです。）
@komagata レビューの対応しましたので、よろしくお願いします！
https://github.com/fjordllc/bootcamp/pull/7272#issuecomment-1987267780 こちらのレビューになります。
@komagata 修正しました。再レビューよろしくお願いします。 https://github.com/fjordllc/bootcamp/pull/7272#discussion_r1536809020 こちらコメント返信してます。
@komagata 修正しましたのでよろしくお願いします。
- UpcomingEventsGroup
- UpcomingEvent
の２つのクラスを利用して、近日開催イベントを表示するようにしました。
初めとだいぶ様相も変わったので、練習も兼ねてコミットも整理し直しましたm(_ _)m
@komagata 以下のコミットでご指摘修正いたしました。改めてよろしくお願いいたします。 811753c6424991ec0ea4cd390f646ab25d2282b6 318d82e2019811dfe2c31fd1528877c9f065f2f8
@komagata 間空いてしまってすみません。 こちらのご指摘について修正いたしました。
クラス図は以下のような形です。
- 今回は特に開催予定日に関する処理を
EventScheduleという形で切り出しました。 -
schedule = EventSchedule.load(event_object)とした場合、schedule.tentative_next_event_dateやshedule.held_next_event_dateのように利用できます。- 内部的には
EventSchedule::SpecialEventScheduleかEventSchedule::RegularEventScheduleをinitializeして、それぞれのインスタンスメソッドを呼んでいます。
- 内部的には
作成したファイルは以下です。
-
app/models/event_schedule.rb -
app/models/event_schedule/special_event_schedule.rb -
app/models/event_schedule/regular_event_schedule.rb -
app/models/event_schedule/date_calculator.rb
お手数ですが再レビューのほどよろしくお願いいたします。