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

USER_ACTIVATEDイベントを追加 #2266

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Conversation

mehm8128
Copy link
Member

@mehm8128 mehm8128 commented Jan 16, 2024

close #2030

質問: 動作確認って何か方法ありますか?

@logica0419
Copy link
Member

logica0419 commented Jan 16, 2024

  1. 手元環境でBotを作る
  2. Botにactivate userイベントをsubscribeさせる
    • 上2つはURL直だたきになるかも
    • bot-consoleをcompose内で起動しても良いとは思う
  3. Botのイベント送信先URLをダミーサーバーに当てる
  4. 手元環境で凍結・解除をしてみて正常にリクエストが飛ぶか確かめる

って感じでデバッグはできると思います
ちょっと大変だけど…頑張ってみてもらえると嬉しいです

@logica0419
Copy link
Member

bot_debuggerにもともと全てのログが送られている可能性もある気はするので、ちょっと一回bot_debuggerの方確認してもらってからの方がいいかもしれないです

@mehm8128
Copy link
Member Author

分かりました、やってみます

@mehm8128
Copy link
Member Author

bot_debuggerの方でやり方が分からなくて(http modeでやったのですが、activateが上手くいかなかったりendpointにlocalhostのurlを指定できなかったり)、結局postmanでやり、ちゃんと受け取れていることが確認できました
image

Copy link
Member

@logica0419 logica0419 left a comment

Choose a reason for hiding this comment

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

ありがとう!実装に不備はなさそうです!素晴らしい
ちょっとだけリファクタのお願いしたいのでそれだけお願いします

repository/gorm/user.go Outdated Show resolved Hide resolved
repository/gorm/user.go Outdated Show resolved Hide resolved
Copy link
Member

@logica0419 logica0419 left a comment

Choose a reason for hiding this comment

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

一応確認
activatedの時UserUpdateイベントが発行されていたのが今後発行されなくなる(クライアントにWebSocketで行かなくなる)のだけれど、そこに関しては大丈夫?

repository/gorm/user.go Outdated Show resolved Hide resolved
@mehm8128
Copy link
Member Author

activatedの時UserUpdateイベントが発行されていたのが今後発行されなくなる(クライアントにWebSocketで行かなくなる)のだけれど、そこに関しては大丈夫?

これちょっと考えていて、USER_ACTIVATEDイベントを追加でlistenするようにすればいいかなって思ってたんですけど、やっぱりUSER_UPDATEDももらえるようにした方がよさそうなので直します
直すとしたらactivateのcaseに

r.hub.Publish(hub.Message{
			Name: event.UserUpdated,
			Fields: hub.Fields{
				"user_id": id,
			},
		})

を追加することになりそうですかね?defaultと重複しちゃうので上手くまとめられるならまとめたい気がしてます

@mehm8128
Copy link
Member Author

調べてみたらfallthroughでbreakしないようにできるんですね
activateのcaseでfallthroughすればdefaultのも実行できそうですが、上で書いたように普通に追加するのとどっちがよさそうですか?

@mehm8128
Copy link
Member Author

一旦fallthroughする方針でやってみました

Copy link
Member

@logica0419 logica0419 left a comment

Choose a reason for hiding this comment

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

LGTM!
対応ありがとう~

@mehm8128 mehm8128 merged commit 3a8dd29 into master Jan 23, 2024
6 checks passed
@mehm8128 mehm8128 deleted the feat/bot_event_user_activated branch January 23, 2024 23:39
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.

アカウント凍結解除のBOTイベントを発行できるようにする
2 participants