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 project schemas and environment schemas along with their types to @keyshade/schema #538

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 16, 2024

User description

Description

@keyshade/schema

  • Add schemas and types for project and environment.
  • Add tests for project and environment schemas.
  • Export both schemas from index and their types from index.types
  • Remove existing project.ts and environment.ts. Modify project schema to include new CreateEnvironmentRequestSchema.

@keyshade/api-client

  • Remove existing project.types and environment.types.
  • Change imports for project and environment 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.

PR Type

enhancement, tests


Description

  • Migrated environment and project schemas and types from api-client to @keyshade/schema.
  • Added new zod schemas for environment and project in the schema package.
  • Updated api-client to use the new schemas from @keyshade/schema.
  • Removed old type definitions from api-client.
  • Added comprehensive tests for the new environment and project schemas.

Changes walkthrough 📝

Relevant files
Enhancement
10 files
environment.ts
Update environment controller to use new schema imports   

packages/api-client/src/controllers/environment.ts

  • Updated imports to use schemas from @keyshade/schema.
+1/-1     
project.ts
Update project controller to use new schema imports           

packages/api-client/src/controllers/project.ts

  • Updated imports to use schemas from @keyshade/schema.
+1/-1     
environment.types.d.ts
Remove environment type definitions from api-client           

packages/api-client/src/types/environment.types.d.ts

  • Removed environment type definitions.
+0/-55   
project.types.d.ts
Remove project type definitions from api-client                   

packages/api-client/src/types/project.types.d.ts

  • Removed project type definitions.
+0/-87   
environment.ts
Add environment schemas using zod                                               

packages/schema/src/environment/environment.ts

  • Added environment schemas using zod.
+56/-0   
environment.types.ts
Add inferred types for environment schemas                             

packages/schema/src/environment/environment.types.ts

  • Added inferred types for environment schemas.
+54/-0   
index.ts
Update index exports for environment and project                 

packages/schema/src/index.ts

  • Updated exports for environment and project schemas.
+5/-2     
index.types.ts
Update index type exports for environment and project       

packages/schema/src/index.types.ts

  • Updated type exports for environment and project.
+4/-11   
project.ts
Add project schemas using zod                                                       

packages/schema/src/project/project.ts

  • Added project schemas using zod.
+99/-0   
project.types.ts
Add inferred types for project schemas                                     

packages/schema/src/project/project.types.ts

  • Added inferred types for project schemas.
+62/-0   
Tests
2 files
environment.spec.ts
Add tests for environment schemas                                               

packages/schema/tests/environment.spec.ts

  • Added comprehensive tests for environment schemas.
+312/-8 
project.spec.ts
Add tests for project schemas                                                       

packages/schema/tests/project.spec.ts

  • Added comprehensive tests for project schemas.
+558/-21

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

Copy link
Contributor

codiumai-pr-agent-free bot commented Nov 16, 2024

PR Reviewer Guide 🔍

(Review updated until commit 00211b4)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

519 - Fully compliant

Compliant requirements:

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

Code Smell
The ProjectSchema validation logic for isForked and forkedFromId could be simplified by using zod's built-in conditional validation instead of a custom refine function

Possible Bug
The UpdateEnvironmentRequestSchema omits projectId but doesn't validate that at least one field is provided for update

Copy link
Contributor

codiumai-pr-agent-free bot commented Nov 16, 2024

PR Code Suggestions ✨

Latest suggestions up to 00211b4
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Security
Add array size validation to prevent potential abuse through excessive data

Add validation for the maximum number of environments that can be created in a
single project request to prevent potential abuse.

packages/schema/src/project/project.ts [29-36]

 export const CreateProjectRequestSchema = z.object({
   name: z.string(),
   workspaceSlug: z.string(),
   description: z.string().optional(),
   storePrivateKey: z.boolean().optional(),
-  environments: CreateEnvironmentRequestSchema.array().optional(),
+  environments: CreateEnvironmentRequestSchema.array().max(10).optional(),
   accessLevel: projectAccessLevelEnum
 })
  • Apply this suggestion
Suggestion importance[1-10]: 8

Why: This security enhancement prevents potential DoS attacks by limiting the number of environments that can be created in a single request, protecting server resources from abuse.

8
Best practice
Add length validation for string fields to enforce reasonable constraints

Add validation for the environment name length to ensure it meets reasonable
constraints (e.g., between 1 and 100 characters).

packages/schema/src/environment/environment.ts [15-19]

 export const CreateEnvironmentRequestSchema = z.object({
-  name: z.string(),
+  name: z.string().min(1).max(100),
   description: z.string().optional(),
   projectId: z.string()
 })
  • Apply this suggestion
Suggestion importance[1-10]: 7

Why: Adding string length validation is important for data integrity and security, preventing potential issues with extremely long or empty strings that could affect system performance or stability.

7

💡 Need additional feedback ? start a PR chat


Previous suggestions

Suggestions up to commit 00211b4
CategorySuggestion                                                                                                                                    Score
Possible issue
Add format validation for key fields to ensure data integrity

Add URL validation for publicKey and privateKey fields if they are expected to be
URLs, or add pattern validation if they should follow a specific format.

packages/schema/src/project/project.ts [14-15]

-publicKey: z.string(),
-privateKey: z.string(),
+publicKey: z.string().regex(/^[A-Za-z0-9+/=]+$/, 'Invalid public key format'),
+privateKey: z.string().regex(/^[A-Za-z0-9+/=]+$/, 'Invalid private key format'),
Suggestion importance[1-10]: 8

Why: Adding regex validation for public and private keys is crucial for security and data integrity, ensuring that these sensitive fields contain only valid base64 characters.

8
Best practice
Add maximum length validation for text fields to prevent data issues

Add validation for description field length to prevent excessively long
descriptions. Consider adding a max length constraint of 500-1000 characters.

packages/schema/src/project/project.ts [11]

-description: z.string(),
+description: z.string().max(500, 'Description must be 500 characters or less'),
Suggestion importance[1-10]: 7

Why: Adding a maximum length constraint for the description field is a good practice to prevent potential database or UI issues from excessively long text. This is particularly important for fields that store user input.

7
Add input validation rules for name fields to prevent invalid data

Add validation for name field to ensure it contains valid characters and reasonable
length limits.

packages/schema/src/environment/environment.ts [6]

-name: z.string(),
+name: z.string().min(1).max(100).regex(/^[a-zA-Z0-9-_\s]+$/, 'Name can only contain alphanumeric characters, spaces, hyphens and underscores'),
Suggestion importance[1-10]: 7

Why: Adding length and character constraints for the name field helps prevent invalid or malformed environment names, improving data quality and user experience.

7

@muntaxir4 muntaxir4 requested a review from rajdip-b November 16, 2024 18:10
@muntaxir4 muntaxir4 changed the title feat(schema, api-client): Migrate environment types and schemas to @keyshade/schema feat(schema, api-client): Migrate environment types and schemas to @keyshade/schema Nov 16, 2024
packages/schema/src/index.ts Outdated Show resolved Hide resolved
packages/schema/src/index.types.ts Outdated Show resolved Hide resolved
@muntaxir4 muntaxir4 changed the title feat(schema, api-client): Migrate environment types and schemas to @keyshade/schema feat(schema, api-client): Migrate project schemas and environment schemas along with their types to @keyshade/schema Nov 18, 2024
@muntaxir4 muntaxir4 closed this Nov 18, 2024
@muntaxir4 muntaxir4 deleted the environment-schemas branch November 18, 2024 07:21
@muntaxir4 muntaxir4 restored the environment-schemas branch November 18, 2024 07:22
@muntaxir4 muntaxir4 reopened this Nov 18, 2024
Copy link
Contributor

Persistent review updated to latest commit 00211b4

@rajdip-b
Copy link
Member

If you can make the changes, I'll get this merged asap.

@rajdip-b rajdip-b force-pushed the environment-schemas branch from 00211b4 to c6cf55e Compare November 18, 2024 10:14
@muntaxir4
Copy link
Contributor Author

If you can make the changes, I'll get this merged asap.

Made the required changes.

@muntaxir4 muntaxir4 requested a review from rajdip-b November 18, 2024 11:22
@rajdip-b rajdip-b merged commit c468af0 into keyshade-xyz:develop Nov 18, 2024
5 checks passed
@muntaxir4 muntaxir4 deleted the environment-schemas branch November 18, 2024 12:01
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