Skip to content

Conversation

afitzek
Copy link
Contributor

@afitzek afitzek commented Oct 9, 2025

Summary

Display names for roles should be unique to help avoid confusion for customers. This adds a unique constraint into the database to ensure this and a migration path to fix current duplicates. The migration was inspired by 1620821879465-UniqueWorkflowNames.

Related Linear tickets, Github issues, and Community forum posts

closes https://linear.app/n8n/issue/PAY-3948/make-role-name-unique

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@n8n-assistant n8n-assistant bot added the n8n team Authored by the n8n team label Oct 9, 2025
@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch from 6b963bd to 37b3e55 Compare October 10, 2025 09:43
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 30.95238% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...migrations/common/1760020838000-UniqueRoleNames.ts 3.44% 28 Missing ⚠️
packages/cli/src/services/role.service.ts 90.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch 2 times, most recently from 1200fab to 6fdc7fc Compare October 10, 2025 15:35
@afitzek afitzek marked this pull request as ready for review October 13, 2025 07:07
@afitzek afitzek requested a review from a team as a code owner October 13, 2025 07:07
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 15 files

Prompt for AI agents (all 5 issues)

Understand the root cause of the following 5 issues and fix them.


<file name="packages/cli/test/integration/shared/db/roles.ts">

<violation number="1" location="packages/cli/test/integration/shared/db/roles.ts:167">
The cleanup helper now propagates delete failures for system or referenced roles, so tests depending on cleanupRolesAndScopes() will start failing once a role can’t be removed.</violation>

<violation number="2" location="packages/cli/test/integration/shared/db/roles.ts:177">
Removing the try/catch around scope deletion causes cleanupRolesAndScopes() to throw whenever a test scope is still referenced, breaking the cleanup helper used by integration tests.</violation>
</file>

<file name="packages/@n8n/backend-test-utils/src/migration-test-helpers.ts">

<violation number="1" location="packages/@n8n/backend-test-utils/src/migration-test-helpers.ts:99">
Respect migrations that disable transactions when undoing the last migration by falling back to transaction: &#39;none&#39; instead of hard-coding &#39;each&#39;.</violation>
</file>

<file name="packages/@n8n/db/src/migrations/common/1760020838000-UniqueRoleNames.ts">

<violation number="1" location="packages/@n8n/db/src/migrations/common/1760020838000-UniqueRoleNames.ts:30">
Renaming duplicates with a fixed “ +&lt;n&gt;” suffix only guarantees uniqueness within that duplicate set. If another role already uses the same suffixed display name, this update will create a fresh conflict and the unique index creation will fail. Please ensure the generated name is unique across the entire table (e.g., track used display names and keep incrementing until a free value is found).</violation>
</file>

<file name="packages/cli/src/services/role.service.ts">

<violation number="1" location="packages/cli/src/services/role.service.ts:177">
Rule violated: **Prefer Typeguards over Type casting**

Please replace the new `error as Error` cast with a proper type guard to comply with the &quot;Prefer Typeguards over Type casting&quot; rule.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch from 8060761 to a7d7d58 Compare October 13, 2025 07:42
cursor[bot]

This comment was marked as outdated.

@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch from a7d7d58 to 6020bdd Compare October 13, 2025 11:30
cursor[bot]

This comment was marked as outdated.

@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch from 6020bdd to 8d574de Compare October 13, 2025 18:36
@afitzek afitzek force-pushed the pay-3948-make-role-name-unique branch from 8d574de to f0a6de0 Compare October 13, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

n8n team Authored by the n8n team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant