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

add user controller #99

Merged
merged 1 commit into from
Mar 27, 2024
Merged

add user controller #99

merged 1 commit into from
Mar 27, 2024

Conversation

Faouzijedidi1
Copy link
Contributor

@Faouzijedidi1 Faouzijedidi1 commented Mar 27, 2024

User description

Add user controller

#93


Type

enhancement


Description

  • Introduced a new UserController for handling user-related operations, secured with JWT authentication.
  • Simplified the code in spotChecks.ts by removing unnecessary line breaks in the assignment of variables.
  • Registered the new UserController within the UserModule to enable its functionalities.

Changes walkthrough

Relevant files
Formatting
spotChecks.ts
Simplify Spot Order and Exchange Index Validity Checks     

server/src/common/helpers/checks/spotChecks.ts

  • Simplified the assignment of validSpotOrderTypes and
    validExchangeIndexes by removing unnecessary line breaks.
  • +2/-4     
    Enhancement
    user.controller.ts
    Implement User Controller with JWT Authentication               

    server/src/modules/mixin/user/user.controller.ts

  • Introduced a new UserController class with JWT authentication guard.
  • Added a getAllUsers method to fetch all users.
  • +15/-0   
    user.module.ts
    Register UserController in UserModule                                       

    server/src/modules/mixin/user/user.module.ts

    • Registered UserController in the UserModule.
    +2/-0     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    vercel bot commented Mar 27, 2024

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Updated (UTC)
    mr-market ✅ Ready (Inspect) Visit Preview Mar 27, 2024 3:11am

    Copy link

    railway-app bot commented Mar 27, 2024

    This PR is being deployed to Railway 🚅

    Mr.Market: ◻️ REMOVED

    @github-actions github-actions bot added the enhancement New feature or request label Mar 27, 2024
    Copy link

    PR Description updated to latest commit (e8ec891)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the PR introduces a new feature with a moderate amount of code across three files. The changes are straightforward, involving the addition of a new controller and minor refactoring in an existing file. The logic seems simple, and the code modifications are not extensive.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The UserController lacks method-level security checks. While the controller is secured with JwtAuthGuard at the class level, individual methods might require additional permissions or roles checks depending on the application's security requirements.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileserver/src/modules/mixin/user/user.controller.ts
    suggestion      

    Consider implementing role-based access control (RBAC) for different endpoints within the UserController. For instance, you might want to restrict certain actions to admin users only. This can be achieved by using NestJS's built-in @Roles decorator or a custom decorator that suits your application's needs. [important]

    relevant line@UseGuards(JwtAuthGuard)

    relevant fileserver/src/common/helpers/checks/spotChecks.ts
    suggestion      

    It's good practice to ensure that the values being checked against the enums/maps are sanitized and validated to prevent potential issues such as type mismatches or injection attacks. Consider adding a validation layer if not already present. [medium]

    relevant lineconst validSpotOrderTypes: SpotOrderType[] = Object.keys(SPOT_ORDER_TYPE_MAP);


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    github-actions bot commented Mar 27, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Improve type safety with as const assertion in Object.keys().

    Consider using as const assertion to ensure Object.keys() returns a more specific type
    (readonly ["key1", "key2", ...]) instead of a general string[]. This can help with type
    safety when working with enums or known object keys.

    server/src/common/helpers/checks/spotChecks.ts [17-21]

    -const validSpotOrderTypes: SpotOrderType[] = Object.keys(SPOT_ORDER_TYPE_MAP);
    -const validExchangeIndexes: ExchangeIndex[] = Object.keys(SPOT_EXCHANGE_MAP);
    +const validSpotOrderTypes: readonly SpotOrderType[] = Object.keys(SPOT_ORDER_TYPE_MAP) as const;
    +const validExchangeIndexes: readonly ExchangeIndex[] = Object.keys(SPOT_EXCHANGE_MAP) as const;
     
    Implement pagination or limit the number of users returned.

    To enhance the security and flexibility of the getAllUsers method, consider implementing
    pagination or limiting the number of users returned in a single request. This can help
    prevent potential performance issues with large datasets and improve the overall security
    posture.

    server/src/modules/mixin/user/user.controller.ts [12-13]

    -async getAllUsers(): Promise<MixinUser[]> {
    -  return this.userService.getAllUsers();
    +async getAllUsers(@Query('limit') limit: number = 10): Promise<MixinUser[]> {
    +  return this.userService.getAllUsers(limit);
     }
     
    Best practice
    Use @Inject decorator for better service injection.

    To ensure that the UserService is correctly injected and can be easily mocked for testing,
    consider using the @Inject decorator with a custom provider token instead of directly
    injecting the UserService class.

    server/src/modules/mixin/user/user.controller.ts [9]

    -constructor(private userService: UserService) {}
    +constructor(@Inject('USER_SERVICE') private userService: UserService) {}
     
    Avoid exporting controllers in modules.

    To ensure that the UserController is only accessible to modules that explicitly import the
    UserModule, consider removing the UserController from the exports array. Controllers
    should typically not be exported as they are meant to be route handlers rather than shared
    services.

    server/src/modules/mixin/user/user.module.ts [13]

    -controllers: [UserController],
    +controllers: [UserController], // No need to export controllers
     
    Specify explicit HTTP status codes in controller responses.

    To improve the readability and maintainability of the routes, consider using explicit HTTP
    status codes in the controller's response decorators. For example, specifying
    @HttpCode(HttpStatus.OK) for the getAllUsers method can make the intended response status
    clear.

    server/src/modules/mixin/user/user.controller.ts [11-13]

     @Get()
    +@HttpCode(HttpStatus.OK)
     async getAllUsers(): Promise<MixinUser[]> {
       return this.userService.getAllUsers();
     }
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @zed-wong zed-wong merged commit 23fd3ed into main Mar 27, 2024
    10 checks passed
    @zed-wong zed-wong deleted the user-controller branch April 1, 2024 13:46
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants