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

WIPの質問は未解決一覧には含めず、全ての質問一覧にだけ含める #4604

Merged
merged 2 commits into from
Apr 17, 2022
Merged

WIPの質問は未解決一覧には含めず、全ての質問一覧にだけ含める #4604

merged 2 commits into from
Apr 17, 2022

Conversation

NorifumiOgawa
Copy link
Contributor

@NorifumiOgawa NorifumiOgawa commented Apr 12, 2022 •
edited
Loading

Issue: WIPの質問は未解決一覧には含めず、全ての質問一覧にだけ含めるようにしたい。 #4474

概要

以下の質問一覧表示時はWIP状態の質問を表示しないようにした。

  • 「未解決の質問一覧」
  • 「プラクティス」の個別ページの「質問」「未解決」タブ選択時

確認方法

  1. 本ブランチをローカルに取り込む。
  2. rails s を実行し開発環境を開く。
  3. 適当なユーザーでログインし、「OS X Mountain Lionをクリーンインストールする」のプラクティスを選択してWIPの質問を作成する。
  4. 左メニューの「Q&A」から未解決の質問一覧を開き、3で作成したWIPの質問が表示されていないことを確認する。
  5. 左メニューの「プラクティス」から「OS X Mountain Lionをクリーンインストールする」のプラクティスを開き、「質問」→「未解決」タブを選択して、3で作成したWIPの質問が表示されていないことを確認する。

変更前

未解決の質問一覧

WIPの質問が表示されています。
スクリーンショット 2022-04-12 13 59 39

プラクティスの質問ページ

WIPの質問が表示されています。
スクリーンショット 2022-04-12 13 59 17

変更後

未解決の質問一覧

WIPの質問が表示されなくなりました。
スクリーンショット 2022-04-12 14 00 17

プラクティスの質問ページ

WIPの質問が表示されなくなりました。
スクリーンショット 2022-04-12 14 00 26

@NorifumiOgawa NorifumiOgawa changed the title WIPの質問を未解決に表示しない WIPの質問は未解決一覧には含めず、全ての質問一覧にだけ含める Apr 12, 2022
@NorifumiOgawa NorifumiOgawa self-assigned this Apr 12, 2022
@NorifumiOgawa NorifumiOgawa marked this pull request as ready for review April 12, 2022 05:10
@NorifumiOgawa NorifumiOgawa requested a review from tksmasaki April 12, 2022 05:10
@NorifumiOgawa
Copy link
Contributor Author

@tksmasaki さんお疲れさまです！お手すきの際にチームレビューをお願いできますでしょうか。よろしくおねがいします🙏

tksmasaki
tksmasaki reviewed Apr 13, 2022
View reviewed changes
Copy link
Contributor

@tksmasaki tksmasaki left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@NorifumiOgawa
レビューさせていただきました！
いくつかコメントしたので、ご確認よろしくお願いします🙏

また、概要の内容について気になった点があったので、以下のコメントもご確認お願いします。

  • 確認手順の説明が以下のようになっていましたが、任意のプラクティスで作成するのならば、確認するのも選択したプラクティスになると思うので、3と5の内容を一致させたほうがいいかなと思いました。
  1. 適当なユーザーでログインし、任意のプラクティスを選択してWIPの質問を作成する。
  2. 左メニューの「Q&A」から未解決の質問一覧を開き、3で作成したWIPの質問が表示されていないことを確認する。
  3. 左メニューの「プラクティス」から「OS X Mountain Lionをクリーンインストールする」のプラクティスを開き、「質問」→「未解決」タブを選択して、3で作成したWIPの質問が表示されていないことを確認する。

test/system/practice/questions_test.rb

test 'not show a WIP question on the unsolved questions list ' do
visit_with_auth "/practices/#{practices(:practice1).id}/questions?not_solved=true", 'hatsuno'
assert_no_text 'wipテスト用の質問(wip中)'
Copy link
Contributor

Choose a reason for hiding this comment

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

何かがないことをテストする際は、そのページ特有のものがあることを一緒にアサーションしたほうがよいかなと思いました。

参考 : システムスペックやフィーチャスペックで「〜が表示されないこと」だけを検証するのはちょっと危険、という話 - Qiita

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます！こちら、「未解決」のラベルがアクティブになっていることの検証も追加してみました。

test/system/questions_test.rb
assert_text 'wipテスト用の質問(wip中)'
element = all('.thread-list-item').find { |component| component.has_text?('wipテスト用の質問(wip中)') }
within element do
assert_selector '.thread-list-item-title__icon.is-wip', text: 'WIP'
end
end

test 'not show a WIP question on the unsolved Q&A list page' do
visit_with_auth questions_path, 'kimura'
assert_no_text 'wipテスト用の質問(wip中)'
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.

こちら、「未解決の質問一覧」の文言が表示されていることの検証も追加してみました。

@NorifumiOgawa
Copy link
Contributor Author

@tksmasaki
レビューありがとうございます。ご指摘いただいた「何かがないことをテストする際は、そのページ特有のものがあることを一緒にアサーション」をそれぞれ追加してみました。
また、概要説明は質問作成時のプラクティスを任意ではなく「OS X Mountain Lionをクリーンインストールする」に指定して修正いたしました。
ご確認の程よろしくお願いいたします！

tksmasaki
tksmasaki approved these changes Apr 14, 2022
View reviewed changes
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.

@NorifumiOgawa
ご対応ありがとうございます！確認しました、僕からはOKです🙆

@NorifumiOgawa
Copy link
Contributor Author

@tksmasaki レビューありがとうございました！🙏
@komagata チームレビューを完了しました。レビューをよろしくお願いします。

@NorifumiOgawa NorifumiOgawa requested a review from komagata April 14, 2022 05:48
komagata
komagata approved these changes Apr 17, 2022
View reviewed changes
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 eae4957 into main Apr 17, 2022
@komagata komagata deleted the feature/wip-questions-are-not-included-in-the-list-of-unresolved-questions branch April 17, 2022 05:56
@github-actions github-actions bot mentioned this pull request Apr 17, 2022
Merged
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tksmasaki tksmasaki tksmasaki approved these changes

@komagata komagata komagata approved these changes

Assignees

@NorifumiOgawa NorifumiOgawa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NorifumiOgawa @komagata @tksmasaki