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.

Sign up for GitHub

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

Jump to bottom

feat: Reconnect logic #9453

Open
wants to merge 51 commits into
base: develop
Choose a base branch
from
Open

feat: Reconnect logic #9453

wants to merge 51 commits into from

Conversation

iamsivin
Copy link
Member

@iamsivin iamsivin commented May 10, 2024 •
edited

Pull Request Template

Description

Fixes https://linear.app/chatwoot/issue/CW-1957/web-app-disconnects-and-doesnt-notify-about-incoming-messages-anymore

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Loom Video
https://www.loom.com/share/076018aa84e248ae806fe3bd865165a1?sid=4e25a798-8796-4b1f-8f02-3c0013862d2f

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@linear Linear
Copy link

linear bot commented May 10, 2024

CW-1957 Web app disconnects and doesn't notify about incoming messages anymore

@iamsivin iamsivin requested a review from pranavrajs May 10, 2024 13:47
@iamsivin iamsivin temporarily deployed to chatwoot-pr-9453 May 10, 2024 17:03 Inactive
@iamsivin iamsivin marked this pull request as ready for review May 14, 2024 07:53
@iamsivin iamsivin mentioned this pull request May 14, 2024
Open
7 tasks
@iamsivin iamsivin changed the title feat: Updates reconnect logic feat: Reconnect logic May 14, 2024
muhsin-k pushed a commit that referenced this pull request May 30, 2024
@iamsivin
feat: Split reconnect logic PR (store) (#9520)
e3eca47 
# Pull Request Template

## Description

This PR includes store filter parts split from this [Reconnect
PR](#9453)
@muhsin-k muhsin-k self-requested a review May 30, 2024 10:32
muhsin-k
muhsin-k requested changes May 30, 2024
View reviewed changes
Copy link
Member

@muhsin-k muhsin-k left a comment

Choose a reason for hiding this comment

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

@iamsivin Please see the inline comments.

@iamsivin iamsivin requested a review from muhsin-k May 30, 2024 12:20
@muhsin-k
Copy link
Member

@iamsivin Message are updating after the disconnect. But the loader is not closing for some reason.

chat-message-loading.mov

muhsin-k
muhsin-k reviewed May 31, 2024
View reviewed changes
app/javascript/dashboard/components/NetworkNotification.vue
// If the user goes online, close the notification

// Case: Websocket is disconnected
// If the user goes offline, show the notification
Copy link
Member

@muhsin-k muhsin-k May 31, 2024 •
edited

Choose a reason for hiding this comment

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

Here, it is the app that is going offline, not the user. It's better to keep the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muhsin-k muhsin-k muhsin-k requested changes

@pranavrajs pranavrajs Awaiting requested review from pranavrajs

@sojan-official sojan-official Awaiting requested review from sojan-official

@scmmishra scmmishra Awaiting requested review from scmmishra

Requested changes must be addressed to merge this pull request.

Assignees

@scmmishra scmmishra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@iamsivin @muhsin-k @vishnu-narayanan @scmmishra