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

(Refactor) Move welcome message to listener and add tests #4296

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

IonBazan
Copy link
Contributor

This PR moves the logic responsible for sending welcome message to newly registered user into a listener. This makes it much easier to test the registration flow as it no longer depends on Bot and Chatroom classes and allows to test that particular logic separately instead.

Using event listeners instead of controllers/actions makes it easier to decouple the logic and allows to conditionally run it in the future if needed. It is also easy to mock it using Event::fake() in tests.

@HDVinnie
Copy link
Collaborator

HDVinnie commented Oct 31, 2024

@IonBazan great pull request. Fully agree with the changes here and really appreciate the test coverage which has been lacking unfortunately. Hopefully soon we can get a new core dev on board to manage tests and get our coverage 75% or greater. Thanks again.

@HDVinnie HDVinnie merged commit 2618ebf into HDInnovations:8.x.x Oct 31, 2024
5 checks passed
@IonBazan IonBazan deleted the feature/register-tests branch October 31, 2024 18:18
@IonBazan
Copy link
Contributor Author

Thanks for quickly merging the change. Glad to make a helpful contribution. I'll try to keep providing more similar improvements and hope to increase the coverage as the part of refactoring or fixing bugs too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants