ユーザー一覧ページの絞り込みにおける現役生と研修生のルールを整理 #7619

unikounio · 2024-03-30T10:32:22Z

Issue

ユーザー一覧の絞り込みのルールを変更。 #7614

概要

ユーザー一覧ページの絞り込みタブを次のように整理しました。

変更前
「現役生」タブを選択すると現役生と研修生が表示される。
変更後
- 「現役 + 研修生」タブを選択すると現役生と研修生が表示される
- 「現役生」タブを選択すると現役生だけが表示される
- 「研修生」タブの表示位置を現役生の隣に変更

変更確認方法

feature/update-users-filtering-rules-about-student-and-traineeをローカルに取り込む
bin/setupを実行
foreman start -f Procfile.devでサーバーを立ち上げる
メンターアカウント（komagata等）でログインし、/usersにアクセス
絞り込みタブから「現役生」をクリックし、次の作業を行う
1. ページタイトル横の数値をメモする
2. 現役生のユーザーのみが表示されることを確認する
絞り込みタブから「研修生」をクリックし、ページタイトル横の数値をメモする
絞り込みタブから「現役 + 研修生」をクリックし次の点を確認する
1. ページタイトル横の数値が現役生 + 研修生となっている
2. 現役生と研修生のユーザーが表示される

Screenshot

変更前

変更後

unikounio · 2024-03-30T11:06:17Z

@88-99 さん
お疲れ様です！
こちらのPRのレビューをお願いさせていただきたくてご連絡させていただきました🙏
ご対応可能なようでしたら、お手すきの際にでもご確認いただけますと幸いです。
よろしくお願いいたします。

88-99 · 2024-03-30T23:20:57Z

@unikounio
お疲れ様です。
今週中でも大丈夫でしたらお受けできますがいかがでしょうか？
もし急ぐようでしたら、他の方にお願いいたします。

unikounio · 2024-03-31T08:42:12Z

@88-99 さん
ご連絡ありがとうございます～
急ぎませんので、是非よろしくお願いします🙏

88-99

@unikounio
お待たせして申し訳ございません。
フィルタリングの挙動は問題ないように思いました。
テストで気になった点を3つコメントさせていただきました。ご確認よろしくお願いいたします。

88-99 · 2024-04-06T08:03:50Z

test/system/users_test.rb

+    visit_with_auth '/users', 'komagata'
+    click_link '現役 + 研修生'
+
+    assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')


.is-activeが抜けていませんでしょうか？ 現役 + 研修生以外のタグに変更してもテストが通りました。

おっしゃる通り、抜けていました😅
修正させていただきました～🙏

88-99 · 2024-04-06T08:11:06Z

test/system/users_test.rb

+    click_link '現役生'
+
+    assert_selector('a.tab-nav__item-link.is-active', text: '現役生')
+    assert_text '現役生'


assert_text '現役 + 研修生'と同じで、そちらにコメントさせていただきました。

88-99 · 2024-04-06T09:17:49Z

test/system/users_test.rb

+    click_link '現役 + 研修生'
+
+    assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')
+    assert_text '現役 + 研修生'


こちらの意図をお伺いしたいです。ページ内には'現役 + 研修生'以外にも全てのタグが表示されているので、どのタグでもassert_textはtrueを返すと思うのですが、いかがでしょうか？

申し訳ないのですが、ご質問の内容についてわからない部分があったので確認させてください🙏

こちらは「assert_selectorでは現役 + 研修生という文字列の表示を確認しているわけではないので、assert_textで表示について確認する必要がある」という意図で書いたテストコードです。

ページ内には'現役 + 研修生'以外にも全てのタグが表示されているので、どのタグでもassert_textはtrueを返す

こちらおっしゃる通り、assert_textの対象文字列を変えればどのタグでもtrueは返すと思います。
テスト対象であるタグの表示が確認できればいいと考えて書いたのですが、すべてのタグについてテストすべきということでしょうか？あるいはこのテストコードは不要ということでしょうか？

@unikounio
ご確認ありがとうございます。
私もunikounioさんがされようとしていることが理解できていなかもしれません。

あるいはこのテストコードは不要ということでしょうか？

不要かなと思いました。

こちらは「assert_selectorでは現役 + 研修生という文字列の表示を確認しているわけではないので、

『現役 + 研修生という文字列の表示』とは画像②のことを仰っていますか？

私の認識ですが、

assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')ではテキストが現役 + 研修生のリンクがあること(画像①)を確認している。

assert_text '現役 + 研修生'では画像①と画像②どちらにもヒットしてしまうので、②が「卒業生」でも①でtrueになる。

click_link '現役 + 研修生'で現役 + 研修生のリンク(タグ?)があることはわりますし(なければクリックできずエラーになる)、assert_selector('h1.page-main-header__title', text: '現役 + 研修生')で現役 + 研修生のフィルタがかかってますよというのだけ(画像②)確認すれば良いのではないかと思いました。

- assert_selector('a.tab-nav__item-link', text: '現役 + 研修生') - assert_text '現役 + 研修生' + assert_selector('h1.page-main-header__title', text: '現役 + 研修生')

間違ってたらすみません。

丁寧なご説明ありがとうございます！

assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')ではテキストが現役 + 研修生のリンクがあること(画像①)を確認している。

この点について誤解していました💦
display: noneが設定されている等で画面に表示されていなくてもテストが通ってしまうものと思っていました😅
お返事するときも確認したはずなのですが気づけず😭ご迷惑をおかけしました🙏

ご提示いただいた形で修正させていただきました～

unikounio

@88-99 さん
ご確認ありがとうございます！✨
コメントいただいた点について対応させていただきました。
お手すきの際にご確認をお願いいたします🙏

unikounio · 2024-04-07T08:58:46Z

app/views/users/_user.html.slim

@@ -17,7 +17,7 @@
          .users-item__header-start
            .users-item__icon
              = link_to user.url
-                span class="a-user-role"


#7426 のマージに伴いユーザーロールが渡されなくなっていたようなので修正しました。
「期生別」タブではユーザーロールが渡されているようなので変更しても大丈夫だろうと判断したのですが、こちらについてもご確認いただけますと幸いです（駒形さんのレビュー時にもご確認をお願いする予定です）。

【4/24追記】
同様の内容である#7665 がマージされたため、本PRで変更を行う必要がなくなりました。

元々あったのであれば、あっても良い気がしますね。

unikounio · 2024-04-07T09:12:17Z

test/system/users_test.rb

+    click_link '現役 + 研修生'
+
+    assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')
+    assert_text '現役 + 研修生'


申し訳ないのですが、ご質問の内容についてわからない部分があったので確認させてください🙏

こちらは「assert_selectorでは現役 + 研修生という文字列の表示を確認しているわけではないので、assert_textで表示について確認する必要がある」という意図で書いたテストコードです。

ページ内には'現役 + 研修生'以外にも全てのタグが表示されているので、どのタグでもassert_textはtrueを返す

こちらおっしゃる通り、assert_textの対象文字列を変えればどのタグでもtrueは返すと思います。
テスト対象であるタグの表示が確認できればいいと考えて書いたのですが、すべてのタグについてテストすべきということでしょうか？あるいはこのテストコードは不要ということでしょうか？

unikounio · 2024-04-07T09:18:24Z

test/system/users_test.rb

+    visit_with_auth '/users', 'komagata'
+    click_link '現役 + 研修生'
+
+    assert_selector('a.tab-nav__item-link', text: '現役 + 研修生')


おっしゃる通り、抜けていました😅
修正させていただきました～🙏

unikounio

@88-99 さん
再度のご確認ありがとうございます！
修正させていただきましたのでご確認をお願いいたします🙏

88-99

@unikounio
修正ありがとうございました。確認しました。
承認させていただきます。

unikounio · 2024-04-13T12:24:22Z

@88-99 さん
丁寧にご確認いただきありがとうございました！
引き続きチームメイトとしてよろしくお願いいたします～😄

unikounio · 2024-04-13T12:27:56Z

@komagata さん
お疲れさまです！
チームメイトからApproveいただきましたので、お手すきの際にレビューをお願いします🙏

Issueへの直接的な対応ではないのですが下記コメントの箇所を追加で変更しておりますので、ご確認いただけますと幸いです。
#7619 (comment)

komagata

conflictの修正をお願い致します～。

unikounio · 2024-04-24T05:48:32Z

@komagata さん
ご確認ありがとうございます！
コンフリクト対応させていただきましたので、お手すきの際に再度ご確認をお願いいたします🙏

komagata · 2024-04-26T05:42:36Z

test/system/users_test.rb

+    visit_with_auth '/users', 'komagata'
+    click_link '現役 + 研修生'
+
+    assert_selector('h1.page-main-header__title', text: '現役 + 研修生')


上のクリック前にもこのselectorの要素が存在する（中身は違う）とすると、JSでデータを取ってくる前の状態をチェックしてFlakyなテストになっちゃいそうな気がしました。

@komagata さん
ご連絡ありがとうございます🙏
おっしゃる通りですね。テスト対象のタブがアクティブになっていることを確認できるように修正しました！
１つ前のコミットに戻すような形になったので、コミットの整理も行わせていただきました～

ロケールファイルの変更が影響するテストも修正した。

併せて関連するリスト等の要素も並び替えた

unikounio · 2024-05-11T08:59:26Z

@komagata さん
お疲れ様です！
コメントいただいいた点について対応させていただきましたので、お手隙の際にご確認をお願いいたします🙏
#7619 (comment)

komagata

確認させていただきました。OKです～👌

unikounio self-assigned this Mar 30, 2024

unikounio marked this pull request as ready for review March 30, 2024 11:02

unikounio requested a review from 88-99 March 30, 2024 11:04

88-99 reviewed Apr 6, 2024

View reviewed changes

unikounio force-pushed the feature/update-users-filtering-rules-about-student-and-trainee branch from 11fdebe to b7321b2 Compare April 7, 2024 08:06

unikounio commented Apr 7, 2024

View reviewed changes

unikounio force-pushed the feature/update-users-filtering-rules-about-student-and-trainee branch from b7321b2 to b59593a Compare April 10, 2024 12:08

unikounio commented Apr 10, 2024

View reviewed changes

88-99 approved these changes Apr 11, 2024

View reviewed changes

unikounio requested a review from komagata April 13, 2024 12:28

komagata reviewed Apr 24, 2024

View reviewed changes

unikounio force-pushed the feature/update-users-filtering-rules-about-student-and-trainee branch from b59593a to debd573 Compare April 24, 2024 05:20

komagata reviewed Apr 26, 2024

View reviewed changes

unikounio added 4 commits May 2, 2024 00:57

ユーザー一覧ページの絞り込みタブに関するシステムテストを追加

8cb8880

ユーザー一覧ページの「現役生」タブを「現役 + 研修生」タブに変更

9a6939f

ロケールファイルの変更が影響するテストも修正した。

ユーザー一覧ページの絞り込みタブに「現役生」を追加

e61fdac

ユーザー一覧ページの絞り込みタブの並び替え

6984d3c

併せて関連するリスト等の要素も並び替えた

unikounio force-pushed the feature/update-users-filtering-rules-about-student-and-trainee branch from debd573 to 6984d3c Compare May 1, 2024 16:12

komagata approved these changes May 28, 2024

View reviewed changes

komagata merged commit b79fc6c into main May 28, 2024
3 checks passed

komagata deleted the feature/update-users-filtering-rules-about-student-and-trainee branch May 28, 2024 19:17

github-actions bot mentioned this pull request May 28, 2024

Release 2024-06-04 14:22:37 +0000 #7802

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ユーザー一覧ページの絞り込みにおける現役生と研修生のルールを整理 #7619

ユーザー一覧ページの絞り込みにおける現役生と研修生のルールを整理 #7619

unikounio commented Mar 30, 2024 •

edited

Loading

unikounio commented Mar 30, 2024

88-99 commented Mar 30, 2024

unikounio commented Mar 31, 2024

88-99 left a comment

88-99 Apr 6, 2024

unikounio Apr 7, 2024

88-99 Apr 6, 2024

88-99 Apr 6, 2024

unikounio Apr 7, 2024

88-99 Apr 7, 2024

unikounio Apr 10, 2024

unikounio left a comment

unikounio Apr 7, 2024 •

edited

Loading

88-99 Apr 7, 2024

unikounio Apr 7, 2024

unikounio Apr 7, 2024

unikounio left a comment

88-99 left a comment

unikounio commented Apr 13, 2024

unikounio commented Apr 13, 2024

komagata left a comment

unikounio commented Apr 24, 2024

komagata Apr 26, 2024

unikounio May 1, 2024

unikounio commented May 11, 2024

komagata left a comment

ユーザー一覧ページの絞り込みにおける現役生と研修生のルールを整理 #7619

ユーザー一覧ページの絞り込みにおける現役生と研修生のルールを整理 #7619

Conversation

unikounio commented Mar 30, 2024 • edited Loading

Issue

概要

変更確認方法

Screenshot

変更前

変更後

unikounio commented Mar 30, 2024

88-99 commented Mar 30, 2024

unikounio commented Mar 31, 2024

88-99 left a 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

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

unikounio left a comment

Choose a reason for hiding this comment

unikounio Apr 7, 2024 • 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

unikounio left a comment

Choose a reason for hiding this comment

88-99 left a comment

Choose a reason for hiding this comment

unikounio commented Apr 13, 2024

unikounio commented Apr 13, 2024

komagata left a comment

Choose a reason for hiding this comment

unikounio commented Apr 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unikounio commented May 11, 2024

komagata left a comment

Choose a reason for hiding this comment

unikounio commented Mar 30, 2024 •

edited

Loading

unikounio Apr 7, 2024 •

edited

Loading