-
Notifications
You must be signed in to change notification settings - Fork 66
[Refactor]: Migrate apps/web/app/services to Typed OOP Classes in apps/web/core/services #3785
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
[Refactor]: Migrate apps/web/app/services to Typed OOP Classes in apps/web/core/services #3785
Conversation
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 67 files out of 174 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR implements a significant architectural change, migrating the API service layer to a typed OOP class structure for better organization and type safety.
Key changes:
- Created new APIService base class with standardized HTTP methods and error handling
- Migrated all API functions to service classes (e.g. AuthService, TimerService) that extend APIService
- Implemented proper TypeScript interfaces and removed redundant 'API' suffixes from method names
- Added conditional endpoint handling based on GAUZY_API_BASE_SERVER_URL configuration
- Consolidated related API methods into cohesive service classes with consistent patterns
Potential issues:
- Several services have inconsistent error handling approaches
- Some services use 'any' types instead of proper interfaces
- The RequestToJoinTeam service has a typo in filename ('joi' vs 'join')
- The TimeLogsService has swapped startDate/endDate parameters that could cause incorrect queries
170 file(s) reviewed, 94 comment(s)
Edit PR Review Bot Settings | Greptile
apps/web/core/services/client/api/timer/timer-status.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
Based on the recent changes and previous review, I'll focus on the most important new or modified aspects not covered in the last summary:
Significant security and type safety improvements have been implemented across the authentication and API service layer.
- Replaced Math.random() with crypto.randomBytes() in
generate-token.ts
for cryptographically secure token generation - Fixed critical Promise.reject handling in APIService request interceptor for proper error propagation
- Corrected parameter mapping in TimeLogsService where startDate/todayStart were previously swapped
- Fixed inconsistent null checks between HTTP methods in APIService base class
- Added proper tenant and organization ID handling in request headers across all service classes
The changes demonstrate a focus on security and reliability while maintaining the architectural improvements from the previous review. These modifications address some of the potential issues mentioned earlier while introducing important security enhancements.
32 file(s) reviewed, 19 comment(s)
Edit PR Review Bot Settings | Greptile
apps/web/core/services/client/api/organization-team/organization-team.service.ts
Show resolved
Hide resolved
apps/web/core/services/client/api/organization-team/organization-team.service.ts
Outdated
Show resolved
Hide resolved
@CREDO23 could you please fix bots suggestions and git conflicts ? |
570b9cf
to
e2409fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Based on the recent changes and previous reviews, I'll focus on the most significant new modifications not covered in earlier summaries:
Fixed several critical typos and inconsistencies across the codebase to improve reliability and maintainability.
- Fixed typo in
task-linked-issue.ts
by renamingcreateTaskLinkedIsssue
tocreateTaskLinkedIssue
- Added missing words 'Blandit', 'lobortis', and 'neque' to .cspell.json dictionary
- Corrected duplicate 'I' prefix in
IIInviteRequest
interface name in IEmployee.ts - Fixed incorrect import path in request-to-join-team/index.ts ('joi' to 'join')
The changes focus on code quality improvements and bug fixes while maintaining the architectural improvements from previous reviews. These modifications help ensure consistent naming and proper spell checking across the codebase.
174 file(s) reviewed, 36 comment(s)
Edit PR Review Bot Settings | Greptile
Description
Type of Change
Checklist