chore: db migration for adding slug to workflow - #1136
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds a ChangesWorkflow Slug Column Addition
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
Adds a workspace-scoped slug column to the workflow table so workflows can be addressed by a human-readable identifier. The migration is hand-edited to add the column nullable, backfill from name via a SQL slugify expression, then mark NOT NULL and add a UNIQUE(workspace_id, slug) constraint. Drizzle schema, snapshot/journal metadata, and the workspace-engine sqlc-generated model/query are updated accordingly.
Changes:
- Add
slugcolumn and unique(workspace_id, slug)index toworkflow(Drizzle schema + migration + snapshot/journal). - Backfill existing rows in SQL using
lower(name)→ replace non-alphanumerics with-→ trim-. - Update workspace-engine
Workflowmodel,GetWorkflowByIDquery, and raw schema to includeslug.
Reviewed changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema/workflow.ts | Adds slug column and unique(workspaceId, slug) constraint to the Drizzle schema. |
| packages/db/drizzle/0195_left_silverclaw.sql | New migration: add nullable slug, backfill, set NOT NULL, add unique constraint. |
| packages/db/drizzle/meta/_journal.json | Registers migration 0195. |
| packages/db/drizzle/meta/0195_snapshot.json | Drizzle snapshot reflecting new column and unique constraint. |
| apps/workspace-engine/pkg/db/queries/schema.sql | Mirrors slug column and unique constraint in workspace-engine schema. |
| apps/workspace-engine/pkg/db/models.go | Adds Slug field to Workflow struct. |
| apps/workspace-engine/pkg/db/workflows.sql.go | Selects and scans the new slug column in GetWorkflowByID. |
Files not reviewed (2)
- apps/workspace-engine/pkg/db/models.go: Language not supported
- apps/workspace-engine/pkg/db/workflows.sql.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,4 @@ | |||
| ALTER TABLE "workflow" ADD COLUMN "slug" text;--> statement-breakpoint | |||
| UPDATE "workflow" SET "slug" = trim(both '-' from regexp_replace(lower("name"), '[^a-z0-9]+', '-', 'g'));--> statement-breakpoint | |||
| @@ -0,0 +1,4 @@ | |||
| ALTER TABLE "workflow" ADD COLUMN "slug" text;--> statement-breakpoint | |||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/db/drizzle/0195_left_silverclaw.sql`:
- Around line 1-4: Add a pre-flight collision check before creating the "slug"
column and adding the UNIQUE constraint "workflow_workspace_id_slug_unique": run
a query that slugifies existing workflow names (using trim(both '-' from
regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g')) as slug) grouped by
workspace_id and fail the migration if any workspace_id/slug has count > 1,
returning the conflicting names so operators can resolve them; ensure this check
runs prior to the ALTER TABLE and aborts the migration (or exits with a non-zero
status) if any collisions are found.
- Line 2: The slug update can produce empty strings for names with no
alphanumerics (e.g., "!!!"); before applying the UPDATE in the migration that
sets "slug" on table "workflow", detect and handle empty slugs by either (A)
running a pre-check query to find rows where trim(both '-' from
regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g')) yields an empty string and
fixing them (e.g., set a fallback slug or update the name), or (B) add a CHECK
constraint on workflow.slug (e.g., CHECK (length(slug) > 0)) after populating
safe slugs to prevent future empty values; refer to the migration UPDATE
statement that assigns trim(...regexp_replace...) to "slug" and the "workflow"
table and "slug" column when making these changes.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b13e0b83-8948-470b-9be7-cb9afd582fa7
📒 Files selected for processing (7)
apps/workspace-engine/pkg/db/models.goapps/workspace-engine/pkg/db/queries/schema.sqlapps/workspace-engine/pkg/db/workflows.sql.gopackages/db/drizzle/0195_left_silverclaw.sqlpackages/db/drizzle/meta/0195_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/workflow.ts
| ALTER TABLE "workflow" ADD COLUMN "slug" text;--> statement-breakpoint | ||
| UPDATE "workflow" SET "slug" = trim(both '-' from regexp_replace(lower("name"), '[^a-z0-9]+', '-', 'g'));--> statement-breakpoint | ||
| ALTER TABLE "workflow" ALTER COLUMN "slug" SET NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "workflow" ADD CONSTRAINT "workflow_workspace_id_slug_unique" UNIQUE("workspace_id","slug"); |
There was a problem hiding this comment.
Missing pre-flight collision check for duplicate slugs.
The PR objectives explicitly state: "Pre-flight check required: verify no two existing workflow names in the same workspace slugify to the same value before running the migration in production."
However, the migration doesn't include this verification. If two workflows in the same workspace have names that slugify to the same value (e.g., "My Workflow" and "My-Workflow" both become "my-workflow"), the ADD CONSTRAINT on line 4 will fail, potentially leaving the database in an inconsistent state during production deployment.
🛡️ Recommended pre-flight validation query
Run this query before applying the migration to detect collisions:
WITH slugified AS (
SELECT
workspace_id,
name,
trim(both '-' from regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g')) as slug
FROM workflow
)
SELECT workspace_id, slug, array_agg(name) as conflicting_names, count(*) as count
FROM slugified
GROUP BY workspace_id, slug
HAVING count(*) > 1;If this returns any rows, you must resolve the name conflicts before running the migration.
🤖 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 `@packages/db/drizzle/0195_left_silverclaw.sql` around lines 1 - 4, Add a
pre-flight collision check before creating the "slug" column and adding the
UNIQUE constraint "workflow_workspace_id_slug_unique": run a query that
slugifies existing workflow names (using trim(both '-' from
regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g')) as slug) grouped by
workspace_id and fail the migration if any workspace_id/slug has count > 1,
returning the conflicting names so operators can resolve them; ensure this check
runs prior to the ALTER TABLE and aborts the migration (or exits with a non-zero
status) if any collisions are found.
| @@ -0,0 +1,4 @@ | |||
| ALTER TABLE "workflow" ADD COLUMN "slug" text;--> statement-breakpoint | |||
| UPDATE "workflow" SET "slug" = trim(both '-' from regexp_replace(lower("name"), '[^a-z0-9]+', '-', 'g'));--> statement-breakpoint | |||
There was a problem hiding this comment.
Consider validating against empty slugs.
If a workflow name consists entirely of non-alphanumeric characters (e.g., "!!!"), the slugification logic produces an empty string. While the NOT NULL constraint allows this, an empty slug may violate business logic expectations and could cause issues in URL routing or API endpoints.
🛡️ Proposed validation to detect empty slugs
Add a check constraint to prevent empty slugs:
ALTER TABLE "workflow" ADD CONSTRAINT "workflow_slug_not_empty" CHECK (length(slug) > 0);Or pre-validate before migration:
SELECT id, name, trim(both '-' from regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g')) as slug
FROM workflow
WHERE length(trim(both '-' from regexp_replace(lower(name), '[^a-z0-9]+', '-', 'g'))) = 0;🤖 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 `@packages/db/drizzle/0195_left_silverclaw.sql` at line 2, The slug update can
produce empty strings for names with no alphanumerics (e.g., "!!!"); before
applying the UPDATE in the migration that sets "slug" on table "workflow",
detect and handle empty slugs by either (A) running a pre-check query to find
rows where trim(both '-' from regexp_replace(lower(name), '[^a-z0-9]+', '-',
'g')) yields an empty string and fixing them (e.g., set a fallback slug or
update the name), or (B) add a CHECK constraint on workflow.slug (e.g., CHECK
(length(slug) > 0)) after populating safe slugs to prevent future empty values;
refer to the migration UPDATE statement that assigns trim(...regexp_replace...)
to "slug" and the "workflow" table and "slug" column when making these changes.
fixes #1130
Summary by CodeRabbit