Conversation
sirisjo
requested review from
AD7ZJ,
LawsonWillard,
brandonshearin,
kpowderly and
mistahj67
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a PL/pgSQL backfill routine to the v8.8.0 migration that scans non-built-in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/database/migration/migrations/v8.8.0.sql`:
- Around line 100-106: The FOR loop is selecting rows from schema_node_kinds
without excluding soft-deleted entries, so modify the SELECT used by the FOR ...
IN to skip rows where schema_node_kinds.deleted_at IS NOT NULL (e.g., add "AND
schema_node_kinds.deleted_at IS NULL" to the WHERE clause) so that
schema_node_kind_record never represents a soft-deleted kind; keep the rest of
the logic (INSERT into custom_node_kinds and UPDATE of schema_node_kind_id)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7abb41f0-69a7-4bcd-8028-44b5970a8098
📒 Files selected for processing (1)
cmd/api/src/database/migration/migrations/v8.8.0.sql
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/database/migration/migrations/v8.8.0.sql`:
- Around line 118-124: The current migration overwrites custom_node_kinds.config
when a row matches kind_name; instead, only set schema_node_kind_id for
pre-existing rows and leave config untouched. Modify the UPDATE for
custom_node_kinds (referencing table custom_node_kinds, columns
schema_node_kind_id and config, the kind_name match and schema_node_kind_record)
so it only assigns schema_node_kind_id = schema_node_kind_record.id and only for
rows that do not already have a schema_node_kind_id (e.g., add a WHERE ... AND
schema_node_kind_id IS NULL), keeping the INSERT branch unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d25825a7-e5cb-4f01-831a-142f080bc15c
📒 Files selected for processing (1)
cmd/api/src/database/migration/migrations/v8.8.0.sql
mistahj67
left a comment
mistahj67
left a comment
There was a problem hiding this comment.
Code LGTM. Tested as well 🚀
3 participants
Description
When fixing the schema upload logic to add icon definitions to the
custom_node_kindstable, I missed that we also need to backfill thecustom_node_kindstable with icon definitions that exist in theschema_node_kindstable before that change was made. This change adds a migration to do so.Motivation and Context
Resolves BED-7610
Required so that existing OpenGraph schema icons will show up across the app without having to either re-upload the schema or use the custom-node-kinds API suite.
How Has This Been Tested?
Tested locally by uploading a schema, deleting the icons that are auto-added, and then running the migration to confirm they are re-populated.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit