[Bugfix:Forum] Fix Clear Messages Socket#12072
Merged
Conversation
Passed locally.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12072 +/- ##
=========================================
Coverage 21.68% 21.68%
Complexity 9591 9591
=========================================
Files 268 268
Lines 36618 36614 -4
Branches 475 475
=========================================
Hits 7940 7940
+ Misses 28207 28203 -4
Partials 471 471
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Member
|
I looked into this test when fixing the sidebar tests, and I came to the same conclusion that it is the websocket for clearing while on the page that is problematic. If you refresh the page the messages are gone, but that doesn't help when you want to clear it with websockets and clear it live. |
Contributor
|
@JManion32 I believe I just pushed a change to this branch that should fix the test |
Contributor
Author
|
It works! Thank you @lavalleeale, please approve on my behalf. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
The live chat clearing test module of
live_chat.spec.jsis failing on CI. This failure was introduced in commit b66259f of #12008 when I merged main into it. There were other live lecture chat features (namely #11863) introduced in this merge, however, I could not locate a failing test for the clear socket on any of them. NOTE: The clear feature worked as expected up until that commit.What is the New Behavior?
N / A. I assume the change has to come from
ChatroomController.php. It is not the Cypress test that is broken, but rather the websocket for live chat clearing.How to Test
Delete all of the tests in
live_chat_spec.jsexcept for the bottom so you don't have to wait 5 minutes for it to pass.Other information
This is not a breaking change. This PR is currently marked as a draft since I have been unable to come up with a fix. The current diff was from when I thought the issue was timing before discovering it was unrelated to Cypress.
Error on 10/03/2025 11:30:00
Looks promising, will continue to investigate.
Found another PR that touches live chat sockets:
#11634 (Add Verification of WebSocket Pages) - a lot of types were changed. Seems like a small bug was introduced that broke websockets for in #12008 (Clear Button for Live Chat) - still investigating, but narrowing it down.