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(cli): Add functionality to operate on Workspace Membership #589

Merged

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Dec 12, 2024

User description

Description

Improves CLI functionality to now work with workspace membership.

Fixes #548

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


Description

Added new CLI functionality to manage workspace memberships:

  • Created new workspace membership command structure with multiple subcommands
  • Implemented commands for:
    • Inviting/removing users from workspace
    • Accepting/declining/canceling invitations
    • Transferring workspace ownership
    • Updating member roles
    • Listing workspace members
  • Added WorkspaceMembershipController integration
  • Fixed formatting in email templates

Changes walkthrough 📝

Relevant files
Enhancement
membership.workspace.ts
Add workspace membership command structure                             

apps/cli/src/commands/workspace/membership.workspace.ts

  • Created new WorkspaceMembershipCommand class to manage workspace
    memberships
  • Added subcommands for invite, remove, transfer ownership, and other
    membership operations
  • +34/-0   
    *.ts
    Implement workspace membership command handlers                   

    apps/cli/src/commands/workspace/membership/*.ts

  • Implemented individual command classes for each membership operation
  • Added accept/decline invitation, cancel invitation, list members
    functionality
  • Added transfer ownership, invite user, remove user, update roles
    commands
  • controller-instance.ts
    Add workspace membership controller support                           

    apps/cli/src/util/controller-instance.ts

  • Added WorkspaceMembershipController to the controller instance
  • Initialized workspace membership controller with base URL
  • +14/-1   
    Formatting
    *
    Format email templates code                                                           

    apps/api/src/mail/emails/*

  • Fixed formatting and indentation in email templates
  • Improved text wrapping in workspace invitation and removal emails

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

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    548 - Partially compliant

    Compliant requirements:

    • All required CLI commands implemented
    • Files created in correct locations
    • workspaceMembershipController integrated correctly

    Non-compliant requirements:

    • No test cases added (as indicated in PR checklist)
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Bug
    The roleSlugs parameter is split with comma but command description says space-separated. This inconsistency could cause issues.

    Incorrect Description
    Command description says "List all roles" but this command actually lists members

    File Naming
    File name contains "copy" which seems unintentional and should be removed

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Dec 12, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fixed incorrect array splitting that would break command line argument handling

    The roleSlugs argument is incorrectly split - it's already an array from the command
    line arguments, so splitting it with comma will break the array into incorrect
    segments.

    apps/cli/src/commands/workspace/membership/invite.membership.ts [49]

    -roleSlugs: roleSlugs.split(',')
    +roleSlugs: roleSlugs
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion fixes a critical bug where command line arguments would be incorrectly processed, potentially breaking the invite functionality by splitting already-split arguments.

    9
    General
    ✅ Fixed misleading command description that referenced incorrect functionality

    The command description and log message incorrectly refer to "roles" instead of
    "members", which is misleading since this command lists workspace members.

    apps/cli/src/commands/workspace/membership/get-all-members.membership.ts [16-18]

     getDescription(): string {
    -  return 'List all roles of a workspace'
    +  return 'List all members of a workspace'
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    Why: The suggestion corrects misleading documentation that could confuse users by referring to "roles" instead of "members", improving command usability and clarity.

    7
    Fixed incorrect file naming that suggests unintended file duplication

    The file name contains "copy" which appears to be a mistake from file duplication.
    Rename to remove the "copy" suffix.

    apps/cli/src/commands/workspace/membership/transfer-ownership.membership copy.ts [1]

    -'apps/cli/src/commands/workspace/membership/transfer-ownership.membership copy.ts'
    +'apps/cli/src/commands/workspace/membership/transfer-ownership.membership.ts'
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion addresses a file naming issue that indicates accidental duplication, which could cause confusion in maintenance and potentially lead to wrong file references.

    6

    Copy link

    codecov bot commented Dec 13, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 87.19%. Comparing base (ce50743) to head (86b6fa7).
    Report is 247 commits behind head on develop.

    Additional details and impacted files
    @@             Coverage Diff             @@
    ##           develop     #589      +/-   ##
    ===========================================
    - Coverage    91.71%   87.19%   -4.52%     
    ===========================================
      Files          111      110       -1     
      Lines         2510     2828     +318     
      Branches       469      426      -43     
    ===========================================
    + Hits          2302     2466     +164     
    - Misses         208      362     +154     
    Flag Coverage Δ
    api-e2e-tests 87.19% <100.00%> (-4.52%) ⬇️

    Flags with carried forward coverage won't be shown. Click here to find out more.

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @muntaxir4 muntaxir4 requested a review from rajdip-b December 16, 2024 07:27
    @rajdip-b rajdip-b merged commit 0fde62b into keyshade-xyz:develop Dec 16, 2024
    5 checks passed
    muntaxir4 added a commit to muntaxir4/keyshade that referenced this pull request Jan 1, 2025
    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.

    CLI: Add functionality to operate on Workspace Membership
    2 participants