-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CMM-839 odie link UI with wordpress rs #22285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Project dependencies changeslist! Upgraded Dependencies
net.java.dev.jna:jna:5.18.1, (changed from 5.18.0)
rs.wordpress.api:android:trunk-1a64cb921601fd34bfe6030919960676d45a19c0, (changed from trunk-d0c9eebab77e8701810077ac1fba7d39ef8d121f)
rs.wordpress.api:kotlin:trunk-1a64cb921601fd34bfe6030919960676d45a19c0, (changed from trunk-d0c9eebab77e8701810077ac1fba7d39ef8d121f) tree +--- project :libs:fluxc
-| \--- rs.wordpress.api:android:trunk-d0c9eebab77e8701810077ac1fba7d39ef8d121f
-| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
-| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
-| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.0 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
-| +--- net.java.dev.jna:jna:5.18.0
-| +--- rs.wordpress.api:kotlin:trunk-d0c9eebab77e8701810077ac1fba7d39ef8d121f
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
-| | +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
-| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.20 (*)
-| \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.20 (*)
+| \--- rs.wordpress.api:android:trunk-1a64cb921601fd34bfe6030919960676d45a19c0
+| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
+| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
+| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.0 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
+| +--- net.java.dev.jna:jna:5.18.1
+| +--- rs.wordpress.api:kotlin:trunk-1a64cb921601fd34bfe6030919960676d45a19c0
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.0 (*)
+| | +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
+| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.20 (*)
+| \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.20 (*)
-+--- rs.wordpress.api:android:trunk-d0c9eebab77e8701810077ac1fba7d39ef8d121f (*)
++--- rs.wordpress.api:android:trunk-1a64cb921601fd34bfe6030919960676d45a19c0 (*)
\--- com.automattic:encryptedlogging:1.1.1
- \--- net.java.dev.jna:jna:5.17.0 -> 5.18.0
+ \--- net.java.dev.jna:jna:5.17.0 -> 5.18.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Links the new AI Bot Support UI to the WordPress.rs backend and wires it into the Help flow. Key updates include integrating a repository + ViewModel for conversations/messages, replacing time strings with Android plurals, and adding UI states (empty list, pull-to-refresh, typing indicator, and progress).
- Introduces AIBotSupportRepository with WordPress.rs client and mapping functions.
- Updates UI: conversations list with empty state and pull-to-refresh; conversation detail with typing indicator and send gating.
- Replaces relative time formatting with plurals and moves non-Compose time util to use Resources.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
File | Description |
---|---|
gradle/libs.versions.toml | Bumps wordpress-rs to a new trunk hash to enable API usage. |
WordPress/src/main/res/values/strings.xml | Adds AI bot empty state and generic error strings; removes singular time strings in favor of plurals. |
WordPress/src/main/res/values/plurals.xml | Adds minutes/hours/days/weeks plurals for relative time. |
WordPress/src/main/java/org/wordpress/android/ui/accounts/HelpActivity.kt | Temporarily routes Contact Us to AI Bot screen; adds startActivity call with token/user. |
WordPress/src/main/java/org/wordpress/android/support/aibot/util/ConversationUtils.kt | Switches relative time formatting to use Resources and plurals. |
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/ConversationsListScreen.kt | Adds empty state, pull-to-refresh, resources wiring, and previews. |
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/ConversationDetailScreen.kt | Adds typing indicator, loading overlay, Resources usage, and send gating. |
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/AIBotSupportViewModel.kt | Adds repository integration, UI state flows, and message send/refresh/init flows. |
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/AIBotSupportActivity.kt | Wires ViewModel to composables, collects UI states, navigation, and error toasts. |
WordPress/src/main/java/org/wordpress/android/support/aibot/repository/AIBotSupportRepository.kt | New repository with rs client, request helpers, and mapping to UI models. |
WordPress/src/main/java/org/wordpress/android/support/aibot/model/BotMessage.kt | Removes unused field from BotMessage. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
import androidx.compose.runtime.getValue | ||
import androidx.compose.ui.Alignment | ||
import androidx.compose.ui.Modifier | ||
import androidx.compose.ui.platform.LocalResources |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalResources is not a Compose platform ambient. Use LocalContext.current.resources instead. Replace this import with import androidx.compose.ui.platform.LocalContext and derive Resources via LocalContext.current.resources.
import androidx.compose.ui.platform.LocalResources | |
import androidx.compose.ui.platform.LocalContext |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a warning in AS saying just the opposite...
onConversationClick: (BotConversation) -> Unit | ||
) { | ||
val conversations by conversations.collectAsState() | ||
val resources = LocalResources.current |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalResources.current is invalid. Get Resources from LocalContext: val resources = LocalContext.current.resources.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import androidx.compose.ui.tooling.preview.Preview | ||
import androidx.compose.ui.unit.dp | ||
import android.content.res.Configuration.UI_MODE_NIGHT_YES | ||
import androidx.compose.ui.platform.LocalResources |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalResources isn't a valid Compose ambient. Replace with LocalContext and retrieve resources via LocalContext.current.resources.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
} | ||
|
||
val resources = LocalResources.current |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use LocalContext.current.resources instead of LocalResources.current.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
listState.animateScrollToItem(conversation.messages.size + 1) // +1 for spacer | ||
// +2 for welcome header and spacer, +1 if typing indicator is showing | ||
val itemCount = conversation.messages.size + 2 + if (isBotTyping) 1 else 0 | ||
listState.animateScrollToItem(itemCount) |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off-by-one when scrolling to the bottom: animateScrollToItem expects a valid index (0..lastIndex). Use itemCount - 1 (clamped) to avoid IndexOutOfBounds when there are exactly itemCount items.
listState.animateScrollToItem(itemCount) | |
listState.animateScrollToItem((itemCount - 1).coerceAtLeast(0)) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because of extra UI elements not includued in the list (see the comment)
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/ConversationsListScreen.kt
Outdated
Show resolved
Hide resolved
private var userId: Long = 0 | ||
|
||
private val wpComApiClient: WpComApiClient by lazy { | ||
check(accessToken != null || userId != 0L) { "Repository not initialized" } |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both accessToken and userId are required for the client; the OR condition allows initialization with only one set, leading to a possible NPE on accessToken!!. Change to logical AND: check(accessToken != null && userId != 0L) { "Repository not initialized" }.
check(accessToken != null || userId != 0L) { "Repository not initialized" } | |
check(accessToken != null && userId != 0L) { "Repository not initialized" } |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emmm, that's not the way it works... It wull fail if "condition 1 OR condition 2" fail. So, it looks correct to me...
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/AIBotSupportViewModel.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/support/aibot/ui/AIBotSupportViewModel.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/accounts/HelpActivity.kt
Show resolved
Hide resolved
} else { | ||
createNewZendeskTicket() | ||
} | ||
// TODO: Again, this TODO is preventing the PR to pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, don't pay attention to this code. It will be removed before merging the PR
|
Description
This PR links the new AI Bot Support UI to the rust layer.
Now the user can completely interact and create bot conversations in the new chat.
Some improvements to the UI/UX have been done as well like adding UI states, empty list, pull-to-refresh, typing indicator, progress indicator... Also there's a better handling of the plurals when showing the ellapsed time sinde the message was created.
Note: Pagination is not supported yet.
Testing instructions
Note: the entry point to this screen is hardcoded, and a TODO has been added to avoid merging this PR until those hardcoded lines are removed. So, don't pay attention to that part of the code.
Screen.Recording.2025-10-15.at.12.40.49.mov