From 5702b118b07559109a9b78f5faefd1304ba75bf8 Mon Sep 17 00:00:00 2001
From: Nuno Duque Nunes <nunoduquenunes@gmail.com>
Date: Tue, 12 May 2026 01:43:43 +0000
Subject: [PATCH] refactor: remove/rename helpers, test --fn mode, param
 defaults, WGCTL_BINARY

---
 commands/remove.command.sh | 49 +++++++++--------------------
 commands/rename.command.sh | 56 ++++++++++++++-------------------
 commands/test.command.sh   | 63 ++++++++++++++++++++++++++++++++++----
 3 files changed, 95 insertions(+), 73 deletions(-)

diff --git a/commands/remove.command.sh b/commands/remove.command.sh
index da5a13c..5cbe593 100644
--- a/commands/remove.command.sh
+++ b/commands/remove.command.sh
@@ -82,39 +82,20 @@ function cmd::remove::run() {
   local was_blocked=false
   peers::is_blocked "$name" && was_blocked=true
 
-  # Unapply rule if assigned
-  local assigned_rule
-  assigned_rule=$(peers::get_meta "$name" "rule")
-
-  if [[ -z "$assigned_rule" ]]; then
-    assigned_rule=$(peers::default_rule "$name")
-  fi
-
-  # Flush all iptables rules for this peer IP
-  if [[ -n "$client_ip" ]]; then
-    fw::flush_peer "$client_ip"
-  fi
-
-  # Remove peer from server config
-  peers::remove_from_server "$name" || return 1
-
-  # Remove client config
-  peers::remove_client_config "$name" || return 1
-
-  # Remove keys
-  keys::remove "$name" || return 1
-
-  # Remove block rules only if client was fully blocked
-  if [[ -n "$client_ip" ]] && $was_blocked; then
-    fw::unblock_all "$client_ip"
-  fi
-
-  fw::remove_block_file "$name" 2>/dev/null || true
-
-  peers::remove_meta "$name" 2>/dev/null || true
-
-  # Reload WireGuard
-  peers::reload || return 1
-
+  cmd::remove::_cleanup "$name" "$client_ip" "$was_blocked" || return 1
   log::wg_success "Client removed: ${name}"
 }
+
+function cmd::remove::_cleanup() {
+  local name="${1:-}" client_ip="${2:-}" was_blocked="${3:-false}"
+  
+  [[ -n "$client_ip" ]] && fw::flush_peer "$client_ip"
+  peers::remove_from_server "$name"     || return 1
+  peers::remove_client_config "$name"   || return 1
+  keys::remove "$name"                  || return 1
+
+  [[ -n "$client_ip" ]] && $was_blocked && fw::unblock_all "$client_ip"
+  fw::remove_block_file "$name"         2>/dev/null || true
+  peers::remove_meta "$name"            2>/dev/null || true
+  peers::reload                         || return 1
+}
\ No newline at end of file
diff --git a/commands/rename.command.sh b/commands/rename.command.sh
index fe4d794..123f77f 100644
--- a/commands/rename.command.sh
+++ b/commands/rename.command.sh
@@ -70,15 +70,7 @@ function cmd::rename::run() {
   fi
 
   name=$(peers::resolve_and_require "$name" "$type") || return 1
-  
-  # Resolve new name if provided
-  if [[ -n "$new_name" || -n "$new_type" ]]; then
-    # If only new_type provided, keep same base name
-    if [[ -z "$new_name" ]]; then
-      new_name=$(echo "$name" | cut -d'-' -f2-)
-    fi
-    new_name=$(peers::resolve_name "$new_name" "$new_type") || return 1
-  fi
+  new_name=$(peers::resolve_name "$new_name" "$new_type") || return 1
 
   local dir
   dir="$(ctx::clients)"
@@ -95,33 +87,31 @@ function cmd::rename::run() {
 
   log::section "Renaming client: ${name} → ${new_name}"
 
-  # Rename client files
-  mv "${dir}/${name}.conf"        "${dir}/${new_name}.conf"
-  mv "${dir}/${name}_private.key" "${dir}/${new_name}_private.key"
-  mv "${dir}/${name}_public.key"  "${dir}/${new_name}_public.key"
-
-  log::fs_write "Renamed client files"
-
-  # Update comment in wg0.conf
-  sed -i "s/^# ${name}$/# ${new_name}/" "$(config::config_file)"
-
-  log::fs_write "Updated server config"
-
-  # Update block file if exists
-  local block_file
-  block_file="$(ctx::block::path "${name}.block")"
-  if [[ -f "$block_file" ]]; then
-    mv "$block_file" "$(ctx::block::path "${new_name}.block")"
-    log::fs_write "Renamed block file"
-  fi
-
-  local old_meta new_meta
-  old_meta=$(peers::meta_path "$name")
-  new_meta=$(peers::meta_path "$new_name")
-  [[ -f "$old_meta" ]] && mv "$old_meta" "$new_meta"
+  cmd::rename::_rename_files "$name" "$new_name"
 
   # Reload WireGuard
   peers::reload
 
   log::wg_success "Client renamed: ${name} → ${new_name}"
 }
+
+function cmd::rename::_rename_files() {
+  local name="${1:-}" new_name="${2:-}"
+  local dir
+  dir="$(ctx::clients)"
+
+  mv "${dir}/${name}.conf"        "${dir}/${new_name}.conf"
+  mv "${dir}/${name}_private.key" "${dir}/${new_name}_private.key"
+  mv "${dir}/${name}_public.key"  "${dir}/${new_name}_public.key"
+
+  sed -i "s/^# ${name}$/# ${new_name}/" "$(config::config_file)"
+
+  local block_file
+  block_file="$(ctx::block::path "${name}.block")"
+  [[ -f "$block_file" ]] && mv "$block_file" "$(ctx::block::path "${new_name}.block")"
+
+  local old_meta new_meta
+  old_meta=$(peers::meta_path "$name")
+  new_meta=$(peers::meta_path "$new_name")
+  [[ -f "$old_meta" ]] && mv "$old_meta" "$new_meta"
+}
\ No newline at end of file
diff --git a/commands/test.command.sh b/commands/test.command.sh
index a9c2a85..0a5dde0 100644
--- a/commands/test.command.sh
+++ b/commands/test.command.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-WGCTL_BINARY="/usr/local/bin/wgctl"
+WGCTL_BINARY="$(command -v wgctl)"
 
 # ============================================
 # Lifecycle
@@ -115,10 +115,12 @@ function cmd::test::run_function() {
   log::section "Function Test: ${fn}"
 
   case "$fn" in
-    cmd::block::run)       cmd::test::fn_block      ;;
-    cmd::unblock::run)     cmd::test::fn_unblock    ;;
-    cmd::remove::run)      cmd::test::fn_remove     ;;
-    cmd::rule::assign)     cmd::test::fn_rule_assign ;;
+    cmd::block::run)       cmd::test::fn_block        ;;
+    cmd::unblock::run)     cmd::test::fn_unblock      ;;
+    cmd::remove::run)      cmd::test::fn_remove       ;;
+    cmd::rule::assign)     cmd::test::fn_rule_assign  ;;
+    cmd::rename::run)      cmd::test::fn_rename       ;;
+    cmd::remove::run)      cmd::test::fn_remove       ;;
     *)
       log::error "No function test defined for: ${fn}"
       return 1
@@ -263,7 +265,7 @@ function cmd::test::fn_block() {
 
   "$WGCTL_BINARY" unblock --name phone-testunit > /dev/null 2>&1 || true
   cmd::test::run_cmd "block with --type" "blocked" block --name testunit --type phone
-  
+
   cmd::test::run_cmd_fails "block nonexistent"            block --name truly-nonexistent-xyz
 
   # Cleanup
@@ -271,6 +273,55 @@ function cmd::test::fn_block() {
   "$WGCTL_BINARY" remove --name phone-testunit --force > /dev/null 2>&1 || true
 }
 
+function cmd::test::fn_remove() {
+  test::section "cmd::remove::run"
+
+  # Setup
+  "$WGCTL_BINARY" remove --name phone-testunit --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" add --name testunit --type phone \
+    > /dev/null 2>&1
+
+  # Tests
+  # Skip the interactive prompt test — it hangs waiting for input
+  # cmd::test::run_cmd_fails "remove without --force" \
+  #   remove --name phone-testunit
+  cmd::test::run_cmd "remove with --force" "removed" \
+    remove --name phone-testunit --force
+  cmd::test::run_cmd_fails "remove nonexistent" \
+    remove --name nonexistent-peer --force
+  cmd::test::run_cmd_fails "remove missing --name" \
+    remove --force
+
+  # Cleanup already done by tests
+}
+
+function cmd::test::fn_rename() {
+  test::section "cmd::rename::run"
+
+  # Setup
+  "$WGCTL_BINARY" remove --name phone-testunit --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" remove --name phone-testunit2 --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" add --name testunit --type phone \
+    > /dev/null 2>&1
+
+  # Tests
+  cmd::test::run_cmd "rename peer" "renamed" \
+    rename --name phone-testunit --new-name phone-testunit2
+  cmd::test::run_cmd_fails "rename to existing" \
+    rename --name phone-testunit2 --new-name phone-nuno
+  cmd::test::run_cmd_fails "rename nonexistent" \
+    rename --name phone-nonexistent --new-name phone-testunit
+  cmd::test::run_cmd_fails "rename missing --new-name" \
+    rename --name phone-testunit2
+
+  # Cleanup
+  /usr/local/bin/wgctl remove --name phone-testunit2 --force \
+    > /dev/null 2>&1 || true
+}
+
 # ============================================
 # Run
 # ============================================