-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
お知らせのメール通知に作成者のアカウントID(ログイン名)を表示する #7353
Conversation
お手隙のときに、こちらのデザインをお願いいたします🙏 |
@88-99 お待たせしました🙇♂️ デザイン入れました!! |
ありがとうございます!🙇♂️ |
8808ca7
to
11741ce
Compare
急ぎませんので、お手隙のときにこちらのレビューをお願いできませんでしょうか? |
@88-99 |
@a-kuroki-gs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@88-99
お待たせしました!
こちら問題なく動作することを確認いたしましたので、私からはApproveさせていただきます!
ただPRがDraft状態のままなので、Ready for review
を押していただければと思います🙏
1つ個人的に気になった点としては、PRの変更確認方法において
「+ お知らせ作成」までの流れが長いかな?と思いました。
単純に http://localhost:3000/announcements/ にアクセスするか、左上の「お知らせ」タブをクリックし一覧画面に移動する等でもよさそうですし、
もしくは http://localhost:3000/announcements/new にアクセスするのように、お知らせ作成画面のリンクを載せるものアリかなと思いました。
参考までに🙇♀️
お忙しいところご確認いただきまして、ありがとうございました🙇♂️
すみません、押しました。気をつけます。
おっしゃるとおりで長かったですね。ご指摘いただきありがとうございます🙏 |
11741ce
to
2eda3cf
Compare
こちらのレビューよろしくお願いいたします🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確認させて頂きました。OKです〜🙆♂️
ご確認いただき、ありがとうございました! |
ステージングや本番環境でのみ出現する不具合についてはどのようなアプローチを取れば良いのか、ご教示いただけないでしょうか? 反映されない画像が記述されているファイルのBlameで、特に最近修正した形跡がないことは確認しました。 |
疑問点としてあるのは、修正をステージング環境で確認するには、駒形さんにマージしていただかないといけないでしょうか? |
@88-99 もう少し調べてみて、どのように調べたか、どのように考えたかをまとめてみてください〜
もうちょっと何が行われているかをブレイクダウンして考えてみた方がいいかもです。問題に対する知識の解像度が荒い状態かもしれません。ちょっと上記の文章だけでは情報が足りず、応えるのが難しいです。 まず、githubにあるコードがステージング環境・本番環境にデプロイされています。 上記の情報をもとにもう一度質問をいただければありがたいです。 |
コメントいただきまして、ありがとうございます。
すみません、かしこまりました。考えてみます! |
調べたことをご報告させていただきます。 【これまでの状況】
【考えられること】
もし 1. であれば、開発環境やステージング環境の違いに関わらず同じ現象になりそうなので、2. の可能性の方が高いでしょうか... 【確認したこと】開発者ツールで、該当箇所のコードを見比べました。 letter_opener(開発環境)の通知メールには
|
<h3 | ||
style="font-family: sans-serif; font-size: 20px; color:#4638a0; line-height: 1.5; margin-bottom: 0; font-weight: bold"> | ||
<%= title %></h3> | ||
<td style="word-wrap: break-word; font-size: 0px; padding: 16px 20px 0; text-align: left;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@88-99 画像が表示されないのはどの行になりますか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/fjordllc/bootcamp/blame/main/app/views/layouts/mailer.html.erb
こちらのファイルの119行目です。
今回のログイン名を表示する実装をしました app/views/notification_mailer/_notification_mailer_template.html.erb は、上記ファイル130行目の<%= yield %>
で指定して表示されています。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@komagata
ご確認いただきありがとうございます。
あ、今回変更していない箇所なんですね?
そうです。
今までは表示されていたんでしょうか?
今まで表示されていたかどうかについては、私がリリース日までメール通知をOFFにしており確認できていないため、他の方にメールを確認していただこうと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue
お知らせのメール通知に作者を表示してほしい。 #6305
概要
お知らせを作成したとき送信されるメールにお知らせ作成者の記載がないので、作成者のアカウントID(ログイン名)を表示する。
変更確認方法
feature/add-author-name-to-notification-email をローカルに取り込む。
bootcamp を起動する。
bundle exec foreman start -f Procfile.dev
任意のユーザでログインする。
お知らせを作成する。
http://localhost:3000/announcements/new
http://localhost:3000/letter_opener に接続する。
届いたメールにお知らせの作成者名が記載されていることを確認する。
Screenshot
変更前
変更後