Backend refactoring #1454
Backend refactoring #1454
Conversation
…ion.controller.ts
….json and yarn.lock
There was a problem hiding this comment.
Pull request overview
This PR performs a comprehensive backend refactoring focused on removing unused code and improving code cleanliness. The main changes include:
- Removal of unused DTOs, interfaces, helper functions, and test utilities
- Addition of Knip tool for automated dead code detection
- Dependency cleanup (removed
@nestjs/microservicesandexpress-rate-limit) - Function visibility adjustments (converting exported functions to internal where appropriate)
Reviewed changes
Copilot reviewed 74 out of 77 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated dependencies including removal of @nestjs/microservices, express-rate-limit, addition of knip and its dependencies, and version bumps for @types/node, js-yaml, and @emnapi packages |
| backend/package.json | Added knip script and dependency, removed unused @nestjs/microservices and express-rate-limit, updated TypeScript to use caret versioning |
| backend/knip.json | New Knip configuration file to manage dead code detection settings |
| backend/src/guards/index.ts | Removed PaidFeatureGuard export ( |
| backend/src/helpers/index.ts | Removed exports for several unused helper functions (pauseCode still in use) |
| backend/src/entities//dto/.ts | Removed multiple unused DTO files across various entities (user, group, permission, connection, table-actions, etc.) |
| backend/src/entities//application/data-structures/.ts | Removed unused data structure classes |
| backend/src/helpers/*.ts | Deleted entire unused helper utility files |
| backend/test/utils/*.ts | Removed unused test utility functions |
| backend/test/mock.factory.ts | Replaced imported CreateGroupDto with local class definition |
| backend/src/enums/index.ts | Removed unused enum exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| class CreateGroupDto { | ||
| title: string; | ||
| permissions?: Array<any>; | ||
| isMain?: boolean; | ||
| users?: Array<any>; | ||
| } |
There was a problem hiding this comment.
Creating a local CreateGroupDto class in the test file rather than importing from the entity is inconsistent with how other DTOs are used in this file (e.g., CreateConnectionDto which is still imported on line 11). If the original DTO was deleted, this creates a maintenance issue as the local class definition may drift from any actual requirements. Consider either keeping the original DTO or ensuring all test DTOs follow the same pattern.
| "ts-node": "^10.9.2", | ||
| "tsconfig-paths": "^4.2.0", | ||
| "typescript": "5.9.3" | ||
| "typescript": "^5.9.3" |
There was a problem hiding this comment.
[nitpick] The TypeScript version constraint has been changed from an exact version (5.9.3) to a caret range (^5.9.3). This allows minor and patch version updates automatically. Ensure this is intentional, as it could introduce unexpected behavior if a new minor version of TypeScript is released with breaking changes. For consistency with other dependencies in the file, consider keeping exact versions for critical tooling like TypeScript.
| "typescript": "^5.9.3" | |
| "typescript": "5.9.3" |
| export { TableAddGuard } from './table-add.guard.js'; | ||
| export { TableDeleteGuard } from './table-delete.guard.js'; | ||
| export { TableEditGuard } from './table-edit.guard.js'; | ||
| export { TableReadGuard } from './table-read.guard.js'; |
There was a problem hiding this comment.
The PaidFeatureGuard is still being used in backend/src/entities/company-info/company-info.controller.ts (lines 504, 556, 609). Removing it from the guards index export will break the imports in that file.
| export { replaceTextInCurlies } from './operate-values-between-curlies.js'; | ||
| export { slackPostMessage } from './slack/slack-post-message.js'; | ||
| export { tableSettingsFieldValidator } from './validators/table-settings-field-validator.js'; | ||
| export { toPrettyErrorsMsg } from './to-pretty-errors-msg.js'; |
There was a problem hiding this comment.
The pauseCode function is still being used in test files (e.g., backend/test/ava-tests/non-saas-tests/non-saas-user-with-table-only-permissions-e2e.test.ts line 23). Removing it from the index export will break test imports.
| "migration:run": "yarn run typeorm migration:run -d dist/shared/config/datasource.config.js", | ||
| "migration:revert": "npm run typeorm -- migration:revert -d dist/shared/config/datasource.config.js" | ||
| "migration:revert": "npm run typeorm -- migration:revert -d dist/shared/config/datasource.config.js", | ||
| "knip": "knip" |
There was a problem hiding this comment.
[nitpick] A new knip script has been added to help identify unused code. Ensure that the team is aware of this new tool and how to use it. Consider adding documentation about running yarn knip as part of the development or CI/CD process to maintain code cleanliness.
No description provided.