-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: add multi-org domain tracking #40938
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
Conversation
WalkthroughThis update introduces a cookie-based mechanism for tracking recently visited multi-organization domains, integrates recent domain suggestions into the sign-up page, and updates UI constants for sign-in messaging. The Applications page now tracks the current domain on mount. No existing exports or logic were removed or modified. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignUpPage
participant multiOrgDomainsUtils
User->>SignUpPage: Loads SignUp page
SignUpPage->>multiOrgDomainsUtils: getRecentDomains()
multiOrgDomainsUtils-->>SignUpPage: Returns recent domains list
SignUpPage-->>User: Displays recent domains section (if applicable)
User->>SignUpPage: Clicks "Open" on a domain
SignUpPage->>User: Redirects to selected domain's login page
sequenceDiagram
participant ApplicationsPage
participant multiOrgDomainsUtils
ApplicationsPage->>multiOrgDomainsUtils: trackCurrentDomain()
multiOrgDomainsUtils->>multiOrgDomainsUtils: Update recent domains cookie
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/client/src/pages/UserAuth/SignUp.tsx (1)
69-80
:⚠️ Potential issueDefer
getRecentDomains()
until after module load
getRecentDomains()
toucheswindow
/document
at module top-level.
This breaks when the file is required in a non-browser context (SSR, Jest without JSDOM) and also freezes the list for the entire lifetime of the bundle.-const recentDomains = getRecentDomains(); +// inside the component so that `window` is guaranteed to exist +const recentDomains = useMemo(() => getRecentDomains(), []);Place it right after the other React hooks in
SignUp
.
🧹 Nitpick comments (2)
app/client/src/ce/constants/messages.ts (1)
90-91
: Check trailing space/punctuation for multi-org login messages.
YOU_VE_ALREADY_SIGNED_INTO returns"You've already signed into"
without a trailing space or punctuation. When appending domain names, this could lead to text likeYou've already signed intomyorg.com
. Consider including a space or colon at the end:-export const YOU_VE_ALREADY_SIGNED_INTO = () => `You've already signed into`; +export const YOU_VE_ALREADY_SIGNED_INTO = () => `You've already signed into: `;app/client/src/pages/UserAuth/SignUp.tsx (1)
253-265
: Minor: inline tailwind-style string escapes are noisyThe literal
text-[color:var(--ads-v2\-color-fg)]
class works, but the escaped backslash breaks readability and is easy to mistype.
Consider extracting a CSS module or styled-component instead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/src/ce/constants/messages.ts
(1 hunks)app/client/src/ce/pages/Applications/index.tsx
(2 hunks)app/client/src/pages/UserAuth/SignUp.tsx
(7 hunks)app/client/src/utils/multiOrgDomains.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/client/src/ce/pages/Applications/index.tsx (1)
app/client/src/utils/multiOrgDomains.ts (1)
trackCurrentDomain
(55-78)
app/client/src/pages/UserAuth/SignUp.tsx (5)
app/client/src/utils/multiOrgDomains.ts (1)
getRecentDomains
(80-89)app/client/packages/design-system/widgets/src/components/Text/src/Text.tsx (1)
Text
(66-66)app/client/src/ce/constants/messages.ts (6)
YOU_VE_ALREADY_SIGNED_INTO
(91-91)USING_APPSMITH
(90-90)SIGN_IN_TO_AN_EXISTING_ORGANISATION
(92-93)ALREADY_HAVE_AN_ACCOUNT
(86-86)SIGNUP_PAGE_LOGIN_LINK_TEXT
(83-83)SIGNUP_PAGE_TITLE
(74-74)app/client/packages/design-system/widgets/src/components/Button/src/Button.tsx (1)
Button
(86-86)app/client/src/constants/routes/baseRoutes.ts (2)
ORG_LOGIN_PATH
(32-32)AUTH_LOGIN_URL
(25-25)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/utils/multiOrgDomains.ts (2)
36-43
: Consider adding theSecure
attribute when possibleThe cookie is always scoped to
.appsmith.com
and markedSameSite=Lax
, which is great.
However, it is never markedSecure
, so it will also be sent over plain-HTTP connections if such endpoints ever exist. That weakens the protection we gain fromSameSite
.- document.cookie = `${COOKIE_NAME}=${cookieValue}; expires=${expires.toUTCString()}; domain=.appsmith.com; path=/; SameSite=Lax`; + const secure = window.location.protocol === "https:" ? "; Secure" : ""; + document.cookie = `${COOKIE_NAME}=${cookieValue}; expires=${expires.toUTCString()}; domain=.appsmith.com; path=/; SameSite=Lax${secure}`;(Bump the same flag in
clearRecentDomains()
for symmetry.)
[ suggest_essential_refactor ]
55-78
: Filter expired entries before enforcingMAX_DOMAINS
domains.slice(0, MAX_DOMAINS)
is executed before the 30-day expiry filter.
If the top N items are stale, the final list can end up shorter than intended and the fresh entry might be pushed out.- domains = domains.slice(0, MAX_DOMAINS); - - const thirtyDaysAgo = currentTime - 30 * 24 * 60 * 60 * 1000; - domains = domains.filter((entry) => entry.timestamp > thirtyDaysAgo); + const THIRTY_DAYS_MS = EXPIRY_DAYS * 24 * 60 * 60 * 1000; + + // prune old entries first + domains = domains.filter( + (entry) => currentTime - entry.timestamp < THIRTY_DAYS_MS, + ); + + // then cap the list length + domains = domains.slice(0, MAX_DOMAINS);This keeps the list dense with live domains and avoids an unnecessary cookie write churn.
[ suggest_optional_refactor ]app/client/src/ce/pages/Applications/index.tsx (1)
143-156
: Import + invocation look good
trackCurrentDomain()
is imported once and invoked insidecomponentDidMount
, guaranteeing a single write per page visit – fits the intended tracking cadence.
No further action required.
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15672926050. |
Deploy-Preview-URL: https://ce-40938.dp.appsmith.com |
🏢 Multi-Org Domain Tracking System
🎯 Problem Statement
Users with access to multiple Appsmith organizations often forget the exact domain names of their different workspaces (e.g.,
company1.appsmith.com
,team2.appsmith.com
). This creates friction during login as users need to remember or guess the specific subdomain for each organization they belong to.✨ Solution Overview
This PR implements a lightweight domain tracking system that remembers the organization domains users have successfully accessed in the last 30 days. When users visit
login.appsmith.com
or signup pages, they can see and quickly navigate to their recently visited organizations.🏗️ Technical Approach
Why Cookies Over localStorage?
We chose cookies with
.appsmith.com
domain over localStorage for the following critical reasons:.appsmith.com
🛠️ Implementation Details
Core Utility (
utils/multiOrgDomains.ts
)📊 Domain Tracking Logic
/applications
pagecompany.appsmith.com
) + timestamplogin.appsmith.com
)Automation
/ok-to-test tags="@tag.Authentication, @tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15676451597
Commit: 935ccef
Cypress dashboard.
Tags:
@tag.Authentication, @tag.Sanity
Spec:
Mon, 16 Jun 2025 09:43:36 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit