管理者の場合、ユーザー情報変更ページでコース変更できるようにした #4596

Saki-htr · 2022-04-09T10:10:44Z

ユーザー情報変更ページで、管理者の場合はコース変更ができるようにしたい #4138

概要

現在、DBを直接変更しないとユーザーのコース変更ができないため、管理者でログインした時、ユーザー情報変更ページでコースを変更できるようにしました。

変更前

管理者でログイン時、ユーザー情報変更ページに、コース変更の項目が無い。

変更後

管理者でログイン時、ユーザー情報変更ページの「以下管理者のみ操作ができます」の下の一番目に、コースの項目が有る。

動作確認の手順

feature/enable-change-courses-at-user-info-edit-page-when-logined-as-adminブランチをローカルに持ってきて、立ち上げる
管理者 (machida)でログインする。
kensyu のユーザー個別ページにアクセスする。(http://localhost:3000/users/301971253)
プロフィールのコースが「Rails Webプログラマー」になっていることを確認する。
プロフィールの「管理者として情報変更」をクリックする。
「以下管理者のみ操作ができます」の下の一番目に、「コース」の項目があることを確認する。
コースを「iOSプログラマー」に変更し、「更新する」をクリックする。
kensyuのプロフィールのコースが、「iOSプログラマー」に変更されていることを確認する。

Saki-htr · 2022-04-11T04:40:51Z

app/views/users/form/_course.html.slim

  = f.label :course_id, class: 'a-form-label is-required'
  .a-button.is-md.is-secondary.is-select.is-block
-    = f.collection_select :course_id, Course.order(:created_at), :id, :title, {}
+    = f.collection_select :course_id, Course.order(:created_at), :id, :title
  .a-form-help
    p コースの一覧は#{link_to 'こちら', courses_path, trget: '_blank'}


新しくコース変更用のパーシャルを作成しようとしたところ、すでにusers/form下に_course.html.slimが存在しており、中身を確認したところ、そのままコース変更の項目として利用できそうでしたので、こちらのパーシャルを使うことにし、app/views/users/_form.html.slimでrenderしています。

こちらのパーシャルが他のファイルで呼び出されているか確認したところ、リポジトリ内を「users/form/course」や「form/course」で検索しても出てこず、コードに変更を加えても他の場所に影響がないと考え、以下2点を変更いたしました。

{}は削除しても動作に問題がなかったため、不要と考え削除しました。

.form-item.is-hiddenですと、renderしても表示されないため、users/form/job_seekingやusers/form/companyなどのCSSセレクタに合わせ、.form-itemに変更しました。

Saki-htr · 2022-04-11T05:03:46Z

@saeyama さん
お疲れさまです☺️
ユーザーの登録情報変更ページまわりのissueを担当されていらしたので、こちらのレビューをお願いさせていただければと思います🙏
急いでいないので、いつでも大丈夫です🙆‍♀️
また、レビュー依頼が複数きている等でお忙しいようでしたら、他の方に依頼しますので、遠慮なく仰ってください~
よろしくお願いいたします🙇‍♀️

saeyama

@Saki-htr さん、レビュー依頼ありがとうございます！
説明とてもわかりやすかったです。ありがとうございます😊
動作およびテストが通ったことを確認出来ました！

1点、お手数ですがDraft Pull RequestになっていたのでPull Requestに変更お願いします…！
私の方ではApprove致します！よろしくお願いします🙏

saeyama · 2022-04-11T16:33:40Z

app/views/users/form/_course.html.slim

  = f.label :course_id, class: 'a-form-label is-required'
  .a-button.is-md.is-secondary.is-select.is-block
-    = f.collection_select :course_id, Course.order(:created_at), :id, :title, {}


{}は削除しても動作に問題がなかったため、不要と考え削除しました。

余談なんですが、以前レビューさせてもらった、#4385のapp/views/users/form/_company.html.slim で{}がないと、その後ろに記述されるHTML属性が効かなかったので、今後、Choices-jsを使って、インクリメンタルサーチできる仕様にする可能性ありそうな気がしたので、残しておいてもいいのかな〜って一瞬思ったのですが、現状なくても動くので問題ないと思います…！

参考にしたサイト

{}は、Choices-jsを使って、インクリメンタルサーチできるようにする際に必要なのですね〜
勉強になります。ありがとうございます！

今は問題なく動いているので、削除しておくか

今後、Choices-jsを使って、インクリメンタルサーチできる仕様にする可能性があるので削除しないでおくか

で非常に迷ったのですが、今後に備えて残しておくことにしました。
61e71e5のコミットで修正しましたので、ご確認をお願いいたします🙏

戻して頂いたんですね…！ありがとうございます🙏

{}は、Choices-jsを使って、インクリメンタルサーチできるようにする際に必要なのですね〜

Choices-jsに限らず、javascriptとかで何かイベントを発火させたい時に、記述の順番として
HTML属性={} or イベント属性={}の指定の前にはオプション={}が必要そうな感じがしました…！
もしかしたら、なくても内容によっては動くかもしれませんが😅

参考にしたサイトより↓

f.collection_select(メソッド名, 要素の配列, value属性の項目, テキストの項目, オプション={}, HTML属性={} or イベント属性={}) 

私が以前レビューさせてもらったissueの場合だと、company-select.jsの4行目のgetElementById('js-company-select')を _company.html.slimの= f.collection_select に指定した時に{}を外すと動かなかったんですよね〜。

bootcamp/app/javascript/company-select.js

Lines 3 to 15 in a55ba9f

document.addEventListener('DOMContentLoaded', () => {

const element = document.getElementById('js-company-select')

if (element) {

return new Choices(element, {

removeItemButton: true,

allowHTML: true,

searchResultLimit: 10,

searchPlaceholderValue: '検索ワード',

noResultsText: '一致する情報は見つかりません',

itemSelectText: '選択'

})

}

})

bootcamp/app/views/users/form/_company.html.slim

Line 4 in a55ba9f

= f.collection_select :company_id, all_companies_with_empty, :id, :name, {}, { id: 'js-company-select' }

Saki-htr · 2022-04-12T07:01:47Z

@saeyama さん

早速レビューしてくださってありがとうございます🙏 ✨
伊藤さんより、こちらの日報で、システムテストでfindでなく reload を使うべきと教えていただきましたので、修正いたしました。
既に見てくださったのに大変申し訳ないのですが、再度レビューをrequestさせていただきました。
お手すきの際にご確認いただけますと幸いです🙇‍♀️

変更点は以下の3つです。

1点目：Draft Pull Request を Pull Requestに変更

Draft Pull Request を Pull Requestに変更にするのを失念しており、申し訳ありませんでした🙏
変更いたしました!

2点目：`{}`を再入力しました

こちらに詳細をコメントいたしましたが、削除した{}を再度入力することにいたしました。

3点目：システムテストの`find`を`reload`に変更しました。

findとreloadどちらも、DBからデータを再読み込みし、最新の値を取得しているのですが、以下2点の理由から、reloadに変更しました。

User.find(user.id)を使うと冗長で、reloadを使った方がシンプルなため
user.reloadを使ったほうが、「userのデータを再読み込み（reload）する」という意図が分かりやすいため

b3bbf37 のコミットで修正いたしました。

saeyama

@Saki-htr さん、変更頂いた３点確認できました！
テストのreloadとても勉強になりました…！
日報も読ませていただきました〜！ありがとうございます🙏

改めて、Approve致します！よろしくお願いします😁

saeyama · 2022-04-12T12:53:18Z

app/views/users/form/_course.html.slim

  = f.label :course_id, class: 'a-form-label is-required'
  .a-button.is-md.is-secondary.is-select.is-block
-    = f.collection_select :course_id, Course.order(:created_at), :id, :title, {}


戻して頂いたんですね…！ありがとうございます🙏

{}は、Choices-jsを使って、インクリメンタルサーチできるようにする際に必要なのですね〜

Choices-jsに限らず、javascriptとかで何かイベントを発火させたい時に、記述の順番として
HTML属性={} or イベント属性={}の指定の前にはオプション={}が必要そうな感じがしました…！
もしかしたら、なくても内容によっては動くかもしれませんが😅

参考にしたサイトより↓

f.collection_select(メソッド名, 要素の配列, value属性の項目, テキストの項目, オプション={}, HTML属性={} or イベント属性={}) 

私が以前レビューさせてもらったissueの場合だと、company-select.jsの4行目のgetElementById('js-company-select')を _company.html.slimの= f.collection_select に指定した時に{}を外すと動かなかったんですよね〜。

bootcamp/app/javascript/company-select.js

Lines 3 to 15 in a55ba9f

document.addEventListener('DOMContentLoaded', () => {

const element = document.getElementById('js-company-select')

if (element) {

return new Choices(element, {

removeItemButton: true,

allowHTML: true,

searchResultLimit: 10,

searchPlaceholderValue: '検索ワード',

noResultsText: '一致する情報は見つかりません',

itemSelectText: '選択'

})

}

})

bootcamp/app/views/users/form/_company.html.slim

Line 4 in a55ba9f

= f.collection_select :company_id, all_companies_with_empty, :id, :name, {}, { id: 'js-company-select' }

Saki-htr · 2022-04-13T00:36:58Z

@saeyama さん
早速レビューしてくださってありがとうございました！
f.collection_selectについて、とても丁寧に教えてくださってありがとうございます☺️

@komagata さん
チームメンバーのレビューを通りましたので、お手すきの際にレビューしていただけますと幸いです。
よろしくお願いいたします🙇‍♀️

komagata · 2022-04-13T12:30:26Z

test/system/admin/users_test.rb

@@ -182,4 +182,14 @@ class Admin::UsersTest < ApplicationSystemTestCase
      assert_text 'アドバイザー'
    end
  end
+
+  test 'admin can change user course' do


逆に一般ユーザーで変更できると困るので、「一般ユーザーは変更できない」というテストもあるとありがたいです〜

@komagata さん
仰る通り、「一般ユーザーは変更できない」テストも追加したほうが良いと思いましたので、追加いたしました。
16b7905で変更しております。

テスト名ですが、'not admin cannot change user course' ですと、2つ否定が入っていて分かりづらいと思いましたので、 'general user cannot change user course' という名前にしました。
よろしくお願いいたします🙏

komagata

確認させて頂きました。OKです〜🙆‍♂️

Saki-htr · 2022-04-23T06:00:54Z

@komagata @machida
お疲れ様です。
お手すきの際に、本番環境での動作確認をお願いいたします。
よろしくお願いいたします🙇‍♀️

komagata · 2022-04-23T12:38:42Z

@Saki-htr 本番環境で動作することを確認しました。

Saki-htr · 2022-04-23T12:41:48Z

@komagata さん
ご確認いただき、ありがとうございます！
issueをcloseいたしました🙏

Saki-htr changed the title ~~Feature/enable change courses at user info edit page when logined as admin~~ 管理者の場合、ユーザー情報変更ページでコース変更できるようにした Apr 9, 2022

Saki-htr force-pushed the feature/enable-change-courses-at-user-info-edit-page-when-logined-as-admin branch from cfb8a11 to 4ca22ac Compare April 10, 2022 05:33

Saki-htr self-assigned this Apr 11, 2022

Saki-htr commented Apr 11, 2022

View reviewed changes

Saki-htr requested a review from saeyama April 11, 2022 04:57

saeyama approved these changes Apr 11, 2022

View reviewed changes

Saki-htr marked this pull request as ready for review April 12, 2022 01:17

Saki-htr force-pushed the feature/enable-change-courses-at-user-info-edit-page-when-logined-as-admin branch from 4ca22ac to 61e71e5 Compare April 12, 2022 05:54

Saki-htr requested a review from saeyama April 12, 2022 06:36

saeyama approved these changes Apr 12, 2022

View reviewed changes

Saki-htr requested a review from komagata April 13, 2022 00:35

komagata reviewed Apr 13, 2022

View reviewed changes

Saki-htr added 6 commits April 17, 2022 13:13

ユーザー情報変更ページの管理者が変更できる項目に、コースを追加

2cc338f

不要な{}を削除

41f5e07

管理者が登録情報変更ページで、コース変更できることを確認するテストを追加

e552597

findよりreloadを使ったほうが、シンプルかつuserのデータを再読み込みしているとすぐに理解できるため、変更

1a89100

Choices-jsを使って、インクリメンタルサーチできる仕様にする際に必要になるため、{}を再追加

935a07a

管理者以外の一般ユーザーは、ユーザーのコースを変更できないテストを追加

16b7905

Saki-htr force-pushed the feature/enable-change-courses-at-user-info-edit-page-when-logined-as-admin branch from dc297a6 to 16b7905 Compare April 17, 2022 04:42

komagata approved these changes Apr 17, 2022

View reviewed changes

komagata merged commit de027c6 into main Apr 17, 2022

komagata deleted the feature/enable-change-courses-at-user-info-edit-page-when-logined-as-admin branch April 17, 2022 06:43

github-actions bot mentioned this pull request Apr 17, 2022

Release 2022-04-21 11:15:40 #4627

Merged

20 tasks

Saki-htr added the 2 label Jul 5, 2022

Saki-htr removed the 2 label Aug 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

管理者の場合、ユーザー情報変更ページでコース変更できるようにした #4596

管理者の場合、ユーザー情報変更ページでコース変更できるようにした #4596

Saki-htr commented Apr 9, 2022

Saki-htr Apr 11, 2022

Saki-htr commented Apr 11, 2022

saeyama left a comment •

edited

Loading

saeyama Apr 11, 2022

Saki-htr Apr 12, 2022

saeyama Apr 12, 2022

Saki-htr commented Apr 12, 2022

saeyama left a comment

saeyama Apr 12, 2022

Saki-htr commented Apr 13, 2022

komagata Apr 13, 2022

Saki-htr Apr 17, 2022

komagata left a comment

Saki-htr commented Apr 23, 2022

komagata commented Apr 23, 2022

Saki-htr commented Apr 23, 2022

	document.addEventListener('DOMContentLoaded', () => {
	const element = document.getElementById('js-company-select')
	if (element) {
	return new Choices(element, {
	removeItemButton: true,
	allowHTML: true,
	searchResultLimit: 10,
	searchPlaceholderValue: '検索ワード',
	noResultsText: '一致する情報は見つかりません',
	itemSelectText: '選択'
	})
	}
	})

管理者の場合、ユーザー情報変更ページでコース変更できるようにした #4596

管理者の場合、ユーザー情報変更ページでコース変更できるようにした #4596

Conversation

Saki-htr commented Apr 9, 2022

概要

変更前

変更後

動作確認の手順

Saki-htr Apr 11, 2022

Choose a reason for hiding this comment

Saki-htr commented Apr 11, 2022

saeyama left a comment • edited Loading

Choose a reason for hiding this comment

saeyama Apr 11, 2022

Choose a reason for hiding this comment

Saki-htr Apr 12, 2022

Choose a reason for hiding this comment

saeyama Apr 12, 2022

Choose a reason for hiding this comment

Saki-htr commented Apr 12, 2022

1点目：Draft Pull Request を Pull Requestに変更

2点目：{}を再入力しました

3点目：システムテストのfindをreloadに変更しました。

saeyama left a comment

Choose a reason for hiding this comment

saeyama Apr 12, 2022

Choose a reason for hiding this comment

Saki-htr commented Apr 13, 2022

komagata Apr 13, 2022

Choose a reason for hiding this comment

Saki-htr Apr 17, 2022

Choose a reason for hiding this comment

komagata left a comment

Choose a reason for hiding this comment

Saki-htr commented Apr 23, 2022

komagata commented Apr 23, 2022

Saki-htr commented Apr 23, 2022

saeyama left a comment •

edited

Loading

2点目：`{}`を再入力しました

3点目：システムテストの`find`を`reload`に変更しました。