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

Added Zod Schema Validation and Error Handling #8

Merged

Conversation

Gabriel-Mbugua
Copy link
Contributor

This PR introduces Zod schema validation and error handling to the project. This is to address issue #4.

1. Zod Schema Validation:

  • Added validate middleware that uses Zod to validate request bodies.
  • Created specific Zod schemas for user registration and recall petition endpoints.

2: Zod Error Handling:

  • Introduced a zodErrorHandler middleware to handle Zod validation errors specifically for routes that require schema validation.

Changes Made

### 1:Middleware:

        - middleware/validate.ts: Middleware to validate request bodies using Zod schemas.
        - middleware/zodErrorHandler.ts: Middleware to handle errors thrown by Zod validation.

### 2: Validation:
        - validation/user.validate.ts: Schema for validating user registration data.
        - validation/recall.validate.ts: Schema for validating recall petition data.
        - validation/index.ts: Export file for all schemas.

### 3: Routes:

        - routes/user.route.ts: Applied Zod validation and error handling for user registration.

Request for Review

Please review the changes to ensure they align with our project's best practices. Specifically, I'd appreciate feedback on:

  1. Schema Definitions: Are the Zod schemas comprehensive and correct?
  2. Middleware Logic: Is the separation of validation and error handling logic appropriate?
  3. Error Handling: Does the approach to error handling maintain clarity and consistency?
  4. Best Practices: Are there any areas where we can improve or align better with our coding standards?

Looking forward to your feedback and suggestions for any improvements.

Thank you!

@@ -0,0 +1,14 @@
import { Request, Response, NextFunction, Router } from 'express';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move the middlewares to a commons package. Only retain the module-specific middleware in the recall api

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also use the folder structure in the packages/recall-api/module/example to create modules.

@@ -82,7 +83,7 @@ export default async ({ app }: { app: Application }) => {
* Add more versions of the api below
*/

// app.use('/api/v1');
app.use('/api/v1/users', userRoutes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The routes are auto-registered if you use the decorators in the routing-controllers package. You can read more here

@Gabriel-Mbugua
Copy link
Contributor Author

Updated Changes: Refactor User Module and Common Middlewares

In addition to the previous changes, this update includes:

Recent Changes

  • Restructured the user module to include registration functionality
  • Moved common middlewares (validate) to the shared package.
  • Removed zodErrorHandler and handling errors directly in the validate middleware.
  • Updated user controller, service, and schemas
  • Aligned the module structure with the collaborator's suggestions

Reasoning for Changes

These changes were made to improve code organization, reusability, and maintainability. By incorporating registration into the auth module, we've kept related functionality together. Moving common middlewares to a shared package allows for easier reuse across different parts of the application.

Impact on Docker Instances

The changes should not significantly impact the Docker setup. However, the Docker containers should be rebuilt and tested on the staging server to ensure compatibility.

Testing Done

  • Tested user registration functionality locally
  • Ensured that other parts of the application were not negatively impacted

Please review these additional changes and let me know if any further modifications are needed.

Copy link
Collaborator

@timothygachengo timothygachengo left a comment

Choose a reason for hiding this comment

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

Approved. You can create another PR for tests.

@timothygachengo timothygachengo merged commit 11d291c into Friendsofthepeople:main Jun 27, 2024
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.

2 participants