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

メンター登録ページからメンターが登録できるようにした #8079

Merged

Conversation

MikotoMakizuru
Copy link
Contributor

@MikotoMakizuru MikotoMakizuru commented Sep 19, 2024

Issue

概要

背景

Issue「招待URL作成ページが欲しい」(#7424) で、管理者は管理ページの招待 URL タブから招待ページを作成できるようになりました。招待 URL を生成するためのロールをメンターに設定して、生成された URL にアクセスすると受講生登録用の「フィヨルドブートキャンプ参加登録画面」が表示されていました。また、メンターとして参加してほしい人がいる場合は、一度アドバイザーなどで参加登録してもらい、管理者が区分を「メンター」に変更していました。

やったこと

  1. 招待 URL を生成するためのロールをメンターに設定して、生成された URL にアクセスするとメンター登録ページ遷移するようにしました。
  2. 1 のメンター登録ページから登録を行うとメンターとして参加できるようにしました。

変更確認方法

  1. feature/enable-mentor-register-from-mentor-registration-screen をローカルに取り込む
  2. foreman start -f Procfile.dev でローカル環境を立ち上げる
  3. komagata でログイン
  4. 画面右上ユーザアイコン>管理者メニュー>管理ページ>招待 URL タブ
  5. 企業を設定
  6. ロールをメンターに設定、生成された URL に(同じブラウザだと Cookie が残っているため)別 web ブラウザ(Safari, Firefoxなど)でアクセス
  7. メンター登録ページが表示されることを確認
  8. 「所属企業」の項目に 5 で設定した企業が設定されていることを確認
  9. 必須項目を入力して「メンター登録」ボタンを押下
  10. 作成したメンターでログインして、プロフィールの区分が「メンター」になっていることを確認
  11. Chrome (komagata でログインしている web ブラウザ) でメンターが参加したことを確認

Screenshot

変更前

2024-09-19.21.22.31.mov

変更後

2024-09-30.0.45.14.mov

./bin/lint 実行時に slim-lint の Warning が発生するため、引数を明示的に指定する形にしています

./bin/lint を実行した際、app/views/users/_form.html.slim:29 で slim-lint による Warning が表示されました。詳細は以下の通りです。

  • ローカルでの実行結果
$ ./bin/lint                                                                    
Inspecting 738 files
..................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

738 files inspected, no offenses detected
app/views/users/_form.html.slim:29 [W] RuboCop: Lint/Syntax: unexpected token tSTRING
(Using Ruby 3.1 parser; configure using `TargetRubyVersion` parameter, under `AllCops`)
  • CI での実行結果

https://app.circleci.com/pipelines/github/fjordllc/bootcamp/10647/workflows/3596c345-a152-4335-a0c1-99dc35dbf868/jobs/38622?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary

原因としては、slim-lint の構文解析が正しく処理できていない可能性が高いため、回避策として引数を明示的に指定する方法に変更しました。詳細な対応は以下のコミットをご参照ください。
7da88b0

今回の対応は、以下の Issue と bootcamp の Q&A ページを参考に判断しました。

@MikotoMakizuru MikotoMakizuru self-assigned this Sep 19, 2024
@MikotoMakizuru
Copy link
Contributor Author

@kyokucho1989
お疲れ様です。PR のレビューをお願いすることは可能でしょうか
よろしくお願いします。

@kyokucho1989
Copy link
Contributor

@MikotoMakizuru
りょうかいしました!
一週間ほどかかるかと思います。

@kyokucho1989 kyokucho1989 self-requested a review September 19, 2024 20:23
@MikotoMakizuru
Copy link
Contributor Author

@kyokucho1989
ありがとうございます!よろしくお願いします🙇‍♂️

@MikotoMakizuru MikotoMakizuru requested review from kyokucho1989 and removed request for kyokucho1989 September 20, 2024 00:56
Copy link
Contributor

@kyokucho1989 kyokucho1989 left a comment

Choose a reason for hiding this comment

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

@MikotoMakizuru
レビューしました!

確認したいことを書いてます。

動作自体は問題ありません。マージされたら、招待URLがうまく生成されるかステージング環境で確認をお願いします。

- elsif @user.mentor?
- title 'メンター登録'
- set_meta_tags(site: 'FJORD BOOT CAMP(フィヨルドブートキャンプ)',
description: 'フィヨルドブートキャンプメンター登録フォームです。オンラインプログラミングスクールのフィヨルドブートキャンプにアドバイザーとして登録する際はこちらのページからお申し込みください。')
Copy link
Contributor

Choose a reason for hiding this comment

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

オンラインプログラミングスクールのフィヨルドブートキャンプにアドバイザーとして

コピー元の文章のままでした。「メンターとして」と修正をお願いします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます、完全に見落としてましたmm

修正しました15bd222

= f.check_box :mentor, class: 'a-toggle-checkbox'
= f.label :mentor, class: 'a-block-check__label is-ta-left' do
= f.check_box :mentor, class: 'a-toggle-checkbox', id: 'checkbox_mentor'
= f.label :mentor, class: 'a-block-check__label is-ta-left', for: 'checkbox_mentor' do
Copy link
Contributor

Choose a reason for hiding this comment

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

「管理画面からメンター権限を付与するためのチェックボックスにid・for属性を追加してチェックボックスが活性化するように修正」
とcommitがありますが、これは今回のIssueで必要な対応でしょうか?
これができないと招待用のURLがうまく生成できない、という問題が出てくるとか。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

このコミットしたとき、このテストが通らなくて原因探ったところ、参加登録ページの管理者でログインしたときのみ表示される以下の部分のメンターのチェックボックス私の変更が原因で非活性になっていました。

スクリーンショット 2024-09-22 6 23 35

main ブランチでも活性になっていましたし、こちらの PR で管理者は特殊ユーザー属性でメンター権限を付けられるようにしたみたいなので、活性になっているのが正しい挙動です。上記の画像の3つのチェックボックスは以下3つのファイルがapp/views/users/_form.html.slimから render されて表示されるのでが_mentor.html.slimにのみid・for 属性が入っていないという状況でした。

  • app/views/users/form/_adviser.html.slim
  • app/views/users/form/_trainee.html.slim
  • app/views/users/form/_mentor.html.slim

同じチェックボックスのアイテムなのに_adviser.html.slim_trainee.html.slim には属性があって_mentor.html.slimにのみ属性がないのはおかしいじゃないのかと思い、属性を入れたところ活性となりテストも通るようになったので今回は id・for の属性を入れました。

@MikotoMakizuru MikotoMakizuru force-pushed the feature/enable-mentor-register-from-mentor-registration-screen branch 2 times, most recently from 822aaf9 to 15bd222 Compare September 21, 2024 20:42
Copy link
Contributor

@kyokucho1989 kyokucho1989 left a comment

Choose a reason for hiding this comment

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

@MikotoMakizuru
対応ありがとうございました!Approveしますー

@MikotoMakizuru
Copy link
Contributor Author

@kyokucho1989 ありがとうございます🙇‍♂️

@komagata お疲れ様です、メンバーレビューが終わりましたのでレビューお願いしたいです!

@komagata
Copy link
Member

@MikotoMakizuru テキストエリアのplaceholderやその説明文、採用する項目などは基本的にアドバイザーのものを選択してください。

@MikotoMakizuru MikotoMakizuru force-pushed the feature/enable-mentor-register-from-mentor-registration-screen branch from 15bd222 to c201fc0 Compare September 29, 2024 15:35
@MikotoMakizuru
Copy link
Contributor Author

@komagata
レビュー内容修正しました。プルリクの変更後の動画は今回の修正を取り入れた内容に更新しています。

また .bin/lint 実行時に slim-lint で Warning が出るので引数を明示的に指定する書き方にしています。( 7da88b0 ) プルリクの下の方に判断に至るまでの背景も記載しています。

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 69fcbf8 into main Oct 1, 2024
3 checks passed
@komagata komagata deleted the feature/enable-mentor-register-from-mentor-registration-screen branch October 1, 2024 18:26
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