-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
cleanup pg_graphql #1 #7673
cleanup pg_graphql #1 #7673
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.
Great step forward!!! TODO: remove from setup-db in a followup PR after deploy
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 pull request focuses on removing components related to pg_graphql as part of a larger effort to deprecate it and streamline the codebase. The changes primarily affect the workspace-query-builder and workspace-query-runner modules.
- Removed multiple factory classes in
packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/
, including those for creating, updating, and deleting queries - Deleted
WorkspaceQueryRunnerService
and its associated module, impacting database operations previously handled by pg_graphql - Removed
AISQLQueryModule
and related files, affecting AI-assisted SQL query generation functionality - Updated
TRACK
mutation parameters in analytics module, simplifying from 'type', 'sessionId', and 'data' to 'action' and 'payload' - Simplified
workspaceQueryBuilderFactories
array, retaining onlyRecordPositionQueryFactory
andForeignDataWrapperServerQueryFactory
30 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
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.
logic: Removing CreateManyQueryFactory may break bulk record creation functionality. Ensure alternative implementation is in place.
@@ -1,34 +1,8 @@ | |||
import { ForeignDataWrapperServerQueryFactory } from 'src/engine/api/graphql/workspace-query-builder/factories/foreign-data-wrapper-server-query.factory'; |
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.
style: Consider updating import path to use relative import for consistency with line 3
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.
logic: Removing FieldsStringFactory may break GraphQL query generation. Ensure all dependencies are updated.
providers: [ | ||
WorkspaceQueryRunnerService, | ||
...workspaceQueryRunnerFactories, | ||
EntityEventsToDbListener, | ||
TelemetryListener, | ||
RecordPositionBackfillCommand, | ||
], |
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.
logic: Removal of WorkspaceQueryRunnerService may impact module functionality. Ensure all dependencies are handled.
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.
logic: Removing this entire file is a major change. Ensure all dependencies and references to WorkspaceQueryRunnerService are removed or replaced throughout the codebase.
Context
This PR removes workspace-query-runner/builder in preparation for fully deprecating pg_graphql
next steps: Remove from the setup and make a command to remove comments on schema/tables related to pg_graphql