-
Notifications
You must be signed in to change notification settings - Fork 32
feat: B2B Node SDK #182
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: B2B Node SDK #182
Conversation
anmol-stytch
left a comment
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.
Nit in LoginOrSignupByEmailResponse but other than that it looks good to me
lib/b2b/magic_links.ts
Outdated
| export interface LoginOrSignupByEmailResponse extends BaseResponse { | ||
| member_id: string; | ||
| member: Member; | ||
| member_creared: boolean; |
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.
nit: member_created
taronish-stytch
left a comment
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.
lgtm
| super(config); | ||
|
|
||
| if (!this.fetchConfig.baseURL.endsWith("b2b/")) { | ||
| this.fetchConfig.baseURL += "b2b/"; |
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.
Is this because we share a baseURL with b2c? Prefer just having two completely separate variables in that case like baseB2BURL to avoid having to do something like this
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.
Our base URL is our env string - stytch.envs.live or stytch.envs.test - I didn't want to introduce a separate set of envs for B2B, I think that would make things more confusing for end users / harder to document. One other option would be to embed the b2b/ URL in all the b2b-scoped client endpoints
| connection_id: string; | ||
| idp_entity_id?: string; | ||
| display_name?: string; | ||
| attribute_mapping?: Record<string, string>; |
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.
Unrelated to these changes - should we be adding validation in api that the value types in attribute_mapping are strings? We reuse the same entitymetadata that allows numeric values for trusted and untrusted metadata on other entities
danny-stytch
left a comment
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.
LGMT
Uh oh!
There was an error while loading. Please reload this page.