ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 #7304

masyuko0222 · 2024-02-02T10:23:24Z

Issue

ブログ記事にサムネイルを投稿にも表示というチェックボックスを追加。 #7264

概要

変更確認方法

feature/checkbox_displaying_thumbnail_in_articleをローカルに取り込む
rails db:migrate
foreman start -f Procfile.devで起動する
komagataでログイン
/articles/newに移動
「サムネイル画像を記事に表示」のチェックボックスがデフォルトでONになっていることを確認する
任意の画像を選択する。タイトルや内容の項目は適当に入力をしておく。
公開するボタンを押下する
リダイレクト先のブログ本文内に、選択したサムネイルが表示されていることを確認する
内容修正ボタンを押下する
「サムネイル画像を記事に表示」のチェックボックスをクリックし、OFFにする
更新するボタンを押下する
リダイレクト先のブログ本文内に、サムネイルが表示されていないことを確認する
再度、内容修正ボタンを押下する
「サムネイル画像を記事に表示」のチェックボックスがOFFのままであることを確認する

Screenshot

変更前

変更後

masyuko0222 · 2024-02-02T14:43:21Z

app/views/articles/_form.html.slim

+    .form-item
+      = f.check_box :insert_thumbnail, { check: @article.new_record? || @article.insert_thumbnail }
+      = f.label :insert_thumbnail, 'サムネイル画像を記事に表示'
+      .a-form-help
+        p チェックを入れると、ブログ記事の本文内にもサムネイル画像が表示されます。
+


@machida
お手数ですが、お手すきでデザインのほどよろしくお願いいたしますm(__)m

machida · 2024-02-13T08:55:42Z

@masyuko0222
お待たせしました！デザインやヘルプ文章を入れました。
最新の main を rebase したので（最新のメインにもサムネイル関係の機能追加があったので）、

git pull --rebase origin feature/checkbox_displaying_thumbnail_in_article

をお願いします。

masyuko0222 · 2024-02-17T09:20:04Z

@omochiumaiumai
お疲れ様です。ご都合よろしければレビューお願いしてもよろしいでしょうかー🙇‍♂️

もしチーム開発既に終わっちゃってたり、忙しかったらお気兼ねなく仰ってくださいー。

omochiumaiumai · 2024-02-18T15:36:19Z

@masyuko0222
お疲れ様です！返信が遅くなってしまいすみません。
ご依頼いただきありがとうございます〜申し訳ありませんが少々立て込んでおりまして、十分なレビューの時間をとるのが難しいため他の方にお願いしていただいてもよろしいでしょうか😭
お手数をおかけしますが、よろしくお願いします🙇‍♂💦

masyuko0222 · 2024-02-18T23:51:35Z

@omochiumaiumai
承知しました！いえいえ、こちらこそご返信ありがとうございましたー🙆🏻‍♀️

@SuzukaHori
お疲れ様です！ご都合良い時にレビューをお願いしたいです！🙇‍♂️
お時間的に難しそうでしたら、遠慮なく仰ってください〜！

SuzukaHori · 2024-02-19T03:48:57Z

@masyuko0222
お疲れさまです、レビュー承知しました👍
数日中にお返事できると思いますので、少々お待ちください！

SuzukaHori

お待たせしました！
記事内のサムネイル画像の表示・非表示が切り替えられることを確認しました。動作はバッチリでした👍（手順を詳しく書いてくださっていたのでわかりやすかったです！）

コードの方で数点コメントしましたので、ご確認をお願いします🙏

SuzukaHori · 2024-02-20T03:15:35Z

db/migrate/20240202131554_add_insert_thumbnail_to_articles.rb

@@ -0,0 +1,5 @@
+class AddInsertThumbnailToArticles < ActiveRecord::Migration[6.1]
+  def change
+    add_column :articles, :insert_thumbnail, :boolean, default: true, null: false


insertは「データを挿入する」イメージが強いため、article.insert_thumbnailというコードを見たときに「サムネイルのデータを記事に追加するメソッド」という誤解を招きそうです👀

「サムネイルを本文中に入れるかどうかを示すboolean」というのが複雑なので、良い代案が浮かばないのですが、いくつか思いついたのを一応書いておきます💦

inline_thumbnail

body_thumbnail

thumbnail_required_in_body

baf6c90
こちらのコミットで修正しました！
一旦、真っ直ぐにdisplay_thumbnail_in_body、というカラム名にしました。

時間経ってみて、今回の変更含めどれが一番しっくり来そうでしょうか～？😅

修正ありがとうございます！
私は「display_thumbnail_in_body」のままがいいと思いました😊

app/helpers/articles_helper.rb

SuzukaHori · 2024-02-20T08:01:19Z

app/views/articles/show.html.slim

-                    = image_tag @article.prepared_thumbnail_url, class: 'article__image'
-                  - elsif !thumbnail_blank?(@article)
-                    = image_tag @article.selected_thumbnail_url, class: 'article__image'
+                  = display_thumbnail(@article)
              .article__body


ヘルパーメソッドからimage_tagを返すと、HTMLの構造がぱっと見でわかりにくくなるかもと思いました。

複雑な処理をhelperに移したい場合、条件分岐の部分（@article.thumbnail.attached? && @article.prepared_thumbnail?と!thumbnail_blank?(@article)）をヘルパーに置き換えて、image_tag自体はshow.html.slimで直接で表示するのはいかがでしょうか？？（app/helpers/layout_helper.rbのコードのようなイメージです）

69d90a6
こちらでリファクタリングをして、そもそもhelperを使わなくいい書き方に変えました。

こっちの会話も同じく、解決していますm(_ _)m

SuzukaHori · 2024-02-20T10:59:21Z

app/views/articles/_form.html.slim

+      .checkboxes
+        .checkboxes__items
+          .checkboxes__item
+            = f.check_box :insert_thumbnail, { checked: @article.new_record? || @article.insert_thumbnail, class: 'a-toggle-checkbox' }


初期値がtrueなので、f.check_box :insert_thumbnail, class: 'a-toggle-checkbox'でも動きそうに見えました👀（テストは通りました）

この書き方をしている理由がありましたら教えていただきたいです🙇‍♀️

おっしゃる通り不要ですので削除しました！

masyuko0222

おまたせしました！
ご指摘点修正しましたので、ご確認お願いしますm(_ _)m
(てすとまだでした

masyuko0222 · 2024-02-23T11:51:15Z

db/migrate/20240202131554_add_insert_thumbnail_to_articles.rb

@@ -0,0 +1,5 @@
+class AddInsertThumbnailToArticles < ActiveRecord::Migration[6.1]
+  def change
+    add_column :articles, :insert_thumbnail, :boolean, default: true, null: false


baf6c90
こちらのコミットで修正しました！
一旦、真っ直ぐにdisplay_thumbnail_in_body、というカラム名にしました。

時間経ってみて、今回の変更含めどれが一番しっくり来そうでしょうか～？😅

masyuko0222 · 2024-02-23T11:52:59Z

app/views/articles/_form.html.slim

+      .checkboxes
+        .checkboxes__items
+          .checkboxes__item
+            = f.check_box :insert_thumbnail, { checked: @article.new_record? || @article.insert_thumbnail, class: 'a-toggle-checkbox' }


おっしゃる通り不要ですので削除しました！

masyuko0222 · 2024-02-23T12:06:20Z

app/views/articles/show.html.slim

-                    = image_tag @article.prepared_thumbnail_url, class: 'article__image'
-                  - elsif !thumbnail_blank?(@article)
-                    = image_tag @article.selected_thumbnail_url, class: 'article__image'
+                  = display_thumbnail(@article)
              .article__body


69d90a6
こちらでリファクタリングをして、そもそもhelperを使わなくいい書き方に変えました。

こっちの会話も同じく、解決していますm(_ _)m

SuzukaHori

修正お疲れ様です！🍵

前回の修正箇所が直っている事を確認しました👍
数点コメントを追加しましたので、お手数ですがご確認をお願いします🙇‍♀️

以下、0d22823 以降の変更について質問です。

recent_articlesに関わる変更の意図は、ファットコントローラーの解消という解釈でいいでしょうか？

PR自体は「本文中にサムネイル画像を表示するチェックボックスの追加」を目的としたものですが、「最新記事の取得」の処理がコントローラーからモデルに移動しているなど、少し大きめの変更がされているように思えました。

「最新記事の取得」に関わる変更がこのPRに入って良いものなのか、判断がつきませんでした。このことについてご意見を伺いたいです！🤔

app/models/article.rb

test/models/article_test.rb

masyuko0222 · 2024-02-25T11:47:02Z

@SuzukaHori

少し大きめの変更がされているように思えました。

おっしゃる通り今回はPRの範囲からは超えてやりすぎてしまいました😅

どうしてもやり取りが発生してしまいそうで、PRに関係ない所にレビュワーの労力を割かせてしまうのは良くないと思い、関係ない修正（ファットコントローラーの解消）は一旦なかったことにしました。
定数のご指摘などはとてもタメになったのですが、すみません、次の機会で活かさせて頂きます😢

お手数ですが、上記踏まえた上で改めてご確認いただけますと幸いです・・・！
なお、「本文中にサムネイル画像を表示するチェックボックスの追加」の部分は何も変えていないですm(_ _)m

ちなみにファットコントローラーの解消については、下記の記事を参考にしていました。
https://qiita.com/jnchito/items/2b57316b39d92ca0e8dc

SuzukaHori

お返事と修正ありがとうございます！

ファットコントローラー解消の件、承知しました。
コードを読んでいて、私もとても勉強になりました🙏ありがとうございます！

変数名について一点だけ追加でコメントしましたので、ご検討ください（前回見逃していてすみません🙇‍♀️）
ごく些細な内容ですので、こちらでApproveとさせていただきます🎉

SuzukaHori · 2024-02-25T12:38:07Z

test/models/article_test.rb

@@ -13,6 +13,14 @@ class ArticleTest < ActiveSupport::TestCase
    assert_equal '/ogp/ruby_on_rails.png', article.selected_thumbnail_url
  end

+  test '#published?' do
+    public = articles(:article1)


publicは予約語ではないようですが、 public・private・protectedの特別な意味を持つpublicと紛らわしいため、特にこだわりがないならpublic_articlesのような変数名の方が良いかなと思いました！

masyuko0222 · 2024-02-26T12:49:58Z

@SuzukaHori
細かくありがとうございました！
testの命名についても修正いたしました。

@komagata
Approveいただいたので、レビューのほどよろしくお願いいたします。

…bnailカラムを確認するように

article.display_thumbnail_in_body?がtrueの時点で、article.thumbnail.attached?も必ずtrueになります。画像アップロードしていなくても、それ用のサムネイルを表示するためです。それを利用したリファクタリング

以下のコミットで追加した、".fetch_recent_articles"メソッド。１０個の記事を取ってくることを確認。 0d22823

komagata

確認させて頂きました。OKです〜🙆‍♂️

masyuko0222 · 2024-03-06T11:00:35Z

@komagata
すみません。mainにマージされた後、ステージング環境での確認が現状取れていません。実装した機能が乗ってないという意味です。
mainブランチをローカルで起動した際は、確認が取れました。

mainブランチでの、build_and_test とcloud-run-deploy-stagingのチェックがRedであることが原因だと思っています。
明日リリースを控えていると思うのですが、生徒がRerun実行してしまってもよろしいものでしょうか？

追記：
2024/03/06 22:40
ステージング環境での確認が取れましたので、こちらのコメントは解決しました。

masyuko0222 assigned masyuko0222 and machida Feb 2, 2024

masyuko0222 changed the title ~~ブログ記事に「サムネイルを投稿にも表示」というチェックボックスを追加。~~ ブログ記事に「サムネイルを記事に表示」というチェックボックスを追加。 Feb 2, 2024

masyuko0222 changed the title ~~ブログ記事に「サムネイルを記事に表示」というチェックボックスを追加。~~ ブログ記事に「サムネイル画像を記事に表示」というチェックボックスを追加。 Feb 2, 2024

masyuko0222 commented Feb 2, 2024

View reviewed changes

masyuko0222 changed the title ~~ブログ記事に「サムネイル画像を記事に表示」というチェックボックスを追加。~~ ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 Feb 2, 2024

masyuko0222 force-pushed the feature/checkbox_displaying_thumbnail_in_article branch from 73fc2dc to 250186f Compare February 8, 2024 13:23

machida force-pushed the feature/checkbox_displaying_thumbnail_in_article branch from dbd0f56 to e1879fb Compare February 13, 2024 08:54

masyuko0222 force-pushed the feature/checkbox_displaying_thumbnail_in_article branch 2 times, most recently from 9bab6dc to 9e2548e Compare February 17, 2024 04:28

masyuko0222 marked this pull request as ready for review February 17, 2024 09:19

masyuko0222 requested a review from omochiumaiumai February 17, 2024 09:19

masyuko0222 removed the request for review from omochiumaiumai February 18, 2024 23:50

masyuko0222 requested a review from SuzukaHori February 18, 2024 23:51

masyuko0222 force-pushed the feature/checkbox_displaying_thumbnail_in_article branch from ec41c43 to 16e6b8a Compare February 19, 2024 11:21

SuzukaHori reviewed Feb 20, 2024

View reviewed changes

machida removed their assignment Feb 22, 2024

masyuko0222 commented Feb 23, 2024

View reviewed changes

masyuko0222 requested a review from SuzukaHori February 23, 2024 13:52

SuzukaHori reviewed Feb 25, 2024

View reviewed changes

app/models/article.rb Outdated Show resolved Hide resolved

app/models/article.rb Outdated Show resolved Hide resolved

test/models/article_test.rb Outdated Show resolved Hide resolved

masyuko0222 requested a review from SuzukaHori February 25, 2024 11:50

SuzukaHori approved these changes Feb 25, 2024

View reviewed changes

masyuko0222 force-pushed the feature/checkbox_displaying_thumbnail_in_article branch from 1097460 to b250a1e Compare February 26, 2024 11:03

masyuko0222 and others added 24 commits March 5, 2024 20:18

チェックボックス文言を変更

b25948d

サムネイル画像を記事本文に表示する説明を追加

bd7a000

サムネイル画像についての説明を追加

ce56e20

rabaseミス+mainのサムネイル関連が更新された。とりあえずチェックを入れたら、適切な画像が表示されるように

a116b3a

公式ドキュメントに則り、正しいキー名に修正

1dd9495

チェックを外した際のテストを追加。本当はuncheckメソッドを使いたいが、一時的にclickメソッドでチェックを外すような処理。

c3675e9

checkboxを操作する処理は、checkかuncheckメソッドを使うようにした

58197ca

サムネが本文に表示されるには、insert_thumbnailカラムがtrueであることが前提なので、条件式も初めにinsert_thum…

0bd1d2e

…bnailカラムを確認するように

テスト内の不要な期待結果を削除

d466a71

viewの複雑な処理をhelperに移動

a3746a6

SQLのINSERTのイメージが先行してしまうので、カラム名を変えた

4e126be

checkboxの初期値は、カラムのboolean値で決められるから削除

ccb5c8c

一旦helperに移動していたのを元に戻した

dbb4825

showメソッド内がファットになっていたので、最後の理想形

f7bee4e

理想形に基づいて、ロジックをモデルやビューに移動した。

aa2e625

最新の記事がすべて公開済みであることは、モデルテストで担保済

95adc08

テストで確認し忘れていたところを追加

d9d1d84

以下のコミットで追加した、".fetch_recent_articles"メソッド。１０個の記事を取ってくることを確認。 0d22823

mainブランチとコンフリクトを起こさないように。

a039f7a

PRのスコープ外となってしまう修正をやっぱ元に戻す

fcdf050

省略しておく意味もないので、ちゃんと書く

352eb66

リベースミスで消してしまった行を追加

09b0840

viewの可読性を上げた

c95e7a6

テストの書き方をプロジェクト全体の書き方に合わせた

a480d61

masyuko0222 force-pushed the feature/checkbox_displaying_thumbnail_in_article branch from 286c555 to a480d61 Compare March 5, 2024 11:18

komagata approved these changes Mar 5, 2024

View reviewed changes

komagata merged commit a84b956 into main Mar 5, 2024
1 of 2 checks passed

komagata deleted the feature/checkbox_displaying_thumbnail_in_article branch March 5, 2024 11:26

github-actions bot mentioned this pull request Mar 5, 2024

Release 2024-03-05 11:26:43 #7465

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 #7304

ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 #7304

masyuko0222 commented Feb 2, 2024 •

edited

Loading

masyuko0222 Feb 2, 2024 •

edited

Loading

machida commented Feb 13, 2024

masyuko0222 commented Feb 17, 2024

omochiumaiumai commented Feb 18, 2024

masyuko0222 commented Feb 18, 2024 •

edited

Loading

SuzukaHori commented Feb 19, 2024

SuzukaHori left a comment •

edited

Loading

SuzukaHori Feb 20, 2024

masyuko0222 Feb 23, 2024

SuzukaHori Feb 25, 2024

SuzukaHori Feb 20, 2024

masyuko0222 Feb 23, 2024 •

edited

Loading

SuzukaHori Feb 20, 2024

masyuko0222 Feb 23, 2024

masyuko0222 left a comment •

edited

Loading

masyuko0222 Feb 23, 2024

masyuko0222 Feb 23, 2024

masyuko0222 Feb 23, 2024 •

edited

Loading

SuzukaHori left a comment

masyuko0222 commented Feb 25, 2024

SuzukaHori left a comment

SuzukaHori Feb 25, 2024

masyuko0222 commented Feb 26, 2024

komagata left a comment

masyuko0222 commented Mar 6, 2024 •

edited

Loading

ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 #7304

ブログ記事に「サムネイル画像を本文に表示」というチェックボックスを追加。 #7304

Conversation

masyuko0222 commented Feb 2, 2024 • edited Loading

Issue

概要

変更確認方法

Screenshot

変更前

変更後

masyuko0222 Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

machida commented Feb 13, 2024

masyuko0222 commented Feb 17, 2024

omochiumaiumai commented Feb 18, 2024

masyuko0222 commented Feb 18, 2024 • edited Loading

SuzukaHori commented Feb 19, 2024

SuzukaHori left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masyuko0222 Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masyuko0222 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masyuko0222 Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

SuzukaHori left a comment

Choose a reason for hiding this comment

masyuko0222 commented Feb 25, 2024

SuzukaHori left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masyuko0222 commented Feb 26, 2024

komagata left a comment

Choose a reason for hiding this comment

masyuko0222 commented Mar 6, 2024 • edited Loading

masyuko0222 commented Feb 2, 2024 •

edited

Loading

masyuko0222 Feb 2, 2024 •

edited

Loading

masyuko0222 commented Feb 18, 2024 •

edited

Loading

SuzukaHori left a comment •

edited

Loading

masyuko0222 Feb 23, 2024 •

edited

Loading

masyuko0222 left a comment •

edited

Loading

masyuko0222 Feb 23, 2024 •

edited

Loading

masyuko0222 commented Mar 6, 2024 •

edited

Loading