Skip to content

Commit d1d630c

Browse files
neelanshsahaineelanshsahai
committed
Fix code review changes
1 parent 4c67e30 commit d1d630c

File tree

8 files changed

+95
-29
lines changed

8 files changed

+95
-29
lines changed

‎Shrine/app/src/main/java/com/authentication/shrine/CredentialManagerUtils.kt‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import com.authentication.shrine.repository.AuthRepository.Companion.USER_ID_KEY
4646
import com.authentication.shrine.repository.AuthRepository.Companion.read
4747
import com.authentication.shrine.repository.SERVER_CLIENT_ID
4848
import com.google.android.libraries.identity.googleid.GetGoogleIdOption
49+
import org.json.JSONArray
4950
import org.json.JSONObject
5051
import javax.inject.Inject
5152

@@ -264,8 +265,11 @@ class CredentialManagerUtils @Inject constructor(
264265
credentialId: String,
265266
) {
266267
credentialManager.signalCredentialState(
267-
SignalUnknownCredentialRequest(
268-
"""{"rpId":"${dataStore.read(RP_ID_KEY)}", "credentialId":"$credentialId"}""",
268+
request = SignalUnknownCredentialRequest(
269+
requestJson = JSONObject().apply {
270+
put("rpId", dataStore.read(RP_ID_KEY))
271+
put("credentialId", credentialId)
272+
}.toString()
269273
),
270274
)
271275
}
@@ -275,8 +279,12 @@ class CredentialManagerUtils @Inject constructor(
275279
credentialIds: List<String>,
276280
) {
277281
credentialManager.signalCredentialState(
278-
SignalAllAcceptedCredentialIdsRequest(
279-
"""{"rpId":"${dataStore.read(RP_ID_KEY)}","userId":"${dataStore.read(USER_ID_KEY)}","allAcceptedCredentialIds":["${credentialIds.joinToString(",")}"]}""",
282+
request = SignalAllAcceptedCredentialIdsRequest(
283+
requestJson = JSONObject().apply {
284+
put("rpId", dataStore.read(RP_ID_KEY))
285+
put("userId", dataStore.read(USER_ID_KEY))
286+
put("allAcceptedCredentialIds", JSONArray(credentialIds))
287+
}.toString()
280288
),
281289
)
282290
}
@@ -287,8 +295,13 @@ class CredentialManagerUtils @Inject constructor(
287295
newDisplayName: String,
288296
) {
289297
credentialManager.signalCredentialState(
290-
SignalCurrentUserDetailsRequest(
291-
"""{"rpId":"${dataStore.read(RP_ID_KEY)}","userId":"${dataStore.read(USER_ID_KEY)}", "name":"$newName","displayName":"$newDisplayName"}""",
298+
request = SignalCurrentUserDetailsRequest(
299+
requestJson = JSONObject().apply {
300+
put("rpId", dataStore.read(RP_ID_KEY))
301+
put("userId", dataStore.read(USER_ID_KEY))
302+
put("name", newName)
303+
put("displayName", newDisplayName)
304+
}.toString()
292305
),
293306
)
294307
}

‎Shrine/app/src/main/java/com/authentication/shrine/ui/MainMenuScreen.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ private fun MainMenuButtonsList(
180180

181181
ShrineButton(
182182
onClick = navigateToUpdateProfile,
183-
buttonText = "Update Profile",
183+
buttonText = stringResource(R.string.update_profile),
184184
)
185185
}
186186
}

‎Shrine/app/src/main/java/com/authentication/shrine/ui/PasskeyManagementScreen.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ fun PasskeyManagementScreen(
191191

192192
ShrineButton(
193193
onClick = onSignal,
194-
buttonText = "Delete selected credentials",
194+
buttonText = stringResource(R.string.delete_selected_credentials),
195195
modifier = Modifier.fillMaxWidth(),
196196
)
197197
} else {

‎Shrine/app/src/main/java/com/authentication/shrine/ui/UpdateProfileScreen.kt‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,16 @@ import com.authentication.shrine.ui.common.ShrineToolbar
3333
import com.authentication.shrine.ui.theme.ShrineTheme
3434
import com.authentication.shrine.ui.viewmodel.UpdateProfileViewModel
3535

36+
/**
37+
* A stateful Composable screen that allows users to update their profile information,
38+
* specifically their username and display name.
39+
*
40+
* @param onBackClicked Lambda to be invoked when the back button in the toolbar is clicked.
41+
* @param viewModel An instance of [UpdateProfileViewModel] used to handle the business logic
42+
*/
3643
@Composable
3744
fun UpdateProfileScreen(
45+
onBackClicked: () -> Unit,
3846
viewModel: UpdateProfileViewModel,
3947
) {
4048
var username by remember { mutableStateOf("") }
@@ -43,24 +51,39 @@ fun UpdateProfileScreen(
4351
UpdateProfileScreen(
4452
username = username,
4553
onUsernameChanged = { username = it },
46-
email = email,
47-
onEmailChanged = { email = it },
54+
displayName = email,
55+
onDisplayNameChanged = { email = it },
4856
onMetadataUpdate = viewModel::updateMetadata,
57+
onBackClicked = onBackClicked,
4958
)
5059
}
5160

61+
/**
62+
* A stateless Composable screen that provides the UI for updating user profile information.
63+
*
64+
* @param username The current value of the username to be displayed in the text field.
65+
* @param onUsernameChanged Lambda to update the username on change.
66+
* @param displayName The current value of the display name to be displayed in the text field.
67+
* @param onDisplayNameChanged Lambda to update the display name on change
68+
* @param onMetadataUpdate Lambda function that is invoked when the update button is clicked.
69+
* It receives the current username and display name strings as parameters,
70+
* @param onBackClicked Lambda function to be invoked when the back button in the toolbar is clicked
71+
*/
5272
@Composable
5373
fun UpdateProfileScreen(
5474
username: String,
5575
onUsernameChanged: (String) -> Unit,
56-
email: String,
57-
onEmailChanged: (String) -> Unit,
76+
displayName: String,
77+
onDisplayNameChanged: (String) -> Unit,
5878
onMetadataUpdate: (String, String) -> Unit,
79+
onBackClicked: () -> Unit,
5980
) {
6081
Column(
6182
modifier = Modifier.fillMaxSize(),
6283
) {
63-
ShrineToolbar(onBackClicked = {})
84+
ShrineToolbar(
85+
onBackClicked = onBackClicked
86+
)
6487

6588
ShrineTextField(
6689
value = username,
@@ -70,29 +93,33 @@ fun UpdateProfileScreen(
7093
)
7194

7295
ShrineTextField(
73-
value = email,
74-
onValueChange = onEmailChanged,
75-
hint = stringResource(R.string.email_address),
96+
value = displayName,
97+
onValueChange = onDisplayNameChanged,
98+
hint = stringResource(R.string.display_name),
7699
modifier = Modifier.fillMaxWidth(),
77100
)
78101

79102
ShrineButton(
80-
onClick = { onMetadataUpdate(username, email) },
103+
onClick = { onMetadataUpdate(username, displayName) },
81104
buttonText = stringResource(R.string.update_user_info),
82105
)
83106
}
84107
}
85108

109+
/**
110+
* Preview Composable function of [UpdateProfileScreen]
111+
*/
86112
@Preview(showBackground = true, showSystemUi = true)
87113
@Composable
88114
fun TestPreview() {
89115
ShrineTheme {
90116
UpdateProfileScreen(
91-
"",
92-
{},
93-
"",
94-
{},
95-
{ _, _ -> },
117+
username = "",
118+
onUsernameChanged = { },
119+
displayName = "",
120+
onDisplayNameChanged = { },
121+
onMetadataUpdate = { _, _ -> },
122+
onBackClicked = { }
96123
)
97124
}
98125
}

‎Shrine/app/src/main/java/com/authentication/shrine/ui/navigation/ShrineNavGraph.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ fun ShrineNavGraph(
181181

182182
composable(route = ShrineAppDestinations.UpdateProfileRoute.name) {
183183
UpdateProfileScreen(
184+
onBackClicked = { navController.popBackStack() },
184185
viewModel = hiltViewModel(),
185186
)
186187
}

‎Shrine/app/src/main/java/com/authentication/shrine/ui/viewmodel/PasskeyManagementViewModel.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,9 @@ class PasskeyManagementViewModel @Inject constructor(
275275
}
276276
}
277277

278+
/**
279+
* Update list of items
280+
*/
278281
fun updateItem(index: Int, passkeysList: List<PasskeyCredential>) {
279282
_uiState.update {
280283
it.copy(

‎Shrine/app/src/main/java/com/authentication/shrine/ui/viewmodel/UpdateProfileViewModel.kt‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,38 @@ package com.authentication.shrine.ui.viewmodel
1818
import androidx.datastore.core.DataStore
1919
import androidx.datastore.preferences.core.Preferences
2020
import androidx.lifecycle.ViewModel
21+
import androidx.lifecycle.viewModelScope
2122
import com.authentication.shrine.CredentialManagerUtils
2223
import com.authentication.shrine.repository.AuthRepository.Companion.CRED_ID
2324
import com.authentication.shrine.repository.AuthRepository.Companion.RP_ID_KEY
2425
import com.authentication.shrine.repository.AuthRepository.Companion.USER_ID_KEY
2526
import com.authentication.shrine.repository.AuthRepository.Companion.read
2627
import dagger.hilt.android.lifecycle.HiltViewModel
27-
import kotlinx.coroutines.CoroutineScope
2828
import kotlinx.coroutines.flow.MutableStateFlow
2929
import kotlinx.coroutines.flow.asStateFlow
3030
import kotlinx.coroutines.flow.update
3131
import kotlinx.coroutines.launch
3232
import javax.inject.Inject
3333

34+
/**
35+
* ViewModel responsible for managing the state and business logic for the user profile
36+
* update screen.
37+
*
38+
* @property credentialManagerUtils Utilities for interacting with the Credential Manager.
39+
* @property dataStore The DataStore instance used for reading persisted user and credential identifiers.
40+
*/
3441
@HiltViewModel
3542
class UpdateProfileViewModel @Inject constructor(
3643
private val credentialManagerUtils: CredentialManagerUtils,
3744
private val dataStore: DataStore<Preferences>,
38-
private val coroutineScope: CoroutineScope,
3945
) : ViewModel() {
40-
private val _uiState = MutableStateFlow(TestState())
46+
private val _uiState = MutableStateFlow(UpdateProfileState())
4147
val uiState = _uiState.asStateFlow()
4248

4349
init {
44-
coroutineScope.launch {
50+
viewModelScope.launch {
4551
_uiState.update {
46-
TestState(
52+
UpdateProfileState(
4753
userId = dataStore.read(USER_ID_KEY) ?: "",
4854
rpId = dataStore.read(RP_ID_KEY) ?: "",
4955
credentialId = dataStore.read(CRED_ID) ?: "",
@@ -52,17 +58,31 @@ class UpdateProfileViewModel @Inject constructor(
5258
}
5359
}
5460

61+
/**
62+
* Signals an update to the user's metadata (name and display name) through the
63+
* [CredentialManagerUtils].
64+
*
65+
* @param newName The new name for the user.
66+
* @param newDisplayName The new display name for the user.
67+
*/
5568
fun updateMetadata(
5669
newName: String,
5770
newDisplayName: String,
5871
) {
59-
coroutineScope.launch {
72+
viewModelScope.launch {
6073
credentialManagerUtils.signalUserDetails(newName, newDisplayName)
6174
}
6275
}
6376
}
6477

65-
data class TestState(
78+
/**
79+
* Represents the state of the user profile update screen.
80+
*
81+
* @property userId The unique identifier for the user
82+
* @property rpId The identifier for the Relying Party
83+
* @property credentialId The identifier for the credential that needs to be updated
84+
*/
85+
data class UpdateProfileState(
6686
val userId: String = "",
6787
val rpId: String = "",
6888
val credentialId: String = "",

‎Shrine/app/src/main/res/values/strings.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,6 @@
104104
<string name="error_invalid_credentials">Invalid credentials. Please check your username and password.</string>
105105
<string name="update_user_info">Update User Info</string>
106106
<string name="update_profile">Update Profile</string>
107+
<string name="delete_selected_credentials">Delete selected credentials</string>
108+
<string name="display_name">Display Name</string>
107109
</resources>

0 commit comments

Comments
 (0)