[PM-43240] fix: preserve custom authenticator refresh periods when editing

authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreen.kt

@@ -156,7 +156,7 @@ fun EditItemContent(
     onTypeOptionClicked: (AuthenticatorItemType) -> Unit = {},
     onTotpCodeTextChange: (String) -> Unit = {},
     onAlgorithmOptionClicked: (AuthenticatorItemAlgorithm) -> Unit = {},
-    onRefreshPeriodOptionClicked: (AuthenticatorRefreshPeriodOption) -> Unit = {},
+    onRefreshPeriodOptionClicked: (Int) -> Unit = {},
     onNumberOfDigitsChanged: (Int) -> Unit = {},
     onExpandAdvancedOptionsClicked: () -> Unit = {},
 ) {
@@ -261,7 +261,7 @@ private fun LazyListScope.advancedOptions(
     viewState: EditItemState.ViewState.Content,
     onAlgorithmOptionClicked: (AuthenticatorItemAlgorithm) -> Unit,
     onTypeOptionClicked: (AuthenticatorItemType) -> Unit,
-    onRefreshPeriodOptionClicked: (AuthenticatorRefreshPeriodOption) -> Unit,
+    onRefreshPeriodOptionClicked: (Int) -> Unit,
     onNumberOfDigitsChanged: (Int) -> Unit,
 ) {
     item(key = "OtpItemTypeSelector") {
@@ -312,11 +312,16 @@ private fun LazyListScope.advancedOptions(
 
     item(key = "RefreshPeriodItemTypePicker") {
         val possibleRefreshPeriodOptions = AuthenticatorRefreshPeriodOption.entries
+            .map { it.seconds }
+            .plus(viewState.itemData.originalRefreshPeriod)
+            .plus(viewState.itemData.refreshPeriod)
+            .distinct()
+            .sorted()
         val refreshPeriodOptionsWithStrings = possibleRefreshPeriodOptions.associateWith {
             pluralStringResource(
                 id = BitwardenPlurals.refresh_period_seconds,
-                count = it.seconds,
-                formatArgs = arrayOf(it.seconds),
+                count = it,
+                formatArgs = arrayOf(it),
             )
         }
         BitwardenMultiSelectButton(
@@ -406,7 +411,8 @@ private fun EditItemContentExpandedOptionsPreview() {
         viewState = EditItemState.ViewState.Content(
             isAdvancedOptionsExpanded = true,
             itemData = EditItemData(
-                refreshPeriod = AuthenticatorRefreshPeriodOption.THIRTY,
+                refreshPeriod = 45,
+                originalRefreshPeriod = 45,
                 totpCode = "123456",
                 type = AuthenticatorItemType.TOTP,
                 username = "account name",
@@ -428,7 +434,8 @@ private fun EditItemContentCollapsedOptionsPreview() {
         viewState = EditItemState.ViewState.Content(
             isAdvancedOptionsExpanded = false,
             itemData = EditItemData(
-                refreshPeriod = AuthenticatorRefreshPeriodOption.THIRTY,
+                refreshPeriod = 30,
+                originalRefreshPeriod = 30,
                 totpCode = "123456",
                 type = AuthenticatorItemType.TOTP,
                 username = "account name",

authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemViewModel.kt

@@ -136,7 +136,7 @@ class EditItemViewModel @Inject constructor(
                     accountName = content.itemData.username?.trim(),
                     type = content.itemData.type,
                     algorithm = content.itemData.algorithm,
-                    period = content.itemData.refreshPeriod.seconds,
+                    period = content.itemData.refreshPeriod,
                     digits = content.itemData.digits,
                     issuer = content.itemData.issuer.trim(),
                     favorite = content.itemData.favorite,
@@ -354,8 +354,8 @@ class EditItemViewModel @Inject constructor(
         minDigitsAllowed = MIN_ALLOWED_CODE_DIGITS,
         maxDigitsAllowed = MAX_ALLOWED_CODE_DIGITS,
         itemData = EditItemData(
-            refreshPeriod = AuthenticatorRefreshPeriodOption.fromSeconds(period)
-                ?: AuthenticatorRefreshPeriodOption.THIRTY,
+            refreshPeriod = period,
+            originalRefreshPeriod = period,
             totpCode = key.toUpperCase(Locale.current),
             type = type,
             username = accountName,
@@ -504,7 +504,7 @@ sealed class EditItemAction {
      * The user has selected a refresh period option.
      */
     data class RefreshPeriodOptionClick(
-        val period: AuthenticatorRefreshPeriodOption,
+        val period: Int,
     ) : EditItemAction()
 
     /**
@@ -552,13 +552,4 @@ enum class AuthenticatorRefreshPeriodOption(val seconds: Int) {
     THIRTY(seconds = 30),
     SIXTY(seconds = 60),
     NINETY(seconds = 90),
-    ;
-
-    @Suppress("UndocumentedPublicClass")
-    companion object {
-        /**
-         * Returns a [AuthenticatorRefreshPeriodOption] with the provided [seconds], or null.
-         */
-        fun fromSeconds(seconds: Int) = entries.find { it.seconds == seconds }
-    }
 }

authenticator/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/model/EditItemData.kt

@@ -3,13 +3,13 @@ package com.bitwarden.authenticator.ui.authenticator.feature.edititem.model
 import android.os.Parcelable
 import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemAlgorithm
 import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemType
-import com.bitwarden.authenticator.ui.authenticator.feature.edititem.AuthenticatorRefreshPeriodOption
 import kotlinx.parcelize.Parcelize
 
 /**
  * The data relating to the verification code.
  *
- * @property refreshPeriod The period for the verification code.
+ * @property refreshPeriod The selected period for the verification code in seconds.
+ * @property originalRefreshPeriod The loaded period in seconds, retained as a picker option.
  * @property totpCode The totp code for the item.
  * @property username Account or username for this item.
  * @property issuer Name of the item provider.
@@ -18,7 +18,8 @@ import kotlinx.parcelize.Parcelize
  */
 @Parcelize
 data class EditItemData(
-    val refreshPeriod: AuthenticatorRefreshPeriodOption,
+    val refreshPeriod: Int,
+    val originalRefreshPeriod: Int,
     val totpCode: String,
     val type: AuthenticatorItemType,
     val username: String?,

authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreenTest.kt

@@ -1,10 +1,15 @@
 package com.bitwarden.authenticator.ui.authenticator.feature.edititem
 
 import androidx.compose.ui.test.assert
+import androidx.compose.ui.test.assertCountEquals
 import androidx.compose.ui.test.assertIsDisplayed
+import androidx.compose.ui.test.assertIsSelected
+import androidx.compose.ui.test.filter
 import androidx.compose.ui.test.filterToOne
 import androidx.compose.ui.test.hasAnyAncestor
 import androidx.compose.ui.test.hasContentDescription
+import androidx.compose.ui.test.hasText
+import androidx.compose.ui.test.isSelectable
 import androidx.compose.ui.test.isDialog
 import androidx.compose.ui.test.onAllNodesWithText
 import androidx.compose.ui.test.onChildren
@@ -318,25 +323,111 @@ class EditItemScreenTest : AuthenticatorComposeTest() {
             .onAllNodesWithText(text = "Refresh period")
             .filterToOne(hasAnyAncestor(isDialog()))
             .assertIsDisplayed()
+        composeTestRule
+            .onAllNodes(isSelectable() and hasAnyAncestor(isDialog()))
+            .assertCountEquals(3)
+        listOf(30, 60, 90).forEach { period ->
+            composeTestRule
+                .onAllNodesWithText(text = "$period seconds")
+                .filter(hasAnyAncestor(isDialog()))
+                .assertCountEquals(1)
+        }
         composeTestRule
             .onNodeWithText(text = "60 seconds")
             .performClick()
         composeTestRule.assertNoDialogExists()
 
         verify(exactly = 1) {
             viewModel.trySendAction(
-                EditItemAction.RefreshPeriodOptionClick(AuthenticatorRefreshPeriodOption.SIXTY),
+                EditItemAction.RefreshPeriodOptionClick(60),
             )
         }
     }
 
     @Test
-    fun `refresh period click should display dialog and cancel should dismiss the dialog`() {
+    fun `custom refresh period should remain selectable after choosing a preset`() {
+        val content = DEFAULT_CONTENT.copy(
+            isAdvancedOptionsExpanded = true,
+            itemData = DEFAULT_ITEM_DATA.copy(refreshPeriod = 45, originalRefreshPeriod = 45),
+        )
+        mutableStateFlow.update { it.copy(viewState = content) }
+        composeTestRule
+            .onNodeWithContentDescriptionAfterScroll(label = "45 seconds. Refresh period")
+            .assertIsDisplayed()
+        composeTestRule.onNodeWithText(text = "45 seconds").assertIsDisplayed()
+        composeTestRule
+            .onNodeWithContentDescription(label = "45 seconds. Refresh period")
+            .performClick()
+
+        val options = composeTestRule
+            .onAllNodes(isSelectable() and hasAnyAncestor(isDialog()))
+            .assertCountEquals(4)
+        listOf(30, 45, 60, 90).forEachIndexed { index, period ->
+            options[index].assert(hasText("$period seconds"))
+        }
+        options[1].assertIsSelected()
+        options[2].performClick()
+        composeTestRule.assertNoDialogExists()
+        verify(exactly = 1) {
+            viewModel.trySendAction(EditItemAction.RefreshPeriodOptionClick(60))
+        }
+
         mutableStateFlow.update {
-            it.copy(viewState = DEFAULT_CONTENT.copy(isAdvancedOptionsExpanded = true))
+            it.copy(viewState = content.copy(itemData = content.itemData.copy(refreshPeriod = 60)))
         }
         composeTestRule
-            .onNodeWithContentDescriptionAfterScroll(label = "30 seconds. Refresh period")
+            .onNodeWithContentDescriptionAfterScroll(label = "60 seconds. Refresh period")
+            .performClick()
+        composeTestRule
+            .onAllNodesWithText(text = "60 seconds")
+            .filterToOne(hasAnyAncestor(isDialog()))
+            .assertIsSelected()
+        composeTestRule.onNodeWithText(text = "45 seconds").performClick()
+        composeTestRule.assertNoDialogExists()
+        verify(exactly = 1) {
+            viewModel.trySendAction(EditItemAction.RefreshPeriodOptionClick(45))
+        }
+
+        mutableStateFlow.update { it.copy(viewState = content) }
+        composeTestRule
+            .onNodeWithContentDescriptionAfterScroll(label = "45 seconds. Refresh period")
+            .assertIsDisplayed()
+    }
+
+    @Test
+    fun `restored selection should be included alongside the original custom period`() {
+        mutableStateFlow.update {
+            it.copy(
+                viewState = DEFAULT_CONTENT.copy(
+                    isAdvancedOptionsExpanded = true,
+                    itemData = DEFAULT_ITEM_DATA.copy(
+                        refreshPeriod = 120,
+                        originalRefreshPeriod = 45,
+                    ),
+                ),
+            )
+        }
+        composeTestRule
+            .onNodeWithContentDescriptionAfterScroll(label = "120 seconds. Refresh period")
+            .performClick()
+        val options = composeTestRule
+            .onAllNodes(isSelectable() and hasAnyAncestor(isDialog()))
+            .assertCountEquals(5)
+        listOf(30, 45, 60, 90, 120).forEachIndexed { index, period ->
+            options[index].assert(hasText("$period seconds"))
+        }
+        options[4].assertIsSelected()
+    }
+
+    @Test
+    fun `refresh period click should display dialog and cancel should dismiss the dialog`() {
+        val content = DEFAULT_CONTENT.copy(
+            isAdvancedOptionsExpanded = true,
+            itemData = DEFAULT_ITEM_DATA.copy(refreshPeriod = 45, originalRefreshPeriod = 45),
+        )
+        mutableStateFlow.update { it.copy(viewState = content) }
+        composeTestRule
+            .onNodeWithContentDescriptionAfterScroll(label = "45 seconds. Refresh period")
             .performClick()
 
         composeTestRule
@@ -347,6 +438,36 @@ class EditItemScreenTest : AuthenticatorComposeTest() {
             .onNodeWithText(text = "Cancel")
             .performClick()
         composeTestRule.assertNoDialogExists()
+        composeTestRule
+            .onNodeWithTextAfterScroll(text = "Additional options")
+            .performClick()
+        mutableStateFlow.update {
+            it.copy(viewState = content.copy(isAdvancedOptionsExpanded = false))
+        }
+        composeTestRule.onNodeWithText(text = "45 seconds").assertDoesNotExist()
+        composeTestRule
+            .onNodeWithTextAfterScroll(text = "Name")
+            .performTextInput(text = "New issuer")
+        val renamedContent = content.copy(itemData = content.itemData.copy(issuer = "New issuer"))
+        mutableStateFlow.update {
+            it.copy(viewState = renamedContent.copy(isAdvancedOptionsExpanded = false))
+        }
+        composeTestRule
+            .onNodeWithTextAfterScroll(text = "Additional options")
+            .performClick()
+        mutableStateFlow.update { it.copy(viewState = renamedContent) }
+        composeTestRule
+            .onNodeWithContentDescriptionAfterScroll(label = "45 seconds. Refresh period")
+            .assertIsDisplayed()
+        verify(exactly = 0) {
+            viewModel.trySendAction(any<EditItemAction.RefreshPeriodOptionClick>())
+        }
+        verify(exactly = 2) {
+            viewModel.trySendAction(EditItemAction.ExpandAdvancedOptionsClick)
+        }
+        verify(exactly = 1) {
+            viewModel.trySendAction(EditItemAction.IssuerNameTextChange("New issuer"))
+        }
     }
 
     @Test
@@ -391,7 +512,8 @@ private val DEFAULT_STATE: EditItemState =
 
 private val DEFAULT_ITEM_DATA: EditItemData =
     EditItemData(
-        refreshPeriod = AuthenticatorRefreshPeriodOption.THIRTY,
+        refreshPeriod = 30,
+        originalRefreshPeriod = 30,
         totpCode = "",
         type = AuthenticatorItemType.TOTP,
         username = null,