8000 Topic/delete user account by TsurutaYoshiki · Pull Request #768 · nttcom/threatconnectome · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Topic/delete user account #768

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

Merged
merged 4 commits into from
Jun 12, 2025
Merged

Topic/delete user account #768

merged 4 commits into from
Jun 12, 2025

Conversation

TsurutaYoshiki
Copy link
Collaborator

PR の目的

  • ユーザを削除する機能を追加しました

経緯・意図・意思決定

  • API
    • 既存のdelete_user APIはTC内のDBにあるaccoutテーブルのみ削除していました。今回Firebase, SupabaseのDBからもユーザを削除するように実装しました。
    • Firebase, Supabaseからユーザを削除する機能をapi/app/auth/firebase_auth_module.py、api/app/auth/supabase_auth_module.pyにそれぞれ実装しました
    • Supabaseのユーザ削除はANON_KEYでは無く、SERVICE_ROLE_KEYでないとできないため、docker-compose-supabase-local.ymlのapiコンテナにSERVICE_ROLE_KEYを追加しました
  • Web
    • ユーザ削除時にログイン画面に戻るように実装しました。(UserMe.jsxのログアウトを行なっている処理を参考にしました)
    • ユーザ削除時にRTKQの状態をリセットするため、今回deleteUserに関してタグ設計をしていません

参考文献

@TsurutaYoshiki
Copy link
Collaborator Author

テストでエラーになっていた箇所を修正しました。

  • 原因

    • delete_user APIをテストで使用することにより、Firebase側からアカウントが消えてしまいます。
      アカウントが消えているため他のテストで401: Unauthorized: EMAIL_NOT_FOUNDが出てしまいます。
  • 対策

    • delete_user API関連のテスをを行う際にFirebaseAuthModuleとSupabaseAuthModuleにあるdelete_userをモック化しました
    • firebase, supabase両方のテストで動作するようにutils.pyにjudge_whether_firebase_or_supabase()を作成しました

@TsurutaYoshiki TsurutaYoshiki marked this pull request as ready for review June 10, 2025 07:56
Copy link
Collaborator
@dejima-shikou dejima-shikou left a comment

Choose a reason for hiding this comment

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

Firebaseのエラー周りで指摘です

@mshim03 mshim03 requested a review from Copilot June 10, 2025 08:28
Copy link
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

このPRはユーザ削除機能を実装することを目的としており、以下の変更が含まれます:

  • API側でFirebaseおよびSupabaseの認証モジュールを利用してユーザ削除を実装し、既存のDB削除処理と連携
  • Web側において、ユーザ削除用の新たなRTKQミューテーションやダイアログの削除処理を追加
  • 関連テストやdocker-compose設定の更新を行い、サービス用のキーを追加するなどの環境整備を実施

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
web/src/services/tcApi.js deleteUserミューテーションの追加
web/src/pages/App/UserMenu/DeleteAccountDialog.jsx ユーザ削除処理とサインアウト処理を統合したダイアログの更新
docker-compose-supabase-local.yml SupabaseのSERVICE_ROLE_KEYの設定追加
api/app/routers/users.py ユーザ削除エンドポイントの更新(認証モジュール経由で削除実施)
api/app/auth/supabase_auth_module.py, firebase_auth_module.py 認証モジュールにdelete_userの実装追加
各テストファイル ユーザ削除に関するモックパッチの挿入とテストの更新
Comments suppressed due to low confidence (2)

web/src/pages/App/UserMenu/DeleteAccountDialog.jsx:49

  • 削除処理が成功または失敗した後、ダイアログが閉じられないため、handleClose()を呼び出してUIの状態をリセットすることを検討してください。
const handleDeleteAccount = async () => { ... }

api/app/auth/auth_module.py:31

  • delete_userの実装がpassになっています。基底クラスとして意図する場合はNotImplementedErrorを明示的に投げるようにすると、オーバーライド忘れを防げます。
def delete_user(self, uid): pass

@TsurutaYoshiki
Copy link
Collaborator Author

Copy link
Collaborator
@dejima-shikou dejima-shikou left a comment

Choose a reason for hiding this comment

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

LGTM

@shihakata shihakata merged commit d1b0d44 into main Jun 12, 2025
7 checks passed
@shihakata shihakata deleted the topic/delete-user-account branch June 12, 2025 05:57
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
0