Merge pull request #364 from fmasa/362-duplicate-name

fmasa · web-flow · commit f88b06b17b0a · 2024-11-05T19:00:25.000+01:00
Ensure duplicated item name fits into limit
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Blessing.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Blessing.kt
@@ -4,6 +4,7 @@ import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -40,5 +41,5 @@ data class Blessing(
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = !isVisibleToPlayers)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Career.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Career.kt
@@ -6,6 +6,7 @@ import cz.frantisekmasa.wfrp_master.common.core.domain.SocialClass
 import cz.frantisekmasa.wfrp_master.common.core.domain.character.Race
 import cz.frantisekmasa.wfrp_master.common.core.domain.character.SocialStatus
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelable
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
@@ -49,7 +50,7 @@ data class Career(
         )
     }
 
-    override fun duplicate(): Career = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate(): Career = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/CompendiumItem.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/CompendiumItem.kt
@@ -1,7 +1,6 @@
 package cz.frantisekmasa.wfrp_master.common.compendium.domain
 
 import com.benasher44.uuid.Uuid
-import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelable
 
 sealed class CompendiumItem<T : CompendiumItem<T>> : Parcelable {
@@ -14,6 +13,4 @@ sealed class CompendiumItem<T : CompendiumItem<T>> : Parcelable {
     abstract fun replace(original: T): T
 
     abstract fun changeVisibility(isVisibleToPlayers: Boolean): T
-
-    protected fun duplicateName(): String = duplicateName(name)
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Disease.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Disease.kt
@@ -4,6 +4,7 @@ import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -35,7 +36,7 @@ data class Disease(
     override fun duplicate() =
         copy(
             id = uuid4(),
-            name = duplicateName(),
+            name = duplicateName(name, NAME_MAX_LENGTH),
         )
 
     override fun replace(original: Disease) = copy(id = original.id)
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/JournalEntry.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/JournalEntry.kt
@@ -4,6 +4,7 @@ import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
@@ -36,7 +37,7 @@ data class JournalEntry(
             gmText = gmText.ifEmpty { original.gmText },
         )
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Miracle.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Miracle.kt
@@ -4,6 +4,7 @@ import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -41,7 +42,7 @@ data class Miracle(
 
     override fun replace(original: Miracle) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Skill.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Skill.kt
@@ -5,6 +5,7 @@ import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.domain.Characteristic
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -32,7 +33,7 @@ data class Skill(
 
     override fun replace(original: Skill) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Spell.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Spell.kt
@@ -3,6 +3,7 @@ package cz.frantisekmasa.wfrp_master.common.compendium.domain
 import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
@@ -47,7 +48,7 @@ data class Spell(
 
     override fun replace(original: Spell) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Talent.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Talent.kt
@@ -4,6 +4,7 @@ import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.common.requireMaxLength
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -36,7 +37,7 @@ data class Talent(
 
     override fun replace(original: Talent) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Trait.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Trait.kt
@@ -3,6 +3,7 @@ package cz.frantisekmasa.wfrp_master.common.compendium.domain
 import androidx.compose.runtime.Immutable
 import com.benasher44.uuid.uuid4
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.icerock.moko.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -25,7 +26,7 @@ data class Trait(
 
     override fun replace(original: Trait) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Trapping.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/compendium/domain/Trapping.kt
@@ -21,6 +21,7 @@ import cz.frantisekmasa.wfrp_master.common.core.domain.trappings.WeaponQuality
 import cz.frantisekmasa.wfrp_master.common.core.domain.trappings.WeaponRangeExpression
 import cz.frantisekmasa.wfrp_master.common.core.serialization.NullableSerializer
 import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString
+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName
 import dev.gitlive.firebase.FirebaseClassDiscriminator
 import dev.icerock.moko.parcelize.Parcelable
 import dev.icerock.moko.parcelize.Parcelize
@@ -52,7 +53,7 @@ data class Trapping(
 
     override fun replace(original: Trapping) = copy(id = original.id)
 
-    override fun duplicate() = copy(id = uuid4(), name = duplicateName())
+    override fun duplicate() = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))
 
     override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/skills/Skill.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/skills/Skill.kt
@@ -40,7 +40,7 @@ data class Skill(
     override fun unlinkFromCompendium() = copy(compendiumId = null)
 
     companion object {
-        const val NAME_MAX_LENGTH = 50
+        const val NAME_MAX_LENGTH = CompendiumSkill.NAME_MAX_LENGTH
         val DESCRIPTION_MAX_LENGTH get() = CompendiumSkill.DESCRIPTION_MAX_LENGTH
         const val MIN_ADVANCES = 0
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/spells/Spell.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/spells/Spell.kt
@@ -58,7 +58,7 @@ data class Spell(
     override fun unlinkFromCompendium() = copy(compendiumId = null)
 
     companion object {
-        const val NAME_MAX_LENGTH = 50
+        const val NAME_MAX_LENGTH = CompendiumSpell.NAME_MAX_LENGTH
         const val RANGE_MAX_LENGTH = 50
         const val TARGET_MAX_LENGTH = 50
         const val DURATION_MAX_LENGTH = 50
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/talents/Talent.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/talents/Talent.kt
@@ -62,7 +62,7 @@ data class Talent(
     }
 
     companion object {
-        const val NAME_MAX_LENGTH = 50
+        const val NAME_MAX_LENGTH = CompendiumTalent.NAME_MAX_LENGTH
         const val DESCRIPTION_MAX_LENGTH = 1500
         const val MAX_TIMES_TAKEN_MAX_LENGTH = CompendiumTalent.MAX_TIMES_TAKEN_MAX_LENGTH
 
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/trappings/InventoryItem.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/domain/trappings/InventoryItem.kt
@@ -95,7 +95,7 @@ data class InventoryItem(
     fun duplicate(): InventoryItem =
         copy(
             id = uuid4(),
-            name = if (compendiumId == null) duplicateName(name) else name,
+            name = if (compendiumId == null) duplicateName(name, NAME_MAX_LENGTH) else name,
         )
 
     fun addToContainer(containerId: InventoryItemId): InventoryItem {
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/utils/DuplicateName.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/core/utils/DuplicateName.kt
@@ -1,3 +1,18 @@
 package cz.frantisekmasa.wfrp_master.common.core.utils
 
-fun duplicateName(name: String): String = "$name (Copy)"
+const val SUFFIX = " (Copy)"
+
+fun duplicateName(
+    name: String,
+    maxLength: Int,
+): String =
+    buildString {
+        if (name.length + SUFFIX.length <= maxLength) {
+            append(name)
+        } else {
+            append(name.substring(0, maxLength - SUFFIX.length - 1))
+            append('…')
+        }
+
+        append(SUFFIX)
+    }
diff --git a/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/npcs/NpcsScreenModel.kt b/common/src/commonMain/kotlin/cz/frantisekmasa/wfrp_master/common/npcs/NpcsScreenModel.kt
@@ -1,6 +1,7 @@
 package cz.frantisekmasa.wfrp_master.common.npcs
 
 import cafe.adriel.voyager.core.model.ScreenModel
+import cz.frantisekmasa.wfrp_master.common.core.domain.character.Character
 import cz.frantisekmasa.wfrp_master.common.core.domain.character.CharacterNotFound
 import cz.frantisekmasa.wfrp_master.common.core.domain.character.CharacterRepository
 import cz.frantisekmasa.wfrp_master.common.core.domain.character.CharacterType
@@ -49,7 +50,7 @@ class NpcsScreenModel(
             DuplicateCharacterRequest(
                 partyId = partyId,
                 characterId = npcId,
-                newName = duplicateName(originalName),
+                newName = duplicateName(originalName, Character.NAME_MAX_LENGTH),
             ),
         )
     }
diff --git a/common/src/commonTest/kotlin/cz/frantisekmasa/wfrp_master/common/core/utils/DuplicateNameTest.kt b/common/src/commonTest/kotlin/cz/frantisekmasa/wfrp_master/common/core/utils/DuplicateNameTest.kt
@@ -0,0 +1,30 @@
+package cz.frantisekmasa.wfrp_master.common.core.utils
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class DuplicateNameTest {
+    @Test
+    fun `appends (Copy) to the name`() {
+        assertEquals(
+            "Name (Copy)",
+            duplicateName("Name", Int.MAX_VALUE),
+        )
+    }
+
+    @Test
+    fun `shortens original name it's too long`() {
+        assertEquals(
+            "Na… (Copy)",
+            duplicateName("Name", 10),
+        )
+    }
+
+    @Test
+    fun `does not shorten original if it's not necessary`() {
+        assertEquals(
+            "Name (Copy)",
+            duplicateName("Name", 11),
+        )
+    }
+}
diff --git a/functions/src/functions/duplicateCharacter.ts b/functions/src/functions/duplicateCharacter.ts
@@ -14,7 +14,21 @@ type Character = {
     avatarUrl: string | null;
 }
 
+const isBlank = (str: string): boolean => {
+    return str.replace(/\s/g, '').length === 0;
+}
+
+const NAME_MAX_LENGTH = 50;
+
 export const duplicateCharacter = characterChange(RequestBody, async (body, character) => {
+    if (body.newName.length > NAME_MAX_LENGTH || isBlank(body.newName)) {
+        return {
+            status: "error",
+            error: 400,
+            message: `Invalid character name, it must be non-blank and less than ${NAME_MAX_LENGTH} characters long.`,
+        };
+    }
+
     const batch = firestore().batch();
     const newCharacterId = uuidv4();
     const newCharacter = character.parent.doc(newCharacterId);

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@ import cz.frantisekmasa.wfrp_master.common.core.domain.SocialClass`
`6`	`6`	`import cz.frantisekmasa.wfrp_master.common.core.domain.character.Race`
`7`	`7`	`import cz.frantisekmasa.wfrp_master.common.core.domain.character.SocialStatus`
`8`	`8`	`import cz.frantisekmasa.wfrp_master.common.core.serialization.UuidAsString`
	`9`	`+import cz.frantisekmasa.wfrp_master.common.core.utils.duplicateName`
`9`	`10`	`import dev.icerock.moko.parcelize.Parcelable`
`10`	`11`	`import dev.icerock.moko.parcelize.Parcelize`
`11`	`12`	`import kotlinx.serialization.Serializable`
`@@ -49,7 +50,7 @@ data class Career(`
`49`	`50`	`)`
`50`	`51`	`}`
`51`	`52`
`52`		`- override fun duplicate(): Career = copy(id = uuid4(), name = duplicateName())`
	`53`	`+ override fun duplicate(): Career = copy(id = uuid4(), name = duplicateName(name, NAME_MAX_LENGTH))`
`53`	`54`
`54`	`55`	`override fun changeVisibility(isVisibleToPlayers: Boolean) = copy(isVisibleToPlayers = isVisibleToPlayers)`
`55`	`56`