キャンペーン名を動的に表示する #4228

ot0m1 · 2022-02-15T13:58:55Z

要件

未ログイン時のトップページに表示に、現在実施中のキャンペーン情報を動的に取得して表示させる。

画面イメージ

確認方法

feature/display-the-campaign-name-dynamically ブランチをローカルに持ってくる（参考：https://qiita.com/great084/items/ad74dd064a2c2bc47cff ）

git fetch origin pull/{このプルリクエストのid}/head:{任意のブランチ名}
git checkout {上記でfetchした任意のブランチ名}

管理者（komagata or machida）のアカウントでログイン
お試し延長画面（ http://localhost:3000/admin/campaigns ）にアクセスしてキャンペーン名を変更
ログアウト後、（ http://localhost:3000/ ）にアクセスしてがキャンペーン名が変更になっていることを確認

ref: #4130

ot0m1 · 2022-02-16T04:57:04Z

お試し期間の動的な変更は #4232 で進められているので、このプルリクエストではキャンペーン名とキャンペーン期間のみの動的な変更を行う。

aim2bpg · 2022-02-16T05:43:00Z

@ot0m1 さん、おつかれさまです。

このプルリクエストではキャンペーン名とキャンペーン期間のみの動的な変更を行う。

キャンペーン期間の方は、#4196 で自分の方で対応予定です。

PRにメンションがあったので、覗いてみたらキャンペーン期間の言及があり
被ってはいけないな〜と思いましたので、一応、ご連絡させていただきました。。🙇🏻‍♂️

ot0m1 · 2022-02-16T10:03:32Z

@aim2bpg
なんと〜キャンペーン期間の実装もされていたんですね。
ではキャンペーン名だけの取得にします🙏 ご連絡ありがとうございました。

ot0m1 · 2022-02-16T11:02:24Z

@aim2bpg こちらShimokawaさんに見ていただくのが色々良さそうなのでレビュー依頼させていただきます〜🙏 よろしくお願いします。

aim2bpg

@ot0m1 さん
確認しました、OKですー🙆‍♀️

テストが頭をよぎりましたが、タイトルは目立つし、仮に間違っていても影響は少なそうです。キャンペーン入れた後には大体確認するでしょうから問題なさそうだと思いました👍

↓手元の確認結果

ot0m1 · 2022-02-16T13:54:28Z

@aim2bpg レビューありがとうございました。
@komagata レビューお願いいたします🙏

komagata · 2022-02-20T17:58:55Z

app/models/campaign.rb

@@ -25,6 +25,12 @@ def self.today_is_campaign?
    recently_campaign.cover?(Time.current)
  end

+  def self.current_title


クラスメソッドが複数になったらclass << selfでまとめるのがいいかもです。

あと、これは @ot0m1 さんが書いたところじゃないですが、today_is_campaign?って英語的に間違ってるので変えたいですね〜

クラスメソッドのまとめ & today_is_campaign? の修正を行いました。
is_today_campaign? だと Rubocop にメソッド名の修正を推奨されるので、Rubocop にサジェストされる today_campaign? にしています。

komagata · 2022-02-26T16:26:48Z

app/models/campaign.rb

-  def self.today_is_campaign?
-    return if recently_campaign.nil?
+    def current_title
+      return unless today_campaign?


メソッド名素晴らしいです〜！

これってメソッドの定義はどこでかわっていますか？（今回のdiffの中に見当たらないので）

aim2bpg · 2022-03-03T14:02:13Z

@komagata さん、@ot0m1 さん、おつかれさまです〜
こちらのPR、結構修正が入っており、並行して進めている自分の方のPRとコンフリクトを起こすものと思われます。

もし、可能であれば、先にこちらのPRを早めにmainにマージしていただき、自分の方のPRでコンフリクトを解決したいと考えております。何卒ご配慮のほど、お願い申し上げます🙇🏻‍♂️

ot0m1 · 2022-03-03T14:24:44Z

@aim2bpg お待たせしております🙏
@komagata #4228 (comment) こちらご確認お願いいたします🙇‍♂️

komagata · 2022-03-05T13:37:27Z

@ot0m1

#4228 (comment) こちらご確認お願いいたします

こちら私が質問しているように見えるのですが、それに対して「確認お願いします」というのは何をすればいい感じでしょうか〜

aim2bpg · 2022-03-05T14:05:14Z

@komagata さん、自分の方で分かりましたので回答いたします🙇🏻‍♂️

これってメソッドの定義はどこでかわっていますか？（今回のdiffの中に見当たらないので）

23行目でかわっています〜

ot0m1 · 2022-03-05T23:59:35Z

@komagata

で質問されているのを、

と回答したので、それをご確認いただきたいという意味です。

komagata · 2022-03-07T13:56:41Z

@ot0m1 なるほどです、僕が見逃しておりました〜。

komagata · 2022-03-07T13:58:09Z

app/models/campaign.rb

+    end
+
+    def today_campaign?
+      return if recently_campaign.nil?


このifは不要かもと思いました〜。

Merge branch 'main' into feature/display-the-campaign-name-dynamically

ot0m1 · 2022-03-08T12:29:14Z

@komagata レビューいただいた点修正いたしました。再レビューお願いいたします。

komagata · 2022-03-10T04:06:38Z

app/models/campaign.rb

@@ -21,8 +21,6 @@ def recently_campaign
    end

    def today_campaign?
-      return if recently_campaign.nil?


すみません、こちら僕が読み間違っていました。

引数の無いreturnなんですね。だとしたら不要では無いかと思います。すみません。
（if recently_campaign.nil?をreturnしてるのかと読み間違えちゃっていました。）

ot0m1 · 2022-03-10T11:57:24Z

@komagata

#4228 (comment) にコメント追加できないのでこちらで回答します。

たしかに current_title 内で return unless today_campaign? で判断しているので、today_campaign? では return で抜ける必要がないと考え、この行を削除しました。

上記のような回答をしていましたが、today_campaign?は current_title メソッド以外の箇所でも呼ばれているので必要そうでしたので、return if recently_campaign.nil? を復活させました。

komagata

読み間違えてややこしくしてしまってすみませんでした〜。
確認しました、OKですー🙆‍♂️

キャンペーン名を動的に表示する

961da1d

ref: #4130

ot0m1 added 2 commits February 16, 2022 19:04

キャンペーン名を動的に取得し反映

f4d921f

rubocop通した

7e908dc

ot0m1 requested a review from aim2bpg February 16, 2022 11:01

ot0m1 marked this pull request as ready for review February 16, 2022 11:01

aim2bpg approved these changes Feb 16, 2022

View reviewed changes

ot0m1 requested a review from komagata February 16, 2022 13:54

aim2bpg mentioned this pull request Feb 17, 2022

[お試し延長開始・終了機能] キャンペーン期間を動的に表示する #4247

Merged

komagata reviewed Feb 20, 2022

View reviewed changes

ot0m1 added 4 commits February 22, 2022 19:03

クラスメソッドをまとめた

1ff8bb4

メソッド名を文法的に正しく修正

9780140

rubocopがサジェストするメソッド名に変更

f942ccc

テストが通るよう古いメソッド名の箇所を変更

a1e4074

ot0m1 requested a review from komagata February 22, 2022 10:47

komagata reviewed Feb 26, 2022

View reviewed changes

ot0m1 requested a review from komagata March 1, 2022 10:49

komagata requested changes Mar 7, 2022

View reviewed changes

app/models/campaign.rb

end

def today_campaign?

return if recently_campaign.nil?

Copy link

Member

komagata Mar 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このifは不要かもと思いました〜。

ot0m1 added 2 commits March 8, 2022 20:16

最新の origin/main を取り込み

6b496a5

Merge branch 'main' into feature/display-the-campaign-name-dynamically

不要な return 文を削除

2421c5e

ot0m1 requested a review from komagata March 8, 2022 12:29

komagata reviewed Mar 10, 2022

View reviewed changes

ot0m1 added 2 commits March 10, 2022 20:57

return if を復活

0fa9432

不要なスペースを削除

6288731

ot0m1 requested a review from komagata March 10, 2022 13:17

komagata approved these changes Mar 10, 2022

View reviewed changes

komagata merged commit 5db79f9 into main Mar 10, 2022

komagata deleted the feature/display-the-campaign-name-dynamically branch March 10, 2022 14:35

github-actions bot mentioned this pull request Mar 10, 2022

Release 2022-03-18 01:41:12 #4380

Merged

32 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

キャンペーン名を動的に表示する #4228

キャンペーン名を動的に表示する #4228

ot0m1 commented Feb 15, 2022 •

edited

Loading

ot0m1 commented Feb 16, 2022

aim2bpg commented Feb 16, 2022

ot0m1 commented Feb 16, 2022

ot0m1 commented Feb 16, 2022

aim2bpg left a comment •

edited

Loading

ot0m1 commented Feb 16, 2022

komagata Feb 20, 2022

ot0m1 Feb 22, 2022

komagata Feb 26, 2022

aim2bpg commented Mar 3, 2022

ot0m1 commented Mar 3, 2022

komagata commented Mar 5, 2022

aim2bpg commented Mar 5, 2022 •

edited

Loading

ot0m1 commented Mar 5, 2022 •

edited

Loading

komagata commented Mar 7, 2022

komagata Mar 7, 2022

ot0m1 commented Mar 8, 2022

komagata Mar 10, 2022

ot0m1 commented Mar 10, 2022

komagata left a comment

キャンペーン名を動的に表示する #4228

キャンペーン名を動的に表示する #4228

Conversation

ot0m1 commented Feb 15, 2022 • edited Loading

要件

画面イメージ

確認方法

ot0m1 commented Feb 16, 2022

aim2bpg commented Feb 16, 2022

ot0m1 commented Feb 16, 2022

ot0m1 commented Feb 16, 2022

aim2bpg left a comment • edited Loading

Choose a reason for hiding this comment

ot0m1 commented Feb 16, 2022

komagata Feb 20, 2022

Choose a reason for hiding this comment

ot0m1 Feb 22, 2022

Choose a reason for hiding this comment

komagata Feb 26, 2022

Choose a reason for hiding this comment

aim2bpg commented Mar 3, 2022

ot0m1 commented Mar 3, 2022

komagata commented Mar 5, 2022

aim2bpg commented Mar 5, 2022 • edited Loading

ot0m1 commented Mar 5, 2022 • edited Loading

komagata commented Mar 7, 2022

komagata Mar 7, 2022

Choose a reason for hiding this comment

ot0m1 commented Mar 8, 2022

komagata Mar 10, 2022

Choose a reason for hiding this comment

ot0m1 commented Mar 10, 2022

komagata left a comment

Choose a reason for hiding this comment

ot0m1 commented Feb 15, 2022 •

edited

Loading

aim2bpg left a comment •

edited

Loading

aim2bpg commented Mar 5, 2022 •

edited

Loading

ot0m1 commented Mar 5, 2022 •

edited

Loading