Skip to content
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

feat: Ensure Unique Usernames During Profile Updates #322

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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("[email protected]")
// 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) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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<Void>,
onSuccess: () -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,7 +33,6 @@ fun ProfileInformationScreen(
profileViewModel: ProfileViewModel,
navigationActions: NavigationActions
) {

val context = LocalContext.current

// Lock the screen orientation to portrait mode.
Expand All @@ -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")) {
Expand Down Expand Up @@ -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),
Expand All @@ -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
}
Expand Down Expand Up @@ -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 =
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,6 +20,10 @@ class ProfileViewModel(private val repository: ProfileRepository) : ViewModel()
private val _error = MutableStateFlow<String?>(null)
val error: StateFlow<String?> = _error.asStateFlow()

// ADDED: To indicate if username is taken
private val _usernameError = MutableStateFlow<String?>(null)
val usernameError: StateFlow<String?> = _usernameError.asStateFlow()

init {
repository.init { fetchUserProfile() }
}
Expand All @@ -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(
Expand All @@ -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 <T : ViewModel> create(modelClass: Class<T>): T {
return ProfileViewModel(ProfileRepositoryFirestore(Firebase.firestore, Firebase.auth))
return ProfileViewModel(
com.github.lookupgroup27.lookup.model.profile.ProfileRepositoryFirestore(
Firebase.firestore, Firebase.auth))
as T
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
Loading