From edc9de5516e0e57a600a954a1303b145d7f3c228 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Thu, 12 Nov 2026 22:20:37 +0800 Subject: [PATCH] refactor(ui): extract duplicated form rules and module labels A cross-file duplicate-block scan found three places where the same knowledge was copied into two files: - LostFoundEditViewModel / LostFoundPublishViewModel each carried the same eleven-branch validation chain (title/description/location/contact limits). - MarketplaceEditViewModel / MarketplaceViewModel did the same for price and contact rules. The two copies could drift when a limit changes. - InteractionListScreen / MessagesScreen each defined an identical interactionModuleLabel() mapping of module ids to string resources. Extract LostFoundFormValidator, MarketplaceFormValidator and a shared InteractionModuleLabels composable; all six call sites now use them. The publish-only image check stays in the publish view model, and the marketplace validator exposes a non-null price only after validation succeeds (the old code relied on a Kotlin smart cast that no longer applies across the call). Add unit tests for both validators (rule order, trimming, invalid prices). ANDROID_HOME=~/Library/Android/sdk ./gradlew --offline :app:testDebugUnitTest -> 233 tests, 0 failures. --- .../ui/lostfound/LostFoundEditViewModel.kt | 26 +++------ .../ui/lostfound/LostFoundFormValidator.kt | 51 +++++++++++++++++ .../ui/lostfound/LostFoundPublishViewModel.kt | 26 +++------ .../marketplace/MarketplaceEditViewModel.kt | 26 +++------ .../marketplace/MarketplaceFormValidator.kt | 56 +++++++++++++++++++ .../ui/marketplace/MarketplaceViewModel.kt | 26 +++------ .../ui/messages/InteractionListScreen.kt | 14 ----- .../ui/messages/InteractionModuleLabels.kt | 19 +++++++ .../ui/messages/MessagesScreen.kt | 14 ----- .../lostfound/LostFoundFormValidatorTest.kt | 51 +++++++++++++++++ .../MarketplaceFormValidatorTest.kt | 47 ++++++++++++++++ 11 files changed, 260 insertions(+), 96 deletions(-) create mode 100644 app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidator.kt create mode 100644 app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidator.kt create mode 100644 app/src/main/java/cn/gdeiassistant/ui/messages/InteractionModuleLabels.kt create mode 100644 app/src/test/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidatorTest.kt create mode 100644 app/src/test/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidatorTest.kt diff --git a/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundEditViewModel.kt b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundEditViewModel.kt index 05100d3..dd20444 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundEditViewModel.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundEditViewModel.kt @@ -100,25 +100,17 @@ class LostFoundEditViewModel @Inject constructor( wechat: String, phone: String ) { - val normalizedTitle = title.trim() - val normalizedDescription = description.trim() - val normalizedLocation = location.trim() - val normalizedQq = qq.trim() - val normalizedWechat = wechat.trim() - val normalizedPhone = phone.trim() + val form = LostFoundFormValidator.normalize(title, description, location, qq, wechat, phone) + val normalizedTitle = form.title + val normalizedDescription = form.description + val normalizedLocation = form.location + val normalizedQq = form.qq + val normalizedWechat = form.wechat + val normalizedPhone = form.phone + val errorRes = LostFoundFormValidator.errorRes(form) when { - normalizedTitle.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_title_required)) - normalizedTitle.length > 25 -> emitMessage(context.getString(R.string.lost_found_publish_title_too_long)) - normalizedDescription.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_description_required)) - normalizedDescription.length > 100 -> emitMessage(context.getString(R.string.lost_found_publish_description_too_long)) - normalizedLocation.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_location_required)) - normalizedLocation.length > 30 -> emitMessage(context.getString(R.string.lost_found_publish_location_too_long)) - normalizedQq.isBlank() && normalizedWechat.isBlank() && normalizedPhone.isBlank() -> - emitMessage(context.getString(R.string.lost_found_publish_contact_required)) - normalizedQq.length > 20 -> emitMessage(context.getString(R.string.lost_found_publish_qq_too_long)) - normalizedWechat.length > 20 -> emitMessage(context.getString(R.string.lost_found_publish_wechat_too_long)) - normalizedPhone.length > 11 -> emitMessage(context.getString(R.string.lost_found_publish_phone_too_long)) + errorRes != null -> emitMessage(context.getString(errorRes)) else -> { viewModelScope.launch { _state.update { it.copy(isSubmitting = true, error = null) } diff --git a/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidator.kt b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidator.kt new file mode 100644 index 0000000..c8e1729 --- /dev/null +++ b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidator.kt @@ -0,0 +1,51 @@ +package cn.gdeiassistant.ui.lostfound + +import cn.gdeiassistant.R + +/** + * The publish and edit screens accept the same form. Keeping the rules here means a new + * field or limit is changed once instead of in two view models that can silently drift. + */ +data class LostFoundForm( + val title: String, + val description: String, + val location: String, + val qq: String, + val wechat: String, + val phone: String +) + +object LostFoundFormValidator { + + fun normalize( + title: String, + description: String, + location: String, + qq: String, + wechat: String, + phone: String + ): LostFoundForm = LostFoundForm( + title = title.trim(), + description = description.trim(), + location = location.trim(), + qq = qq.trim(), + wechat = wechat.trim(), + phone = phone.trim() + ) + + /** String resource of the first failed rule, or null when the form is valid. */ + fun errorRes(form: LostFoundForm): Int? = when { + form.title.isBlank() -> R.string.lost_found_publish_title_required + form.title.length > 25 -> R.string.lost_found_publish_title_too_long + form.description.isBlank() -> R.string.lost_found_publish_description_required + form.description.length > 100 -> R.string.lost_found_publish_description_too_long + form.location.isBlank() -> R.string.lost_found_publish_location_required + form.location.length > 30 -> R.string.lost_found_publish_location_too_long + form.qq.isBlank() && form.wechat.isBlank() && form.phone.isBlank() -> + R.string.lost_found_publish_contact_required + form.qq.length > 20 -> R.string.lost_found_publish_qq_too_long + form.wechat.length > 20 -> R.string.lost_found_publish_wechat_too_long + form.phone.length > 11 -> R.string.lost_found_publish_phone_too_long + else -> null + } +} diff --git a/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundPublishViewModel.kt b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundPublishViewModel.kt index 6cb6e6b..3b16d53 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundPublishViewModel.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/lostfound/LostFoundPublishViewModel.kt @@ -65,25 +65,17 @@ class LostFoundPublishViewModel @Inject constructor( phone: String, images: List ) { - val normalizedTitle = title.trim() - val normalizedDescription = description.trim() - val normalizedLocation = location.trim() - val normalizedQq = qq.trim() - val normalizedWechat = wechat.trim() - val normalizedPhone = phone.trim() + val form = LostFoundFormValidator.normalize(title, description, location, qq, wechat, phone) + val normalizedTitle = form.title + val normalizedDescription = form.description + val normalizedLocation = form.location + val normalizedQq = form.qq + val normalizedWechat = form.wechat + val normalizedPhone = form.phone + val errorRes = LostFoundFormValidator.errorRes(form) when { - normalizedTitle.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_title_required)) - normalizedTitle.length > 25 -> emitMessage(context.getString(R.string.lost_found_publish_title_too_long)) - normalizedDescription.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_description_required)) - normalizedDescription.length > 100 -> emitMessage(context.getString(R.string.lost_found_publish_description_too_long)) - normalizedLocation.isBlank() -> emitMessage(context.getString(R.string.lost_found_publish_location_required)) - normalizedLocation.length > 30 -> emitMessage(context.getString(R.string.lost_found_publish_location_too_long)) - normalizedQq.isBlank() && normalizedWechat.isBlank() && normalizedPhone.isBlank() -> - emitMessage(context.getString(R.string.lost_found_publish_contact_required)) - normalizedQq.length > 20 -> emitMessage(context.getString(R.string.lost_found_publish_qq_too_long)) - normalizedWechat.length > 20 -> emitMessage(context.getString(R.string.lost_found_publish_wechat_too_long)) - normalizedPhone.length > 11 -> emitMessage(context.getString(R.string.lost_found_publish_phone_too_long)) + errorRes != null -> emitMessage(context.getString(errorRes)) images.isEmpty() -> emitMessage(context.getString(R.string.lost_found_publish_images_required)) else -> { viewModelScope.launch { diff --git a/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceEditViewModel.kt b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceEditViewModel.kt index d3c1090..65b13f4 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceEditViewModel.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceEditViewModel.kt @@ -98,24 +98,16 @@ class MarketplaceEditViewModel @Inject constructor( qq: String, phone: String ) { - val normalizedTitle = title.trim() - val normalizedDescription = description.trim() - val normalizedLocation = location.trim() - val normalizedQq = qq.trim() - val normalizedPhone = phone.trim() - val price = priceText.trim().toDoubleOrNull() + val form = MarketplaceFormValidator.normalize(title, priceText, description, location, qq, phone) + val normalizedTitle = form.title + val normalizedDescription = form.description + val normalizedLocation = form.location + val normalizedQq = form.qq + val normalizedPhone = form.phone + val errorRes = MarketplaceFormValidator.errorRes(form) when { - normalizedTitle.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_title_required)) - normalizedTitle.length > 25 -> emitMessage(context.getString(R.string.marketplace_publish_title_too_long)) - price == null || price <= 0.0 || price > 9999.99 -> emitMessage(context.getString(R.string.marketplace_publish_price_invalid)) - normalizedDescription.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_description_required)) - normalizedDescription.length > 100 -> emitMessage(context.getString(R.string.marketplace_publish_description_too_long)) - normalizedLocation.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_location_required)) - normalizedLocation.length > 30 -> emitMessage(context.getString(R.string.marketplace_publish_location_too_long)) - normalizedQq.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_qq_required)) - normalizedQq.length > 20 -> emitMessage(context.getString(R.string.marketplace_publish_qq_too_long)) - normalizedPhone.length > 11 -> emitMessage(context.getString(R.string.marketplace_publish_phone_too_long)) + errorRes != null -> emitMessage(context.getString(errorRes)) else -> { viewModelScope.launch { _state.update { it.copy(isSubmitting = true, error = null) } @@ -123,7 +115,7 @@ class MarketplaceEditViewModel @Inject constructor( id = itemId, draft = MarketplaceUpdateDraft( title = normalizedTitle, - price = price, + price = MarketplaceFormValidator.price(form), description = normalizedDescription, location = normalizedLocation, typeId = typeId, diff --git a/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidator.kt b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidator.kt new file mode 100644 index 0000000..259d844 --- /dev/null +++ b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidator.kt @@ -0,0 +1,56 @@ +package cn.gdeiassistant.ui.marketplace + +import cn.gdeiassistant.R + +/** + * Shared rules for the marketplace publish and edit forms. Both screens used to carry the + * same eleven-branch validation chain, so a limit change had to be applied twice. + */ +data class MarketplaceForm( + val title: String, + val price: Double?, + val description: String, + val location: String, + val qq: String, + val phone: String +) + +object MarketplaceFormValidator { + + fun normalize( + title: String, + priceText: String, + description: String, + location: String, + qq: String, + phone: String + ): MarketplaceForm = MarketplaceForm( + title = title.trim(), + price = priceText.trim().toDoubleOrNull(), + description = description.trim(), + location = location.trim(), + qq = qq.trim(), + phone = phone.trim() + ) + + /** String resource of the first failed rule, or null when the form is valid. */ + fun errorRes(form: MarketplaceForm): Int? = when { + form.title.isBlank() -> R.string.marketplace_publish_title_required + form.title.length > 25 -> R.string.marketplace_publish_title_too_long + form.price == null || form.price <= 0.0 || form.price > 9999.99 -> + R.string.marketplace_publish_price_invalid + form.description.isBlank() -> R.string.marketplace_publish_description_required + form.description.length > 100 -> R.string.marketplace_publish_description_too_long + form.location.isBlank() -> R.string.marketplace_publish_location_required + form.location.length > 30 -> R.string.marketplace_publish_location_too_long + form.qq.isBlank() -> R.string.marketplace_publish_qq_required + form.qq.length > 20 -> R.string.marketplace_publish_qq_too_long + form.phone.length > 11 -> R.string.marketplace_publish_phone_too_long + else -> null + } + + /** Non-null price; call only after [errorRes] returned null. */ + fun price(form: MarketplaceForm): Double = requireNotNull(form.price) { + "price() requires a validated form" + } +} diff --git a/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceViewModel.kt b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceViewModel.kt index f827cb6..39458b2 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceViewModel.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/marketplace/MarketplaceViewModel.kt @@ -133,24 +133,16 @@ class MarketplacePublishViewModel @Inject constructor( phone: String, images: List ) { - val normalizedTitle = title.trim() - val normalizedDescription = description.trim() - val normalizedLocation = location.trim() - val normalizedQq = qq.trim() - val normalizedPhone = phone.trim() - val price = priceText.trim().toDoubleOrNull() + val form = MarketplaceFormValidator.normalize(title, priceText, description, location, qq, phone) + val normalizedTitle = form.title + val normalizedDescription = form.description + val normalizedLocation = form.location + val normalizedQq = form.qq + val normalizedPhone = form.phone + val errorRes = MarketplaceFormValidator.errorRes(form) when { - normalizedTitle.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_title_required)) - normalizedTitle.length > 25 -> emitMessage(context.getString(R.string.marketplace_publish_title_too_long)) - price == null || price <= 0.0 || price > 9999.99 -> emitMessage(context.getString(R.string.marketplace_publish_price_invalid)) - normalizedDescription.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_description_required)) - normalizedDescription.length > 100 -> emitMessage(context.getString(R.string.marketplace_publish_description_too_long)) - normalizedLocation.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_location_required)) - normalizedLocation.length > 30 -> emitMessage(context.getString(R.string.marketplace_publish_location_too_long)) - normalizedQq.isBlank() -> emitMessage(context.getString(R.string.marketplace_publish_qq_required)) - normalizedQq.length > 20 -> emitMessage(context.getString(R.string.marketplace_publish_qq_too_long)) - normalizedPhone.length > 11 -> emitMessage(context.getString(R.string.marketplace_publish_phone_too_long)) + errorRes != null -> emitMessage(context.getString(errorRes)) images.isEmpty() -> emitMessage(context.getString(R.string.marketplace_publish_images_required)) else -> { viewModelScope.launch { @@ -158,7 +150,7 @@ class MarketplacePublishViewModel @Inject constructor( marketplaceRepository.publish( draft = MarketplaceDraft( title = normalizedTitle, - price = price, + price = MarketplaceFormValidator.price(form), description = normalizedDescription, location = normalizedLocation, typeId = typeId, diff --git a/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionListScreen.kt b/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionListScreen.kt index d4e8661..7c9f0af 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionListScreen.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionListScreen.kt @@ -204,17 +204,3 @@ private fun InteractionListRow( } } -@Composable -private fun interactionModuleLabel(module: String?): String { - return when (module) { - "secret" -> stringResource(R.string.messages_module_secret) - "secondhand" -> stringResource(R.string.messages_module_marketplace) - "lostandfound" -> stringResource(R.string.messages_module_lost_found) - "topic" -> stringResource(R.string.messages_module_topic) - "express" -> stringResource(R.string.messages_module_express) - "dating" -> stringResource(R.string.messages_module_dating) - "delivery" -> stringResource(R.string.messages_module_delivery) - "photograph" -> stringResource(R.string.messages_module_photograph) - else -> stringResource(R.string.messages_interaction_badge) - } -} diff --git a/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionModuleLabels.kt b/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionModuleLabels.kt new file mode 100644 index 0000000..465de50 --- /dev/null +++ b/app/src/main/java/cn/gdeiassistant/ui/messages/InteractionModuleLabels.kt @@ -0,0 +1,19 @@ +package cn.gdeiassistant.ui.messages + +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.stringResource +import cn.gdeiassistant.R + +/** One mapping from an interaction module id to its display name, shared by both lists. */ +@Composable +fun interactionModuleLabel(module: String?): String = when (module) { + "secret" -> stringResource(R.string.messages_module_secret) + "secondhand" -> stringResource(R.string.messages_module_marketplace) + "lostandfound" -> stringResource(R.string.messages_module_lost_found) + "topic" -> stringResource(R.string.messages_module_topic) + "express" -> stringResource(R.string.messages_module_express) + "dating" -> stringResource(R.string.messages_module_dating) + "delivery" -> stringResource(R.string.messages_module_delivery) + "photograph" -> stringResource(R.string.messages_module_photograph) + else -> stringResource(R.string.messages_interaction_badge) +} diff --git a/app/src/main/java/cn/gdeiassistant/ui/messages/MessagesScreen.kt b/app/src/main/java/cn/gdeiassistant/ui/messages/MessagesScreen.kt index d73f733..29ac2c0 100644 --- a/app/src/main/java/cn/gdeiassistant/ui/messages/MessagesScreen.kt +++ b/app/src/main/java/cn/gdeiassistant/ui/messages/MessagesScreen.kt @@ -369,20 +369,6 @@ private fun FestivalCard(festival: Festival) { } } -@Composable -private fun interactionModuleLabel(module: String?): String { - return when (module) { - "secret" -> stringResource(R.string.messages_module_secret) - "secondhand" -> stringResource(R.string.messages_module_marketplace) - "lostandfound" -> stringResource(R.string.messages_module_lost_found) - "topic" -> stringResource(R.string.messages_module_topic) - "express" -> stringResource(R.string.messages_module_express) - "dating" -> stringResource(R.string.messages_module_dating) - "delivery" -> stringResource(R.string.messages_module_delivery) - "photograph" -> stringResource(R.string.messages_module_photograph) - else -> stringResource(R.string.messages_interaction_badge) - } -} @Composable private fun InfoSectionCard( diff --git a/app/src/test/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidatorTest.kt b/app/src/test/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidatorTest.kt new file mode 100644 index 0000000..a0ca017 --- /dev/null +++ b/app/src/test/java/cn/gdeiassistant/ui/lostfound/LostFoundFormValidatorTest.kt @@ -0,0 +1,51 @@ +package cn.gdeiassistant.ui.lostfound + +import cn.gdeiassistant.R +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class LostFoundFormValidatorTest { + + private fun form( + title: String = "Found keys", + description: String = "Near the library", + location: String = "Library", + qq: String = "12345", + wechat: String = "", + phone: String = "" + ) = LostFoundFormValidator.normalize(title, description, location, qq, wechat, phone) + + @Test + fun trimsEveryField() { + val result = form(title = " keys ", qq = " 123 ", wechat = " wx ") + assertEquals("keys", result.title) + assertEquals("123", result.qq) + assertEquals("wx", result.wechat) + } + + @Test + fun validFormHasNoError() { + assertNull(LostFoundFormValidator.errorRes(form())) + } + + @Test + fun reportsRulesInTheSameOrderAsTheOriginalScreens() { + assertEquals( + R.string.lost_found_publish_title_required, + LostFoundFormValidator.errorRes(form(title = " ")) + ) + assertEquals( + R.string.lost_found_publish_title_too_long, + LostFoundFormValidator.errorRes(form(title = "x".repeat(26))) + ) + assertEquals( + R.string.lost_found_publish_contact_required, + LostFoundFormValidator.errorRes(form(qq = "", wechat = "", phone = "")) + ) + assertEquals( + R.string.lost_found_publish_phone_too_long, + LostFoundFormValidator.errorRes(form(phone = "1".repeat(12))) + ) + } +} diff --git a/app/src/test/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidatorTest.kt b/app/src/test/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidatorTest.kt new file mode 100644 index 0000000..ea68a04 --- /dev/null +++ b/app/src/test/java/cn/gdeiassistant/ui/marketplace/MarketplaceFormValidatorTest.kt @@ -0,0 +1,47 @@ +package cn.gdeiassistant.ui.marketplace + +import cn.gdeiassistant.R +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class MarketplaceFormValidatorTest { + + private fun form( + title: String = "Bike", + priceText: String = "12.50", + qq: String = "12345" + ) = MarketplaceFormValidator.normalize( + title, priceText, "A used bike", "Dorm", qq, "" + ) + + @Test + fun validFormHasNoErrorAndExposesParsedPrice() { + val result = form() + assertNull(MarketplaceFormValidator.errorRes(result)) + assertEquals(12.50, MarketplaceFormValidator.price(result), 0.0) + } + + @Test + fun rejectsInvalidPrices() { + for (price in listOf("", "abc", "0", "-1", "10000")) { + assertEquals( + "price=$price", + R.string.marketplace_publish_price_invalid, + MarketplaceFormValidator.errorRes(form(priceText = price)) + ) + } + } + + @Test + fun reportsFieldRulesInTheSameOrderAsTheOriginalScreens() { + assertEquals( + R.string.marketplace_publish_title_required, + MarketplaceFormValidator.errorRes(form(title = " ")) + ) + assertEquals( + R.string.marketplace_publish_qq_required, + MarketplaceFormValidator.errorRes(form(qq = "")) + ) + } +}