Skip to content

Conversation

@arakabCL
Copy link

@arakabCL arakabCL commented Sep 5, 2025

P1B: Starter Task: Refactoring PR

1. Issue

#242

/workspaces/NodeBB/src/middleware/headers.js

What do you think this file does?

It sets security/cors headers, adds a devonly header, and auto detects guest language.

What is the scope of your refactoring within that file?
Header and locale logic were split into small helpers, inline handlers removed, and exports simplified.

Which Qlty‑reported issue did you address?
The main function’s complexity (34) was reduced and now exports are simple with no warnings.

2. Refactoring

How did the specific issue you chose impact the codebase’s adaptability?

One large function made headers logic hard to read, change, or test.

What changes did you make to resolve the issue?

Split logic into small helpers buildBaseHeaders, applyCspHeaders, CORS helpers, applyAdditionalHeaders, writeHeaders. Created addHeadersHandler and autoLocaleHandler. Split locale flow into handleLangQuery and maybeDetectAnonLang. Kept listCodes as a shared helper.

How do your changes improve adaptability? Did you consider alternatives?

Easier to change one piece (e.g., CORS) without touching others.

3. Validation

How did you trigger the refactored code path from the UI?

Accessed NodeBB Through Browser and code path triggered automatically.

Attach a screenshot of the logs and UI demonstrating the trigger.
Screenshot 2025-09-04 at 10 49 27 PM

(Run ./nodebb log; include the relevant UI view. Temporary logs should be removed before final commit.)

../../../bootstrap/scss/forms/_form-control.scss 40:7  @import
bootstrap/scss/_forms.scss 3:9                         @import
- 19:9                                                 root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
../../../bootstrap/scss/mixins/_box-shadow.scss 10:9 box-shadow()
../../../bootstrap/scss/forms/_form-select.scss 32:7 @import
bootstrap/scss/_forms.scss 4:9 @import
- 19:9 root stylesheet

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.js> showing fewer reported issues after the changes.
Screenshot 2025-09-04 at 11 05 31 PM

@coveralls
Copy link

Pull Request Test Coverage Report for Build 17482500309

Details

  • 60 of 74 (81.08%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 78.536%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/middleware/headers.js 60 74 81.08%
Totals Coverage Status
Change from base Build 17435863312: 0.007%
Covered Lines: 24708
Relevant Lines: 29623

💛 - 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