Skip to content

Feature/7 fetch user profile #30

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

akkkr
Copy link
Contributor

@akkkr akkkr commented Jul 21, 2020

概要

• Firebaseから名前と画像を取得して表示する処理を追加

レビュー観点

レビューレベル

  • Lv0: まったく見ないでAcceptする

  • Lv1: ぱっとみて違和感がないかチェックしてAcceptする

  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証してAcceptする

  • Lv3: 実際に環境で動作確認したうえでAcceptする

スクリーンショット

スクリーンショット 2020-07-21 16 09 01

備考

Copy link
Contributor

@ojun9 ojun9 left a comment

Choose a reason for hiding this comment

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

コメントしましたー!

func setUserProfileImage(imageData: Data) {
DispatchQueue.main.async {
self.editProfileButton.isEnabled = false
self.profileImageView.image = UIImage(data: imageData)!
Copy link
Contributor

Choose a reason for hiding this comment

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

self.profileImageView.image = UIImage(data: imageData)!の右辺は
UIImage(data: imageData) ?? UIImage()が良さそうです

Comment on lines 19 to 21
editProfileButton.isEnabled = false
editProfileButton.layer.cornerRadius = 10.0
profileImageView.layer.cornerRadius = profileImageView.frame.height / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

ここの3行はそれぞれの変数に対して関数で切り分けたほうがいいと思います!

Copy link
Contributor

@shinya-todaka shinya-todaka left a comment

Choose a reason for hiding this comment

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

レビューしました!

Comment on lines 30 to 51
self.firestore.collection("message/v1/users").document("y783WJnXJqDfDED0nBvK").getDocument { (document, error) in
if let error = error {
print("Error: \(error.localizedDescription)")
return
}

guard let document = document, document.exists else {
print("The document doesn't exist.")
return
}

do {
let user = try Firestore.Decoder().decode(User.self, from: document.data()!)
self.presenter.successFetchUser(user: user)
self.downloadProfile(downLoadURL: user.profileImageURL ?? "")
} catch {
fatalError()
}


}

Copy link
Contributor

Choose a reason for hiding this comment

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

ここはlistenerの方がuserのdisplayNameやprofileImageURLを変えた時に更新しなくて良くなるのでいいと思います!
それとusersのdocumentIDはauthのuidと一緒なので

 guard let uid = Auth.auth().currentUser?.uid else { return }
 let userReference = Firestore.firestore()collection("message/v1/users").document(uid)
listner = userReference.addSnapshotListener { (snapshot,error) in 
  エラーじゃない場合はpresenterのsuccessFetchUserを呼ぶ
} 
っていう感じになると思います

Comment on lines 54 to 67
private func downloadProfile(downLoadURL: String) {
let httpsReference = Storage.storage().reference(forURL: downLoadURL)
httpsReference.getData(maxSize: 1 * 512 * 512) { data, error in
if let error = error {
print(error.localizedDescription)
return
}

guard let data = data else {
return
}

self.presenter.successFetchImageData(imageData: data)
}
Copy link
Contributor

@shinya-todaka shinya-todaka Jul 24, 2020

Choose a reason for hiding this comment

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

userを取ってくる処理だけしてimageViewにuserのプロフィール画像を入れる処理はNukeに任せたほうがいいです!
import Nuke
して

let options = ImageLoadingOptions(placeholder: デフォルトの画像, failureImage: 失敗した時の画像)
loadImage(with: userのprofileImageURL, options: options, into: profileImageView, progress: nil, completion: nil)

こんな感じでしょうか
Nukeについてはoptionsとかも色々あるので調べた方が良いかもです!

Comment on lines +18 to +19
var userName = ""
var profileImage = UIImage()
Copy link
Contributor

Choose a reason for hiding this comment

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

UserProfileViewControllerからEditProfileViewControllerにへの値渡しはViewBuilderを介して行った方が良いです!
UserProfileViewControllerで

func presentEditProfileViewController(user: User) {
 let editProfileVC = EditProfileViewBuilder.create(user: user)
 let navigationController = UINavigationController(rootViewController:  editProfileVC)
 navigationController.modalPresentationStyle = .fullScreen
 present(navigationController, animated: true, completion: nil)
}

こんな感じになるようにして

struct EditProfileViewBuilder {
    static func create(user: User) -> UIViewController {
        guard let EditProfileViewController = EditProfileViewController.loadFromStoryboard() as? EditProfileViewController else {
            fatalError("fatal: Failed to initialize the EditProfileViewController")
        }
        let model = EditProfileModel()
        let presenter = EditProfileViewPresenter(model: model, user: user)
        EditProfileViewController.inject(with: presenter)
        return EditProfileViewController
    }
}

EditProfileViewBuilderはこんな感じでしょうか

Copy link
Contributor Author

Choose a reason for hiding this comment

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

値の受け渡し、もう少し調べてから頑張ります。。

@@ -42,10 +43,12 @@ final class EditProfileViewController: UIViewController {
}

func setupNameTextField() {
self.nameTextField.text = self.userName
Copy link
Contributor

Choose a reason for hiding this comment

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

presenterにuserを渡すことになるので
nameTextField.text = presenter.user.displayName になります!

self.nameTextField.addBorderBottom(borderWidth: 1.0, color: .gray)
}

func setupImageView() {
self.imageView.image = self.profileImage
Copy link
Contributor

Choose a reason for hiding this comment

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

ここもuserのprofileImageURLを使ってnukeでimageViewの画像を変えたほうがいいです!

@shinya-todaka
Copy link
Contributor

それとprofileImageの保存は
storageの
message/v1/users/authのuid/profileImage.jpgでお願いします!

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