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

refactor(cli): Replace arguments with options #615

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

rajdip-b
Copy link
Member

@rajdip-b rajdip-b commented Jan 9, 2025

PR Type

Enhancement, Documentation


Description

  • Refactored CLI commands to replace arguments with options.

  • Updated command implementations to use descriptive option flags.

  • Added new subcommands and improved workspace role management.

  • Updated version to 2.1.0 and documented changes in the changelog.


Changes walkthrough 📝

Relevant files
Enhancement
13 files
list.secret.ts
Refactored secret list command to use options                       
+15/-6   
revisions.secret.ts
Refactored secret revisions command to use options             
+14/-6   
rollback.secret.ts
Updated secret rollback command to use options                     
+7/-6     
revisions.variable.ts
Refactored variable revisions command to use options         
+17/-4   
rollback.variable.ts
Updated variable rollback command to use options                 
+6/-6     
cancel-invitation.membership.ts
Refactored cancel invitation command to use options           
+17/-9   
invite.membership.ts
Updated invite membership command to use options                 
+20/-12 
remove.membership.ts
Refactored remove membership command to use options           
+19/-11 
transfer-ownership.membership copy.ts
Updated transfer ownership command to use options               
+19/-7   
update-role.membership.ts
Refactored update role command to use options                       
+26/-9   
role.workspace.ts
Added create role subcommand to workspace role                     
+2/-0     
create.role.ts
Added new create role command                                                       
+8/-8     
update.role.ts
Updated workspace role update command to use options         
+6/-6     
Documentation
1 files
CHANGELOG.md
Documented changes for version 2.1.0                                         
+6/-0     
Configuration changes
2 files
package-lock.json
Updated package-lock.json for version 2.1.0                           
+2/-2     
package.json
Updated package.json version to 2.1.0                                       
+1/-1     

💡 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:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Validation Missing

The code accepts environment slugs and project slugs as comma-separated lists but lacks validation for valid slugs format and characters before processing.

const authoritiesArray = authorities?.split(',')
const projectSlugsArray = projects?.split(',')
const environmentSlugsArray = environments?.split(',')
Error Handling

The error message in the Logger.error call could be more specific about which emails failed to be removed when processing multiple emails.

Logger.error(`Failed to remove users: ${error.message}`)

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Add validation to ensure required command options are provided before execution

Add input validation to ensure that the 'emails' option is provided and not empty
before attempting to remove users.

apps/cli/src/commands/workspace/membership/remove.membership.ts [42-44]

 async action({ args, options }: CommandActionData): Promise<void> {
   const [workspaceSlug] = args
   const { emails } = options
+  
+  if (!emails?.trim()) {
+    Logger.error('The --emails option is required')
+    return
+  }
  • Apply this suggestion
Suggestion importance[1-10]: 8

Why: The suggestion adds critical input validation to prevent runtime errors when the required emails option is missing, which could cause the API call to fail unexpectedly.

8
Validate and sanitize role input before processing command options

Add validation to ensure that the roles string is properly formatted before
splitting it into an array.

apps/cli/src/commands/workspace/membership/invite.membership.ts [48-61]

 const { email, roles } = options
+
+if (!roles?.trim()) {
+  Logger.error('The --roles option is required')
+  return
+}
+
 const { error, success } =
   await ControllerInstance.getInstance().workspaceMembershipController.inviteUsers(
     {
       workspaceSlug,
       members: [
         {
           email,
-          roleSlugs: roles.split(',')
+          roleSlugs: roles.trim().split(',')
         }
       ]
     },
  • Apply this suggestion
Suggestion importance[1-10]: 8

Why: The suggestion prevents potential runtime errors by validating the roles option and adds input sanitization, which is important for command-line parameter processing.

8
Validate hex color code format before processing the command

Add validation to ensure the color code format is a valid hex code before making the
API request.

apps/cli/src/commands/workspace/role/create.role.ts [66-73]

 const {
   name,
-  description, 
+  description,
   colorCode,
   authorities,
   projects,
   environments
 } = options
 
+if (colorCode && !/^#[0-9A-F]{6}$/i.test(colorCode)) {
+  Logger.error('Invalid color code format. Must be a valid hex code (e.g. #FF0000)')
+  return
+}
+
  • Apply this suggestion
Suggestion importance[1-10]: 7

Why: Adding color code validation prevents invalid data from being sent to the API and provides clear feedback to users when they input an incorrect format.

7

@rajdip-b rajdip-b merged commit 498f44e into develop Jan 9, 2025
3 of 4 checks passed
@rajdip-b rajdip-b deleted the feat/refactor-cli-arguments-into-options branch January 9, 2025 19:00
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.

1 participant