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.

Sign up for GitHub

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

Jump to bottom

日報が確認OKされていない場合、メンターがコメントした時にアラートが出るように実装 #8061

Merged
merged 7 commits into from
Nov 7, 2024
Merged

日報が確認OKされていない場合、メンターがコメントした時にアラートが出るように実装 #8061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7eff41c
日報が確認OKされていない場合、メンターがコメントした時にアラートが出るように実装
su-su-su-su Sep 10, 2024
280110e
日報が未確認の場合、ダイアログを表示するか確認するテストを追加
su-su-su-su Sep 23, 2024
fabeb51
rubocopに対する修正
su-su-su-su Sep 23, 2024
0b37e61
テストのアラートが出るタイミングをコメントした後になるように修正
su-su-su-su Sep 29, 2024
2075770
テストを元のコードに修正
su-su-su-su Oct 1, 2024
c9a5f15
confirmをcheckに統一、冗長な処理を簡潔化
su-su-su-su Oct 16, 2024
02d7e9d
不必要なブロックの入れ子を避ける為に修正
su-su-su-su Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/javascript/comments.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,15 @@ export default {
}
},
postComment() {
if (this.commentableType === 'Report' && this.isRole('mentor')) {
const notChecked = !this.checkId
if (
notChecked &&
!window.confirm('日報を確認済みにしていませんがよろしいですか？')
) {
return
}
}
this.createComment()
if (this.isUnassignedAndUnchekedProduct) {
this.checkProduct(
Expand Down
12 changes: 12 additions & 0 deletions test/system/comments_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,4 +347,16 @@ class CommentsTest < ApplicationSystemTestCase
end
assert_equal '.new-comment-file-input', find('textarea.a-text-input')['data-input']
end

test 'show confirm dialog if report is not confirmed' do
visit_with_auth "/reports/#{reports(:report2).id}", 'machida'
assert_text '確認OKにする'
within('.thread-comment-form__form') do
fill_in('new_comment[description]', with: 'comment test')
end
accept_confirm '日報を確認済みにしていませんがよろしいですか？' do
click_button 'コメントする'
end
assert_text 'comment test'
end
Comment on lines +350 to +361
Copy link
Contributor

Choose a reason for hiding this comment

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

テスト追加ありがとうございます🙏

  1. 未チェックの日報をクリック
  2. コメントを書いてコメントするをクリック
  3. 「日報を確認済みにしていませんがよろしいですか？」とアラートが出ることを確認

を確認するテストだと思うので、ちょっと意図とずれている印象です😓

この記述だと、

  1. フォームにコメントを入力する
  2. アラートが出る
  3. 「コメントする」ボタンを押す

ように見えます🤔
お手数ですが、もう一度確認いただけると幸いです🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Judeeeee

ご指摘ありがとうございます。
ご指摘の通りで

  1. フォームにコメントを入力する
  2. アラートが出る
  3. 「コメントする」ボタンを押す

となっておりました。
確認不足でした。お手間をおかけしました🙇‍♂️

  1. 未チェックの日報をクリック
  2. コメントを書いてコメントするをクリック
  3. 「日報を確認済みにしていませんがよろしいですか？」とアラートが出ることを確認

となるように

  test 'show confirm dialog if report is not confirmed' do
    visit_with_auth "/reports/#{reports(:report2).id}", 'machida'
    assert_text '確認OKにする'
    within('.thread-comment-form__form') do
      fill_in('new_comment[description]', with: 'comment test')
      click_button 'コメントする'
    end
    accept_confirm '日報を確認済みにしていませんがよろしいですか？' do
    end
    assert_text 'comment test'
  end

と修正しました。

再度ご確認いただけますと幸いです。
よろしくお願いいたします🙇‍♂️

Copy link
Contributor

@Judeeeee Judeeeee Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@su-su-su-su

ご対応いただきありがとうございます🙏
こちらですが、私のaccept_confirmの認識が間違っていたため、su-su-su-suさんの以前の変更が正しいです💦

公式ドキュメントの説明では、

Execute the block, accepting a confirm.
Expects a block whose actions will trigger the display modal to appear.

とあります。
https://www.rubydoc.info/gems/capybara/Capybara%2FSession:accept_confirm

ややこしいのですが、accept_confirmのブロック内に書く処理は「モーダルを出現させる処理」なので、以前の変更のままで十分意図通りだと思います😓

折角ご対応いただいたのに混乱させてしまって申し訳ないです🙇🙇

私の方からはApproveさせていただきますので、修正次第komagataさんにレビュー依頼をお願いいたします🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Judeeeee
ご確認ありがとうございます！
そうなのですね。
私こそ自分の書いたコードに自信がなく、ご指摘を頂いたまま修正してしまい申し訳ございません🙇‍♂️
こちら元のコードに修正いたしました。

end