-
Notifications
You must be signed in to change notification settings - Fork 282
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(clerk-js): Add ability to set active organization by slug #3825
Changes from 5 commits
7ff078e
7c61adf
3414e6f
3aae4eb
fdb2c08
a9c1051
3483068
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
"@clerk/clerk-js": patch | ||
"@clerk/types": patch | ||
--- | ||
|
||
Introduce ability to set an active organization by slug |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -75,6 +75,7 @@ import { | |||||||||||||||
inBrowser, | ||||||||||||||||
isDevAccountPortalOrigin, | ||||||||||||||||
isError, | ||||||||||||||||
isOrganizationId, | ||||||||||||||||
isRedirectForFAPIInitiatedFlow, | ||||||||||||||||
noOrganizationExists, | ||||||||||||||||
noUserExists, | ||||||||||||||||
|
@@ -707,9 +708,18 @@ export class Clerk implements ClerkInterface { | |||||||||||||||
// However, if the `organization` parameter is not given (i.e. `undefined`), we want | ||||||||||||||||
// to keep the organization id that the session had. | ||||||||||||||||
const shouldSwitchOrganization = organization !== undefined; | ||||||||||||||||
|
||||||||||||||||
if (newSession && shouldSwitchOrganization) { | ||||||||||||||||
const organizationId = typeof organization === 'string' ? organization : organization?.id; | ||||||||||||||||
newSession.lastActiveOrganizationId = organizationId || null; | ||||||||||||||||
const organizationIdOrSlug = typeof organization === 'string' ? organization : organization?.id; | ||||||||||||||||
|
||||||||||||||||
if (isOrganizationId(organizationIdOrSlug!)) { | ||||||||||||||||
newSession.lastActiveOrganizationId = organizationIdOrSlug || null; | ||||||||||||||||
izaaklauer marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
} else { | ||||||||||||||||
const matchingOrganization = newSession.user.organizationMemberships.find( | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have two concerns here, borne of my own experience: 1: 🤔 How confident are we that 2: 🤔 Even if we can depend on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be happy to pair and trace to find those answers with you @wobsoriano . And I think it's likely that someone with more experience may know the answer to these off-hand - they strike me as foundational-sdk-philosophy questions 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I happened to chat with @BRKalow , and I got my answers! 1: The magic here is that, when someone calls useClerk, they're getting IsomorphicClerk, who'se implementation of setActive is here: javascript/packages/react/src/isomorphicClerk.ts Lines 602 to 608 in 427fcde
If clerk hasn't loaded the client by the time the user calls setActive, it'll return 2: This isn't client piggybacking supplying this data. Client piggybacking is us returning the latest client data on api calls other than the explicit call to load the client, but it's the explicit call that we're making when clerk loads. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good client piggybacking intro. Thanks izaak! |
||||||||||||||||
mem => mem.organization.slug === organizationIdOrSlug, | ||||||||||||||||
); | ||||||||||||||||
newSession.lastActiveOrganizationId = matchingOrganization?.organization.id || null; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// If this.session exists, then signOut was triggered by the current tab | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { isOrganizationId } from '../organization'; | ||
|
||
describe('isOrganizationId(string)', () => { | ||
it('should return true for strings starting with `org_`', () => { | ||
expect(isOrganizationId('org_123')).toBe(true); | ||
expect(isOrganizationId('org_abc')).toBe(true); | ||
}); | ||
|
||
it('should return false for strings not starting with `org_`', () => { | ||
expect(isOrganizationId('user_123')).toBe(false); | ||
expect(isOrganizationId('123org_')).toBe(false); | ||
expect(isOrganizationId('ORG_123')).toBe(false); | ||
expect(isOrganizationId('')).toBe(false); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/** | ||
* Checks and assumes a string is an organization ID if it starts with 'org_', specifically for | ||
disambiguating with slugs. `_` is a disallowed character in slug names, so slugs cannot | ||
start with `org_`. | ||
*/ | ||
export function isOrganizationId(id: string) { | ||
izaaklauer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return id.startsWith('org_'); | ||
} |
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.
I'd probably opt for removing the
!
and addfalsy
handling toisOrganizationId
+ add tests for passing inundefined
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.
Updated!