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: update spec #45

Merged
merged 2 commits into from
Nov 19, 2024
Merged

feat: update spec #45

merged 2 commits into from
Nov 19, 2024

Conversation

DanielRivers
Copy link
Contributor

Explain your changes

Updates to the latest spec

Checklist

🛟 If you need help, consider asking for advice over in the Kinde community.

Copy link
Contributor

coderabbitai bot commented Nov 19, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

The pull request introduces significant modifications across several TypeScript files, primarily focusing on schema definitions, service methods, and type definitions. A new schema for environment responses is added, and existing schemas are updated with new properties and type changes. Service methods are simplified by removing unnecessary parameters, enhancing usability. Additionally, type definitions are refined with new properties and stricter type constraints, improving type safety. Overall, these changes aim to enhance the clarity and functionality of the API.

Changes

File Change Summary
lib/api/schemas.gen.ts - New schema $get_environment_response added.
- Updated timestamp type in $get_event_response from string to integer.
- Updated exp and iat types in $token_introspect from string to integer.
- Deprecated logo in $get_organization_response; added logo_dark, favicon_svg, favicon_fallback.
lib/api/services.gen.ts - Removed data parameter from getApIs, getBusiness, getIndustries, getTimezones, getEventTypes.
- New method getEnvironment added.
- Default values added for data parameters in several methods, maintaining backward compatibility.
lib/api/types.gen.ts - New types get_environment_response, theme_code, and color_scheme added.
- Added properties has_clickwrap, has_kinde_branding, created_on to get_business_response.
- Updated timestamp type in get_event_response from string to number.
- Updated type in get_application_response to a specific union type.
lib/main.test.ts - New export $get_environment_response added to test expected exports in main.ts.

Possibly related PRs

  • test: test exported items #41: The changes in this PR involve the addition of a test for the $get_environment_response export in lib/main.test.ts, which directly relates to the new schema added in the main PR.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (6)
lib/api/schemas.gen.ts (5)

1227-1504: Ensure all date properties include the 'format' field.

In the newly added $get_environment_response schema, properties like created_on represent dates but lack the format: "date-time" field. Including the format improves schema accuracy and developer experience.

For example, update the created_on property:

            created_on: {
              type: "string",
+             format: "date-time",
              description: "Date of environment creation in ISO 8601 format.",
              example: "2021-01-01T00:00:00Z",
            },

1717-1754: Add 'format' to 'created_on' property and ensure consistency across date fields.

The created_on property in the $get_organization_response schema is missing the format: "date-time" field. Including this enhances clarity and aligns with OpenAPI standards.

Update the created_on property:

        created_on: {
          type: "string",
+         format: "date-time",
          description: "Date of organization creation in ISO 8601 format.",
          example: "2021-01-01T00:00:00Z",
        },

2527-2537: Secure handling of 'client_secret' in examples.

Including real-looking client_secret examples may pose a security risk or cause confusion. It's best to use placeholders that clearly indicate they are examples.

Apply this diff to update the client_secret example:

            client_secret: {
              description: "The application's client secret.",
              type: "string",
-             example: "sUJSHI3ZQEVTJkx6hOxdOSHaLsZkCBRFLzTNOI791rX8mDjgt7LC",
+             example: "<client_secret>",
            },

2561-2597: Consistent use of examples and inclusion of 'format' where applicable.

Ensure that all properties, especially dates and URLs, include appropriate formats and consistent example values for clarity.

For instance, add format: "uri" to URL fields and format: "date-time" to date fields.

            homepage_uri: {
              description: "The homepage link to your application.",
              type: "string",
+             format: "uri",
              example: "https://yourapp.com",
            },
            created_on: {
              type: "string",
+             format: "date-time",
              description: "Date of application creation in ISO 8601 format.",
              example: "2021-01-01T00:00:00Z",
            },

303-317: General suggestion: Add 'format' to all date-time string properties.

Across multiple schemas, properties representing dates and times are of type string but lack the format: "date-time" attribute. Adding this format improves schema compliance and developer understanding.

Consider updating all such properties accordingly.

Also applies to: 1227-1504, 1546-1754

lib/api/types.gen.ts (1)

1515-1519: Redundant Type Declaration for type

The type export type type = "m2m" | "reg" | "spa"; appears to duplicate the type definition already assigned to the type property in get_application_response. Consider removing this redundant type declaration to avoid confusion and potential conflicts.

Apply this diff to remove the redundant type:

-export type type = "m2m" | "reg" | "spa";
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 40e4eb0 and 61371e0.

⛔ Files ignored due to path filters (1)
  • spec/kinde-mgmt-api-specs.yaml is excluded by !**/*.yaml
📒 Files selected for processing (3)
  • lib/api/schemas.gen.ts (8 hunks)
  • lib/api/services.gen.ts (24 hunks)
  • lib/api/types.gen.ts (9 hunks)
🔇 Additional comments (15)
lib/api/services.gen.ts (3)

75-75: LGTM: New environment-related types

The addition of environment-related types properly supports the new environment functionality.


1355-1376: LGTM: Well-structured new environment endpoint

The new getEnvironment method is well-implemented with:

  • Clear JSDoc documentation
  • Proper permission code requirement (read:environments)
  • Consistent error handling
  • Type-safe response

398-398: ⚠️ Potential issue

Breaking Change: Method signatures updated with optional parameters

Multiple methods have been modified to make their parameters optional with default values. While this improves API usability, it's a breaking change that could affect existing code. Consider:

  1. Incrementing the major version number following semver
  2. Adding migration notes to the changelog
  3. Updating documentation to reflect the new optional parameters

Also applies to: 1848-1848, 1909-1909, 2609-2609, 2640-2640, 2737-2737, 2867-2867, 2966-2966, 2998-2998, 3226-3226, 3329-3329, 3428-3428

lib/api/schemas.gen.ts (2)

2875-2883: Verify consistency of timestamp properties throughout the schemas.

The exp and iat properties are correctly updated to integer types representing Unix epoch times. Ensure that the descriptions accurately reflect this.


1546-1571: 🛠️ Refactor suggestion

Deprecate the 'logo' property as indicated in the summary.

According to the AI-generated summary, the logo property is marked as deprecated. However, the schema lacks the deprecated: true attribute. To properly indicate deprecation, add this attribute.

Apply this diff to mark logo as deprecated:

        logo: {
          type: "string",
          nullable: true,
+         deprecated: true,
          description: "The organization's logo URL.",
          example: "https://yoursubdomain.kinde.com/logo?org_code=...",
        },

Likely invalid or redundant comment.

lib/api/types.gen.ts (10)

189-200: New Properties in get_business_response

The addition of has_clickwrap, has_kinde_branding, and created_on to the get_business_response type enhances the information available about a business entity. Ensure that all functions and components consuming this type are updated to handle these new optional properties appropriately.


784-907: Addition of get_environment_response Type

A new type get_environment_response has been added, providing comprehensive details about an environment. This includes properties for analytics tags, branding options, and theme settings. Ensure that any API calls or components that require environment data are updated to utilize this new type.


909-918: New Types for Theme Management

The types theme_code and color_scheme have been introduced to standardize theme settings across the application. They define specific allowed string literals, ensuring type safety when assigning theme-related properties.


945-959: Additional Branding Properties in get_organization_response

New optional properties such as logo_dark, favicon_svg, and favicon_fallback have been added to get_organization_response. These properties enhance the branding capabilities for organizations. Verify that the UI components render these assets correctly when provided.


1000-1023: Inclusion of Style Properties in get_organization_response

Properties for border radii (button_border_radius, card_border_radius, input_border_radius), theme_code, color_scheme, and created_on have been added. This allows for more granular control over the organization's styling. Ensure that these properties are appropriately utilized and that default values are in place when they are null.


1491-1491: Type Restriction for type in get_application_response

The type property in get_application_response has been changed from a general string to a union of specific string literals: "m2m" | "reg" | "spa". This enhances type safety by restricting possible values to known application types.


1508-1511: New Property has_cancel_button in get_application_response

The addition of the has_cancel_button property, marked as a beta feature, allows applications to indicate if they support a cancel button in the authentication flow. Ensure that this property is documented and properly handled in the application logic.


1708-1712: Addition of exp and iat in token_introspect

The properties exp (token expiration timestamp) and iat (token issuance timestamp) have been added to token_introspect. Ensure that any logic relying on token validation appropriately handles these new properties and that timestamp values are correctly interpreted.


2318-2318: Exported Type GetEnvironmentResponse

The type alias export type GetEnvironmentResponse = get_environment_response; has been added. This provides a more consistent naming convention for responses. Ensure that any references to environment responses use this new type alias where appropriate.


640-640: ⚠️ Potential issue

Type Change for timestamp in get_event_response

The timestamp property type in get_event_response has been changed from string to number, representing UNIX timestamps. Verify that all usages of this property in the codebase correctly handle the new number type and adjust any formatting or parsing logic accordingly.

Run the following script to identify places where timestamp is used as a string:

✅ Verification successful

Let's search for actual usage of the timestamp property to understand how it's being handled in the codebase. This will help us verify if there are any string-based operations or formatting being performed on the timestamp value.


Type change for timestamp appears consistent with schema definition

Based on the search results, the type change from string to number is actually consistent with the schema definition in lib/api/schemas.gen.ts which shows timestamp is defined as type: "integer". The JSDoc comment "Timestamp in ISO 8601 format" appears to be outdated and should be updated, but the type change itself is correct. No instances of string-based timestamp handling were found in the codebase that would be affected by this change.

  • Note: The JSDoc comment in lib/api/types.gen.ts should be updated to reflect that it's a UNIX timestamp number, not an ISO 8601 string format.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find occurrences where `timestamp` is used as a string.

# Search for `timestamp` properties typed as `string`
rg --type=typescript 'timestamp\s*:\s*string'

Length of output: 82


Script:

#!/bin/bash
# Search for timestamp usage with ripgrep, including TypeScript and JavaScript files
rg -t ts -t js 'timestamp\s*:\s*string'

# Also search for any timestamp property usage to understand the context
rg -t ts -t js 'timestamp[?:]' -A 2 -B 2

Length of output: 560


Script:

#!/bin/bash
# Search for timestamp property access or manipulation
ast-grep --pattern 'timestamp'

# Search for any string operations or Date operations that might be using timestamp
rg -t ts -t js '(new Date|\.toString|String)\s*\(\s*\w+\.timestamp'

Length of output: 159

public static deleteEnvironementFeatureFlagOverrides(
data: DeleteEnvironementFeatureFlagOverridesData,
): CancelablePromise<DeleteEnvironementFeatureFlagOverridesResponse> {
public static deleteEnvironementFeatureFlagOverrides(): CancelablePromise<DeleteEnvironementFeatureFlagOverridesResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typos in method names

The word "Environment" is misspelled as "Environement" in two method names:

  • deleteEnvironementFeatureFlagOverrides
  • getEnvironementFeatureFlags

Apply this fix to correct the spelling:

-  public static deleteEnvironementFeatureFlagOverrides(): CancelablePromise<DeleteEnvironementFeatureFlagOverridesResponse> {
+  public static deleteEnvironmentFeatureFlagOverrides(): CancelablePromise<DeleteEnvironmentFeatureFlagOverridesResponse> {

-  public static getEnvironementFeatureFlags(): CancelablePromise<GetEnvironementFeatureFlagsResponse> {
+  public static getEnvironmentFeatureFlags(): CancelablePromise<GetEnvironmentFeatureFlagsResponse> {

Note: This will require corresponding updates to the response type definitions and any existing usage of these methods.

Also applies to: 1412-1412

Comment on lines +314 to +316
type: "string",
description: "Date of business creation in ISO 8601 format.",
example: "2021-01-01T00:00:00Z",
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add 'format' to the 'created_on' property for consistency.

The created_on property is of type string and represents a date in ISO 8601 format. To comply with OpenAPI specifications and enhance schema clarity, consider adding the format: "date-time" field.

Apply this diff to include the format field:

             created_on: {
               type: "string",
+              format: "date-time",
               description: "Date of business creation in ISO 8601 format.",
               example: "2021-01-01T00:00:00Z",
             },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type: "string",
description: "Date of business creation in ISO 8601 format.",
example: "2021-01-01T00:00:00Z",
type: "string",
format: "date-time",
description: "Date of business creation in ISO 8601 format.",
example: "2021-01-01T00:00:00Z",

Comment on lines +981 to 983
type: "integer",
description: "Timestamp in ISO 8601 format.",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Resolve inconsistency between type and description of 'timestamp'.

The timestamp property is declared as an integer, but the description states "Timestamp in ISO 8601 format," which implies a string. To fix this, either change the type to string with the appropriate format or adjust the description to reflect a Unix epoch timestamp.

If the timestamp is a Unix epoch, update the description:

             timestamp: {
               type: "integer",
-              description: "Timestamp in ISO 8601 format.",
+              description: "Timestamp as a Unix epoch time.",
             },

If the timestamp should be an ISO 8601 string, update the type and add a format:

             timestamp: {
-              type: "integer",
+              type: "string",
+              format: "date-time",
               description: "Timestamp in ISO 8601 format.",
             },

Committable suggestion skipped: line range outside the PR's diff.

@DanielRivers DanielRivers merged commit 4e8f8cd into main Nov 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant