-
Notifications
You must be signed in to change notification settings - Fork 3.6k
switch datasourcing #12825
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
switch datasourcing #12825
Conversation
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
Major architectural shift from TypeORM repository pattern to direct SQL queries via WorkspaceDataSourceService across messaging and calendar modules.
- Database queries in
messaging-message-list-fetch.cron.job.ts
andcalendar-events-import.cron.job.ts
now use string interpolation in SQL queries, introducing potential SQL injection vulnerabilities that need to be addressed - Removal of type safety provided by TypeORM's repository pattern in all modified cron jobs requires additional validation and error handling
- All cron jobs (
messaging-messages-import.cron.job.ts
and others) have simplified database access but lack proper parameterization in SQL queries - Need for comprehensive testing of the new data sourcing mechanism, especially around concurrent workspace access patterns
4 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
const schemaName = this.workspaceDataSourceService.getSchemaName( | ||
activeWorkspace.id, | ||
'calendarChannel', | ||
); |
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.
syntax: Inconsistent indentation in method call (4 spaces instead of 2)
const schemaName = this.workspaceDataSourceService.getSchemaName( | |
activeWorkspace.id, | |
'calendarChannel', | |
); | |
const schemaName = this.workspaceDataSourceService.getSchemaName( | |
activeWorkspace.id, | |
); |
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:35271 This environment will automatically shut down when the PR is closed or after 5 hours. |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
This PR improves error handling in `handleDriverException` by adding a duck-typing check for `MessageImportDriverException` objects. It ensures that errors are correctly identified and processed even if they are received as plain objects rather than class instances. This change prevents missed exception handling and increases the robustness of the message import process.
This PR improves error handling in
handleDriverException
by adding a duck-typing check forMessageImportDriverException
objects. It ensures that errors are correctly identified and processed even if they are received as plain objects rather than class instances. This change prevents missed exception handling and increases the robustness of the message import process.