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

searchables.vueとsearchable.vueをReactコンポーネントに書き換えた #7218

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

2525nicole
Copy link
Contributor

@2525nicole 2525nicole commented Jan 21, 2024

Issue

概要

Vue.jsで実装されていた検索結果一覧表示を、Reactを使って書き換えました。

変更確認方法

React化後も問題なく機能することを確認する

  1. feature/convert-searchables-component-to-reactをローカルに取り込む
  2. foreman start -f Procfile.devを実行し、アプリを起動する
  3. komagataでログインし、トップページ右上にある検索窓から以下それぞれの検索を行う

スクリーンショット 2024-01-21 16 50 00

確認する検索内容

  • 検索対象:検索窓左側のプルダウンで選択
  • 検索ワード:検索窓右側に入力

「すべて」の検索

  • 検索対象:「すべて」
  • 検索ワード:テスト」
  • 複数の検索対象の結果が表示されている
  • 検索結果一覧の中で検索ワードの箇所が太字になっている
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • ページ上下にあるページネーションが機能する

検索結果が見つからない場合

  • 検索対象:任意のもの
  • 検索ワード:「存在しません」
  • 以下の画面が表示される
    image

「お知らせ」の検索

  • 検索対象:お知らせ
  • 検索ワード:お知らせ
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • 「コメント」検索結果において、アイコンまたはコメント投稿ユーザー名(以下、赤で囲んだ箇所)からユーザー個別ページに飛ぶ
  • 「コメント」検索結果において、お知らせ投稿ユーザー名(以下、青で囲んだ箇所)からユーザー個別ページに飛ぶ

スクリーンショット 2024-01-21 16 57 26

「プラクティス」の検索

  • 検索対象:プラクティス
  • 検索ワード:概要欄
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ

「日報」の検索

  • 検索対象:日報
  • 検索ワード:日報
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • 「WIP」と「コメント」の結果も表示される

スクリーンショット 2024-01-21 17 09 03

「提出物」の検索

  • 検索対象:提出物
  • 検索ワード:提出物
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • 「コメント」の検索結果も表示される

「Q&A」の検索

  • 検索対象:Q&A
  • 検索ワード:テスト
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • 「コメント」の検索結果も表示される
  • 「コメント」をクリックすると、質問そのものではなくコメントに飛ぶ(今回のIssue対応に伴って現在の実装から変更した点です。経緯等はこちらに記載しています)

「Docs」の検索

  • 検索対象:Docs
  • 検索ワード:Docs
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ
  • 「コメント」の検索結果も表示される

「イベント」の検索

  • 検索対象:イベント
  • 検索ワード:イベント
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ

「定期イベント」の検索

  • 検索対象:定期イベント
  • 検索ワード:イベント
  • 検索結果をクリックすると、検索結果自体のページに飛ぶ

「ユーザー」の検索(管理者)

  • 検索対象:ユーザー
  • 検索ワード:marumaru
  • 検索結果をクリックすると、ユーザーの個別ページに飛ぶ
  • 相談部屋のリンクが表示され、クリックするとユーザーの個別ページに飛ぶ

Image from Gyazo

「ユーザー」の検索(管理者以外)

kimura(または管理者権限のない任意のユーザー)でログインし、以下のとおり検索する

  • 検索対象:ユーザー
  • 検索ワード:marumaru
  • 相談部屋のリンクは表示されない

Screenshot

画面上の変更はありません。

@2525nicole 2525nicole self-assigned this Jan 21, 2024
@2525nicole 2525nicole marked this pull request as ready for review January 21, 2024 09:29
@2525nicole
Copy link
Contributor Author

@omochiumaiumai
お疲れ様です!
お忙しいところ、また二度目のご依頼になってしまい恐れ入ります🙇‍♀️
急ぎではありませんので、ご都合のよろしいタイミングでレビューをお願いできますでしょうか🙏

@@ -16,8 +16,8 @@ def searchable_url(searchable)
document = searchable.commentable_type.constantize.find(searchable.commentable_id)
"#{polymorphic_url(document)}#comment_#{searchable.id}"
elsif searchable.instance_of?(Answer) || searchable.instance_of?(CorrectAnswer)
document = searchable.question
polymorphic_url(document).to_s
document = Question.find(searchable.question.id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

補足:この箇所の修正について

修正内容

以下の経緯と課題点を踏まえて、
検索結果で表示する質問回答のリンク先を「回答が紐づく質問のURL」から「回答自体のURL」に修正いたしました。

経緯

React化するにあたって検索結果のURLをSearchablesコンポーネントからSearchableコンポーネントにkeyとして渡すにあたり、URLを一意にする必要がありました。

現在の仕様の課題点

検索結果一覧の質問回答のリンク先が「(回答自体ではなく)質問自体のURL」になっているため質問と回答が検索結果に含まれるとkeyが一意のものでなくなってしまうという課題がありました。

Issueの以下メモに記載しております🙇‍♀️
#5140 (comment)

@@ -20,7 +20,7 @@
&.is-unread
background-color: var(--danger)
color: var(--reversal-text)
&.is-serchable
&.is-searchable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

綴りがserchableになっていたのをsearchableに修正いたしました🙏

@@ -20,7 +20,7 @@
&.is-unread
background-color: var(--danger)
color: var(--reversal-text)
&.is-serchable
&.is-searchable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらも綴りをserchableからsearchableに修正した箇所です🙏

@omochiumaiumai
Copy link
Contributor

@2525nicole
お疲れ様です!
レビューの件承知いたしました!少々立て込んでいるためお時間をいただきますが、なるべく早くレビューできるように努めます💪

@2525nicole
Copy link
Contributor Author

@omochiumaiumai
お忙しいところお引き受けいただき本当にありがとうございます😭🙇‍♀️

少々立て込んでいるためお時間をいただきますが
急ぎではないので、問題ございません🙏!!
お手数をおかけしてしまい申し訳ありませんが、どうぞよろしくお願いいたします🙇‍♀️

@2525nicole 2525nicole changed the title searchables.vueとsearchable.vueをReactコンポーネントにに書き換えた searchables.vueとsearchable.vueをReactコンポーネントに書き換えた Jan 28, 2024
Copy link
Contributor

@omochiumaiumai omochiumaiumai left a comment

Choose a reason for hiding this comment

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

@2525nicole
お疲れ様です!お待たせしてすみません💦
動作とコードを確認しました!特に問題はないのですが、変数名について気になった部分があったため2点コメントをしています。
ご確認をお願いいたします🙇‍♂

const canDisplayTalk = searchable.model_name === 'user' && searchable.talk_id
const currentUser = window.currentUser

const labelContet =
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらの変数名は誤字で、恐らくはlabelContentと思うのですがいかがでしょうか?
(意図的だったらすみません💦)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@omochiumaiumai
初歩的なミス、大変失礼いたしました...><!
ご指摘のとおりですので修正いたしました🙇‍♀️
気づいていただきありがとうございます🙏

</span>
)

const badgeContet = searchable.wip ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらも同様にbadgeContentではないかな〜と思いました!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@omochiumaiumai
ありがとうございます!
こちらも同様に、ご指摘いただいたとおりでしたので修正いたしました🙏
お手数をおかけしてしまい申し訳ありません🙇‍♀️🙇‍♀️

@2525nicole 2525nicole force-pushed the feature/convert-searchables-component-to-react branch from 36f2d0a to 3a8da1f Compare January 29, 2024 09:08
@2525nicole
Copy link
Contributor Author

@omochiumaiumai
とんでもないです!!
お忙しいところご確認ありがとうございます🙇‍♀️!!
またこちらの初歩的なお手数をおかけしてしまい申し訳ありませんでした><
修正しましたのでたびたびお手数ですがご確認をお願いできますでしょうか🙏

Copy link
Contributor

@omochiumaiumai omochiumaiumai left a comment

Choose a reason for hiding this comment

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

@2525nicole
お疲れ様です!
修正ありがとうございます〜これでApproveさせていただきます!😊

@2525nicole
Copy link
Contributor Author

@omochiumaiumai
おつかれさまです!
早々にご確認をいただき、
またお忙しいところ引き受けていただき本当にありがとうございました🙇‍♀️
引き続きよろしくお願いいたします😊🍀

@komagata
おつかれさまです!
チームメンバーの方にApproveをいただきましたので、レビューをお願いできますでしょうか🙇‍♀️

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 60a1913 into main Jan 30, 2024
5 checks passed
@komagata komagata deleted the feature/convert-searchables-component-to-react branch January 30, 2024 05:41
@github-actions github-actions bot mentioned this pull request Jan 30, 2024
20 tasks
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