Skip to content
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

Fix engagement started on the chat screen #1194

Conversation

AH-MOC
Copy link
Contributor

@AH-MOC AH-MOC commented Dec 20, 2024

Jira issue:
https://glia.atlassian.net/browse/MOB-3888

What was solved?
When the chat screen opens with the secure conversation state, resubscribe to the engagement streams. This solves the upgrade to chat state issue.

Release notes:

  • Feature
  • Ignore
  • Release notes (Is it clear from the description here?)
  • Migration guide (If changes are needed for integrator already using the SDK - what needs to be communicated? Add underneath please)

Additional info:

  • Is the feature sufficiently tested? All tests fixed? Necessary unit, acceptance, snapshots added? Check that at least new public classes & methods are covered with unit tests
  • Did you add logging beneficial for troubleshooting of customer issues?
  • Did you add new logging? We would like the logging between platforms to be similar. Refer to Logging from Android SDKsThings to consider for newly added logs in Confluence for more information.

Screenshots:

@AH-MOC AH-MOC requested a review from a team as a code owner December 20, 2024 06:41
@AH-MOC AH-MOC force-pushed the MOB-3888_fix_engagement_started branch from 08c9ae4 to 8e928ad Compare December 20, 2024 06:45
@AH-MOC AH-MOC requested a review from andrews-moc December 20, 2024 07:03
Copy link
Contributor

@andrews-moc andrews-moc left a comment

Choose a reason for hiding this comment

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

Nice solution 👍

@AH-MOC AH-MOC force-pushed the MOB-3888_fix_engagement_started branch from 8e928ad to 0eb5075 Compare December 20, 2024 08:05
@AH-MOC AH-MOC requested review from gugalo and DavDo December 27, 2024 08:51
@andrews-moc andrews-moc force-pushed the feature/entry-widget-and-secure-conversations-v2 branch from ae3c8f4 to 274f4c5 Compare December 27, 2024 13:10
@DavDo
Copy link
Collaborator

DavDo commented Dec 27, 2024

@AndriiHorishniiMOC, could you please rebase this branch?

DavDo and others added 21 commits December 27, 2024 17:33
- Migrate Call and Chat Activities to Kotlin
- Refactor CallConfiguration
- Refactor EngagementConfiguration

MOB 3587
- Moved all the Intent-related functionality to the IntentHelper class.
- Abstraction will let us to easily mock this layer.

MOB 3582
- Create ActivityLauncher
- Rename FilePreview... classes to ImagePreview...

MOB 3589
- Create interfaces for EngagementLauncher
- Create basic ConfigurationManager that should hold all the init config

MOB 3588
This custom attributes can be setup by integrators in their theme.xml
This is needed so that customise glia theme settings in integrators theme.xml
- Remove all the configurations in favor of ConfigurationManager
- Get rid of configurations passed through Activity extras
- Move extra keys to internal class
- Make activities internal
- Minor refactoring
Texts have changed, updating according to the Figma designs.

MOB-3615
…idget

Setup queues each time get Entry Widget is called

MOB-3408
DavDo and others added 20 commits December 27, 2024 17:35
Side effect of making the layout flat is that now those buttons have slight animation.

MOB-3844
Last Core SDK staging build got expired/deleted so uploaded new build
- Added inputDisabled Json to UI remote theme configurations
- Switched simple color/drawables to ColorStateList/DrawableStateList

MOB-3855
Remove SecureFileAttachmentRepository and some SC use cases that duplicated the logic.
Refactor FileAttachmentRepository and the file upload use cases. FileAttachmentRepository is used for both SC and Live Engagement.

MOB-3791
Do not start a timer for marking messages as read if the leave secure conversation dialog is shown.

MOB-3833
…type or starts a new engagement during queueing

MOB-3886
@andrews-moc andrews-moc force-pushed the feature/entry-widget-and-secure-conversations-v2 branch from 2ca729a to 35d6916 Compare December 27, 2024 15:56
When the chat screen opens with the secure conversation state, resubscribe to the engagement streams. This solves the upgrade to chat state issue.

MOB-3888
@andrews-moc andrews-moc force-pushed the MOB-3888_fix_engagement_started branch from 0eb5075 to 555bca8 Compare December 27, 2024 16:44
@andrews-moc
Copy link
Contributor

andrews-moc commented Dec 27, 2024

could you please rebase this branch?

@DavDo I've rebased it ✅

Copy link
Collaborator

@DavDo DavDo left a comment

Choose a reason for hiding this comment

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

LGTM

@AH-MOC AH-MOC marked this pull request as draft December 30, 2024 13:24
@@ -258,6 +259,10 @@ internal class ChatController(
emitViewState { chatState.initChat().setSecureMessagingState() }
ensureSecureMessagingAvailable()
observeTopBannerUseCase()

// When the chat screen opens with the secure conversation state, resubscribe to the engagement streams.
// This solves the upgrade to chat state issue.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also add a Jira bug ticket number here. In case someone in the future decides to refactor any related code so that they have reference to find initial steps to make sure that issue didn't re-surface

@andrews-moc andrews-moc force-pushed the feature/entry-widget-and-secure-conversations-v2 branch from ea158b2 to 705f15e Compare January 13, 2025 15:40
@DavDo
Copy link
Collaborator

DavDo commented Jan 14, 2025

Closing because this was fixed in #1210, and our principle is to decide the chat state before opening the screen.

@DavDo DavDo closed this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants