fix: preserve nullable cloud task bindings - #2405
Conversation
📝 WalkthroughWalkthroughThe change adds MySQL ChangesMySQL loop-item alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/db/mysql_loop_items_schema.py`:
- Around line 326-343: The _restore_nullable_constraint_values function restores
only a partial list of nullable columns, but _backfill_mysql_sentinels converts
all mapped nullable fields. Update _restore_nullable_constraint_values to build
the restoration assignments from MYSQL_LOOP_ITEM_SENTINEL_COLUMNS instead of the
hardcoded string_columns and other column lists, ensuring that all
sentinel-backed nullable fields (including task_id, task_user_id, task_title,
linked_at, and unlinked_at) are properly restored to NULL during downgrade.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b6c0a50-3933-4b1a-b057-d2848720e32f
📒 Files selected for processing (8)
backend/alembic/env.pybackend/alembic/versions/20260804_c0d1e2f3a4b5_align_mysql_loop_items_schema.pybackend/app/db/mysql_loop_items_schema.pybackend/app/models/delivery.pybackend/app/services/cloud_projects/service.pybackend/app/services/loop_items/service.pybackend/tests/models/test_loop_items_mysql_schema.pybackend/tests/schemas/test_delivery.py
| def _restore_nullable_constraint_values(connection: Connection) -> None: | ||
| assignments = [] | ||
| string_columns = ( | ||
| "cloud_project_id", | ||
| "parent_id", | ||
| "loop_item_id", | ||
| "delivery_id", | ||
| "public_id", | ||
| "project_key", | ||
| "storage_prefix", | ||
| ) | ||
| for name in string_columns: | ||
| column = _quote(connection, name) | ||
| assignments.append(f"{column} = NULLIF({column}, '')") | ||
| for name in ("local_project_id", "backend_task_id"): | ||
| column = _quote(connection, name) | ||
| assignments.append(f"{column} = NULLIF({column}, 0)") | ||
| connection.exec_driver_sql("UPDATE `loop_items` SET " + ", ".join(assignments)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Restore every sentinel-backed nullable value during downgrade.
_backfill_mysql_sentinels converts every mapped nullable field. This function restores only foreign-key and unique-index fields. A prior NULL task_id, task_user_id, task_title, linked_at, or unlinked_at remains a sentinel after an upgrade and downgrade.
Build restoration from MYSQL_LOOP_ITEM_SENTINEL_COLUMNS instead of this partial list. If a sentinel can be valid business data, preserve the original null state before upgrade because NULLIF cannot reverse COALESCE. Add a round-trip migration test with null and sentinel-valued task bindings.
As per coding guidelines, backend persistent model changes require verification of both upgrade head and rollback before handoff.
🧰 Tools
🪛 Ruff (0.16.0)
[error] 343-343: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/db/mysql_loop_items_schema.py` around lines 326 - 343, The
_restore_nullable_constraint_values function restores only a partial list of
nullable columns, but _backfill_mysql_sentinels converts all mapped nullable
fields. Update _restore_nullable_constraint_values to build the restoration
assignments from MYSQL_LOOP_ITEM_SENTINEL_COLUMNS instead of the hardcoded
string_columns and other column lists, ensuring that all sentinel-backed
nullable fields (including task_id, task_user_id, task_title, linked_at, and
unlinked_at) are properly restored to NULL during downgrade.
Source: Coding guidelines
1 participant
Summary by CodeRabbit
Bug Fixes
Tests