Skip to content

PR: Refactor (src/messaging/notifications.js:105): Complex binary expression #188#189

Open
asrnCMU wants to merge 2 commits intoCMU-313:mainfrom
asrnCMU:msg_notifs_restructure
Open

PR: Refactor (src/messaging/notifications.js:105): Complex binary expression #188#189
asrnCMU wants to merge 2 commits intoCMU-313:mainfrom
asrnCMU:msg_notifs_restructure

Conversation

@asrnCMU
Copy link

@asrnCMU asrnCMU commented Feb 3, 2026

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue:
#188

Full path to the refactored file:
src/messaging/notifications.js

What do you think this file does?
Contains the logic for triggering notifications. It checks various conditions, such as user settings or whether user is a member, in various settings (groups, chats etc.) and triggers a notification accordingly.

What is the scope of your refactoring within that file?
Only changed the binary expression contained in the async function sendNotification. This expression handled the logic of whether a notification should be triggered based on chat room and user settings.

Which Qlty‑reported issue did you address?
Complex binary expression (line 105) in src/messaging/notifications.js

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The original complex binary expression combined several checks (some containing multiple steps) into a large, highly nested expression. With the reasoning or logic behind each check not being commented and the complexity of the expression, it is hard to read and understand the code, especially for future maintenance such as extending the conditions or debugging.

What changes did you make to resolve the issue?
I split the complex binary expression into smaller, more digestable components. Each check has an appropriate const name, and ones with several steps have consts as intermediate combination of terms.

How do your changes improve maintainability? Did you consider alternatives?
The multistage breakdown improves ease of debugging as the purpose of each check is evident and it is much easier to split and individually test each check (or identify which one is failing), as opposed to the less granular combined expression. I tried refactoring by returning after certain checks, to further break the code into sections. However this caused more issues (too many returns in a function), and so I stopped going down this route.

3. Validation

How did you trigger the refactored code path from the UI?
I created new users, then created a chat between them. Sending a message from one user to another present in the chat satisfies the criteria (binary expression) to trigger a notification, and therefore the code path was clearly shown to be triggered in the console log.

Attach a screenshot of the logs and UI demonstrating the trigger.
(If you refactored a public/src/ file (front-end related file), watch logging via DevTools (Ctrl+Shift+I to open and then navigate to the 'Console' tab). If you refactored a src/ file, watch logging via ./nodebb log. Include the relevant UI view. Temporary logs should be removed before final commit.)
Screenshot 2026-02-03 021639

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.js> showing fewer reported issues after the changes.
Screenshot 2026-02-03 024707

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.

1 participant

Comments