Skip to content

Conversation

@syu213
Copy link

@syu213 syu213 commented Sep 5, 2025

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

Please provide a link to the associated GitHub issue:

Link to the associated GitHub issue: #246

Full path to the refactored file: https://github.com/CMU-313/NodeBB/blob/main/src/messaging/edit.js

What do you think this file does?
(Your answer does not have to be 100% correct; give a reasonable, evidence‑based guess.)
This file is responsible for the functionality behind editing your messages to someone/some chat room, with functions that let you edit messages and check permissions to delete/pin messages.
What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
The editMessage 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 many parameters (count = 4): editMessage

2. Refactoring

How did the specific issue you chose impact the codebase’s adaptability?
The specific issue I chose impacted the codebase's adaptability as the old design was highly coupled and difficult to extend in future scenarios.
What changes did you make to resolve the issue?
I resolve the issue by passing in data to editMessage instead of the prior parameters, which was uid, mid, roomId, content, and instead extracted the values inside the function from data.
How do your changes improve adaptability? Did you consider alternatives?
My changes improve adaptability, as the data parameter would result in much higher adaptability to change the function parameters easily in the future. I considered an alternative such as having three parameters instead of four, but one would make it more extendable.

3. Validation

How did you trigger the refactored code path from the UI?
I triggered the refactored code path from the UI by simply editing a message that I had sent.
Attach a screenshot of the logs and UI demonstrating the trigger.

click on the three little dots on the bottom right corner of a message, then click edit
(Run ./nodebb log; include the relevant UI view. Temporary logs should be removed before final commit.)
image

type in your new message, then click save
image

image

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

image

@coveralls
Copy link

Pull Request Test Coverage Report for Build 17483196531

Details

  • 3 of 4 (75.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 78.53%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/activitypub/inbox.js 0 1 0.0%
Totals Coverage Status
Change from base Build 17435863312: 0.001%
Covered Lines: 24693
Relevant Lines: 29607

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