Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Kamila/remove create profile #304

Merged
merged 21 commits into from
May 18, 2022
Merged

Conversation

kamilababayeva
Copy link
Collaborator

@kamilababayeva kamilababayeva commented May 17, 2022

This PR:

  • simplify AuthenticationActivity and its tests
  • remove CreateProfileActivity

@kamilababayeva kamilababayeva marked this pull request as ready for review May 17, 2022 08:54
@MaximeZmt MaximeZmt self-requested a review May 17, 2022 09:13
Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

Nice work !! 👍 Just left some comment below !

import com.google.firebase.auth.GoogleAuthProvider
import com.google.firebase.auth.ktx.auth
import com.google.firebase.ktx.Firebase
import dagger.hilt.android.AndroidEntryPoint
import javax.inject.Inject

import net.datafaker.Faker
Copy link
Owner

Choose a reason for hiding this comment

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

What is the purpose of DataFaker ?

Copy link
Owner

Choose a reason for hiding this comment

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

Is it to create fake username ?

Copy link
Collaborator Author

@kamilababayeva kamilababayeva May 17, 2022

Choose a reason for hiding this comment

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

yes! I have a function that crates username with his library

@@ -52,7 +53,7 @@ class DataGetter @Inject constructor() {
fun updateFieldInt(uid: String, fieldName: String, newVal: Int, method:String) {
dbUserRef.child(uid).child(fieldName)
.get().addOnSuccessListener { t ->
var finalVal = checkValue(t, method, newVal)//newVal TO DELETE IF TESTS PASS, OTHERWISE ROLL BACK!
val finalVal = checkValue(t, method, newVal)//newVal TO DELETE IF TESTS PASS, OTHERWISE ROLL BACK!
Copy link
Owner

Choose a reason for hiding this comment

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

what is the purpose of this comment ? We should not keep it no ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was not me who left it, i am not exaclty sure what it means... I will ask inva group

@jiabaow jiabaow self-requested a review May 17, 2022 09:55
Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

Nice work 😀👍!

@codeclimate
Copy link

codeclimate bot commented May 18, 2022

Code Climate has analyzed commit a0b6218 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 73.5% (80% is the threshold).

This pull request will bring the total coverage in the repository to 84.6%.

View more on Code Climate.

@kamilababayeva kamilababayeva merged commit b9926e2 into main May 18, 2022
@kamilababayeva kamilababayeva deleted the kamila/removeCreateProfile branch May 18, 2022 11:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants