From 9eaf9663f306101bbd8f0224429bde225a28f7e9 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 17:58:41 +0300 Subject: [PATCH] refactor(skills): extract SkillListFilter and SkillScanPresentation Separate pure skill filtering and scan presentation helpers. Wire into SkillsScreen with unit test coverage. --- .../ui/skills/SkillListFilter.kt | 39 ++++++ .../ui/skills/SkillScanPresentation.kt | 75 ++++++++++ .../hermescontrol/ui/skills/SkillsScreen.kt | 69 ++-------- .../ui/skills/SkillListFilterTest.kt | 130 ++++++++++++++++++ .../ui/skills/SkillScanPresentationTest.kt | 64 +++++++++ 5 files changed, 323 insertions(+), 54 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/skills/SkillListFilter.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/skills/SkillScanPresentation.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/skills/SkillListFilterTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/skills/SkillScanPresentationTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillListFilter.kt b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillListFilter.kt new file mode 100644 index 00000000..7f30c87f --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillListFilter.kt @@ -0,0 +1,39 @@ +package com.m57.hermescontrol.ui.skills + +import com.m57.hermescontrol.data.model.Skill + +object SkillListFilter { + /** + * Filters skills by search query, enabled/disabled status, category, and source. + */ + fun filterSkills( + skills: List, + query: String, + selectedStatus: SkillFilter, + selectedCategory: String?, + sourceFilter: String?, + ): List = + skills.filter { skill -> + val matchesQuery = + query.isBlank() || + skill.name.contains(query, ignoreCase = true) || + skill.description?.contains(query, ignoreCase = true) == true + val matchesStatus = + when (selectedStatus) { + SkillFilter.ALL_STATUSES -> true + SkillFilter.ENABLED -> skill.enabled + SkillFilter.DISABLED -> !skill.enabled + } + val matchesCategory = + selectedCategory == null || + selectedCategory == CATEGORY_ALL || + skill.category == selectedCategory + val matchesSource = sourceFilter == null || skill.source == sourceFilter + + matchesQuery && matchesStatus && matchesCategory && matchesSource + } + + fun extractCategories(skills: List): List = skills.mapNotNull { it.category }.distinct().sorted() + + fun extractSources(skills: List): List = skills.mapNotNull { it.source }.distinct().sorted() +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillScanPresentation.kt b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillScanPresentation.kt new file mode 100644 index 00000000..b9726261 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillScanPresentation.kt @@ -0,0 +1,75 @@ +package com.m57.hermescontrol.ui.skills + +import androidx.compose.ui.graphics.Color +import com.m57.hermescontrol.R +import com.m57.hermescontrol.theme.HermesStatusColors + +enum class ScanVerdictKind { + SAFE, + DANGEROUS, + WARNING, +} + +enum class ScanPolicyKind { + ALLOW, + ASK, + BLOCK, + OTHER, +} + +object SkillScanPresentation { + fun resolveVerdictKind(verdict: String?): ScanVerdictKind = + when (verdict) { + "safe" -> ScanVerdictKind.SAFE + "dangerous" -> ScanVerdictKind.DANGEROUS + else -> ScanVerdictKind.WARNING + } + + fun resolveVerdictColor( + verdict: String?, + statusColors: HermesStatusColors, + ): Color = + when (resolveVerdictKind(verdict)) { + ScanVerdictKind.SAFE -> statusColors.success + ScanVerdictKind.DANGEROUS -> statusColors.error + ScanVerdictKind.WARNING -> statusColors.warning + } + + fun resolvePolicyKind(policy: String?): ScanPolicyKind = + when (policy) { + "allow" -> ScanPolicyKind.ALLOW + "ask" -> ScanPolicyKind.ASK + "block" -> ScanPolicyKind.BLOCK + else -> ScanPolicyKind.OTHER + } + + fun resolvePolicyLabelRes(policy: String?): Int? = + when (policy) { + "allow" -> R.string.skills_hub_scan_policy_allow + "ask" -> R.string.skills_hub_scan_policy_ask + "block" -> R.string.skills_hub_scan_policy_block + else -> null + } + + fun resolvePolicyColor( + policy: String?, + statusColors: HermesStatusColors, + ): Color = + when (policy) { + "allow" -> statusColors.success + "ask" -> statusColors.warning + "block" -> statusColors.error + else -> statusColors.neutral + } + + fun resolveSeverityColor( + severity: String?, + statusColors: HermesStatusColors, + ): Color = + when (severity) { + "critical", "high" -> statusColors.error + "medium" -> statusColors.warning + "low" -> statusColors.neutral + else -> statusColors.neutral + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt index b1f06cf2..8c52332d 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt @@ -256,34 +256,22 @@ private fun InstalledSkillsView( val categories = remember(state.skills) { - state.skills - .mapNotNull { it.category } - .distinct() - .sorted() + SkillListFilter.extractCategories(state.skills) } val sources = remember(state.skills) { - state.skills - .mapNotNull { it.source } - .distinct() - .sorted() + SkillListFilter.extractSources(state.skills) } val filteredSkills = remember(state.skills, query, selectedStatus, selectedCategory, sourceFilter) { - state.skills.filter { skill -> - ( - query.isBlank() || skill.name.contains(query, ignoreCase = true) || - skill.description?.contains(query, ignoreCase = true) == true - ) && - ( - selectedStatus == SkillFilter.ALL_STATUSES || - (selectedStatus == SkillFilter.ENABLED && skill.enabled) || - (selectedStatus == SkillFilter.DISABLED && !skill.enabled) - ) && - (selectedCategory == CATEGORY_ALL || skill.category == selectedCategory) && - (sourceFilter == null || skill.source == sourceFilter) - } + SkillListFilter.filterSkills( + skills = state.skills, + query = query, + selectedStatus = selectedStatus, + selectedCategory = selectedCategory, + sourceFilter = sourceFilter, + ) } Column(modifier = Modifier.fillMaxSize()) { @@ -891,26 +879,11 @@ private fun SkillScanSection( @Composable private fun ScanResultCard(result: SkillScanResponse) { val statusColors = LocalHermesStatusColors.current - val verdictColor = - when (result.verdict) { - "safe" -> statusColors.success - "dangerous" -> statusColors.error - else -> statusColors.warning - } + val verdictColor = SkillScanPresentation.resolveVerdictColor(result.verdict, statusColors) val policyLabel = - when (result.policy) { - "allow" -> stringResource(R.string.skills_hub_scan_policy_allow) - "ask" -> stringResource(R.string.skills_hub_scan_policy_ask) - "block" -> stringResource(R.string.skills_hub_scan_policy_block) - else -> result.policy.orEmpty() - } - val policyColor = - when (result.policy) { - "allow" -> statusColors.success - "ask" -> statusColors.warning - "block" -> statusColors.error - else -> statusColors.neutral - } + SkillScanPresentation.resolvePolicyLabelRes(result.policy)?.let { stringResource(it) } + ?: result.policy.orEmpty() + val policyColor = SkillScanPresentation.resolvePolicyColor(result.policy, statusColors) Card( modifier = Modifier.fillMaxWidth(), @@ -979,13 +952,6 @@ private fun ScanResultCard(result: SkillScanResponse) { } // ── Severity tally ── - val severityColors = - mapOf( - "critical" to statusColors.error, - "high" to statusColors.error, - "medium" to statusColors.warning, - "low" to statusColors.neutral, - ) Row( modifier = Modifier.horizontalScroll(rememberScrollState()), horizontalArrangement = Arrangement.spacedBy(6.dp), @@ -993,7 +959,7 @@ private fun ScanResultCard(result: SkillScanResponse) { listOf("critical", "high", "medium", "low").forEach { sev -> val count = result.severityCounts[sev] ?: 0 if (count > 0) { - val sevColor = severityColors[sev] ?: statusColors.neutral + val sevColor = SkillScanPresentation.resolveSeverityColor(sev, statusColors) Surface( color = sevColor.copy(alpha = 0.15f), shape = RoundedCornerShape(50), @@ -1039,12 +1005,7 @@ private fun ScanResultCard(result: SkillScanResponse) { @Composable private fun ScanFindingRow(finding: SkillScanFinding) { val statusColors = LocalHermesStatusColors.current - val severityColor = - when (finding.severity) { - "critical", "high" -> statusColors.error - "medium" -> statusColors.warning - else -> statusColors.neutral - } + val severityColor = SkillScanPresentation.resolveSeverityColor(finding.severity, statusColors) Column(verticalArrangement = Arrangement.spacedBy(2.dp)) { Row( verticalAlignment = Alignment.CenterVertically, diff --git a/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillListFilterTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillListFilterTest.kt new file mode 100644 index 00000000..54ef1626 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillListFilterTest.kt @@ -0,0 +1,130 @@ +package com.m57.hermescontrol.ui.skills + +import com.m57.hermescontrol.data.model.Skill +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class SkillListFilterTest { + private val sampleSkills = + listOf( + Skill( + name = "code-review", + category = "dev", + description = "Automated code review assistant", + enabled = true, + source = "built-in", + ), + Skill( + name = "deploy-bot", + category = "ops", + description = "Deploy to production environments", + enabled = false, + source = "hub", + ), + Skill( + name = "image-gen", + category = "creative", + description = "Generate creative assets", + enabled = true, + source = "hub", + ), + ) + + @Test + fun `filterSkills empty query and all statuses returns all`() { + val result = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "", + selectedStatus = SkillFilter.ALL_STATUSES, + selectedCategory = CATEGORY_ALL, + sourceFilter = null, + ) + assertEquals(3, result.size) + } + + @Test + fun `filterSkills filters by search query on name and description`() { + val nameResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "code", + selectedStatus = SkillFilter.ALL_STATUSES, + selectedCategory = CATEGORY_ALL, + sourceFilter = null, + ) + assertEquals(1, nameResult.size) + assertEquals("code-review", nameResult[0].name) + + val descResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "production", + selectedStatus = SkillFilter.ALL_STATUSES, + selectedCategory = CATEGORY_ALL, + sourceFilter = null, + ) + assertEquals(1, descResult.size) + assertEquals("deploy-bot", descResult[0].name) + } + + @Test + fun `filterSkills filters by enabled and disabled status`() { + val enabledResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "", + selectedStatus = SkillFilter.ENABLED, + selectedCategory = CATEGORY_ALL, + sourceFilter = null, + ) + assertEquals(2, enabledResult.size) + assertTrue(enabledResult.all { it.enabled }) + + val disabledResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "", + selectedStatus = SkillFilter.DISABLED, + selectedCategory = CATEGORY_ALL, + sourceFilter = null, + ) + assertEquals(1, disabledResult.size) + assertEquals("deploy-bot", disabledResult[0].name) + } + + @Test + fun `filterSkills filters by category and source`() { + val catResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "", + selectedStatus = SkillFilter.ALL_STATUSES, + selectedCategory = "ops", + sourceFilter = null, + ) + assertEquals(1, catResult.size) + assertEquals("deploy-bot", catResult[0].name) + + val sourceResult = + SkillListFilter.filterSkills( + skills = sampleSkills, + query = "", + selectedStatus = SkillFilter.ALL_STATUSES, + selectedCategory = CATEGORY_ALL, + sourceFilter = "built-in", + ) + assertEquals(1, sourceResult.size) + assertEquals("code-review", sourceResult[0].name) + } + + @Test + fun `extractCategories and extractSources return distinct sorted values`() { + val categories = SkillListFilter.extractCategories(sampleSkills) + assertEquals(listOf("creative", "dev", "ops"), categories) + + val sources = SkillListFilter.extractSources(sampleSkills) + assertEquals(listOf("built-in", "hub"), sources) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillScanPresentationTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillScanPresentationTest.kt new file mode 100644 index 00000000..a0ee5e2a --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/skills/SkillScanPresentationTest.kt @@ -0,0 +1,64 @@ +package com.m57.hermescontrol.ui.skills + +import androidx.compose.ui.graphics.Color +import com.m57.hermescontrol.R +import com.m57.hermescontrol.theme.HermesStatusColors +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class SkillScanPresentationTest { + private val dummyColors = + HermesStatusColors( + success = Color(0xFF00FF00), + successContainer = Color(0xFF003300), + onSuccess = Color.White, + warning = Color(0xFFFFFF00), + warningContainer = Color(0xFF333300), + onWarning = Color.Black, + error = Color(0xFFFF0000), + errorContainer = Color(0xFF330000), + onError = Color.White, + info = Color(0xFF0000FF), + infoContainer = Color(0xFF000033), + onInfo = Color.White, + neutral = Color.Gray, + ) + + @Test + fun `resolveVerdictKind maps safe dangerous and fallback`() { + assertEquals(ScanVerdictKind.SAFE, SkillScanPresentation.resolveVerdictKind("safe")) + assertEquals(ScanVerdictKind.DANGEROUS, SkillScanPresentation.resolveVerdictKind("dangerous")) + assertEquals(ScanVerdictKind.WARNING, SkillScanPresentation.resolveVerdictKind("unknown")) + assertEquals(ScanVerdictKind.WARNING, SkillScanPresentation.resolveVerdictKind(null)) + } + + @Test + fun `resolveVerdictColor returns corresponding semantic color`() { + assertEquals(dummyColors.success, SkillScanPresentation.resolveVerdictColor("safe", dummyColors)) + assertEquals(dummyColors.error, SkillScanPresentation.resolveVerdictColor("dangerous", dummyColors)) + assertEquals(dummyColors.warning, SkillScanPresentation.resolveVerdictColor("suspicious", dummyColors)) + } + + @Test + fun `resolvePolicyKind and labels map allow ask block and fallback`() { + assertEquals(ScanPolicyKind.ALLOW, SkillScanPresentation.resolvePolicyKind("allow")) + assertEquals(ScanPolicyKind.ASK, SkillScanPresentation.resolvePolicyKind("ask")) + assertEquals(ScanPolicyKind.BLOCK, SkillScanPresentation.resolvePolicyKind("block")) + assertEquals(ScanPolicyKind.OTHER, SkillScanPresentation.resolvePolicyKind("custom")) + + assertEquals(R.string.skills_hub_scan_policy_allow, SkillScanPresentation.resolvePolicyLabelRes("allow")) + assertEquals(R.string.skills_hub_scan_policy_ask, SkillScanPresentation.resolvePolicyLabelRes("ask")) + assertEquals(R.string.skills_hub_scan_policy_block, SkillScanPresentation.resolvePolicyLabelRes("block")) + assertNull(SkillScanPresentation.resolvePolicyLabelRes("other")) + } + + @Test + fun `resolveSeverityColor maps critical high medium low and fallback`() { + assertEquals(dummyColors.error, SkillScanPresentation.resolveSeverityColor("critical", dummyColors)) + assertEquals(dummyColors.error, SkillScanPresentation.resolveSeverityColor("high", dummyColors)) + assertEquals(dummyColors.warning, SkillScanPresentation.resolveSeverityColor("medium", dummyColors)) + assertEquals(dummyColors.neutral, SkillScanPresentation.resolveSeverityColor("low", dummyColors)) + assertEquals(dummyColors.neutral, SkillScanPresentation.resolveSeverityColor("info", dummyColors)) + } +}