feat: custom org domains usage#27798
Open
Amit91848 wants to merge 12 commits intofeat/custom-org-domains-uifrom
Open
feat: custom org domains usage#27798Amit91848 wants to merge 12 commits intofeat/custom-org-domains-uifrom
Amit91848 wants to merge 12 commits intofeat/custom-org-domains-uifrom
Conversation
Contributor
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
Amit91848
changed the title
3 tasks
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Amit91848
force-pushed
the
feat/custom-org-domains-rebased
branch
from
bdaef55 to
e64a30f
Compare
Amit91848
force-pushed
the
feat/custom-org-domains-rebased
branch
from
e64a30f to
e29b951
Compare
Amit91848
added
ready-for-e2e
run-ci
github-actions
bot
added
❗️ migrations
3 tasks
Member
Author
Contributor
|
@Amit91848 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
11 issues found across 73 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/playwright/dynamic-booking-pages.e2e.ts">
<violation number="1" location="apps/web/playwright/dynamic-booking-pages.e2e.ts:234">
P1: Rule violated: **E2E Tests Best Practices**
E2E Tests Best Practices: avoid text locators. These new expectations use `page.getByText(...)` instead of `page.getByTestId(...)`, which violates the rule’s requirement to prefer stable test IDs.</violation>
</file>
<file name="apps/web/playwright/embed-code-generator.e2e.ts">
<violation number="1" location="apps/web/playwright/embed-code-generator.e2e.ts:326">
P1: Rule violated: **E2E Tests Best Practices**
This new test creates a customDomain record but never deletes it, leaving DB state mutated after the test. E2E tests must clean up all created records to avoid side effects and maintain consistent parallel runs (E2E Tests Best Practices: reset DB state after tests).</violation>
</file>
<file name="apps/web/playwright/reschedule.e2e.ts">
<violation number="1" location="apps/web/playwright/reschedule.e2e.ts:440">
P1: Rule violated: **E2E Tests Best Practices**
E2E tests must reset DB state after running. This test inserts a `customDomain` record but never deletes it, leaving side effects that can affect parallel runs and repeatability.</violation>
</file>
<file name="packages/features/eventtypes/lib/getPublicEvent.ts">
<violation number="1" location="packages/features/eventtypes/lib/getPublicEvent.ts:332">
P2: Organization lookup by slug/customDomain should filter to organizations (`isOrganization: true`) to avoid matching a non-org team with the same slug/custom domain.
(Based on your team's feedback about validating team.isOrganization on org slug lookups.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/web/modules/ee/teams/components/MemberList.tsx">
<violation number="1" location="apps/web/modules/ee/teams/components/MemberList.tsx:188">
P2: Remove the console.log debug statement before merging to avoid logging org branding data in production.</violation>
</file>
<file name="apps/web/getNextjsOrgRewriteConfig.ts">
<violation number="1" location="apps/web/getNextjsOrgRewriteConfig.ts:7">
P1: Unguarded `JSON.parse` on an env var at module scope will crash the application at startup if `ALLOWED_HOSTNAMES` is misconfigured (e.g., missing quotes around values). Wrap in a try-catch with a meaningful error message, or parse as a simple comma-separated list instead.
(Based on your team's feedback about centralizing env-derived configuration and avoiding inline parsing.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/web/getNextjsOrgRewriteConfig.ts:103">
P2: Debug `console.log` left at module scope will fire unconditionally in production on every module load (build time, SSR, etc.). This should be removed or at minimum guarded behind a debug/development check.</violation>
</file>
<file name="apps/web/playwright/organization/organization-redirection.e2e.ts">
<violation number="1" location="apps/web/playwright/organization/organization-redirection.e2e.ts:271">
P3: Avoid `include` here and select only the fields used. `include: { hashedLink: true }` pulls all hashedLink columns unnecessarily.</violation>
</file>
<file name="packages/features/ee/organizations/lib/orgDomains.ts">
<violation number="1" location="packages/features/ee/organizations/lib/orgDomains.ts:158">
P2: Debug `console.log` left in production code. This runs on every request hitting a custom domain. Use the existing `log` (structured logger) already imported in this file, or remove it entirely.</violation>
</file>
<file name="apps/web/public/static/locales/en/common.json">
<violation number="1" location="apps/web/public/static/locales/en/common.json:9">
P2: Duplicate translation keys were added here; in JSON the later entry wins, so the newly added strings are silently overwritten and the file becomes ambiguous to maintain. Keep only one set of keys and update the existing entries instead of duplicating them.</violation>
</file>
<file name="apps/web/next.config.ts">
<violation number="1" location="apps/web/next.config.ts:378">
P2: Custom domain rewrites omit embed routes, so custom-domain embed pages won’t be rewritten to org-aware paths (unlike org-domain rewrites). Add custom-domain equivalents for `/embed` and `/:user/:type/embed` to keep embed flows working on custom domains.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| name: "TestOrg", | ||
| }); | ||
| customDomainSlug = `booking-${Math.random().toString(36).substring(7)}.testorg.com`; | ||
| await prisma.customDomain.create({ |
Contributor
There was a problem hiding this comment.
P1: Rule violated: E2E Tests Best Practices
This new test creates a customDomain record but never deletes it, leaving DB state mutated after the test. E2E tests must clean up all created records to avoid side effects and maintain consistent parallel runs (E2E Tests Best Practices: reset DB state after tests).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/playwright/embed-code-generator.e2e.ts, line 326:
<comment>This new test creates a customDomain record but never deletes it, leaving DB state mutated after the test. E2E tests must clean up all created records to avoid side effects and maintain consistent parallel runs (E2E Tests Best Practices: reset DB state after tests).</comment>
<file context>
@@ -316,6 +315,62 @@ test.describe("Embed Code Generator Tests", () => {
+ name: "TestOrg",
+ });
+ customDomainSlug = `booking-${Math.random().toString(36).substring(7)}.testorg.com`;
+ await prisma.customDomain.create({
+ data: { teamId: org.id, slug: customDomainSlug, verified: true },
+ });
</file context>
| name: "TestOrg", | ||
| }); | ||
| const customDomainSlug = `booking-${Math.random().toString(36).substring(7)}.testorg.com`; | ||
| await prisma.customDomain.create({ |
Contributor
There was a problem hiding this comment.
P1: Rule violated: E2E Tests Best Practices
E2E tests must reset DB state after running. This test inserts a customDomain record but never deletes it, leaving side effects that can affect parallel runs and repeatability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/playwright/reschedule.e2e.ts, line 440:
<comment>E2E tests must reset DB state after running. This test inserts a `customDomain` record but never deletes it, leaving side effects that can affect parallel runs and repeatability.</comment>
<file context>
@@ -434,12 +432,50 @@ test.describe("Reschedule Tests", async () => {
+ name: "TestOrg",
+ });
+ const customDomainSlug = `booking-${Math.random().toString(36).substring(7)}.testorg.com`;
+ await prisma.customDomain.create({
+ data: { teamId: org.id, slug: customDomainSlug, verified: true },
+ });
</file context>
| orgDetails = await prisma.team.findFirstOrThrow({ | ||
| where: { | ||
| slug: org, | ||
| OR: [ |
Contributor
There was a problem hiding this comment.
P2: Organization lookup by slug/customDomain should filter to organizations (isOrganization: true) to avoid matching a non-org team with the same slug/custom domain.
(Based on your team's feedback about validating team.isOrganization on org slug lookups.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/eventtypes/lib/getPublicEvent.ts, line 332:
<comment>Organization lookup by slug/customDomain should filter to organizations (`isOrganization: true`) to avoid matching a non-org team with the same slug/custom domain.
(Based on your team's feedback about validating team.isOrganization on org slug lookups.) </comment>
<file context>
@@ -325,33 +325,45 @@ export const getPublicEvent = async (
orgDetails = await prisma.team.findFirstOrThrow({
where: {
- slug: org,
+ OR: [
+ { slug: org },
+ { customDomain: { slug: org, verified: true } },
</file context>
Suggested change
| OR: [ | |
| isOrganization: true, | |
| OR: [ |
| "custom_domain_removed": "Custom domain removed successfully", | ||
| "remove_custom_domain": "Remove custom domain", | ||
| "remove_custom_domain_description": "Are you sure you want to remove {{domain}}? This will make your booking pages unavailable at this domain.", | ||
| "no_custom_domain_configured": "No custom domain configured yet", |
Contributor
There was a problem hiding this comment.
P2: Duplicate translation keys were added here; in JSON the later entry wins, so the newly added strings are silently overwritten and the file becomes ambiguous to maintain. Keep only one set of keys and update the existing entries instead of duplicating them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/public/static/locales/en/common.json, line 9:
<comment>Duplicate translation keys were added here; in JSON the later entry wins, so the newly added strings are silently overwritten and the file becomes ambiguous to maintain. Keep only one set of keys and update the existing entries instead of duplicating them.</comment>
<file context>
@@ -6,6 +6,19 @@
"custom_domain_removed": "Custom domain removed successfully",
"remove_custom_domain": "Remove custom domain",
"remove_custom_domain_description": "Are you sure you want to remove {{domain}}? This will make your booking pages unavailable at this domain.",
+ "no_custom_domain_configured": "No custom domain configured yet",
+ "enter_domain_to_check": "Enter a valid domain to check availability",
+ "checking_availability": "Checking availability...",
</file context>
| ] | ||
| : []), | ||
| // Custom domain rewrites - rewrite to /org/{hostname}/... | ||
| ...(customDomainRewriteConfig |
Contributor
There was a problem hiding this comment.
P2: Custom domain rewrites omit embed routes, so custom-domain embed pages won’t be rewritten to org-aware paths (unlike org-domain rewrites). Add custom-domain equivalents for /embed and /:user/:type/embed to keep embed flows working on custom domains.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/next.config.ts, line 378:
<comment>Custom domain rewrites omit embed routes, so custom-domain embed pages won’t be rewritten to org-aware paths (unlike org-domain rewrites). Add custom-domain equivalents for `/embed` and `/:user/:type/embed` to keep embed flows working on custom domains.</comment>
<file context>
@@ -336,6 +374,29 @@ const nextConfig = (phase: string): NextConfig => {
]
: []),
+ // Custom domain rewrites - rewrite to /org/{hostname}/...
+ ...(customDomainRewriteConfig
+ ? [
+ customDomainMatcherConfig.root
</file context>
| create: [{ link: generateHashedLink(eventType.id) }], | ||
| }, | ||
| }, | ||
| include: { hashedLink: true }, |
Contributor
There was a problem hiding this comment.
P3: Avoid include here and select only the fields used. include: { hashedLink: true } pulls all hashedLink columns unnecessarily.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/playwright/organization/organization-redirection.e2e.ts, line 271:
<comment>Avoid `include` here and select only the fields used. `include: { hashedLink: true }` pulls all hashedLink columns unnecessarily.</comment>
<file context>
@@ -206,3 +206,97 @@ test.describe("Unpublished Organization Redirection", () => {
+ create: [{ link: generateHashedLink(eventType.id) }],
+ },
+ },
+ include: { hashedLink: true },
+ });
+
</file context>
Contributor
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
- Replace page.getByText with page.getByTestId for E2E test locators in dynamic-booking-pages - Wrap ALLOWED_HOSTNAMES JSON.parse in try-catch to prevent startup crashes - Remove debug console.log from getNextjsOrgRewriteConfig.ts module scope - Remove debug console.log from MemberList.tsx - Replace console.log with structured logger in orgDomains.ts Co-Authored-By: unknown <>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Updates since last revision
Addressed Cubic AI review feedback (confidence ≥ 9/10 only):
page.getByText(...)withpage.getByTestId(...)indynamic-booking-pages.e2e.tscustom domain testALLOWED_HOSTNAMESJSON.parsein try-catch ingetNextjsOrgRewriteConfig.tsgetNextjsOrgRewriteConfig.ts(module-scope log firing on every load)MemberList.tsx(logging orgBranding data)orgDomains.ts(replaced with structuredlog.debug)Items for reviewer attention
page.getByTestId(attendee-name-${user.name})— verify that dynamic booking users appear as attendees (not only as hosts) on the success page, since the test IDattendee-name-*is on the attendee rendering path.ALLOWED_HOSTNAMESparse failure now silently falls back to[]with aconsole.error. Consider whether this should be a hard failure in production.Skipped Cubic issues (confidence < 9/10)
embed-code-generator.e2e.tsreschedule.e2e.tsgetPublicEvent.tsisOrganization: truefiltercommon.jsonnext.config.tsorganization-redirection.e2e.tsincludevsselectin testSummary by cubic
Extends custom domain support end-to-end so booking, team, user, forms, and embed pages render and link on verified org domains. Propagates custom domains through API v2 and web metadata so canonical links, embeds, and public URLs resolve on the org domain.
New Features
Refactors
Written for commit 24e9901. Summary will update on new commits.
Link to Devin run: https://app.devin.ai/sessions/862ae4549bc44a239e9f0f4b6ff1ee9c
Requested by: unknown ()