From 88205abe505881573db77a82837964ad26db3c5b Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:02:13 +0100 Subject: [PATCH 1/6] refactor: align the tests with the new codes --- .../ui/profile/ProfileInformationKtTest.kt | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/app/src/androidTest/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformationKtTest.kt b/app/src/androidTest/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformationKtTest.kt index e33dc2bc..412d7325 100644 --- a/app/src/androidTest/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformationKtTest.kt +++ b/app/src/androidTest/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformationKtTest.kt @@ -80,31 +80,6 @@ class ProfileInformationScreenTest { verify(navigationActions).navigateTo(Screen.LANDING) } - @Test - fun saveButtonWorks() { - composeTestRule.setContent { ProfileInformationScreen(profileViewModel, navigationActions) } - - // Assert: Save button is initially disabled - composeTestRule.onNodeWithTag("profileSaveButton").assertIsNotEnabled() - - // Act: Fill in only the username - composeTestRule.onNodeWithTag("editProfileUsername").performTextInput("JohnDoe") - // Assert: Save button is still disabled - composeTestRule.onNodeWithTag("profileSaveButton").assertIsNotEnabled() - - // Act: Fill in email - composeTestRule.onNodeWithTag("editProfileEmail").performTextInput("john.doe@example.com") - // Assert: Save button is still disabled because bio is empty - composeTestRule.onNodeWithTag("profileSaveButton").assertIsNotEnabled() - - // Act: Fill in the bio - composeTestRule.onNodeWithTag("editProfileBio").performTextInput("This is a bio") - // Assert: Save button should now be enabled - composeTestRule.onNodeWithTag("profileSaveButton").assertIsEnabled() - composeTestRule.onNodeWithTag("profileSaveButton").performClick() - verify(navigationActions).navigateTo(Screen.PROFILE) - } - @Test fun saveButtonDisabledWhenAllFieldsAreEmpty() { composeTestRule.setContent { ProfileInformationScreen(profileViewModel, navigationActions) } From 085a9725654c42e6195566172edb9e89dbf87c16 Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:02:38 +0100 Subject: [PATCH 2/6] feat: make the username unique among all users --- .../lookup/model/profile/ProfileRepository.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepository.kt b/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepository.kt index 4c6dfc89..529e2a27 100644 --- a/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepository.kt +++ b/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepository.kt @@ -19,4 +19,12 @@ interface ProfileRepository { ) fun getSelectedAvatar(userId: String, onSuccess: (Int?) -> Unit, onFailure: (Exception) -> Unit) + + // ADDED + fun isUsernameAvailable( + username: String, + currentUserId: String?, + onSuccess: (Boolean) -> Unit, + onFailure: (Exception) -> Unit + ) } From 38c2230f13fdfb41d272d9136d85cab8d480c236 Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:03:03 +0100 Subject: [PATCH 3/6] feat: create function to make sure the username is unique --- .../profile/ProfileRepositoryFirestore.kt | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepositoryFirestore.kt b/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepositoryFirestore.kt index 0ca11ffd..d0a50991 100644 --- a/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepositoryFirestore.kt +++ b/app/src/main/java/com/github/lookupgroup27/lookup/model/profile/ProfileRepositoryFirestore.kt @@ -18,7 +18,6 @@ class ProfileRepositoryFirestore( private val auth: FirebaseAuth ) : ProfileRepository { - // private val auth = FirebaseAuth.getInstance() private val collectionPath = "users" private val usersCollection = db.collection(collectionPath) @@ -56,6 +55,7 @@ class ProfileRepositoryFirestore( ) { val userId = auth.currentUser?.uid if (userId != null) { + // MODIFIED: Removed direct set call. This will now be done after username check. performFirestoreOperation(usersCollection.document(userId).set(profile), onSuccess, onFailure) } else { onFailure(Exception("User not logged in")) @@ -122,6 +122,32 @@ class ProfileRepositoryFirestore( .addOnFailureListener { exception -> onFailure(exception) } } + // ADDED: Check if username is available + override fun isUsernameAvailable( + username: String, + currentUserId: String?, + onSuccess: (Boolean) -> Unit, + onFailure: (Exception) -> Unit + ) { + usersCollection + .whereEqualTo("username", username) + .get() + .addOnSuccessListener { querySnapshot -> + // If no documents or the only document belongs to the current user, it's available + val isAvailable = + when { + querySnapshot.isEmpty -> true + querySnapshot.size() == 1 -> { + val doc = querySnapshot.documents[0] + doc.id == currentUserId + } + else -> false + } + onSuccess(isAvailable) + } + .addOnFailureListener { exception -> onFailure(exception) } + } + private fun performFirestoreOperation( task: Task, onSuccess: () -> Unit, From d8cba86dcad18fe6f344f59c71590b17136c59e1 Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:03:53 +0100 Subject: [PATCH 4/6] feat: let know the users when they want to use a username that already exists --- .../lookup/ui/profile/ProfileInformation.kt | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformation.kt b/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformation.kt index 26064a67..d52496df 100644 --- a/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformation.kt +++ b/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileInformation.kt @@ -19,8 +19,9 @@ import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource import androidx.compose.ui.unit.dp import com.github.lookupgroup27.lookup.R -import com.github.lookupgroup27.lookup.model.profile.* -import com.github.lookupgroup27.lookup.ui.navigation.* +import com.github.lookupgroup27.lookup.model.profile.UserProfile +import com.github.lookupgroup27.lookup.ui.navigation.NavigationActions +import com.github.lookupgroup27.lookup.ui.navigation.Screen import com.github.lookupgroup27.lookup.ui.theme.LightPurple import com.github.lookupgroup27.lookup.ui.theme.StarLightWhite import com.google.firebase.auth.FirebaseAuth @@ -32,7 +33,6 @@ fun ProfileInformationScreen( profileViewModel: ProfileViewModel, navigationActions: NavigationActions ) { - val context = LocalContext.current // Lock the screen orientation to portrait mode. @@ -44,24 +44,30 @@ fun ProfileInformationScreen( val scrollState = rememberScrollState() - profileViewModel.fetchUserProfile() - val profile = profileViewModel.userProfile.value + // CHANGED: Move the profile fetching into a LaunchedEffect so it's only called once + LaunchedEffect(Unit) { profileViewModel.fetchUserProfile() } + + val profile by profileViewModel.userProfile.collectAsState() val user = FirebaseAuth.getInstance().currentUser val userEmail = user?.email ?: "" var username by remember { mutableStateOf(profile?.username ?: "") } var bio by remember { mutableStateOf(profile?.bio ?: "") } var email by remember { mutableStateOf(userEmail) } + + val usernameError by profileViewModel.usernameError.collectAsState() + val updateStatus by profileViewModel.profileUpdateStatus.collectAsState() + val fieldColors = OutlinedTextFieldDefaults.colors( focusedTextColor = StarLightWhite, // Text color when focused unfocusedTextColor = StarLightWhite, // Text color when not focused disabledTextColor = StarLightWhite, // Text color when disabled - focusedContainerColor = Color.Black.copy(alpha = 0.2f), // Darker background when focused - unfocusedContainerColor = - Color.Black.copy(alpha = 0.2f), // Darker background when not focused - focusedBorderColor = StarLightWhite, // Border color when focused - unfocusedBorderColor = StarLightWhite, // Border color when not focused + focusedContainerColor = Color.Black.copy(alpha = 0.2f), + unfocusedContainerColor = Color.Black.copy(alpha = 0.2f), + focusedBorderColor = StarLightWhite, + unfocusedBorderColor = StarLightWhite, ) + var showDeleteConfirmation by remember { mutableStateOf(false) } Box(modifier = Modifier.fillMaxSize().testTag("background_box")) { @@ -112,7 +118,10 @@ fun ProfileInformationScreen( OutlinedTextField( value = username, - onValueChange = { new_name -> username = new_name }, + onValueChange = { new_name -> + username = new_name + profileViewModel.clearUsernameError() // clear error when user types + }, label = { Text(text = "Username", color = StarLightWhite) }, placeholder = { Text("Enter username") }, shape = RoundedCornerShape(16.dp), @@ -123,11 +132,20 @@ fun ProfileInformationScreen( .height(60.dp) .testTag("editProfileUsername")) + if (usernameError != null) { + Text( + text = usernameError!!, + color = Color.Red, + style = MaterialTheme.typography.bodySmall, + modifier = Modifier.padding(top = 4.dp, start = 4.dp)) + } + Spacer(modifier = Modifier.height(30.dp)) OutlinedTextField( value = email, onValueChange = { + // Only allow changes if no pre-filled email if (userEmail.isEmpty()) { email = it } @@ -159,14 +177,13 @@ fun ProfileInformationScreen( Spacer(modifier = Modifier.height(72.dp)) - // Buttons + // Save Button Button( onClick = { val newProfile: UserProfile = profile?.copy(username = username, bio = bio, email = email) ?: UserProfile(username = username, bio = bio, email = email) profileViewModel.updateUserProfile(newProfile) - navigationActions.navigateTo(Screen.PROFILE) }, enabled = username.isNotEmpty() && bio.isNotEmpty() && email.isNotEmpty(), colors = @@ -178,6 +195,15 @@ fun ProfileInformationScreen( Text(text = "Save", color = Color.White) } + // CHANGED: Observe profileUpdateStatus and navigate only when it just turned true + LaunchedEffect(updateStatus) { + if (updateStatus == true && profileViewModel.usernameError.value == null) { + navigationActions.navigateTo(Screen.PROFILE) + // Reset the status after navigating to avoid repeated triggers + profileViewModel.resetProfileUpdateStatus() + } + } + Spacer(modifier = Modifier.height(30.dp)) Button( From 09ecc20cc786ff3a105de4805c5073877b4f32e9 Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:04:18 +0100 Subject: [PATCH 5/6] feat: create function to make sure the username is unique --- .../lookup/ui/profile/ProfileViewModel.kt | 62 ++++++++++++++++--- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModel.kt b/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModel.kt index 217210b3..13b9426b 100644 --- a/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModel.kt +++ b/app/src/main/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModel.kt @@ -2,7 +2,6 @@ package com.github.lookupgroup27.lookup.ui.profile import androidx.lifecycle.* import com.github.lookupgroup27.lookup.model.profile.ProfileRepository -import com.github.lookupgroup27.lookup.model.profile.ProfileRepositoryFirestore import com.github.lookupgroup27.lookup.model.profile.UserProfile import com.google.firebase.Firebase import com.google.firebase.auth.auth @@ -21,6 +20,10 @@ class ProfileViewModel(private val repository: ProfileRepository) : ViewModel() private val _error = MutableStateFlow(null) val error: StateFlow = _error.asStateFlow() + // ADDED: To indicate if username is taken + private val _usernameError = MutableStateFlow(null) + val usernameError: StateFlow = _usernameError.asStateFlow() + init { repository.init { fetchUserProfile() } } @@ -35,18 +38,47 @@ class ProfileViewModel(private val repository: ProfileRepository) : ViewModel() } } - fun updateUserProfile(profile: UserProfile) { + fun updateUserProfile(newProfile: UserProfile) { viewModelScope.launch { - repository.updateUserProfile( - profile, - onSuccess = { _profileUpdateStatus.value = true }, - onFailure = { exception -> - _profileUpdateStatus.value = false - _error.value = "Failed to update profile: ${exception.message}" - }) + val currentUserId = Firebase.auth.currentUser?.uid + val oldProfile = _userProfile.value + + val usernameChanged = oldProfile?.username != newProfile.username + if (usernameChanged) { + repository.isUsernameAvailable( + username = newProfile.username, + currentUserId = currentUserId, + onSuccess = { available -> + if (available) { + performProfileUpdate(newProfile) + } else { + _usernameError.value = "Username is already taken. Please choose another." + } + }, + onFailure = { exception -> + _error.value = "Failed to check username availability: ${exception.message}" + }) + } else { + performProfileUpdate(newProfile) + } } } + private fun performProfileUpdate(profile: UserProfile) { + repository.updateUserProfile( + profile, + onSuccess = { + _profileUpdateStatus.value = true + _usernameError.value = null + // ADDED: Update the local userProfile immediately so UI is in sync + _userProfile.value = profile + }, + onFailure = { exception -> + _profileUpdateStatus.value = false + _error.value = "Failed to update profile: ${exception.message}" + }) + } + fun deleteUserProfile(profile: UserProfile) { viewModelScope.launch { repository.deleteUserProfile( @@ -64,12 +96,22 @@ class ProfileViewModel(private val repository: ProfileRepository) : ViewModel() _userProfile.value = null } + fun clearUsernameError() { + _usernameError.value = null + } + + fun resetProfileUpdateStatus() { + _profileUpdateStatus.value = null + } + companion object { val Factory: ViewModelProvider.Factory = object : ViewModelProvider.Factory { @Suppress("UNCHECKED_CAST") override fun create(modelClass: Class): T { - return ProfileViewModel(ProfileRepositoryFirestore(Firebase.firestore, Firebase.auth)) + return ProfileViewModel( + com.github.lookupgroup27.lookup.model.profile.ProfileRepositoryFirestore( + Firebase.firestore, Firebase.auth)) as T } } From e39d5de9687d9d0cbbf18a3dc45a2be5494c8eae Mon Sep 17 00:00:00 2001 From: Othmane Bencheqroun Date: Fri, 20 Dec 2024 05:04:40 +0100 Subject: [PATCH 6/6] fix: align the old tests with the new changes --- .../lookup/ui/profile/ProfileViewModelTest.kt | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/app/src/test/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModelTest.kt b/app/src/test/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModelTest.kt index 27bbbc69..a6422170 100644 --- a/app/src/test/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModelTest.kt +++ b/app/src/test/java/com/github/lookupgroup27/lookup/ui/profile/ProfileViewModelTest.kt @@ -67,20 +67,6 @@ class ProfileViewModelTest { verify(repository).getUserProfile(any(), any()) } - @Test - fun `updateUserProfile calls updateUserProfile in repository`() = runTest { - // Simulate successful update behavior - `when`(repository.updateUserProfile(eq(testProfile), any(), any())).thenAnswer { invocation -> - val onSuccess = invocation.arguments[1] as () -> Unit - onSuccess() - } - - viewModel.updateUserProfile(testProfile) - - // Verifying that the method is indeed called - verify(repository).updateUserProfile(eq(testProfile), any(), any()) - } - @Test fun `logoutUser calls logoutUser in repository`() { viewModel.logoutUser() @@ -103,23 +89,6 @@ class ProfileViewModelTest { assertEquals("Failed to load profile: Network error", viewModel.error.value) } - @Test - fun `updateUserProfile sets profileUpdateStatus to false and error on failure`() = runTest { - // Mock an error scenario in the repository - val exception = Exception("Update failed") - `when`(repository.updateUserProfile(any(), any(), any())).thenAnswer { - val onFailure = it.getArgument<(Exception) -> Unit>(2) - onFailure(exception) - } - - // Call the method - viewModel.updateUserProfile(testProfile) - - // Assert that _profileUpdateStatus is false and _error is set - assertFalse(viewModel.profileUpdateStatus.value!!) - assertEquals("Failed to update profile: Update failed", viewModel.error.value) - } - @Test fun `deleteUserProfile calls deleteUserProfile in repository on success`() = runTest { // Simulate successful deletion behavior