-
Notifications
You must be signed in to change notification settings - Fork 44
adding three new conection types to node SDK connection type enums #1389
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
base: main
Are you sure you want to change the base?
Conversation
Greptile OverviewGreptile SummaryAdded three new OIDC connection types (
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Enum as ConnectionType Enum
participant SDK as WorkOS SDK
participant API as WorkOS API
Dev->>Enum: Add CleverOIDC, EntraIdOIDC, OktaOIDC
Note over Enum: New connection types<br/>added to enum
SDK->>Enum: Import ConnectionType
SDK->>API: Send connection request<br/>with new type
API-->>SDK: Handle new connection types
SDK-->>Dev: Return connection response
|
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.
1 file reviewed, 1 comment
| Auth0SAML = 'Auth0SAML', | ||
| AzureSAML = 'AzureSAML', | ||
| CasSAML = 'CasSAML', | ||
| CleverOIDC = 'CleverOIDC', |
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.
style: Inconsistent casing: this uses OIDC (all caps) while existing AdpOidc and LoginGovOidc use Oidc (camelCase). GenericOIDC uses caps, so there's already inconsistency in the codebase.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/sso/interfaces/connection-type.enum.ts
Line: 8:8
Comment:
**style:** Inconsistent casing: this uses `OIDC` (all caps) while existing `AdpOidc` and `LoginGovOidc` use `Oidc` (camelCase). `GenericOIDC` uses caps, so there's already inconsistency in the codebase.
How can I resolve this? If you propose a fix, please make it concise.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.
These casings match the API. Looks good
| Auth0SAML = 'Auth0SAML', | ||
| AzureSAML = 'AzureSAML', | ||
| CasSAML = 'CasSAML', | ||
| CleverOIDC = 'CleverOIDC', |
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.
These casings match the API. Looks good
Description
Added the following connection types:
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.