CMM-872 support he conversations and tickets logic #22309
Conversation
…/github.com/wordpress-mobile/WordPress-Android into feat/CMM-843-Create-the-Ask-the-HE-entry-UI
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
WordPress/src/main/java/org/wordpress/android/support/he/repository/HESupportRepository.kt
Show resolved
Hide resolved
...Press/src/main/java/org/wordpress/android/support/common/ui/ConversationsSupportViewModel.kt
Show resolved
Hide resolved
| is CreateConversationResult.Success -> { | ||
| val newConversation = result.conversation | ||
| // update conversations locally | ||
| _conversations.value = listOf(newConversation) + _conversations.value |
There was a problem hiding this comment.
Local State Update Before Remote Operation
The newly created conversation is added to the local list immediately after creation, which is good for UX. However, consider what happens if the user:
- Creates a ticket
- Navigates back before the network completes
- The network call fails or times out
The ticket might appear in the list but not actually exist on the server. Consider adding a "pending" or "syncing" state to distinguish locally-created items from server-confirmed items.
There was a problem hiding this comment.
This is a good catch!
WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt
Outdated
Show resolved
Hide resolved
Project dependencies changeslist! Upgraded Dependencies
rs.wordpress.api:android:trunk-a0864c91b8dc3726b0ad43e22662c4415aca59ce, (changed from trunk-1a64cb921601fd34bfe6030919960676d45a19c0)
rs.wordpress.api:kotlin:trunk-a0864c91b8dc3726b0ad43e22662c4415aca59ce, (changed from trunk-1a64cb921601fd34bfe6030919960676d45a19c0)tree +--- project :libs:fluxc
-| \--- rs.wordpress.api:android:trunk-1a64cb921601fd34bfe6030919960676d45a19c0
-| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.1 (*)
-| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.1 (*)
-| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.1 (*)
-| | \--- 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.1 (*)
-| | +--- 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-a0864c91b8dc3726b0ad43e22662c4415aca59ce
+| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.1 (*)
+| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.1 (*)
+| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.1 (*)
+| | \--- 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-a0864c91b8dc3726b0ad43e22662c4415aca59ce
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.2.1 (*)
+| | +--- 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 (*)
+\--- rs.wordpress.api:android:trunk-a0864c91b8dc3726b0ad43e22662c4415aca59ce (*) |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/base_manifest.txt 2025-10-27 07:32:36.365666867 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/head_manifest.txt 2025-10-27 07:32:46.175765049 +0000
@@ -629,7 +629,8 @@
<activity
android:name="org.wordpress.android.support.aibot.ui.AIBotSupportActivity"
android:label="@string/ai_bot_conversations_title"
- android:theme="@style/WordPress.NoActionBar" />
+ android:theme="@style/WordPress.NoActionBar"
+ android:windowSoftInputMode="adjustResize" />
<activity
android:name="org.wordpress.android.support.main.ui.SupportActivity"
android:label="@string/support_screen_title"
@@ -641,7 +642,8 @@
<activity
android:name="org.wordpress.android.support.he.ui.HESupportActivity"
android:label="@string/support_screen_title"
- android:theme="@style/WordPress.NoActionBar" /> <!-- Reader Activities -->
+ android:theme="@style/WordPress.NoActionBar"
+ android:windowSoftInputMode="adjustResize" /> <!-- Reader Activities -->
<activity
android:name="org.wordpress.android.ui.reader.ReaderPostListActivity"
android:label="@string/reader"Go to https://buildkite.com/automattic/wordpress-android/builds/23741/canvas?sid=019a2491-5e5c-480e-977f-440da2c85f29, click on the |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/base_manifest.txt 2025-10-27 07:32:35.759410398 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/head_manifest.txt 2025-10-27 07:32:43.339407539 +0000
@@ -495,7 +495,8 @@
<activity
android:name="org.wordpress.android.support.aibot.ui.AIBotSupportActivity"
android:label="@string/ai_bot_conversations_title"
- android:theme="@style/WordPress.NoActionBar" />
+ android:theme="@style/WordPress.NoActionBar"
+ android:windowSoftInputMode="adjustResize" />
<activity
android:name="org.wordpress.android.support.main.ui.SupportActivity"
android:label="@string/support_screen_title"
@@ -507,7 +508,8 @@
<activity
android:name="org.wordpress.android.support.he.ui.HESupportActivity"
android:label="@string/support_screen_title"
- android:theme="@style/WordPress.NoActionBar" /> <!-- Deep Linking Activity -->
+ android:theme="@style/WordPress.NoActionBar"
+ android:windowSoftInputMode="adjustResize" /> <!-- Deep Linking Activity -->
<activity
android:name="org.wordpress.android.ui.deeplinks.DeepLinkingIntentReceiverActivity"
android:excludeFromRecents="true"Go to https://buildkite.com/automattic/wordpress-android/builds/23741/canvas?sid=019a2491-5e5a-4ec8-b29e-434ef948810e, click on the |
|
| App Name | Jetpack | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22309-5c157d6 | |
| Commit | 5c157d6 | |
| Direct Download | jetpack-prototype-build-pr22309-5c157d6.apk |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22309-5c157d6 | |
| Commit | 5c157d6 | |
| Direct Download | wordpress-prototype-build-pr22309-5c157d6.apk |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #22309 +/- ##
==========================================
- Coverage 39.43% 39.37% -0.06%
==========================================
Files 2192 2195 +3
Lines 104938 105246 +308
Branches 14926 14955 +29
==========================================
+ Hits 41380 41442 +62
- Misses 60072 60315 +243
- Partials 3486 3489 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dcalhoun
left a comment
dcalhoun
left a comment
There was a problem hiding this comment.
The size of the PR definitely makes it challenging to review this thoroughly without a considerable amount of focus and time. I did my best to review and assess the changes within a normal review time. If you feel we need to slow down and review this thoroughly, please let me know.
I do recommend seeking review from one other Android developer given the size and nature of these changes. Someone with more Android experience may have suggestions on the architecture.
That said, the changes tested well for me. So, I'm approving the changes. Here are few opportunities for improvement I noted:
- When offline, the chat lists state "no conversations." Ideally, we communicate the offline status and that we cannot fetch conversations.
- The Site Address input should likely be lowercase and use a URL keyboard format (optimized for entering common URL characters like / and :, if Android provides that.
- The pencil icon used in the button for creating a new ticket is confusing to me. I interpret that as "edit." Is there a more appropriate Material icon—plus, document, etc?
- The form inputs do no have unique a11y labels. Some are completely empty; others only read aloud the placeholder text, which can be confusing.
- Application Logs toggle as no a11y label; it simply reads aloud "off switch." This can be confusing when navigating by "controls" with VoiceOver.
- Rather than disabling the Send button when fields are missing, a better UX may be allowing form submission and communicating missing field data. Currently, a user has to guess why they cannot submit the form.
- None of the larger text in the UI for "section" are marked as "heading" elements, so navigating the views by "heading" with VoiceOver is not possible.
- We might consider marking each conversation item in the list as a "link" to allow navigating "links" with VoiceOver.
- We might consider grouping elements within each message "bubble" as a single a11y unit so that navigating chats are easier and quicker.
| private val _navigationEvents = MutableSharedFlow<NavigationEvent>() | ||
| val navigationEvents: SharedFlow<NavigationEvent> = _navigationEvents.asSharedFlow() | ||
|
|
||
| @Suppress("VariableNaming") |
There was a problem hiding this comment.
Given we are suppressing this so frequently, it seems we should consider updating or removing this lint rule later.
There was a problem hiding this comment.
This is because the variables are protected instead of private (this is an abstract class), so probably the best here is to wrap them with getter/setter. I'll note it down with all your other suggestions
jkmassel
left a comment
jkmassel
left a comment
There was a problem hiding this comment.
I can confirm this addresses CMM-878.
The conversations loaded as well.
One issue – when I first tried my device wasn't on wifi and it just said 'no conversations' – there was no indication that my device was offline.
Co-authored-by: David Calhoun <github@davidcalhoun.me>
Thank you for your thorough review! I know this one was certainly tricky. |
Thank you, I've included that in an "improvements ticket" |
|
adalpari
deleted the
feat/CMM-872-support-HE-conversations-and-tickets-logic
branch
Description
This PR links the whole "Ask the Happiness Engineers" with the rust layer and improves the general ticket handling.
Now you can: browse current tickets, interact with them, and add new ones. Notice that we still depend on Zendesk in the backend, so some actions can take a while, like a ticker being included in the tickets list (we are doing it locally to mitigate it and improve the UX though)
Sorry about the size of the PR, but it includes some refactor and renaming after putting all the pieces together. So, that's why several files are impacted. Some of the extra work done:
Testing instructions
MODERN_SUPPORTSmoke test creating and answering tickets. Don't forget to mark them as solved in Zendesk after the tests are done.
Also, please smoke test AI bot assistant as well as the code has been refactored a bit.
NOTE: Attachments and logs are not supported yet.
NOTE 2: Please don't pay attention to UI styles, since they are being corrected to look more Android-ish and become homogeneous here.
Screen_recording_20251023_104002.mp4