simplified isGroupTitleValid by adding helper functions for validation checks #169
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
1. Issue
Link to the associated GitHub issue:
#62
Full path to the refactored file:
NodeBB/src/user/profile.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 handles various functions for updating a user's profile and validating the user's information. This includes checking if a user’s email is valid, checking username availability, validating birthday format, updating usernames and passwords. The function I focused on was checking if a user’s group title (badge) is valid by making sure it's not a reserved or privileged group and follows the site’s multiple badge policy.
What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
The scope of my refactoring was to reduce the complexity of the isGroupTitleValid function, which was flagged a complexity of 10.
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 = 10): isGroupTitleValid
2. Refactoring
How did the specific issue you chose impact the codebase’s maintainability?
For a valid group title/badge, there are several requirements. Looking at the function originally, it is clear what those requirements are and how they are checked. The nested function and the multiple if statements added unnecessary complexity and was the source of the qlty smell. For instance, the code was hard to test as the checkTitle() nested function could not be tested separately. There was duplicate validation as the checkTitle() was called in both the if and else statement. Lastly, the parsing, validation and enforcement logic was mixed together, making it difficult to understand the functions behavior.
What changes did you make to resolve the issue?
By refactoring and breaking down this function, I divided the function into 3 main checks. The main function calls on 3 helper functions. The first, parseGroupTitles, normalizes the data into an array format. The second function, validateGroupTitles, checks that the group title is not 'registered-users' or part of a privileged group. The third function, enforceMultipleBadgeLimit, checks if multiple badges are allowed and checks the number of selected badges based on that. These helper functions are called by the main function and eliminates the need for a nested function and various if statements. I didn’t create new solutions or change the functions behavior but rather restructured the existing code to be more testable and maintainable.
How do your changes improve maintainability? Did you consider alternatives?
My changes improved maintainability by breaking down the logic of the function into smaller, single purpose helper functions. This reduces the cognitive load of understanding the original code because the helper functions, including its name, are clear and indicate to a developer its purpose and what it is validating. It also makes future updates safer. For example, if the access is to be changed for the reserved/privilege group badges, only one function is changing without the risk of messing up other unrelated logic. I thought of only making small changes to the structure such as removing duplicate validations and simplifying some boolean logic but I thought that wouldn’t be substantial to significantly reduce the complexity of the function.
3. Validation
How did you trigger the refactored code path from the UI?
I created several groups and added a group title (badge) to each. I enabled the display so that the badges were visible. In the edit profile page, the users group titles are visible. I selected a badge and saved changes which triggered the function reflected in the console logs. To check for multiple badges, I went to the admin page and enabled multiple badges. In the edit profile page, the users group titles are visible. This time, multiple selects were allowed, indicated by the arrows. I selected all badges and saved changes which triggered the function reflected in the console logs.
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.)
Attach a screenshot of
qlty <img width="1512" height="302" alt="qlty smells --no-snippets src:user:profile" src="https://github.com/user-attachments/assets/265513a8-daf8-4628-bc42-d254405013ab" /> <img width="1512" height="982" alt="1_select_group2_badge" src="https://github.com/user-attachments/assets/c829bb9e-53b7-4a89-986e-8653579e9ca8" /> <img width="1512" height="982" alt="4_select_all_badges" src="https://github.com/user-attachments/assets/af4341f4-2f31-4841-9c63-f02f626fe27f" /> <img width="1509" height="336" alt="5_console log_all_badges" src="https://github.com/user-attachments/assets/53c9e238-ca1e-4970-8b58-469af40e8b19" /> <img width="1511" height="853" alt="3_enable_multiple_badges" src="https://github.com/user-attachments/assets/717ddd32-8c0e-4356-97cd-e05ea043f5c1" /> <img width="1512" height="368" alt="2_console log_group2" src="https://github.com/user-attachments/assets/252661cc-a942-4e88-b481-0959fa0ee684" /> smells --no-snippets <full/path/to/file.js>showing fewer reported issues after the changes.