Skip to content

Conversation

@eseudieu
Copy link

@eseudieu eseudieu commented Jan 24, 2026

P1B: Refactor (src/user/bans.js:11): Function with high complexity (count = 22)

Use this pull request template to briefly answer the questions below in one to two sentences each.
Feel free to delete this text at the top after filling out the template.

You are not permitted to use generative AI services (e.g., ChatGPT) to compose the answers.
Any such use will be treated as a violation of academic integrity.

1. Issue

Link to the associated GitHub issue:
#158

Full path to the refactored file:
src/user/bans.js

What do you think this file does?
This file is responsible for handling the actions of banning users and unbanning them either manually or via time expiration, as well as querying the database for various ban-related information such as users banned, reasons users were banned, etc.

What is the scope of your refactoring within that file?
Code was refactored within each method outlined in module.exports, and functions were added at the highest level of the function

Which Qlty‑reported issue did you address?
(Name the rule/metric and include the BEFORE value; e.g., “Cognitive Complexity 18 in render()”.)
Function with high complexity (count = 22) reduced to (count = 12)

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The file proved difficult to read and interpret at times as a result of code breaks and countless conditionals that were not initially intuitive

What changes did you make to resolve the issue?
I made several small changes to individual functions, rearranging code for readability and simplicity and substituting more complex formulas for simpler ones where possible. I also noticed that a recurring source of conditionals were resulting from checking if the input was an array, so I created a high-level function at the top of the file which adapts an input to be an array if requested and another function which takes the given index of an array if an array is not the desired type.

How do your changes improve maintainability? Did you consider alternatives?
Code in places is now easier to follow and less convoluted. Refactoring the array check code into a named function certainly makes it easier to interpret the flow of code within functions.

3. Validation

How did you trigger the refactored code path from the UI?
I added a print statement to a function that I rearranged the most (User.bans.canLoginIfBanned). It seems this function simply is ran whenever the page is refreshed or the user navigates to a new page. This makes sense as the server has to authenticate the user each time they navigate.

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.)
image

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

@eseudieu eseudieu changed the title Bans issue Reduced Code Complexity of src/user/bans.js Jan 24, 2026
@coveralls
Copy link

Pull Request Test Coverage Report for Build 21309125549

Details

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.008%) to 78.833%

Totals Coverage Status
Change from base Build 21306090285: -0.008%
Covered Lines: 25358
Relevant Lines: 30328

💛 - Coveralls

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