search

pj-picbook / picbook

9 stars 0 forks source link

Feature/#74 firebase auth #82

Closed Ta23ka98 closed 2 years ago

Ta23ka98 commented 2 years ago

関連のタスクissue*

PR作成時のチェックリスト*

※対象外の場合は実施していなくてもチェックを入れる

対応したこと*

・ログイン機能 ・新規登録機能 (どちらも名前、生年月日の入力を除く)

未解決の課題

・ログイン・新規登録機能の監視 ・新規登録時のCloudFirestoreへのデータの追加

Ta23ka98 commented 2 years ago

@tokku5552

(追記) すみません、こちらの記事で解決しそうです🙇 以下は一応残してありますが、何とかなりそうです。

UserRepositoryで新規登録機能を実装しようとFirebaseAuthパッケージをimportしたのですが、 「UserはすでにFirebaseAuthとdomain/entyty/user.dartで定義されています」とエラーが出てしまいます。

これではcreateUserWithEmailAndPasswordメソッドが呼べないのですが、どうしたらいいでしょうか？

スクリーンショット 2022-08-08 15 46 34
tokku5552 commented 2 years ago

UserRepositoryで新規登録機能を実装しようとFirebaseAuthパッケージをimportしたのですが、 「UserはすでにFirebaseAuthとdomain/entyty/user.dartで定義されています」とエラーが出てしまいます。

上記解決できそうですかね？ 一応答えておくと、UserRepositoryはusersコレクションを扱うためのリポジトリなので、AuthRepositoryとかでFirebaseAuth用のリポジトリをつくっていただきたいです！

Ta23ka98 commented 2 years ago

@tokku5552 ありがとうございます、大丈夫そうです！👍 まだ理解が浅いですが、とりあえず記事の通りにやれば動きそうです😅

tokku5552 commented 2 years ago

まだドラフトだと思いますが、一点コメントしておくと、get_itは使わずに実装してほしいです！ get_itはDI コンテナのパッケージですが、同じことがriverpodでもできるので、不用意に選択肢を増やさないためです！ 一旦firebaseauth.instanceはリポジトリでそのまま定義しちゃって大丈夫です！

Ta23ka98 commented 2 years ago

@tokku5552 了解です、アドバイスありがとうございます！🙌

tokku5552 commented 2 years ago

@Ta23ka98 急かしているとかではなく念の為確認ですが、こちらまだ作業中ということで大丈夫です？ もしレビュー待ってる感じであれば言ってください！

Ta23ka98 commented 2 years ago

@tokku5552 すみません、作業中です!🙋 新規登録については実装できていて、あとはログイン機能とログイン状態の監視が必要な状態です。

tokku5552 commented 2 years ago

@Ta23ka98 ありがとうございます！ ログイン機能はサクッとできるかもですが、それを監視して画面出し分けるのは結構重いタスクなので、一旦それを含めずにこのPRマージしてしまって、別issueにして改めて対応のほうが良いかなと思ってますがいかがでしょう？ (このPR後でないと #85 ができないということもあり... 😅 )

Ta23ka98 commented 2 years ago

@tokku5552 了解しました！とりあえずサクッとログイン機能を実装します🙋

こちらは質問なのですが、ログイン時の画面遷移をどう実装するか悩んでいます🤔

StatefulWidgetの場合は、スクショのように書いても、エラーが出た段階で処理が止まるはずです。 ただ今回の場合、スクショのように書いてもエラー文が出るだけで、処理が進んでしまいます。

logInメソッドの書き方がよくないのか、あるいはこのログイン画面の画面遷移の書き方がマズイのか.... この辺りについて、アドバイス頂きたいです🙇

・ログイン画面のログインのコード

スクリーンショット 2022-08-11 6 59 36

・AuthServiceのログインメソッド

スクリーンショット 2022-08-11 7 30 43
tokku5552 commented 2 years ago

@Ta23ka98

StatefulWidgetの場合は、スクショのように書いても、エラーが出た段階で処理が止まるはずです。 ただ今回の場合、スクショのように書いてもエラー文が出るだけで、処理が進んでしまいます。

logInメソッドの書き方がよくないのか、あるいはこのログイン画面の画面遷移の書き方がマズイのか.... この辺りについて、アドバイス頂きたいです🙇

これはtry{} catch{}の構造をもう一度見直してほしいです！firebaseのexceptionが発生する箇所で例外をcatchしているんですが、throwしていないので画面側に例外が飛んできておらず、スルーしています！

Ta23ka98 commented 2 years ago

@tokku5552 ありがとうございます、無事に修正できました！👍

あとは不要なコードを削除して、getIt(locator)パッケージの部分をRiverpodで書き換えたいと思います🙋 これらの作業が終わり次第、またご連絡させて頂きます🙇

tokku5552 commented 2 years ago

了解です！ちなみにコンフリクトもしてしまってるのでこちらの解消も合わせてお願いします 🙇

Ta23ka98 commented 2 years ago

@tokku5552 なんとか完成し、ログイン・会員登録機能ともに動作を確認しました🙌 ただ、画面遷移までの動きが遅いところが気になります🤔

右往左往したので、追加→不要と判断して削除したファイルも多いです。 修正も多いかもしれません。お手数ですが、よろしくお願いします🙇

tokku5552 commented 2 years ago

@Ta23ka98

・プルリクのマージ ・別issue(ログイン状態の監視) はどうしましょう🤔

issueは #85 で対応予定です！ マージはまさたかさんがやっていただいて、追加レビューの有無もおまかせします !

Ta23ka98 commented 2 years ago

@Romu1273x 徳田さんにもレビュー＆修正頂きましたが、念の為レビューお願いしたいです!🙋 (お忙しい場合は、また仰って頂けると幸いです🙇)

Romu1273x commented 2 years ago

@Ta23ka98 本件(firebase auth)に関しては経験がなくて見れないので、ボトムナビゲーション絡みの所を見ました。 pushAndRemoveUntilで画面遷移しているので良さそうに見えます。

本件絡みのところが気掛かりなら申し訳ないですが、かわどさんの方が適任かもしれないです。

Ta23ka98 commented 2 years ago

@Romu1273x 了解です、Approveありがとうございました！🙌 かわどさんはお忙しい(Githubのステータス)みたいなので、マージしちゃいます！