Closed muntaxir4 closed 4 days ago
Here are some key observations to aid the review process:
**π« Ticket compliance analysis β ** **[519](https://github.com/keyshade-xyz/keyshade/issues/519) - Fully compliant** Compliant requirements: - Created event folder with required files in schema package - Added zod schemas and types for event module - Updated exports in index files - Updated api-client to use types from schema |
β±οΈ Estimated effort to review: 2 π΅π΅βͺβͺβͺ |
π§ͺ PR contains tests |
π No security concerns identified |
β‘ Recommended focus areas for review Code Smell The metadata object in GetEventsResponseSchema has many optional fields which could indicate a design that's not fully normalized or consistent |
Explore these optional code suggestions:
Category | Suggestion | Score |
General |
Enforce proper date-time format validation for timestamp fields___ **Add validation for the timestamp field to ensure it's a valid ISO 8601 date stringusing z.string().datetime() instead of z.string()** [packages/schema/src/event/index.ts [23]](https://github.com/keyshade-xyz/keyshade/pull/546/files#diff-1b98dc2071de1bad81f6136c16d3d19d4f58c7323e1acf8dd9bea4a6475888bfR23-R23) ```diff -timestamp: z.string(), +timestamp: z.string().datetime(), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8Why: Using z.string().datetime() ensures timestamp values are valid ISO 8601 dates, preventing potential runtime errors from invalid date formats. This is crucial for data integrity and system reliability. | 8 |
Add proper validation for slug format fields___ **Add refinement validation to ensure workspaceSlug is not empty and follows a validslug format using z.string().min(1).regex()** [packages/schema/src/event/index.ts [11]](https://github.com/keyshade-xyz/keyshade/pull/546/files#diff-1b98dc2071de1bad81f6136c16d3d19d4f58c7323e1acf8dd9bea4a6475888bfR11-R11) ```diff -workspaceSlug: z.string(), +workspaceSlug: z.string().min(1).regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: Adding regex validation for workspaceSlug ensures proper slug format and prevents invalid characters, enhancing data validation and URL compatibility. | 7 | |
Prevent unwanted whitespace in string fields___ **Add .trim() validation to string fields that shouldn't contain leading/trailingwhitespace** [packages/schema/src/event/index.ts [34-35]](https://github.com/keyshade-xyz/keyshade/pull/546/files#diff-1b98dc2071de1bad81f6136c16d3d19d4f58c7323e1acf8dd9bea4a6475888bfR34-R35) ```diff -title: z.string(), -description: z.string(), +title: z.string().trim(), +description: z.string().trim(), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 5Why: Adding .trim() validation helps maintain data cleanliness by removing unwanted whitespace from user input, improving data consistency and user experience. | 5 |
π‘ Need additional feedback ? start a PR chat
Fixed ProjectSchema issues. This could be used in #540
User description
Description
@keyshade/schema
index
and its types fromindex.types
enums.ts
."main": "dist/src/index.js"
to"main": "dist/src/index.types.js"
inpackage.json
due to default export beingindex.types.ts
.@keyshade/api-client
event.types.d.ts
.@keyshade/schema
.Related to #519
Developer's checklist
If changes are made in the code:
Documentation Update
PR Type
enhancement, tests
Description
api-client
to@keyshade/schema
.schema
package.ProjectSchema
to useprojectAccessLevelEnum
.package.json
to reflect changes in the main entry point.Changes walkthrough π
8 files
event.ts
Update event type imports to use schema package
packages/api-client/src/controllers/event.ts - Updated imports for event types to use `@keyshade/schema`.
event.types.d.ts
Remove deprecated event types file
packages/api-client/src/types/event.types.d.ts
@keyshade/schema
.enums.ts
Add event-related enums
packages/schema/src/enums.ts - Added enums for event source, triggerer, and severity.
index.ts
Add event schemas using zod
packages/schema/src/event/index.ts
GetEventsRequest
andGetEventsResponse
.index.types.ts
Define event types using zod inference
packages/schema/src/event/index.types.ts
GetEventsRequest
andGetEventsResponse
using zodinference.
index.ts
Export event schemas from index
packages/schema/src/index.ts - Exported event schemas from the main index file.
index.types.ts
Export event types from index types
packages/schema/src/index.types.ts - Exported event types from the main index types file.
index.ts
Use project access level enum in schema
packages/schema/src/project/index.ts - Updated `ProjectSchema` to use `projectAccessLevelEnum`.
3 files
enums.test.ts
Add tests for event enums
packages/schema/tests/enums.test.ts - Added tests for event-related enums.
event.spec.ts
Add tests for event schemas
packages/schema/tests/event.spec.ts
GetEventsRequestSchema
andGetEventsResponseSchema
.project.spec.ts
Update project schema tests for access level enum
packages/schema/tests/project.spec.ts - Updated tests to reflect changes in `ProjectSchema` access level.
1 files
package.json
Update main entry point in package.json
packages/schema/package.json - Changed main entry point to `dist/src/index.types.js`.