Skip to content

Conversation

@do28minh
Copy link

…rs from 4 to 3

P1B: Starter Task: Refactoring PR

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:
#112

Full path to the refactored file:
src/posts/votes.js

What do you think this file does?
(Your answer does not have to be 100% correct; give a reasonable, evidence‑based guess.)
Handles the voting mechanism for posts on NodeBB, including features like up/downvoting, and data mechanisms that update the frontend to user votes.

What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
Very small, just the unvote function parameters and the call to unvote in toggleVote.

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 many parameters (count = 4)

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
Callers need to research what voteStatus is and every call to voteStatus would need to me modified if the method to fetch it changes.

What changes did you make to resolve the issue?
Since the removed parameter entirely relies on the others, it makes sense to compute it internally. I moved the call to calculate voteStatus in unvote, and the calls to unvote to take the expected three parameters.

How do your changes improve maintainability? Did you consider alternatives?
It allows callers to abstract details about voteStatus and the function is self-contained. It could decrease maintainability by forcing bug fixes in additional places or increasing runtime.

3. Validation

How did you trigger the refactored code path from the UI?
I opened the NodeBB server, then clicked 'General Discussion', then selected a post, and upvoted and downvoted. For some reason, some page paths triggered the refactored code and some were broken, although that could be a NodeBB setup error. I did have to create an account to be able to vote.

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-01-23 at 22 57 34
Screenshot 2026-01-23 at 23 17 06

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.js> showing fewer reported issues after the changes.
Screenshot 2026-01-23 at 23 35 01

@coveralls
Copy link

Pull Request Test Coverage Report for Build 21309282231

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 78.841%

Totals Coverage Status
Change from base Build 21306090285: 0.0%
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