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

自分の質問一覧を見るページを追加します #482

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

takaishi
Copy link
Contributor

@takaishi takaishi commented Dec 10, 2023

自分の質問一覧を見るページを追加します(UIの都合上チャットも見えます)

cloudnativedaysjp/dreamkast#2177 と同時にマージが必要。

image

@takaishi takaishi changed the title Qa page 自分の質問一覧を見るページを追加します Dec 10, 2023
Copy link

@github-actions github-actions bot added the reviewapps Build ReviewApp environment automatically if this label is granted label Dec 10, 2023
@takaishi takaishi self-assigned this Dec 10, 2023
@takaishi takaishi marked this pull request as ready for review December 10, 2023 11:46
Copy link
Member

@hrk091 hrk091 left a comment

Choose a reason for hiding this comment

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

https://github.com/cloudnativedaysjp/dreamkast/pull/2177/files のcommitでdkを動かして、質問・回答一覧ページがUIとしてはちゃんと動いていることを確認しました。

一方で、API callにはバグがありました。
ChatのActionCable対応でRTK Queryのgenerated methodをoverrideしていることが原因です。申し訳ありません。
このせいで、誰が「質問・回答一覧」ページを見ても全く同じ画面がでる状態になっています(一つでもコメントがついたら、全員に表示される)

これを直すと、今度は回答数が正しく表示されないバグになります。
replyToで判定するところのロジックが誤っていることが原因です。
おそらくですが、この問題を解消するのはFrontend側では難しく、backend側で対処が必要と思われます。
申し訳ありませんが、上記のあたりの修正をお願いします。

(msg) =>
msg.roomId === talkId &&
msg.replyTo !== undefined &&
data.filter((d) => msg.replyTo === d.id).length > 0,
Copy link
Member

@hrk091 hrk091 Dec 10, 2023

Choose a reason for hiding this comment

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

dataには、自分がChat/QAしたmessageだけが入っています。
この条件にヒットするのは自分が自分に返信した場合だけになります。他の人の返答は、回答数にカウントされません。
(実際に2人のアカウントで、質問と回答をやって試したところ、自分でself replyしたところだけがカウントされ、他の人が回答したところはカウントアップされませんでした)

これをちゃんと実装しようと思うと、backend側で回答がついた質問に対して回答数を導出する方が良さそうです。今のAPI設計でfrontend側でやろうと思うと、全ユーザ分のchatの全件取得が必要に見えます(replyToで判断するしかなさそうなので)

eventAbbr: event.abbr,
roomType: 'talk',
profileId: `${settings.profile.id}`,
})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
})
},
{ skip: !settings.profile.id },
)

skipを入れないと、profileの取得が完了するまでに 0null か何かしらのfalsy valueでAPI callしてしまいます。

@@ -121,6 +152,7 @@ const injectedRtkApi = api
roomId: queryArg.roomId,
roomType: queryArg.roomType,
createdFrom: queryArg.createdFrom,
profileId: queryArg.profileId,
Copy link
Member

Choose a reason for hiding this comment

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

chatは、ActionCableでのwebsocket対応をするために、RTKQueryが自動生成した実装を別の場所でoverrideしています。
https://github.com/cloudnativedaysjp/dreamkast-ui/blob/main/src/components/Chat/Chat.tsx#L53-L102

このため、queryを追加すると、こちらの修正が必要です。具体的には、 https://github.com/cloudnativedaysjp/dreamkast-ui/blob/main/src/components/Chat/Chat.tsx#L67 にprofileIdの追加が必要になります。
(私も完全に失念していて、なぜprofileIdが追加されないんだーと1hほどハマっていました。。。すごく罠でした。申し訳ありません)

このため、現在はqueryのprofileId指定がbackendに到達しておらず、backendはchatの全件を返してきています。
override側の修正をお願いします。

@github-actions github-actions bot removed the reviewapps Build ReviewApp environment automatically if this label is granted label Jul 29, 2024
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.

2 participants