-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor: link to db for stats page #51
Conversation
@@ -12,6 +13,6 @@ open class FirebaseReferences(val env: Env = Env.PROD) { | |||
fun usersFriendRequestOfUid(uid: String): CollectionReference = | |||
usersCollection.document(uid).collection("friend_requests") | |||
|
|||
val usersCollection = root.collection("users") | |||
val usersCollection = FirebaseFirestore.getInstance().collection("users") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not hardcode it like that when we have a root collection. Hardcoding it like that makes it impossible to test with an ENV different than production.
app/src/main/java/ch/epfl/sweng/rps/ui/statistics/StatisticsFragment.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost good, resolve the issues I highlighted in your files and I'll approve it
0405 to merge and update with main
For this sprint, I have refactored the function in FirebaseHelper, which should be compatible with Game and Round Class and Database. Please have a quick check :) Thank you. |
Main issue checkpoint:
If the logic is correct and the code fulfilled the requirements, I will try to write the test :) I really appreciate your time to review my code! |
app/src/main/java/ch/epfl/sweng/rps/ui/statistics/StatisticsFragment.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except these few issues, LGTM.
Here is a basic link to the database: