bootcamp icon indicating copy to clipboard operation
bootcamp copied to clipboard

[動画機能] 動画追加・一覧の追加

Open a-terumoto-gs opened this issue 11 months ago • 29 comments

Issue

  • https://github.com/fjordllc/bootcamp/issues/7420

概要

動画の追加機能、一覧表示機能を新機能として実装しました。

変更確認方法

  1. feature/add-video-addition-and-list-display-functionをローカルに取り込む
  2. bin/setuprails db:migraterails db:seedを実行
  3. foreman start -f Procfile.devでアプリを起動し、ログインする
  4. 左側のサイドバーの「Docs・動画」をクリックして /pagesページが表示されることを確認
  5. 動画一覧ページの確認項目をチェックする
    • 動画タブをクリックすると/moviesページが表示されること
    • 動画が2つあり、各々にサムネイル・タイトル・追加したユーザー名・追加日時が表示されていること
    • mp4動画のほうにはタグが表示されていること
    • 右上に「+動画を追加」ボタンがあり、リンク先が正しいこと
      • movies/newに遷移する
  6. 新規追加ページの確認項目をチェックする
    • 関連プラクティスが選択できる
    • 右上に「<動画一覧」ボタンがあり、リンク先が正しいこと
      • moviesに遷移する
    • 関連プラクティスが選択できる
    • タイトルが入力できる
    • 説明欄ではMarkdownが使用できる
    • 動画データがmp4形式、mov形式で添付できる
    • タグが追加できる
    • 動画の追加を押すと動画詳細ページに遷移する
  7. 個別ページの確認項目をチェックする
    • 上部に「+動画を追加」「<動画一覧」ボタンがあり、リンク先がそれぞれ正しいこと
    • 動画の上部に、Docsと同じような形でプラクティス名、タイトル、公開日、ユーザーアイコン、コメント数、Watchボタン、Bookmarkボタン、タグ欄が表示されていること
    • 動画が再生できること
    • 動画の下に説明欄があること
    • リアクションボタン、コメント欄の表示があり、機能すること
    • 一覧に戻るボタンがあること
    • mp4動画とmov動画で動画の表示サイズがそろっていること
  8. ユーザー削除時の表示を確認する 一般ユーザーで動画を追加した後ログアウトし、管理者権限を持つユーザーでログインしなおし、先ほど動画を追加したユーザーを削除した際、ユーザー欄がghost表示になることを確認する

現時点でのタグの仕様についてですが、以下は現時点での正しい仕様です

  • 一覧ページでのタグをクリックするとそのまま一覧ページを読み込みなおす
  • 個別ページでのタグをクリックするとRouting Errorになる

一覧画面における動画のサムネイルについてですが、今回のissueではサンプル画像をはめる形の仕様で進めることになりましたので、すべて同じ画像が表示されます

Screenshot

既存のデザインからの変更点

左のタブが「Docs」→「Docs・動画」に 「Docs」と「動画」のタブが追加 上部のページタイトルが「Docs」→「Docs・動画」に 動画ページの追加に合わせて「Docs」の表示が出る位置、「+Docs作成」の位置も変更になっております image

無題

新規ページ

一覧ページ image

新規作成ページ image

動画個別ページ image

ユーザー削除時の表示 image

image

a-terumoto-gs avatar Mar 13 '24 07:03 a-terumoto-gs

@machida おつかれさまです! こちらの機能ですが、実装がおおむね終了したのでデザインを依頼したいです! 新しく追加した3画面のデザインをお願いしたいです。

全体としてDocsのデザインに寄せたつもりで実装していますが、だいぶ表示が崩れてしまっております。

  • 一覧画面 image 動画一つ一つの表示はポートフォリオ一覧ページを参考にして実装しております。

  • 新規作成画面 Image from Gyazo

  • 詳細画面 Image from Gyazo

お手数をおかけしますがよろしくお願いいたしますm(__)m

a-terumoto-gs avatar Apr 08 '24 09:04 a-terumoto-gs

@machida 上記で依頼した新規作成画面についてですが、別issueでの編集機能追加のために入力欄をformとして切り出して使うようにしたところ、新規作成画面でもデザインがほぼ崩れないことが判明したので、手間を減らすためにこちらのissueにもそちら適応しております。 といったわけで確認は必要かと思いますが大きな調整は不要になったと思います! 後出しで申し訳ありませんが確認よろしくお願いいたします。

Image from Gyazo

a-terumoto-gs avatar Apr 09 '24 07:04 a-terumoto-gs

@a-terumoto-gs 了解ですー では、formを確認させていただきますー それ以外のページのデザインも進めます!

machida avatar Apr 09 '24 07:04 machida

@machida コメントありがとうございます! 個別ページのデザインも同様にできましたのでheaderとして切り出して追加でコミットしております! 該当ファイル↓ _movie_header.html.slim image

a-terumoto-gs avatar Apr 09 '24 08:04 a-terumoto-gs

@a-terumoto-gs デザイン入れてpush しましたー

machida avatar Apr 09 '24 12:04 machida

レビュー用のファイル差分内訳

修正が入っているファイルが多いので内訳を書いておきます

  • 本issueで新たに追加したもの
    • 設定追加とコントローラ、モデル
      • app/assets/javascripts/applicat
      • app/controllers/movies_controller.rb
      • app/models/movie.rb
      • app/models/practice.rb
      • app/models/user.rb
    • 画面表示関係のファイル
      • app/views/movies/_form.html.slim
      • app/views/movies/_movie_header.html.slim
      • app/views/movies/_movie_list.html.slim
      • app/views/movies/index.html.slim
      • app/views/movies/new.html.slim
      • app/views/movies/show.html.slim
      • app/views/pages/_doc_movie_header.html.slim
      • app/views/pages/_tabs.html.slim
      • config/locales/ja.yml
    • タグのコンポーネント用:app/controllers/api/movies_contoller.rb
    • マイグレーションファイルやセットアップ関係のファイル:
      • db/fixtures/acts_as_taggable_on/taggings.yml
      • db/migrate/20240321092012_create_movies.rb
      • db/seeds.rb
      • lib/bootcamp/setup.rb
      • test/fixtures/active_storage/attachments.yml
      • test/fixtures/active_storage/blobs.yml
      • db/fixtures/files/movies/movie1.mp4
      • db/fixtures/files/movies/movie2.mov
      • db/fixtures/files/movies/thumbnail1.jpg
      • db/fixtures/files/movies/thumbnail2.jpg
      • db/fixtures/movies.yml
    • ルーティング:
      • config/routes.rb
      • config/routes/api.rb
    • テストとテスト用データ:
      • test/system/movies_test.rb
      • test/fixtures/files/movies/movie1.mp4
      • test/fixtures/files/movies/movie2.mov
      • test/fixtures/movies.yml
  • 既存のものの表示を変更したもの
    • Docsページの上部の文言:app/views/works/index.html.slim
    • サイドバーの文言:app/views/application/_global_nav.slim
  • デザイン変更と実装の変更があるもの(動画のものと一部共通化)
    • app/views/pages/index.html.slim
    • app/views/pages/new.html.slim
    • app/views/pages/show.html.slim
  • 町田さんのデザインの変更のみ
    • app/javascript/components/Tags/TagEditButton.jsx
    • app/javascript/components/Tags/TagEditModal.jsx
    • app/javascript/stylesheets/application/blocks/page/_page-header-actions.sass
    • app/javascript/stylesheets/shared/blocks/card/_card-body.sass
    • app/javascript/stylesheets/shared/blocks/card/_thumbnail-card.sass
    • app/views/pages/_doc_header.html.slim
    • app/views/pages/_page.html.slim
    • app/views/works/_work.html.slim
  • デザイン変更に伴う修正が加わったテスト
    • test/system/notification/pages_test.rb
    • test/system/pages_test.rb

a-terumoto-gs avatar Apr 10 '24 09:04 a-terumoto-gs

@kyokucho1989 お疲れ様です! お忙しいところ恐縮ですが、お手すきの際にこちらのissueのレビューをお願いしてもよろしいでしょうか? 急ぎではありません。

新機能なので差分が多くなっているので、内訳をコメントでまとめています。 不明な点があれば遠慮なくお伝えください! よろしくお願いいたしますm(__)m

a-terumoto-gs avatar Apr 10 '24 11:04 a-terumoto-gs

@a-terumoto-gs 了解しました!一週間ほどお待ちくださいー

kyokucho1989 avatar Apr 10 '24 21:04 kyokucho1989

@a-terumoto-gs ざっと確認してますー 質問です。

  • 動画の追加の権限 これは誰でもできる仕様ですか?

  • Movieモデルの関連付けについて MovieモデルはUserモデルやPracticeモデルとは独立した関係の方がよいかと思います。ユーザーやプラクティスが削除されてしまったときの処理が難しくなりそうです。 どうでしょうか?

kyokucho1989 avatar Apr 10 '24 22:04 kyokucho1989

@kyokucho1989 コメントありがとうございます

  • 動画の追加の権限 これは誰でもできる仕様ですか?

そうです。追加者に制限はないです。

  • Movieモデルの関連付けについて MovieモデルはUserモデルやPracticeモデルとは独立した関係の方がよいかと思います。ユーザーやプラクティスが削除されてしまったときの処理が難しくなりそうです。 どうでしょうか?

その点深く考えられていませんでしたが、ユーザーに関しては確かにそうかもしれません。 ユーザーについてはご指摘の通り、独立させる仕様に変更しようかと思います。

プラクティスについては、個人的には関連付いていてよいのではないか、と考えます。 プラクティスを選択する場合はプラクティスに関連付いた動画をあげるときになると思うため、そのプラクティスの削除と同時に動画が消えても支障がないかなと思います。 プラクティスに関係ない動画を追加する際には(LT会の動画など)、プラクティス選択をせずに追加できるため、プラクティス削除で動画も消えることはないので支障はないかと思います。

この点に関しては駒形さんにも確認をしていただこうと思いますm(__)m

@komagata モデル間の関連付けについて質問です。 MovieモデルとUserモデルやPracticeモデルの関連付けについて上記のように考えていますが、いかがでしょうか?

a-terumoto-gs avatar Apr 11 '24 01:04 a-terumoto-gs

@a-terumoto-gs

モデル間の関連付けについて質問です。 MovieモデルとUserモデルやPracticeモデルの関連付けについて上記のように考えていますが、いかがでしょうか?

movieの持ち主(投稿者)という意味でuserとの関連が欲しいです。

また、関連プラクティスという意味でpracticeとの関連が欲しいです。(0~n個のプラクティスと関連)

komagata avatar Apr 18 '24 01:04 komagata

モデル同士の関連付けについて

UserモデルとPracticeモデル、どちらが消えても関連付けられたMovieモデルは残る仕様にする

実装に当たって、dependetntオプションなしとdependent: nullifyのどちらにするか悩んだが、

  • オプションなし:親オブジェクトが削除されても、子オブジェクトの外部キーが残ったままになる、参照先のない外部キーが残る
  • nullify:親オブジェクトが削除されると。子オブジェクトの外部キーがnullになり、他の親オブジェクトと関連付けられるようになる

それぞれの方法の削除後の仕様を考慮して、参照先のない外部キーが残るのはよくないだろう、新しく作成した親オブジェクトとの関連付けをし直すかも、と考えnullifyで実装することとした。 これに伴い、nullでもエラーが出ないようにMovieテーブルを修正

a-terumoto-gs avatar Apr 19 '24 07:04 a-terumoto-gs

@kyokucho1989 お疲れ様です! コメントありがとうございました! 修正していますので確認をお願いいたします!

確認前に実行していただきたいのが、FFmpegのインストールとmovieテーブルの再作成です。

  • FFmpegのインストール こちらのインストール手順を踏むようにしていなかったため、エラーが出て動画が追加できない状態になっていました。 大変お手数ですが、インストールをお願いします。 画像処理ライブラリのインストール

  • movieテーブルの再作成 Userレコードが削除された際にuser_idをnullとして関連したmovieレコードを残す形に変更したため、マイグレーションファイルを書き換えてpushしています 一度migrationファイルをdropしてからupしていただけると問題なく動くと思います。 ↓参考 https://qiita.com/pyon_kiti_jp/items/a23660d20e76fffa5dd4

お手数をおかけしますがよろしくお願いいたしますm(__)m

a-terumoto-gs avatar Apr 19 '24 12:04 a-terumoto-gs

@a-terumoto-gs ありがとうございます!確認します

kyokucho1989 avatar Apr 19 '24 22:04 kyokucho1989

仕様が変わりそうなため、もうしばらくしたらまた確認します。

kyokucho1989 avatar Apr 20 '24 14:04 kyokucho1989

@kyokucho1989 ありがとうございます。 サムネイル関係の修正が反映できましたら、またご連絡させていただきますm(__)m

a-terumoto-gs avatar Apr 22 '24 08:04 a-terumoto-gs

@kyokucho1989 お疲れ様です! 修正が完了いたしましたので、お手すきの際にご確認よろしくお願いいたしますm(__)m

サムネイル関係の処理をすべてなくし、サンプル画像をはめ込む仕様となりました。 それに従い、FFmpegのインストールは不要となりました。

a-terumoto-gs avatar Apr 23 '24 02:04 a-terumoto-gs

@a-terumoto-gs 了解しました! 確認しますー

kyokucho1989 avatar Apr 23 '24 08:04 kyokucho1989

@kyokucho1989 お疲れ様です。 だいぶ時間をいただいてしましましたが、指摘いただいた点と実装が漏れていた点の修正が完了しましたので再度レビューをお願いしたいです。

前回コメントいただき、すでに確認いただいてる点以外の修正としましては、 動画を作成したユーザーが削除された際の表示を変更し、該当ユーザーがいなくなった場合はghostとして表示するようにしております! ユーザー削除の際の挙動を追加で確認いただけますと幸いです。 個別ページのghostアイコンが四角になってしまっているのは後で町田さんに修正いただこうと思っていますので、そこは気にせずでお願いします。

お手数をおかけしますがよろしくお願いいたしますm(__)m

a-terumoto-gs avatar May 16 '24 10:05 a-terumoto-gs

@a-terumoto-gs 了解しました。 確認しますー

kyokucho1989 avatar May 16 '24 10:05 kyokucho1989

@kyokucho1989 途中間があいてしまい申し訳ありませんでしたm(__)m 丁寧にレビューしていただきありがとうございました!

@komagata メンバーの方にApproveいただいたのでレビューをお願いします!

a-terumoto-gs avatar May 22 '24 01:05 a-terumoto-gs

@a-terumoto-gs 動画追加で動画を選択しないでも追加できるようになっているようです~

komagata avatar May 22 '24 11:05 komagata

@komagata 大事な個所の動作確認が漏れていました。申し訳ございません。 バリデーションを追加し、動画が添付されていない際には保存できないようにしました。 また、追加できなかった際のnoticeメッセージが機能していないのも発覚したので、きちんと表示されるようにそちらも合わせて修正しております。

a-terumoto-gs avatar May 23 '24 06:05 a-terumoto-gs

@a-terumoto-gs 何にエラーがでているのかのメッセージがでていないようです。

image

komagata avatar May 24 '24 06:05 komagata

@komagata 申し訳ございません。 他の箇所と同様にエラー内容も出力するように修正しました。

a-terumoto-gs avatar May 24 '24 08:05 a-terumoto-gs

@a-terumoto-gs タブがON状態にならないようです。(Docsページも同じく)

image

komagata avatar May 24 '24 14:05 komagata

@komagata タブがOnにならない点はメンバーのレビューの際に指摘いただいたのですが、動画の編集・削除機能のissueのデザインを町田さんに依頼するタイミングだったのでそちらで合わせて調整入れていただく形にしました このissueの内容のみのリリースはしないという認識だったのでその方法を取ったのですが、そのやり方だと問題ありましたでしょうか?

  • https://github.com/fjordllc/bootcamp/issues/7421 こちらのPRの内容では↓の表示になるようにしております image image

a-terumoto-gs avatar May 27 '24 02:05 a-terumoto-gs

タブがOnにならない点はメンバーのレビューの際に指摘いただいたのですが、動画の編集・削除機能のissueのデザインを町田さんに依頼するタイミングだったのでそちらで合わせて調整入れていただく形にしました

@a-terumoto-gs なるほどです~!

komagata avatar May 28 '24 20:05 komagata