Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solving SonarCloud Critical / Major & Minor Errors - Improving Code Quality #572

Closed
wants to merge 17 commits into from

Conversation

filypsdias
Copy link

@filypsdias filypsdias commented Aug 25, 2023

Description

This pull request addresses several code quality issues identified by SonarCloud and includes various code refactoring improvements. The changes made in this pull request enhance the overall maintainability and readability of the codebase, resolve existing issues, and ensure better adherence to best practices.

Overall, the number of issues fixed was:

  • 15 Major
  • 13 Minor
  • 3 Critical
image

Changes Made

1. Code Refactoring for Readability(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13):

  • Reorganized conditional statements in various components to improve code readability and maintainability.
  • Simplified the use of conditional expressions by utilizing the nullish coalescing operator (??) for default assignments.
  • Replaced multiple nested if-else conditions with early return statements to reduce cognitive complexity.
  • Replaced iterative for loops with for-of loops where applicable to simplify iteration.
  • Namespace Import for Services: This approach makes it evident that these functions are related to the services
  • Descriptive Variable Names: I retained descriptive variable names that help convey the purpose of the variables. For example, whatsappData more clearly represents the data being used in the updateAndNotifyWhatsApp function, and whatsappId represents the ID of the WhatsApp entity being operated on.
  • Comments: I removed or retained the comments as they were in the original code, focusing on clarity and understanding the purpose of each route handler and associated actions.

2. Component Definition Optimization (1, 2, 3, 4):

As part of this pull request, we've addressed an issue related to defining components within the render phase in React. The error message "Do not define components during render" highlights the concern that React may interpret new component types during every render, leading to the destruction of DOM nodes and state within the entire subtree:

  • To resolve this issue, we've extracted component definitions from the parent component's render method and introduced a more efficient approach. By moving these component definitions out of the render phase and passing data as props, I ensure a more stable and performant React application. This change enhances the overall user experience and reduces the risk of unintended behavior caused by frequent component type changes during rendering.

3. Cognitive Complexity Reduction(1, 2, 3):

  • In this pull request, I focused on reducing the cognitive complexity of various methods within the codebase. By simplifying intricate logic and optimizing conditional structures, I've enhanced code readability and maintainability. The changes aim to make the codebase more understandable, making it easier to troubleshoot, maintain, and collaborate on the project.

Impact

  • These changes contribute to a cleaner, more readable, and maintainable codebase. They also align with best practices for structuring code and naming conventions in Node.js and Express.js applications. By improving the organization and naming of functions, eliminating repetition, and centralizing the usage of service functions, this PR enhances the code's long-term maintainability and the team's ability to understand and extend it in the future.
  • Another win, is that we improve the system security, preventing bad code to be executed on the open-source application

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • SonarCloud Integration: The SonarCloud static code analysis tool was employed to identify and address code quality and cognitive complexity issues. All the changes were made to address the issues flagged by SonarCloud, and the codebase was thoroughly reviewed to ensure compliance with best practices.
  • Functional Testing: The application's key functionalities were tested manually to ensure that the changes didn't introduce any regressions. Screens, components, and user interactions were validated to confirm that the user experience remained unaffected.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Additional Notes

  • This PR does not introduce any functional changes; it focuses solely on improving code quality and organization.
  • The changes have been tested locally to ensure they do not introduce regressions.

🥳 BONUS - Unit Tests Fix

  • In this PR, I've tackled the issue of broken test cases that were impacting our codebase. By identifying the underlying causes of these failures, I've managed to reinstate the robustness of our test suite. This ensures that our codebase remains dependable and free of unexpected regressions.

  • In addition to resolving the broken test cases, I've taken the initiative to bolster (reinforce) our code coverage. By strategically introducing new test cases, I've expanded the scope of our testing efforts, encompassing a wider range of scenarios. This proactive approach not only strengthens our code's reliability but also promotes better software quality.

Cool Stuff Added (Click to Open the Toggle)
  1. Sinon.JS: Like Mockito, from Java development, Sinon is a JavaScript library used primarily for testing purposes, specifically in the context of unit testing and mocking. It was used more Stubs & Matchers
  • Stubs: Stubs are functions that replace the behavior of an existing function or method. You can use them to control the output of a function or simulate certain behaviors during testing.
  • Matchers: Sinon.js provides matchers that help you define expectations about function invocations, such as the arguments passed to them. Matchers make it easier to write tests that are more precise and targeted.
  1. Chai.js: Chai.js is another JavaScript library commonly used in testing, specifically in the context of behavior-driven development (BDD) and assertion testing

  2. Changes to Package.json: In the latest version of the project, a significant improvement has been made to the testing process. Previously, it was necessary to run the sequelize seed command as a part of the pretest script. However, this approach led to validation errors if the seed had already been applied. To address this, the configuration has been adjusted. The sequelize seed command has been removed from the pretest script, ensuring a smoother testing experience without encountering validation conflicts due to redundant seeding.

  3. Enhanced Testing Parallelism: A key enhancement has been introduced to the testing workflow. The test command now includes the --maxWorkers=1 flag. This alteration has been made to promote testing singularity and prevent conflicts between different endpoints. By limiting the number of workers to one, the testing process operates in a more isolated manner. This adjustment minimizes the potential for interference and conflicts that might arise when multiple workers are executing tests concurrently.

  4. Added tests for Contact Services: Just introducing some additional tests to the Contact Services to improve test coverage

image

@sonarcloud
Copy link

sonarcloud bot commented Aug 26, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@filypsdias filypsdias changed the title Solving SonarCloud Critial / Major & Minor Errors - Improving Code Quality Solving SonarCloud Critical / Major & Minor Errors - Improving Code Quality Aug 28, 2023
@canove canove closed this Aug 30, 2023
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