-
Notifications
You must be signed in to change notification settings - Fork 50
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
Waiting Queue Implementation #600
Waiting Queue Implementation #600
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request adds functionality for handling in-queue messages. A new method and constant in the channel service enable retrieval of queue information. A constant and logic update in the conversation model allow proper status mapping for in-queue conversations. A dedicated data class represents the queue status, and a new use case fetches and processes this information asynchronously. UI components, including a custom in-queue view, layout modifications, a vector drawable, and string resources, support the display of waiting status. An additional image loading utility method simplifies placeholder image loading. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MobiComConversationFragment
participant UC as WaitingQueueStatusUseCase
participant CS as ChannelClientService
participant HTTP as httpRequestUtils
UI->>UC: updateWaitingStatus()
UC->>CS: getChannelInQueueStatus(teamId)
CS->>HTTP: getResponse(IN_QUEUE_MESSAGE_URL + teamId)
HTTP-->>CS: InQueueData response
CS-->>UC: Return InQueueData
UC-->>UI: APIResult (success/failure)
UI->>UI: Update KmInQueueView display
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 6
🧹 Nitpick comments (8)
kommunicate/src/main/java/io/kommunicate/models/InQueueData.kt (1)
3-7
: Add documentation and consider validation for required fields.The data class properties would benefit from:
- KDoc comments explaining the purpose of each field
- Validation or default values to prevent null pointer issues
data class InQueueData( + /** Status of the queue request (e.g., "success", "error") */ val status: String, + /** Timestamp when the queue status was generated */ val generatedAt: Long, + /** List of response IDs in the queue */ val response: List<Long> ) { + init { + require(status.isNotEmpty()) { "Status cannot be empty" } + require(generatedAt > 0) { "Generated timestamp must be positive" } + } }kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/kommunicate/views/KmInQueueView.kt (1)
17-18
: Remove unnecessary public visibility of awayMessageTv.The property has a private setter but public getter which is unnecessary. Consider making it private.
- lateinit var awayMessageTv: TextView - private set + private lateinit var awayMessageTv: TextViewkommunicate/src/main/java/io/kommunicate/usecase/WaitingQueueStatusUseCase.kt (1)
30-34
: Optimize string comparison and improve error handling.The case-insensitive string comparison can be optimized, and error details should be included in failure case.
- if (response.status.lowercase() == "success") { + if (response.status.equals("success", ignoreCase = true)) { APIResult.success(response.response) }else { - APIResult.failed(INTERNAL_ERR) + APIResult.failed("$INTERNAL_ERR: ${response.status}") }kommunicate/src/main/java/com/applozic/mobicommons/people/channel/Channel.java (1)
293-295
: Consider using a constant for the status value "7".The hardcoded string "7" in the metadata check could be replaced with a constant to improve maintainability and prevent magic numbers.
+private static final String IN_QUEUE_STATUS = "7"; + if (getMetadata().containsKey(CONVERSATION_STATUS) && - ("7".equals(getMetadata().get(CONVERSATION_STATUS)))) { + (IN_QUEUE_STATUS.equals(getMetadata().get(CONVERSATION_STATUS)))) {kommunicate/src/main/java/com/applozic/mobicomkit/channel/service/ChannelClientService.java (1)
261-264
: Enhance error handling in the getChannelInQueueStatus method.While the method correctly handles the basic flow, consider adding more robust error handling:
- Validate the teamId parameter
- Add specific error handling for HTTP and JSON parsing errors
public InQueueData getChannelInQueueStatus(Long teamId) throws Exception { + if (teamId == null || teamId <= 0) { + throw new IllegalArgumentException("Invalid teamId"); + } + try { String response = httpRequestUtils.getResponse(getBaseUrl() + IN_QUEUE_MESSAGE_URL + teamId); + if (TextUtils.isEmpty(response)) { + throw new Exception("Empty response from server"); + } return GsonUtils.getObjectFromJson(response, InQueueData.class); + } catch (Exception e) { + Utils.printLog(context, TAG, "Error fetching queue status: " + e.getMessage()); + throw e; + } }kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/conversation/fragment/MobiComConversationFragment.java (1)
3590-3599
: Consider adding a loading state while fetching queue status.The UI updates for queue status are good, but there's no indication to the user that the status is being fetched. Consider showing a loading indicator before the API call completes.
if (channel.getKmStatus() == Channel.IN_QUEUE_CONVERSATION) { + kmInQueueView.showLoadingState(); toolbarTitleText.setText(R.string.in_queue); toolbarAwayColorDot.setVisibility(GONE); toolbarSubtitleText.setVisibility(GONE); updateWaitingStatus(); }
kommunicateui/src/main/res/values/strings.xml (1)
310-311
: New In-Queue String Resources AddedThe new string resources (
in_queue_position
andin_queue
) are clearly defined to support the waiting queue feature. Please verify that the%1$s
placeholder inin_queue_position
matches the expected data format within your code. Overall, these additions look good and align with the intended UI feedback for queue status.kommunicateui/src/main/res/layout/mobicom_message_list.xml (1)
278-285
: Integration of KmInQueueView into LayoutThe new
<com.applozic.mobicomkit.uiwidgets.kommunicate.views.KmInQueueView>
has been properly inserted within the view hierarchy to display waiting queue information. One detail to note: the attributeandroid:layout_alignParentBottom="true"
is a RelativeLayout-specific property, and since this view is placed within a LinearLayout, it will have no effect. Removing it could simplify the layout and avoid any potential confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
kommunicate/src/main/java/com/applozic/mobicomkit/channel/service/ChannelClientService.java
(3 hunks)kommunicate/src/main/java/com/applozic/mobicommons/people/channel/Channel.java
(2 hunks)kommunicate/src/main/java/io/kommunicate/models/InQueueData.kt
(1 hunks)kommunicate/src/main/java/io/kommunicate/usecase/WaitingQueueStatusUseCase.kt
(1 hunks)kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/conversation/adapter/QuickConversationAdapter.java
(3 hunks)kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/conversation/fragment/MobiComConversationFragment.java
(11 hunks)kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/kommunicate/views/KmInQueueView.kt
(1 hunks)kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/utils/KmViewHelper.kt
(1 hunks)kommunicateui/src/main/res/drawable/km_message_in_queue.xml
(1 hunks)kommunicateui/src/main/res/layout/mobicom_message_list.xml
(1 hunks)kommunicateui/src/main/res/values/strings.xml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- kommunicateui/src/main/res/drawable/km_message_in_queue.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Project
🔇 Additional comments (9)
kommunicate/src/main/java/com/applozic/mobicommons/people/channel/Channel.java (1)
51-51
: LGTM!The constant
IN_QUEUE_CONVERSATION
is appropriately defined with a unique value of4
to represent the in-queue status.kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/conversation/adapter/QuickConversationAdapter.java (3)
202-207
: LGTM!The UI changes for in-queue conversations are well-implemented:
- Setting appropriate text and image resources
- Ensuring proper visibility of UI elements
249-251
: LGTM!The message text view is correctly hidden for in-queue conversations.
304-306
: LGTM!The unread count is appropriately hidden for in-queue conversations.
kommunicate/src/main/java/com/applozic/mobicomkit/channel/service/ChannelClientService.java (1)
39-39
: LGTM!The URL constant is appropriately defined for fetching in-queue messages.
kommunicateui/src/main/java/com/applozic/mobicomkit/uiwidgets/conversation/fragment/MobiComConversationFragment.java (4)
161-162
: LGTM! Field declarations for queue view look good.The import and field declarations for
KmInQueueView
are properly added and follow the existing code structure.Also applies to: 352-352
563-568
: LGTM! Theme setup for queue view follows existing patterns.The theme setup for
kmInQueueView
follows the same pattern as other views, maintaining consistency in the codebase.
1122-1127
: LGTM! Queue status condition check is well implemented.The condition check for showing away message based on queue status is clear and follows good practices:
- Checks for null channel
- Uses proper enum comparison
- Groups related conditions logically
3719-3725
: LGTM! Proper visibility handling for queue status UI elements.The code correctly handles the visibility of status indicators when in queue state:
- Hides all status dots
- Hides subtitle text
- Early returns to prevent further processing
Screen_recording_20250206_160026.webm
Summary by CodeRabbit