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(schema, api-client): Migrate variable schemas and types to @keyshade/schema #545

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 19, 2024

Description

@keyshade/schema

  • Add schemas and types for variable.
  • Add tests for variable schemas.
  • Export variable schemas from index and its types from index.types
  • Add InviteMemberRequest and InviteMemberResponse schemas in workspace.
  • Create .npmignore to exclude source ts files during installing in an app.

@keyshade/api-client

  • Remove existing variable.types.ts.
  • Change imports for variable types to @keyshade/schema.

Related to #519

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

519 - Fully compliant

Compliant requirements:

  • Created variable folder in schema with proper structure
  • Added variable.ts with zod schemas
  • Added variable.types.ts with inferred types
  • Updated api-client to use types from schema
  • Added exports in index files
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Code Smell
The RollBackVariableResponseSchema uses string type for count field which should typically be a number for consistency

Validation Gap
The CreateVariableRequestSchema allows empty arrays for entries which could lead to invalid states

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Add validation to ensure version numbers are positive integers to prevent invalid rollback requests

Add validation for the version number to ensure it's a positive integer in the
RollBackVariableRequestSchema

packages/schema/src/variable/index.ts [78-82]

 export const RollBackVariableRequestSchema = z.object({
   variableSlug: z.string(),
-  version: z.number(),
+  version: z.number().int().positive(),
   environmentSlug: z.string()
 })
  • Apply this suggestion
Suggestion importance[1-10]: 7

Why: The suggestion adds important validation to prevent invalid version numbers in rollback requests, which could cause runtime errors or unexpected behavior. Using positive integers is a logical constraint for version numbers.

7
General
Add validation to prevent empty workspace IDs in project references

Add validation for the project workspaceId to ensure it's a non-empty string in the
VariableSchema

packages/schema/src/variable/index.ts [13-15]

 project: z.object({
-  workspaceId: z.string()
+  workspaceId: z.string().min(1)
 }),
  • Apply this suggestion
Suggestion importance[1-10]: 5

Why: Adding minimum length validation for workspaceId helps prevent empty strings, though the base string validation would already catch null/undefined. The impact is moderate as empty strings are an edge case.

5

💡 Need additional feedback ? start a PR chat

@rajdip-b rajdip-b merged commit 0ee8f9a into keyshade-xyz:develop Nov 20, 2024
9 checks passed
@muntaxir4 muntaxir4 deleted the variable-schemas branch November 23, 2024 21:16
rajdip-b pushed a commit that referenced this pull request Dec 3, 2024
## [2.8.0](v2.7.0...v2.8.0) (2024-12-03)

### 🚀 Features

* **api:** Add workspace removal notification email template ([#476](#476)) ([40b754f](40b754f))
* **cli:** Store `metrics_enabled` key in profile config ([#536](#536)) ([9283b22](9283b22))
* **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([#557](#557)) ([126d024](126d024))
* **platform:** Added screen for CREATE NEW PROJECT ([#540](#540)) ([b644633](b644633))
* **platform:** Updated the empty state of dashboard ([#522](#522)) ([28739d9](28739d9))
* **schema, api-client:** Migrate auth types to @keyshade/schema ([#532](#532)) ([d880098](d880098))
* **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([#546](#546)) ([a3267de](a3267de))
* **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([#547](#547)) ([08868c3](08868c3))
* **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([#538](#538)) ([c468af0](c468af0))
* **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([#539](#539)) ([bc3100b](bc3100b))
* **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([#535](#535)) ([c24695e](c24695e))
* **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([#545](#545)) ([0ee8f9a](0ee8f9a))
* **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([#569](#569)) ([4398969](4398969))
* **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([#568](#568)) ([9efbf2d](9efbf2d))
* **schema:** Add User type inference from UserSchema ([#574](#574)) ([84c1db5](84c1db5))

### 🐛 Bug Fixes

* **api:** Incorrect oauth redirect url ([58d96e5](58d96e5))
* **platform:** Resolve loading SVG blocking input field interaction ([#571](#571)) ([30f4f65](30f4f65))

### 📚 Documentation

* Add pictures to Bruno setup ([#541](#541)) ([210c0fd](210c0fd))
* Migrate to Bruno ([#525](#525)) ([1793d92](1793d92))

### 🔧 Miscellaneous Chores

* **ci:** Add script to validate schema package ([59e4280](59e4280))
* Fixed codecov client version ([a998ae4](a998ae4))
* **package:** Fixed tests and did housekeeping ([#544](#544)) ([40008e3](40008e3))
* Update test coverage settings ([5b27e32](5b27e32))
* Update Turbo to 2.3.1 ([#564](#564)) ([3a63823](3a63823))
* **web:** Update dockerfile ([10d9cc5](10d9cc5))

### 🔨 Code Refactoring

* **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([#520](#520)) ([7c8ee5d](7c8ee5d))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants