Skip to content
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

卒業生のダッシュボードに「卒業後の目標」の入力を促すブロックは表示しないようにした #4597

Merged
merged 5 commits into from
Apr 17, 2022

Conversation

siroemk
Copy link
Contributor

@siroemk siroemk commented Apr 9, 2022

概要

卒業後の目標が未入力の場合、卒業生のダッシュボードにて「フィヨルドブートキャンプを卒業した自分はどうなっていたいかを登録してください。」というアナウンスが表示される。

こちらは性質上受講生向けのものであるため、卒業生のダッシュボードには出てこないようにしたい。

卒業生以外のロールを確認したところ、管理者とメンター、アドバイザーは「未入力の項目」自体が表示されないため、今回は卒業生以外のロールは考慮しない。

修正前

image

修正後

卒業後の目標が未入力の場合でも「フィヨルドブートキャンプを卒業した自分はどうなっていたいかを登録してください。」というアナウンスは出てこない。
image

確認方法

  • ブランチ feature/fix-graduate-dashbord をローカルに取り込む
  • bin/rails sでサーバーを立ち上げる
  • 卒業生のroleでログインする(sotugyouなど)
  • ダッシュボードにアクセスして「未入力の項目」の中に「[フィヨルドブートキャンプを卒業した自分はどうなっていたいかを登録してください」という項目が出てこないことを確認
  • 現役生のroleでログインする(kimura など)
  • ダッシュボードにアクセスして「未入力の項目」の中に「[フィヨルドブートキャンプを卒業した自分はどうなっていたいかを登録してください」という項目が出てくることを確認

@siroemk siroemk marked this pull request as ready for review April 9, 2022 14:58
@siroemk siroemk requested a review from tksmasaki April 9, 2022 14:59
@siroemk
Copy link
Contributor Author

siroemk commented Apr 9, 2022

@tksmasaki
お疲れ様です〜!お手隙の際にレビューをお願いいたします。

Copy link
Contributor

@tksmasaki tksmasaki left a comment

Choose a reason for hiding this comment

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

@siroemk
いくつかコメントさせていただきました。ご確認よろしくお願いします🙏

@@ -82,6 +82,12 @@ class HomeTest < ApplicationSystemTestCase
assert_no_text 'ブログURLを登録してください。'
end

test 'not show messages of after_graduation_hope for gradueted user' do
Copy link
Contributor

Choose a reason for hiding this comment

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

「卒業後の目標のメッセージ」は一つと考えられるかなと思うので、単数の message of after_graduation_hope の方が自然な気がしました。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

本当ですね〜!単数に修正しました。
そして、graduatedのタイプミスもあったので合わせて修正しております。

@@ -10,10 +10,11 @@ class RequiredField
attribute :discord_account, :string
attribute :github_account, :string
attribute :blog_url, :string
attribute :graduated_on, :date
Copy link
Contributor

@tksmasaki tksmasaki Apr 10, 2022

Choose a reason for hiding this comment

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

app/models/user.rbgraduated_on? をラップした graduated? というメソッドがあるので、それを活用してboolean型を扱う方が、値の使用方法的にわかりやすくなるかなと思いました。

Copy link
Contributor

@tksmasaki tksmasaki Apr 10, 2022

Choose a reason for hiding this comment

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

誤)graduate?
正)graduated?

でした🙇‍♂️(修正しました)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

graduated?を使った方が分かりやすそうですね!graduated?に修正いたしました🙌

@siroemk siroemk force-pushed the feature/fix-graduate-dashbord branch from afff780 to 376648d Compare April 12, 2022 03:47
@siroemk siroemk self-assigned this Apr 12, 2022
@siroemk
Copy link
Contributor Author

siroemk commented Apr 12, 2022

@tksmasaki
ありがとうございます。
どれもおっしゃる通りだったので、修正しております〜!
再度ご確認をお願いします😄

@tksmasaki tksmasaki self-requested a review April 12, 2022 05:26
Copy link
Contributor

@tksmasaki tksmasaki left a comment

Choose a reason for hiding this comment

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

@siroemk
確認しました! 僕からはOKです🙆
1点コメントしていますが、参考程度に確認お願いします🙏

@@ -10,10 +10,11 @@ class RequiredField
attribute :discord_account, :string
attribute :github_account, :string
attribute :blog_url, :string
attribute :graduated?, :boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

attribute の名前には ? がないほうが自然かなと思いました。
このままではダメかは判断できなかったので、修正するかはおまかせします🙏

明確な規約などは見つからなかったのですが、参考になりそうと思ったものを貼っておきます!

@siroemk
Copy link
Contributor Author

siroemk commented Apr 12, 2022

@tksmasaki
ありがとうございます!言われてみると他のところも?は使っていないですし、ないほうが自然に感じたので、graduatedに変更しました。
参考のURLもありがとうございます!勉強になりました🙆‍♀️

@siroemk siroemk requested a review from tksmasaki April 12, 2022 14:20
Copy link
Contributor

@tksmasaki tksmasaki left a comment

Choose a reason for hiding this comment

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

@siroemk
Approve しました〜!

@siroemk siroemk requested a review from komagata April 13, 2022 00:43
@siroemk
Copy link
Contributor Author

siroemk commented Apr 13, 2022

@tksmasaki
レビューありがとうございました🙌

@komagata
チームメンバーのレビューが終わったのでご確認をお願いします!

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認しました、OKですー🙆‍♂️

@komagata komagata merged commit 53eeb42 into main Apr 17, 2022
@komagata komagata deleted the feature/fix-graduate-dashbord branch April 17, 2022 01:54
@github-actions github-actions bot mentioned this pull request Apr 17, 2022
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants