Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Single domain setup when only one org in the system #18383

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Dec 27, 2024

What does this PR do?

See Readme update on how to use.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Browse through the app through multiple pages.

@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Dec 27, 2024
Copy link
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:

No release type found in pull request title "Support acme.cal.com to be the org domain even when acme.cal.com is t…". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@github-actions github-actions bot added enterprise area: enterprise, audit log, organisation, SAML, SSO High priority Created by Linear-GitHub Sync Low priority Created by Linear-GitHub Sync organizations area: organizations, orgs self-hosting labels Dec 27, 2024
@keithwillcode keithwillcode added the core area: core, team members only label Dec 27, 2024
@hariombalhara hariombalhara changed the title Support acme.cal.com to be the org domain even when acme.cal.com is t… feat: Single domain setup with one org Dec 27, 2024
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to getNextjsOrgRewriteConfig and made it more robust with better tests.

* app.company.cal.com -> app
* app.company.com -> app
*/
const getLeftMostSubdomain = (url) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed getDefaultSubdomain -> getLeftMostSubdomain

return _url.hostname.match(regex)?.groups?.subdomain || null;
};

const getRegExpNotMatchingLeftMostSubdomain = (url) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed getSubdomainRegExp -> getRegExpNotMatchingLeftMostSubdomain

@@ -124,7 +123,7 @@ const matcherConfigRootPath = {
has: [
{
type: "host",
value: orgHostPath,
value: nextJsOrgRewriteConfig.orgHostPath,
Copy link
Member Author

Choose a reason for hiding this comment

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

if Single org setup isn't enabled the regex here should remain same

@@ -328,23 +328,23 @@ const nextConfig = {
? [
{
...matcherConfigRootPath,
destination: "/team/:orgSlug?isOrgProfile=1",
destination: `/team/${orgSlug}?isOrgProfile=1`,
Copy link
Member Author

Choose a reason for hiding this comment

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

if single org setup isn't enabled, orgSlug = ":orgSlug"

Copy link
Contributor

github-actions bot commented Dec 27, 2024

E2E results are ready!

@hariombalhara hariombalhara removed the Low priority Created by Linear-GitHub Sync label Dec 27, 2024
Copy link

vercel bot commented Dec 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Jan 6, 2025 9:52pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Jan 6, 2025 9:52pm

@hariombalhara
Copy link
Member Author

@calcom/foundation for next.config.js review

@hariombalhara hariombalhara marked this pull request as ready for review December 31, 2024 11:31
@graphite-app graphite-app bot requested a review from a team December 31, 2024 11:32
@dosubot dosubot bot added the ✨ feature New feature or request label Dec 31, 2024
Copy link

graphite-app bot commented Dec 31, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/31/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Comment on lines -123 to -139
const matcherConfigRootPath = {
has: [
{
type: "host",
value: orgHostPath,
},
],
source: "/",
};

const matcherConfigRootPathEmbed = {
has: [
{
type: "host",
value: orgHostPath,
},
],
Copy link
Member Author

Choose a reason for hiding this comment

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

Clubbed them under a single object.

@@ -322,29 +327,33 @@ const nextConfig = {
// These rewrites are other than booking pages rewrites and so that they aren't redirected to org pages ensure that they happen in beforeFiles
...(isOrganizationsEnabled
? [
orgDomainMatcherConfig.root
Copy link
Member Author

Choose a reason for hiding this comment

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

Allows to conditionally enable rewrite to organization for root path.

We disable it for single domain setup of org

@hariombalhara hariombalhara changed the title feat: Single domain setup with one org feat: Single domain setup when only one org in the system Dec 31, 2024
Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

LGTM. Appreciate the tests as well 🙏🏽

@hariombalhara hariombalhara merged commit 6f531d0 into main Jan 7, 2025
38 checks passed
@hariombalhara hariombalhara deleted the single-org-single-domain-setup branch January 7, 2025 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ .env changes contains changes to env variables ✨ feature New feature or request High priority Created by Linear-GitHub Sync organizations area: organizations, orgs ready-for-e2e self-hosting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-4457] Support organization and dashboard on the same domain when there is one organization in the system
4 participants