search

keyshade-xyz / keyshade

Realtime secret and configuration management tool, with the best in class security and seamless integration support
https://keyshade.xyz
Mozilla Public License 2.0
208 stars 105 forks source link

committed #403

Closed surya-moorthy closed 2 months ago

surya-moorthy commented 3 months ago

User description

Description

Give a summary of the change that you have made

Fixes #[ISSUENO]

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

If changes are made in the code:

Documentation Update

PR Type

Enhancement, Documentation

Description

Changes walkthrough 📝

Relevant files
Enhancement
worspacerole.ts
Add EnvironmentController with CRUD operations for environments
packages/api-client/src/controllers/worspacerole.ts
  • Added EnvironmentController class with methods for CRUD operations on
    environments.
  • Implemented methods for creating, updating, retrieving, and deleting
    environments.
  • Utilized apiClient for making HTTP requests.
    		•  +76/-0   
    Documentation
    user.types.d.ts
    Define user-related request and response types                     
    packages/api-client/src/types/user.types.d.ts
  • Added interfaces for user-related requests and responses.
  • Defined types for getting, updating, and deleting user information.
    		•  +42/-0   
    workspace.types.d.ts
    Define workspace role-related request and response types 
    packages/api-client/src/types/workspace.types.d.ts
  • Added interfaces for workspace role-related requests and responses.
  • Defined types for creating, updating, retrieving, and deleting
    workspace roles.
    		•  +77/-0   

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

    codiumai-pr-agent-free[bot] commented 3 months ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Incorrect Class Name
    The class is named `EnvironmentController` but the file name and operations suggest it should be `WorkspaceRoleController`. This could lead to confusion and maintenance issues. Incorrect API Endpoint
    The API endpoints in methods like `createEnvironment` and `updateEnvironment` use `/api/environment/...` which does not align with the expected workspace role operations. This might be a copy-paste error and should be corrected to match the workspace role context. Misleading Method Names
    Methods such as `createEnvironment`, `updateEnvironment`, etc., are misleading in the context of workspace roles. These should be renamed to reflect their actual functionality like `createWorkspaceRole`, `updateWorkspaceRole`, etc.
    codiumai-pr-agent-free[bot] commented 3 months ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Rename the class to match its functionality ___ **The class name EnvironmentController does not reflect the operations performed
    within, which are related to workspace roles. Consider renaming the class to
    WorkspaceRoleController to improve clarity and maintainability.** [packages/api-client/src/controllers/worspacerole.ts [14]](https://github.com/keyshade-xyz/keyshade/pull/403/files#diff-c614a2292955c41ff04e512585439a6361f3886a17aaa482232d7daedc8fbf49R14-R14) ```diff -export default class EnvironmentController { +export default class WorkspaceRoleController { ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: Renaming the class to `WorkspaceRoleController` significantly improves code clarity and maintainability by accurately reflecting the class's purpose.
    		9
    Rename the method to accurately reflect its purpose ___ **The method createEnvironment is incorrectly named as it deals with creating
    workspace roles, not environments. Rename the method to createWorkspaceRole to
    accurately describe its functionality.** [packages/api-client/src/controllers/worspacerole.ts [17]](https://github.com/keyshade-xyz/keyshade/pull/403/files#diff-c614a2292955c41ff04e512585439a6361f3886a17aaa482232d7daedc8fbf49R17-R17) ```diff -static async createEnvironment( +static async createWorkspaceRole( ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: Renaming the method to `createWorkspaceRole` ensures that the method name accurately reflects its functionality, improving code readability and maintainability.
    		9
    Rename the method to better describe its operation ___ **The method updateEnvironment is misleading because it updates workspace roles, not
    environments. Rename this method to updateWorkspaceRole to better align with its
    actual operation.** [packages/api-client/src/controllers/worspacerole.ts [28]](https://github.com/keyshade-xyz/keyshade/pull/403/files#diff-c614a2292955c41ff04e512585439a6361f3886a17aaa482232d7daedc8fbf49R28-R28) ```diff -static async updateEnvironment( +static async updateWorkspaceRole( ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: Renaming the method to `updateWorkspaceRole` aligns the method name with its actual functionality, enhancing code clarity and maintainability.
    		9
    Rename the method to clearly indicate its functionality ___ **The method deleteEnvironment should be renamed to deleteWorkspaceRole since it
    handles the deletion of workspace roles, not environments. This change will make the
    method's purpose clearer.** [packages/api-client/src/controllers/worspacerole.ts [67]](https://github.com/keyshade-xyz/keyshade/pull/403/files#diff-c614a2292955c41ff04e512585439a6361f3886a17aaa482232d7daedc8fbf49R67-R67) ```diff -static async deleteEnvironment( +static async deleteWorkspaceRole( ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: Renaming the method to `deleteWorkspaceRole` clarifies its purpose, improving the readability and maintainability of the code.
    		9
    rajdip-b commented 3 months ago
    kriptonian1 commented 3 months ago

    what is this PR about

    surya-moorthy commented 3 months ago

    @rajdip-b sorry bro kinda busy in college works unable to work on this issue much more which I was scheduled , I will make sure make time to complete issue soon

    rajdip-b commented 2 months ago

    Closing this because of no response.