Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal class AndroidFeatureConventionPlugin : Plugin<Project> {
implementationProject(":core:model")
implementationProject(":feature:screens")

implementation(libs.kotlinx.collections.immutable)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find modules that depend on feature modules but are not feature modules themselves
echo "=== Modules applying AndroidFeatureConventionPlugin ==="
rg -l 'metasearch.android.feature' --include="*.gradle.kts" --include="*.gradle"

echo ""
echo "=== Modules depending on feature modules ==="
rg 'implementation.*:feature:' --include="*.gradle.kts" --include="*.gradle" -A1

Repository: komodgn/meta-android

Length of output: 1066


🏁 Script executed:

# Search for State classes with ImmutableList in feature modules
echo "=== State classes in feature modules ==="
find . -path "*/feature/*/src/*" -name "*State.kt" -type f | head -20

echo ""
echo "=== ImmutableList usage in feature modules ==="
rg 'ImmutableList' --include="*.kt" -l | grep feature

Repository: komodgn/meta-android

Length of output: 1523


🏁 Script executed:

# Check the current dependency scope in the convention plugin
echo "=== AndroidFeatureConventionPlugin content ==="
cat build-logic/src/main/java/AndroidFeatureConventionPlugin.kt | head -60

Repository: komodgn/meta-android

Length of output: 1392


🏁 Script executed:

#!/bin/bash
# Check if State classes actually use ImmutableList
echo "=== Checking State classes for ImmutableList ==="
for file in ./feature/*/src/main/java/com/metasearch/android/feature/*/\*UiState.kt; do
  if [ -f "$file" ]; then
    echo "File: $file"
    head -30 "$file"
    echo "---"
  fi
done | head -150

Repository: komodgn/meta-android

Length of output: 112


🏁 Script executed:

# Search for modules depending on feature modules using correct rg syntax
echo "=== Gradle files with feature dependencies ==="
rg ':feature:' -t gradle -t kotlin | head -30

echo ""
echo "=== App module build file ==="
if [ -f "app/build.gradle.kts" ]; then
  cat app/build.gradle.kts | grep -E 'feature|implementation|dependencies' -A2 -B2 | head -50
fi

Repository: komodgn/meta-android

Length of output: 1701


🏁 Script executed:

# Check what modules apply AndroidFeatureConventionPlugin
echo "=== Feature modules with convention plugin ==="
find . -path "*/feature/*/build.gradle.kts" -exec grep -l "metasearch.android.feature" {} \;

Repository: komodgn/meta-android

Length of output: 413


🏁 Script executed:

#!/bin/bash
# Check individual State files for ImmutableList
echo "=== HomeUiState ===" 
cat ./feature/home/src/main/java/com/metasearch/android/feature/home/HomeUiState.kt

echo ""
echo "=== GraphUiState ===" 
cat ./feature/graph/src/main/java/com/metasearch/android/feature/graph/GraphUiState.kt

echo ""
echo "=== NLSearchUiState ===" 
cat ./feature/search/src/main/java/com/metasearch/android/feature/search/nls/NLSearchUiState.kt

Repository: komodgn/meta-android

Length of output: 4754


🏁 Script executed:

# Search for ImmutableList usage across the codebase
rg 'ImmutableList' -A2 -B2 | head -80

Repository: komodgn/meta-android

Length of output: 1540


🏁 Script executed:

# Check if any non-feature modules use State classes from feature modules
echo "=== Non-feature modules that depend on feature modules ==="
rg 'feature\.' --type gradle --type kotlin | grep -E 'implementation|api' | grep -v feature/ | head -20

Repository: komodgn/meta-android

Length of output: 704


🏁 Script executed:

# Search for all State classes with ImmutableList to get a complete picture
rg 'ImmutableList' -B5 | grep -E 'package|ImmutableList' | head -40

# Also check core:model to see if any model classes use ImmutableList
echo ""
echo "=== ImmutableList in model classes ==="
rg 'ImmutableList' core/model/ -A1 -B1 | head -40

Repository: komodgn/meta-android

Length of output: 424


🏁 Script executed:

# Check if the app module would have compilation issues
# by verifying if kotlinx-collections-immutable is in app module's effective dependencies
echo "=== App module build.gradle.kts dependencies section ==="
sed -n '/dependencies {/,/^}/p' app/build.gradle.kts | grep -i immutable

Repository: komodgn/meta-android

Length of output: 120


🏁 Script executed:

# Verify only FocusingSearchUiState uses ImmutableList
echo "=== Complete ImmutableList usage across feature modules ==="
rg 'ImmutableList' feature/ --include="*.kt" -B2

# Check if app module would compile with current setup
echo ""
echo "=== Gradle dependency resolution check ==="
echo "App module dependencies on feature modules (all implementation scope):"
grep 'feature\.' app/build.gradle.kts | grep implementation | wc -l
echo "Number of feature modules with implementation(libs.kotlinx.collections.immutable):"
echo "9 (all feature modules via convention plugin)"

Repository: komodgn/meta-android

Length of output: 828


🏁 Script executed:

# Check what other core modules might expose ImmutableList
rg 'ImmutableList' core/ --include="*.kt" | head -20

Repository: komodgn/meta-android

Length of output: 505


🏁 Script executed:

# Check if app module or other modules import State classes from features
rg 'FocusingSearchUiState|HomeUiState|GraphUiState|NLSearchUiState' --type kotlin | grep -v 'feature.*src.*' | head -20

echo ""
echo "=== Check FocusingSearchUiState file directly ==="
cat feature/search/src/main/java/com/metasearch/android/feature/search/focusing/FocusingSearchUiState.kt

Repository: komodgn/meta-android

Length of output: 2134


🏁 Script executed:

# Verify if ImmutableList appears in public API of any State classes
rg 'ImmutableList' -B10 -A5 feature/ --type kotlin | grep -E 'package|class|data class|val.*:.*ImmutableList'

Repository: komodgn/meta-android

Length of output: 523


Use api scope for kotlinx-collections-immutable to ensure transitive availability for State types.

FocusingSearchUiState exposes ImmutableList<CircleModel> as a public field—a type that becomes part of the public API. With implementation scope, the dependency is not transitive; modules depending on feature modules won't have ImmutableList on their compile classpath. While most State types are consumed through the Circuit runtime and may not require direct type references, any code that imports these State classes (for testing, navigation setup, or composition) will fail to compile without the dependency.

Changing to api scope ensures the type is available to all consumers of feature modules and eliminates a potential compilation surprise.

Suggested change
-                implementation(libs.kotlinx.collections.immutable)
+                api(libs.kotlinx.collections.immutable)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
implementation(libs.kotlinx.collections.immutable)
api(libs.kotlinx.collections.immutable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@build-logic/src/main/java/AndroidFeatureConventionPlugin.kt` at line 26, The
build uses implementation(libs.kotlinx.collections.immutable) but
FocusingSearchUiState exposes ImmutableList<CircleModel> publicly, so change the
dependency declaration to api(libs.kotlinx.collections.immutable) in the
AndroidFeatureConventionPlugin (the place where kotlinx-collections-immutable is
added) so the ImmutableList type is transitive and available on consumers'
compile classpath; update the dependency configuration from implementation to
api for kotlinx-collections-immutable to fix the missing type at compile time.

implementation(libs.bundles.circuit)
api(libs.circuit.codegen.annotation)
ksp(libs.circuit.codegen.ksp)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ sealed interface MetaSearchEvent {
data class ShowDialog(
val dialogSpec: MetaSearchDialogSpec,
) : MetaSearchEvent

data class ShowToast(
val message: String,
) : MetaSearchEvent
}

data class MetaSearchDialogSpec(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.metasearch.android.core.common.utils

import android.content.Context
import androidx.annotation.StringRes
import androidx.compose.runtime.Composable
import androidx.compose.ui.res.stringResource

@Suppress("SpreadOperator")
sealed interface UiText {
data class DynamicString(val value: String) : UiText

class StringResource(
@StringRes val resId: Int,
vararg val args: Any,
) : UiText

@Composable
fun asString(): String {
return when (this) {
is DynamicString -> value
is StringResource -> stringResource(resId, *args)
}
}

fun asString(context: Context): String {
return when (this) {
is DynamicString -> value
is StringResource -> context.getString(resId, *args)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package com.metasearch.android.core.ui.component

import androidx.compose.foundation.combinedClickable
import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import coil3.compose.AsyncImage
import com.metasearch.android.core.common.extensions.clickableIfNotNull
import com.metasearch.android.core.designsystem.annotation.ComponentPreview
import com.metasearch.android.core.designsystem.theme.MetaSearchTheme

Expand All @@ -17,12 +17,16 @@ fun MetaSearchSquareImage(
model: Any?,
contentDescription: String? = null,
onClick: (() -> Unit)? = null,
onLongClick: (() -> Unit)? = null,
) {
AsyncImage(
modifier = modifier
.aspectRatio(1f)
.padding(1.dp)
.clickableIfNotNull(onClick),
.combinedClickable(
onClick = { onClick?.invoke() },
onLongClick = { onLongClick?.invoke() },
),
model = model,
contentScale = ContentScale.Crop,
contentDescription = contentDescription,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.res.stringResource
import com.metasearch.android.core.common.utils.UiText
import com.metasearch.android.core.data.api.repository.GraphRepository
import com.metasearch.android.feature.detail.R
import com.metasearch.android.feature.screens.GraphDetailScreen
Expand Down Expand Up @@ -38,10 +38,9 @@ class GraphDetailPresenter @AssistedInject constructor(

@Composable
override fun present(): GraphDetailUiState {
var sideEffect by remember { mutableStateOf<GraphDetailSideEffect?>(null) }
var webViewUrl by remember { mutableStateOf("") }
var selectedImages by remember { mutableStateOf(listOf<String>()) }
var errorMessage by remember { mutableStateOf("") }
val failedToFindImage = stringResource(R.string.graph_detail_screen_error)
val maxImages = 10

LaunchedEffect(Unit) {
Expand All @@ -50,6 +49,10 @@ class GraphDetailPresenter @AssistedInject constructor(

fun handleEvent(event: GraphDetailUiEvent) {
when (event) {
GraphDetailUiEvent.InitSideEffect -> {
sideEffect = null
}

is GraphDetailUiEvent.OnPhotoSelected -> {
val scope = MainScope()
scope.launch {
Expand All @@ -60,25 +63,27 @@ class GraphDetailPresenter @AssistedInject constructor(
selectedImages = (listOf(uriString) + selectedImages).take(maxImages)
}
} else {
errorMessage = failedToFindImage
sideEffect = GraphDetailSideEffect.ShowToast(
message = UiText.StringResource(R.string.graph_detail_screen_error),
)
}
}
}

GraphDetailUiEvent.OnBackClick -> navigator.pop()
GraphDetailUiEvent.OnBackClick -> {
navigator.pop()
}

is GraphDetailUiEvent.OnImageClick -> {
navigator.goTo(PhotoDetailScreen(event.uriString))
}

GraphDetailUiEvent.OnErrorDialogDismiss -> errorMessage = ""
}
}

return GraphDetailUiState(
webViewUrl = webViewUrl,
selectedImages = selectedImages,
errorMessage = errorMessage,
sideEffect = sideEffect,
eventSink = ::handleEvent,
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.metasearch.android.feature.detail.graph

import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.platform.LocalContext
import com.metasearch.android.core.common.utils.MetaSearchEvent

@Composable
fun GraphDetailSideEffect(
state: GraphDetailUiState,
eventSink: (GraphDetailUiEvent) -> Unit,
) {
val context = LocalContext.current

LaunchedEffect(state.sideEffect) {
when (state.sideEffect) {
is GraphDetailSideEffect.ShowToast -> {
MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
}
Comment on lines +17 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find all SideEffect-related files and locate the files mentioned in the review
find . -name "*SideEffect*" -type f | head -20

Repository: komodgn/meta-android

Length of output: 818


🏁 Script executed:

# Check the current state of GraphDetailSideEffect.kt, specifically around lines 17-19
find . -name "GraphDetailSideEffect.kt" -type f

Repository: komodgn/meta-android

Length of output: 163


🏁 Script executed:

# Also search for NLSearchSideEffect.kt
find . -name "NLSearchSideEffect.kt" -type f

Repository: komodgn/meta-android

Length of output: 158


🏁 Script executed:

cat -n ./feature/detail/src/main/java/com/metasearch/android/feature/detail/graph/GraphDetailSideEffect.kt

Repository: komodgn/meta-android

Length of output: 1087


🏁 Script executed:

cat -n ./feature/search/src/main/java/com/metasearch/android/feature/search/nls/NLSearchSideEffect.kt

Repository: komodgn/meta-android

Length of output: 1281


🏁 Script executed:

# Search for EventHandler usage pattern to understand the correct dispatch method
rg "EventHandler\.sendEvent" --type kotlin -B 2 -A 2 | head -50

Repository: komodgn/meta-android

Length of output: 4050


🏁 Script executed:

# Check if there are any other ShowToast or EventHandler references in GraphDetailSideEffect.kt
rg "ShowToast|EventHandler" ./feature/detail/src/main/java/com/metasearch/android/feature/detail/graph/GraphDetailSideEffect.kt

Repository: komodgn/meta-android

Length of output: 212


🏁 Script executed:

# Verify the line count of GraphDetailSideEffect.kt to ensure we're seeing the full file
wc -l ./feature/detail/src/main/java/com/metasearch/android/feature/detail/graph/GraphDetailSideEffect.kt

Repository: komodgn/meta-android

Length of output: 166


Add missing EventHandler import and dispatch ShowToast event

MetaSearchEvent.ShowToast is constructed but never dispatched — toast will silently not appear. The event object must be wrapped with EventHandler.sendEvent(), consistent with the correct pattern in NLSearchSideEffect.kt (lines 19-23). The EventHandler import is also missing.

Fix
+import com.metasearch.android.core.common.utils.EventHandler
 import com.metasearch.android.core.common.utils.MetaSearchEvent
 is GraphDetailSideEffect.ShowToast -> {
-    MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
+    EventHandler.sendEvent(
+        MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
+    )
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
is GraphDetailSideEffect.ShowToast -> {
MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
}
import com.metasearch.android.core.common.utils.EventHandler
import com.metasearch.android.core.common.utils.MetaSearchEvent
Suggested change
is GraphDetailSideEffect.ShowToast -> {
MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
}
is GraphDetailSideEffect.ShowToast -> {
EventHandler.sendEvent(
MetaSearchEvent.ShowToast(message = state.sideEffect.message.asString(context))
)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/detail/src/main/java/com/metasearch/android/feature/detail/graph/GraphDetailSideEffect.kt`
around lines 17 - 19, The GraphDetailSideEffect.ShowToast branch builds a
MetaSearchEvent.ShowToast but never dispatches it and lacks the EventHandler
import; update the branch in GraphDetailSideEffect (where
GraphDetailSideEffect.ShowToast is handled) to call
EventHandler.sendEvent(MetaSearchEvent.ShowToast(...)) instead of just
constructing the event, and add the missing import for EventHandler so the
sendEvent call compiles; follow the same pattern used in NLSearchSideEffect
(lines handling EventHandler.sendEvent) as a template.


else -> {}
}

if (state.sideEffect != null) {
eventSink(GraphDetailUiEvent.InitSideEffect)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import com.metasearch.android.core.designsystem.annotation.DevicePreview
import com.metasearch.android.core.designsystem.theme.MetaSearchTheme
import com.metasearch.android.core.designsystem.theme.Neutral500
import com.metasearch.android.core.ui.MetaSearchScaffold
import com.metasearch.android.core.ui.component.MetaSearchDialog
import com.metasearch.android.core.ui.component.MetaSearchHeader
import com.metasearch.android.feature.detail.R
import com.metasearch.android.feature.screens.GraphDetailScreen
Expand All @@ -42,22 +41,18 @@ fun GraphDetailUi(
modifier: Modifier = Modifier,
state: GraphDetailUiState,
) {
GraphDetailSideEffect(
state = state,
eventSink = state.eventSink,
)

MetaSearchScaffold(
modifier = modifier,
) { innerPadding ->
GraphDetailUiContent(
state = state,
innerPadding = innerPadding,
)

if (state.errorMessage.isNotBlank()) {
MetaSearchDialog(
title = stringResource(R.string.graph_detail_screen_dialog_title),
onConfirmRequest = { state.eventSink(GraphDetailUiEvent.OnErrorDialogDismiss) },
content = { Text(state.errorMessage) },
confirmButtonText = stringResource(R.string.graph_detail_screen_dialog_close_button),
)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
package com.metasearch.android.feature.detail.graph

import androidx.compose.runtime.Immutable
import com.metasearch.android.core.common.utils.UiText
import com.slack.circuit.runtime.CircuitUiState
import java.util.UUID

data class GraphDetailUiState(
val isLoading: Boolean = false,
val errorMessage: String = "",
val webViewUrl: String = "",
val selectedImages: List<String> = emptyList(),
val sideEffect: GraphDetailSideEffect? = null,
val eventSink: (GraphDetailUiEvent) -> Unit,
) : CircuitUiState

@Immutable
sealed interface GraphDetailSideEffect {
data class ShowToast(
val message: UiText,
private val key: String = UUID.randomUUID().toString(),
) : GraphDetailSideEffect
}

sealed interface GraphDetailUiEvent {
data object InitSideEffect : GraphDetailUiEvent

data class OnPhotoSelected(
val photoName: String,
) : GraphDetailUiEvent
Expand All @@ -26,6 +39,4 @@ sealed interface GraphDetailUiEvent {
data class OnImageClick(
val uriString: String,
) : GraphDetailUiEvent

data object OnErrorDialogDismiss : GraphDetailUiEvent
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ class PersonDetailPresenter @AssistedInject constructor(

fun handleEvent(event: PersonDetailUiEvent) {
when (event) {
PersonDetailUiEvent.OnHeaderBackClick -> navigator.pop()
PersonDetailUiEvent.OnHeaderBackClick -> {
navigator.pop()
}

PersonDetailUiEvent.OnMenuClick -> {
person?.let {
Expand All @@ -116,13 +118,21 @@ class PersonDetailPresenter @AssistedInject constructor(
}
}

is PersonDetailUiEvent.OnEditHomeDisplayChange -> editIsHomeDisplay = event.isHomeDisplay
is PersonDetailUiEvent.OnEditHomeDisplayChange -> {
editIsHomeDisplay = event.isHomeDisplay
}

is PersonDetailUiEvent.OnEditNameChange -> editName = event.name
is PersonDetailUiEvent.OnEditNameChange -> {
editName = event.name
}

is PersonDetailUiEvent.OnEditPhoneChange -> editPhone = event.phone
is PersonDetailUiEvent.OnEditPhoneChange -> {
editPhone = event.phone
}

PersonDetailUiEvent.OnEditCancel -> showEditDialog = false
PersonDetailUiEvent.OnEditCancel -> {
showEditDialog = false
}

PersonDetailUiEvent.OnConfirmMergeSave -> {
scope.launch {
Expand All @@ -133,7 +143,9 @@ class PersonDetailPresenter @AssistedInject constructor(
}
}

PersonDetailUiEvent.OnDismissMergeDialog -> showMergeConfirmDialog = false
PersonDetailUiEvent.OnDismissMergeDialog -> {
showMergeConfirmDialog = false
}

is PersonDetailUiEvent.OnEditThumbnailClick -> {
val currentPerson = person ?: return
Expand All @@ -147,11 +159,17 @@ class PersonDetailPresenter @AssistedInject constructor(
}
}

PersonDetailUiEvent.OnThumbnailClick -> showPhotoSelectDialog = true
PersonDetailUiEvent.OnThumbnailClick -> {
showPhotoSelectDialog = true
}

PersonDetailUiEvent.OnPhotoSelectCancel -> showPhotoSelectDialog = false
PersonDetailUiEvent.OnPhotoSelectCancel -> {
showPhotoSelectDialog = false
}

is PersonDetailUiEvent.OnGridImageClick -> navigator.goTo(PhotoDetailScreen(event.imageUri.toString()))
is PersonDetailUiEvent.OnGridImageClick -> {
navigator.goTo(PhotoDetailScreen(event.imageUri.toString()))
}
}
}

Expand Down
Loading