Refactor: Extract duplicated validation logic to internal/validation
Extracted repeated priority and ID validation patterns from CLI commands into reusable functions in internal/validation/bead.go. Changes: - Added ValidatePriority(): Combines parsing and error handling - Added ValidateIDFormat(): Validates ID format and extracts prefix - Added ValidatePrefix(): Validates prefix matching with database config - Updated create.go and show.go to use new validation functions - Simplified force flag logic to always call ValidatePrefix() - Added comprehensive tests for all validation functions - Added TODO comment for daemon mode validation enhancement Results: - Reduced code duplication by ~20 lines - Centralized validation logic for easier maintenance - Consistent error messages across all commands - All tests passing Fixes bd-g5p7 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -95,9 +95,9 @@ var createCmd = &cobra.Command{
|
|||||||
|
|
||||||
// Parse priority (supports both "1" and "P1" formats)
|
// Parse priority (supports both "1" and "P1" formats)
|
||||||
priorityStr, _ := cmd.Flags().GetString("priority")
|
priorityStr, _ := cmd.Flags().GetString("priority")
|
||||||
priority := validation.ParsePriority(priorityStr)
|
priority, err := validation.ValidatePriority(priorityStr)
|
||||||
if priority == -1 {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error: invalid priority %q (expected 0-4 or P0-P4)\n", priorityStr)
|
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
if cmd.Flags().Changed("priority") == false && tmpl != nil {
|
if cmd.Flags().Changed("priority") == false && tmpl != nil {
|
||||||
@@ -178,38 +178,29 @@ var createCmd = &cobra.Command{
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Validate explicit ID format if provided
|
// Validate explicit ID format if provided
|
||||||
// Supports: prefix-number (bd-42), prefix-hash (bd-a3f8e9), or hierarchical (bd-a3f8e9.1)
|
|
||||||
if explicitID != "" {
|
if explicitID != "" {
|
||||||
// Must contain hyphen
|
requestedPrefix, err := validation.ValidateIDFormat(explicitID)
|
||||||
if !strings.Contains(explicitID, "-") {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error: invalid ID format '%s' (expected format: prefix-hash or prefix-hash.number, e.g., 'bd-a3f8e9' or 'bd-a3f8e9.1')\n", explicitID)
|
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract prefix (before the first hyphen)
|
// Validate prefix matches database prefix
|
||||||
hyphenIdx := strings.Index(explicitID, "-")
|
ctx := context.Background()
|
||||||
requestedPrefix := explicitID[:hyphenIdx]
|
|
||||||
|
|
||||||
// Validate prefix matches database prefix (unless --force is used)
|
// Get database prefix from config
|
||||||
if !forceCreate {
|
var dbPrefix string
|
||||||
ctx := context.Background()
|
if daemonClient != nil {
|
||||||
|
// TODO(bd-g5p7): Add RPC method to get config in daemon mode
|
||||||
|
// For now, skip validation in daemon mode (needs RPC enhancement)
|
||||||
|
} else {
|
||||||
|
// Direct mode - check config
|
||||||
|
dbPrefix, _ = store.GetConfig(ctx, "issue_prefix")
|
||||||
|
}
|
||||||
|
|
||||||
// Get database prefix from config
|
if err := validation.ValidatePrefix(requestedPrefix, dbPrefix, forceCreate); err != nil {
|
||||||
var dbPrefix string
|
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||||
if daemonClient != nil {
|
os.Exit(1)
|
||||||
// Using daemon - need to get config via RPC
|
|
||||||
// For now, skip validation in daemon mode (needs RPC enhancement)
|
|
||||||
} else {
|
|
||||||
// Direct mode - check config
|
|
||||||
dbPrefix, _ = store.GetConfig(ctx, "issue_prefix")
|
|
||||||
}
|
|
||||||
|
|
||||||
if dbPrefix != "" && dbPrefix != requestedPrefix {
|
|
||||||
fmt.Fprintf(os.Stderr, "Error: prefix mismatch detected\n")
|
|
||||||
fmt.Fprintf(os.Stderr, " This database uses prefix '%s', but you specified '%s'\n", dbPrefix, requestedPrefix)
|
|
||||||
fmt.Fprintf(os.Stderr, " Use --force to create with mismatched prefix anyway\n")
|
|
||||||
os.Exit(1)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -345,9 +345,9 @@ var updateCmd = &cobra.Command{
|
|||||||
}
|
}
|
||||||
if cmd.Flags().Changed("priority") {
|
if cmd.Flags().Changed("priority") {
|
||||||
priorityStr, _ := cmd.Flags().GetString("priority")
|
priorityStr, _ := cmd.Flags().GetString("priority")
|
||||||
priority := validation.ParsePriority(priorityStr)
|
priority, err := validation.ValidatePriority(priorityStr)
|
||||||
if priority == -1 {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error: invalid priority %q (expected 0-4 or P0-P4)\n", priorityStr)
|
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
updates["priority"] = priority
|
updates["priority"] = priority
|
||||||
|
|||||||
@@ -45,3 +45,44 @@ func ParseIssueType(content string) (types.IssueType, error) {
|
|||||||
|
|
||||||
return issueType, nil
|
return issueType, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ValidatePriority parses and validates a priority string.
|
||||||
|
// Returns the parsed priority (0-4) or an error if invalid.
|
||||||
|
// Supports both numeric (0-4) and P-prefix format (P0-P4).
|
||||||
|
func ValidatePriority(priorityStr string) (int, error) {
|
||||||
|
priority := ParsePriority(priorityStr)
|
||||||
|
if priority == -1 {
|
||||||
|
return -1, fmt.Errorf("invalid priority %q (expected 0-4 or P0-P4)", priorityStr)
|
||||||
|
}
|
||||||
|
return priority, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// ValidateIDFormat validates that an ID has the correct format.
|
||||||
|
// Supports: prefix-number (bd-42), prefix-hash (bd-a3f8e9), or hierarchical (bd-a3f8e9.1)
|
||||||
|
// Returns the prefix part or an error if invalid.
|
||||||
|
func ValidateIDFormat(id string) (string, error) {
|
||||||
|
if id == "" {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Must contain hyphen
|
||||||
|
if !strings.Contains(id, "-") {
|
||||||
|
return "", fmt.Errorf("invalid ID format '%s' (expected format: prefix-hash or prefix-hash.number, e.g., 'bd-a3f8e9' or 'bd-a3f8e9.1')", id)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Extract prefix (before the first hyphen)
|
||||||
|
hyphenIdx := strings.Index(id, "-")
|
||||||
|
prefix := id[:hyphenIdx]
|
||||||
|
|
||||||
|
return prefix, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// ValidatePrefix checks that the requested prefix matches the database prefix.
|
||||||
|
// Returns an error if they don't match (unless force is true).
|
||||||
|
func ValidatePrefix(requestedPrefix, dbPrefix string, force bool) error {
|
||||||
|
if force || dbPrefix == "" || dbPrefix == requestedPrefix {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return fmt.Errorf("prefix mismatch: database uses '%s' but you specified '%s' (use --force to override)", dbPrefix, requestedPrefix)
|
||||||
|
}
|
||||||
|
|||||||
@@ -15,23 +15,23 @@ func TestParsePriority(t *testing.T) {
|
|||||||
{"2", 2},
|
{"2", 2},
|
||||||
{"3", 3},
|
{"3", 3},
|
||||||
{"4", 4},
|
{"4", 4},
|
||||||
|
|
||||||
// P-prefix format (uppercase)
|
// P-prefix format (uppercase)
|
||||||
{"P0", 0},
|
{"P0", 0},
|
||||||
{"P1", 1},
|
{"P1", 1},
|
||||||
{"P2", 2},
|
{"P2", 2},
|
||||||
{"P3", 3},
|
{"P3", 3},
|
||||||
{"P4", 4},
|
{"P4", 4},
|
||||||
|
|
||||||
// P-prefix format (lowercase)
|
// P-prefix format (lowercase)
|
||||||
{"p0", 0},
|
{"p0", 0},
|
||||||
{"p1", 1},
|
{"p1", 1},
|
||||||
{"p2", 2},
|
{"p2", 2},
|
||||||
|
|
||||||
// With whitespace
|
// With whitespace
|
||||||
{" 1 ", 1},
|
{" 1 ", 1},
|
||||||
{" P1 ", 1},
|
{" P1 ", 1},
|
||||||
|
|
||||||
// Invalid cases (returns -1)
|
// Invalid cases (returns -1)
|
||||||
{"5", -1}, // Out of range
|
{"5", -1}, // Out of range
|
||||||
{"-1", -1}, // Negative
|
{"-1", -1}, // Negative
|
||||||
@@ -40,7 +40,7 @@ func TestParsePriority(t *testing.T) {
|
|||||||
{"P", -1}, // Just the prefix
|
{"P", -1}, // Just the prefix
|
||||||
{"PP1", -1}, // Double prefix
|
{"PP1", -1}, // Double prefix
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
t.Run(tt.input, func(t *testing.T) {
|
t.Run(tt.input, func(t *testing.T) {
|
||||||
got := ParsePriority(tt.input)
|
got := ParsePriority(tt.input)
|
||||||
@@ -50,3 +50,82 @@ func TestParsePriority(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidatePriority(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
input string
|
||||||
|
wantValue int
|
||||||
|
wantError bool
|
||||||
|
}{
|
||||||
|
{"0", 0, false},
|
||||||
|
{"2", 2, false},
|
||||||
|
{"P1", 1, false},
|
||||||
|
{"5", -1, true},
|
||||||
|
{"abc", -1, true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.input, func(t *testing.T) {
|
||||||
|
got, err := ValidatePriority(tt.input)
|
||||||
|
if (err != nil) != tt.wantError {
|
||||||
|
t.Errorf("ValidatePriority(%q) error = %v, wantError %v", tt.input, err, tt.wantError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if got != tt.wantValue {
|
||||||
|
t.Errorf("ValidatePriority(%q) = %d, want %d", tt.input, got, tt.wantValue)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidateIDFormat(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
input string
|
||||||
|
wantPrefix string
|
||||||
|
wantError bool
|
||||||
|
}{
|
||||||
|
{"", "", false},
|
||||||
|
{"bd-a3f8e9", "bd", false},
|
||||||
|
{"bd-42", "bd", false},
|
||||||
|
{"bd-a3f8e9.1", "bd", false},
|
||||||
|
{"foo-bar", "foo", false},
|
||||||
|
{"nohyphen", "", true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.input, func(t *testing.T) {
|
||||||
|
got, err := ValidateIDFormat(tt.input)
|
||||||
|
if (err != nil) != tt.wantError {
|
||||||
|
t.Errorf("ValidateIDFormat(%q) error = %v, wantError %v", tt.input, err, tt.wantError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if got != tt.wantPrefix {
|
||||||
|
t.Errorf("ValidateIDFormat(%q) = %q, want %q", tt.input, got, tt.wantPrefix)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePrefix(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
requestedPrefix string
|
||||||
|
dbPrefix string
|
||||||
|
force bool
|
||||||
|
wantError bool
|
||||||
|
}{
|
||||||
|
{"matching prefixes", "bd", "bd", false, false},
|
||||||
|
{"empty db prefix", "bd", "", false, false},
|
||||||
|
{"mismatched with force", "foo", "bd", true, false},
|
||||||
|
{"mismatched without force", "foo", "bd", false, true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
err := ValidatePrefix(tt.requestedPrefix, tt.dbPrefix, tt.force)
|
||||||
|
if (err != nil) != tt.wantError {
|
||||||
|
t.Errorf("ValidatePrefix() error = %v, wantError %v", err, tt.wantError)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user