Drop gridPosition column and field (phase 2 of gridPosition removal)#20039
Drop gridPosition column and field (phase 2 of gridPosition removal)#20039
Conversation
There was a problem hiding this comment.
1 issue found across 126 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/engine/core-modules/open-api/utils/components.utils.ts">
<violation number="1" location="packages/twenty-server/src/engine/core-modules/open-api/utils/components.utils.ts:1243">
P2: `PageLayoutWidgetPosition` does not enforce layout-mode-specific required fields, so the OpenAPI contract is weaker than the documented behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content GraphQL Metadata Schema ChangesGraphQL Metadata Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content REST API Analysis ErrorError OutputREST Metadata API Analysis ErrorError Output |
d12ec26 to
aba3aeb
Compare
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:25015 This environment will automatically shut down after 5 hours. |
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts:42">
P1: The 2.2 instance command chain no longer includes the `gridPosition` drop command, so upgrades will skip the column removal step and leave schema drift.</violation>
</file>
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/2-2/2-2-instance-command-slow-1795000003000-backfill-page-layout-widget-position-again.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/2-2/2-2-instance-command-slow-1795000003000-backfill-page-layout-widget-position-again.ts:11">
P1: Guard this backfill query behind a `gridPosition` column-existence check; otherwise the 2.2 upgrade can fail once the column has been dropped.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 21 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/database/commands/upgrade-version-command/2-2/2-2-instance-command-fast-1777473592000-make-page-layout-widget-grid-position-nullable.ts">
<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/2-2/2-2-instance-command-fast-1777473592000-make-page-layout-widget-grid-position-nullable.ts:18">
P2: The `down()` migration can fail because it reapplies `NOT NULL` without backfilling existing `NULL` values in `gridPosition`.</violation>
</file>
<file name="packages/twenty-server/src/engine/metadata-modules/page-layout-widget/dtos/inputs/grid-position.input.ts">
<violation number="1">
P2: Explicitly type numeric GraphQL input fields as `Int`; with `@Field()` they are exposed as `Float` by default, which mismatches the intended integer contract.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts">
<violation number="1">
P1: Using `?? null` on optional `position` turns omitted updates into explicit null writes, which can unintentionally clear existing widget positions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Following #20032
Context
Phase 2 of thegridPositionremoval (Phase 1 was #20032). Now that Phase 1 has rolled out and every widget has a non-nullposition, this PR dropsgridPositionfrom the DB, entity, GraphQL/REST API, manifest type, AI tools, all standard and default layouts, and every test fixture.positionis the single source of truth.Postponing phase 2, some part of the code was still writing in the old column (no real impact since the code was reading
new ?? legacybut in DB position could still be empty