[Testing:Forum] Add Live Chat End to End Testing#11818
Merged
Conversation
…rty, db: fixed migration order
bmcutler
pushed a commit
that referenced
this pull request
Jul 10, 2025
### Why is this Change Important & Necessary? We want to see chatroom information in database so that professors can check chatrooms even after they're deleted. Before deleting a chatroom would delete it from db. ### What is the New Behavior? Now a chatroom is kept in db, but a parameter labeled is_deleted is switched from false to true, which prevents the chatroom from showing. ### What steps should a reviewer take to reproduce or test the bug or new feature? On main, create and delete a chat. Then, go to sql toolbox and query SELECT * FROM chatrooms. That chatroom should not exist in db anymore. Switch to this branch, and do the same. The chatroom should still show, but with a new column labelled "is_deleted" set to true instead of false. ### Automated Testing & Documentation Deleting chats will be tested once #11818 is merged --------- Co-authored-by: Alex Lavallee <73203142+lavalleeale@users.noreply.github.com>
IDzyre
requested changes
Jul 17, 2025
Member
IDzyre
left a comment
There was a problem hiding this comment.
Looks decent to me, apart from a couple of redundant .should() statements.
If you really want to assert that it exists, you can split the statements, but that is redundant as well.
Comment on lines
+139
to
+143
| cy.get('[data-testid="chatroom-name-entry"]').should('exist').type(title, { force: true }); | ||
| cy.get('[data-testid="chatroom-description-entry"]').should('exist').type(description, { force: true }); | ||
| if (!isAnon) { | ||
| cy.get('[data-testid="enable-disable-anon"]').should('exist').click(); | ||
| } |
Member
There was a problem hiding this comment.
.should('exist')... is redundant, because if it doesn't exist, then it will fail anyway, and if it does exist, it will pass without the .should('exist').
jeffrey-cordero
approved these changes
Jul 18, 2025
Contributor
jeffrey-cordero
left a comment
There was a problem hiding this comment.
Live Chat Cypress tests look good.
williamschen23
pushed a commit
that referenced
this pull request
Jul 18, 2025
### Why is this Change Important & Necessary? We want to see chatroom information in database so that professors can check chatrooms even after they're deleted. Before deleting a chatroom would delete it from db. ### What is the New Behavior? Now a chatroom is kept in db, but a parameter labeled is_deleted is switched from false to true, which prevents the chatroom from showing. ### What steps should a reviewer take to reproduce or test the bug or new feature? On main, create and delete a chat. Then, go to sql toolbox and query SELECT * FROM chatrooms. That chatroom should not exist in db anymore. Switch to this branch, and do the same. The chatroom should still show, but with a new column labelled "is_deleted" set to true instead of false. ### Automated Testing & Documentation Deleting chats will be tested once #11818 is merged --------- Co-authored-by: Alex Lavallee <73203142+lavalleeale@users.noreply.github.com>
IDzyre
approved these changes
Jul 19, 2025
Member
IDzyre
left a comment
There was a problem hiding this comment.
Tests look good to me, didn't run them locally. There are a few things you could do differently, but what is here works, and it's not worth getting to the little things.
bmcutler
pushed a commit
that referenced
this pull request
Aug 15, 2025
### Why is this Change Important & Necessary? Before, the way we handled chatroom toggles worked well for the standard usage of chatrooms, with an instructor broadcasting chatroom enable and a student receiving chatroom enabling, which then triggers a row insert. This had the negative side effect of being difficult to update the chatroom row style because we had to make changes in two places. Additionally, since the templates were separate the output was inconsistent depending on what role you had, causing buggy looking chatroom rows. ### What is the New Behavior? Now, a public template named ChatroomRow is added, which is rendered using twig.js. When a chatroom is updated it will delete the row and recreate it using the twig template. ### What steps should a reviewer take to reproduce or test the bug or new feature? Open two chatroom list pages logged in as instructor, create and start a chatroom session. It should look buggy/not add the chatroom at all. On this branch it should function as expected between two instructor windows. ### Automated Testing & Documentation This is now tested through #11818, and I added websocket enable and disable tests, meaning that the websocket features of live chat would be fully tested up to now. ### Other information Potential TODO use vue instead of Twig.js --------- Co-authored-by: Justin Manion <jmanion32@gmail.com>
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
5 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?
Currently Live Chat has no testing. For future development and future developers, end to end testing is necessary.
What is the New Behavior?
Tests are now added at live_chat.spec.js
What steps should a reviewer take to reproduce or test the bug or new feature?
Tests can be run locally through Cypress at live_chat.spec.js
Automated Testing & Documentation
The current e2e documentation is sufficient.
Other information
In the future I would like to directly test the websocket elements of this feature.