johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge refactoring PR #48: Reduce cyclomatic complexity

Browse Source 
Co-authored-by: jjshanks <jjshanks@users.noreply.github.com>
This commit is contained in:
Steve Yegge
2025-10-15 23:39:24 -07:00
parent cf4f11cff7 96f05a79cb
commit cdd24699be
6 changed files with 955 additions and 810 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.golangci.yml
+1
View File
@@ -86,3 +86,4 @@ issues:
- dupl # Test duplication is acceptable
- goconst # Test constants are acceptable
- errcheck # Test cleanup errors are acceptable
- gocyclo # Test complexity is acceptable
cmd/bd/label.go
+28 -46
View File
@@ -18,34 +18,38 @@ var labelCmd = &cobra.Command{
Short: "Manage issue labels",
}
// executeLabelCommand executes a label operation and handles output
func executeLabelCommand(issueID, label, operation string, operationFunc func(context.Context, string, string, string) error) {
ctx := context.Background()
if err := operationFunc(ctx, issueID, label, actor); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
// Schedule auto-flush
markDirtyAndScheduleFlush()
if jsonOutput {
outputJSON(map[string]interface{}{
"status": operation,
"issue_id": issueID,
"label": label,
})
return
}
green := color.New(color.FgGreen).SprintFunc()
// Capitalize first letter manually (strings.Title is deprecated)
capitalizedOp := strings.ToUpper(operation[:1]) + operation[1:]
fmt.Printf("%s %s label '%s' to %s\n", green("✓"), capitalizedOp, label, issueID)
}
var labelAddCmd = &cobra.Command{
Use: "add [issue-id] [label]",
Short: "Add a label to an issue",
Args: cobra.ExactArgs(2),
Run: func(cmd *cobra.Command, args []string) {
issueID := args[0]
label := args[1]
ctx := context.Background()
if err := store.AddLabel(ctx, issueID, label, actor); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
// Schedule auto-flush
markDirtyAndScheduleFlush()
if jsonOutput {
outputJSON(map[string]interface{}{
"status": "added",
"issue_id": issueID,
"label": label,
})
return
}
green := color.New(color.FgGreen).SprintFunc()
fmt.Printf("%s Added label '%s' to %s\n", green("✓"), label, issueID)
executeLabelCommand(args[0], args[1], "added", store.AddLabel)
},
}
@@ -54,29 +58,7 @@ var labelRemoveCmd = &cobra.Command{
Short: "Remove a label from an issue",
Args: cobra.ExactArgs(2),
Run: func(cmd *cobra.Command, args []string) {
issueID := args[0]
label := args[1]
ctx := context.Background()
if err := store.RemoveLabel(ctx, issueID, label, actor); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
// Schedule auto-flush
markDirtyAndScheduleFlush()
if jsonOutput {
outputJSON(map[string]interface{}{
"status": "removed",
"issue_id": issueID,
"label": label,
})
return
}
green := color.New(color.FgGreen).SprintFunc()
fmt.Printf("%s Removed label '%s' from %s\n", green("✓"), label, issueID)
executeLabelCommand(args[0], args[1], "removed", store.RemoveLabel)
},
}
cmd/bd/main.go
+500 -465
View File
File diff suppressed because it is too large Load Diff
cmd/bd/markdown.go
+103 -67
View File
@@ -183,6 +183,102 @@ func validateMarkdownPath(path string) (string, error) {
//
// ### Dependencies
// bd-10, bd-20
// markdownParseState holds state for parsing markdown files
type markdownParseState struct {
issues []*IssueTemplate
currentIssue *IssueTemplate
currentSection string
sectionContent strings.Builder
}
// finalizeSection processes and resets the current section
func (s *markdownParseState) finalizeSection() {
if s.currentIssue == nil || s.currentSection == "" {
return
}
content := s.sectionContent.String()
processIssueSection(s.currentIssue, s.currentSection, content)
s.sectionContent.Reset()
}
// handleH2Header handles H2 headers (new issue titles)
func (s *markdownParseState) handleH2Header(matches []string) {
// Finalize previous section if any
s.finalizeSection()
// Save previous issue if any
if s.currentIssue != nil {
s.issues = append(s.issues, s.currentIssue)
}
// Start new issue
s.currentIssue = &IssueTemplate{
Title: strings.TrimSpace(matches[1]),
Priority: 2, // Default priority
IssueType: "task", // Default type
}
s.currentSection = ""
}
// handleH3Header handles H3 headers (section titles)
func (s *markdownParseState) handleH3Header(matches []string) {
// Finalize previous section
s.finalizeSection()
// Start new section
s.currentSection = strings.TrimSpace(matches[1])
}
// handleContentLine handles regular content lines
func (s *markdownParseState) handleContentLine(line string) {
if s.currentIssue == nil {
return
}
// Content within a section
if s.currentSection != "" {
if s.sectionContent.Len() > 0 {
s.sectionContent.WriteString("\n")
}
s.sectionContent.WriteString(line)
return
}
// First lines after title (before any section) become description
if s.currentIssue.Description == "" && line != "" {
if s.currentIssue.Description != "" {
s.currentIssue.Description += "\n"
}
s.currentIssue.Description += line
}
}
// finalize completes parsing and returns the results
func (s *markdownParseState) finalize() ([]*IssueTemplate, error) {
// Finalize last section and issue
s.finalizeSection()
if s.currentIssue != nil {
s.issues = append(s.issues, s.currentIssue)
}
// Check if we found any issues
if len(s.issues) == 0 {
return nil, fmt.Errorf("no issues found in markdown file (expected ## Issue Title format)")
}
return s.issues, nil
}
// createMarkdownScanner creates a scanner with appropriate buffer size
func createMarkdownScanner(file *os.File) *bufio.Scanner {
scanner := bufio.NewScanner(file)
// Increase buffer size for large markdown files
const maxScannerBuffer = 1024 * 1024 // 1MB
buf := make([]byte, maxScannerBuffer)
scanner.Buffer(buf, maxScannerBuffer)
return scanner
}
func parseMarkdownFile(path string) ([]*IssueTemplate, error) {
// Validate and clean the file path
cleanPath, err := validateMarkdownPath(path)
@@ -199,91 +295,31 @@ func parseMarkdownFile(path string) ([]*IssueTemplate, error) {
_ = file.Close() // Close errors on read-only operations are not actionable
}()
var issues []*IssueTemplate
var currentIssue *IssueTemplate
var currentSection string
var sectionContent strings.Builder
scanner := bufio.NewScanner(file)
// Increase buffer size for large markdown files
const maxScannerBuffer = 1024 * 1024 // 1MB
buf := make([]byte, maxScannerBuffer)
scanner.Buffer(buf, maxScannerBuffer)
// Helper to finalize current section
finalizeSection := func() {
if currentIssue == nil || currentSection == "" {
return
}
content := sectionContent.String()
processIssueSection(currentIssue, currentSection, content)
sectionContent.Reset()
}
state := &markdownParseState{}
scanner := createMarkdownScanner(file)
for scanner.Scan() {
line := scanner.Text()
// Check for H2 (new issue)
if matches := h2Regex.FindStringSubmatch(line); matches != nil {
// Finalize previous section if any
finalizeSection()
// Save previous issue if any
if currentIssue != nil {
issues = append(issues, currentIssue)
}
// Start new issue
currentIssue = &IssueTemplate{
Title: strings.TrimSpace(matches[1]),
Priority: 2, // Default priority
IssueType: "task", // Default type
}
currentSection = ""
state.handleH2Header(matches)
continue
}
// Check for H3 (section within issue)
if matches := h3Regex.FindStringSubmatch(line); matches != nil {
// Finalize previous section
finalizeSection()
// Start new section
currentSection = strings.TrimSpace(matches[1])
state.handleH3Header(matches)
continue
}
// Regular content line - append to current section
if currentIssue != nil && currentSection != "" {
if sectionContent.Len() > 0 {
sectionContent.WriteString("\n")
}
sectionContent.WriteString(line)
} else if currentIssue != nil && currentSection == "" && currentIssue.Description == "" {
// First lines after title (before any section) become description
if line != "" {
if currentIssue.Description != "" {
currentIssue.Description += "\n"
}
currentIssue.Description += line
}
}
}
// Finalize last section and issue
finalizeSection()
if currentIssue != nil {
issues = append(issues, currentIssue)
// Regular content line
state.handleContentLine(line)
}
if err := scanner.Err(); err != nil {
return nil, fmt.Errorf("error reading file: %w", err)
}
// Check if we found any issues
if len(issues) == 0 {
return nil, fmt.Errorf("no issues found in markdown file (expected ## Issue Title format)")
}
return issues, nil
return state.finalize()
}
internal/storage/sqlite/labels.go
+33 -40
View File
@@ -8,26 +8,31 @@ import (
"github.com/steveyegge/beads/internal/types"
)
// AddLabel adds a label to an issue
func (s *SQLiteStorage) AddLabel(ctx context.Context, issueID, label, actor string) error {
// executeLabelOperation executes a label operation (add or remove) within a transaction
func (s *SQLiteStorage) executeLabelOperation(
ctx context.Context,
issueID, actor string,
labelSQL string,
labelSQLArgs []interface{},
eventType types.EventType,
eventComment string,
operationError string,
) error {
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("failed to begin transaction: %w", err)
}
defer tx.Rollback()
_, err = tx.ExecContext(ctx, `
INSERT OR IGNORE INTO labels (issue_id, label)
VALUES (?, ?)
`, issueID, label)
_, err = tx.ExecContext(ctx, labelSQL, labelSQLArgs...)
if err != nil {
return fmt.Errorf("failed to add label: %w", err)
return fmt.Errorf("%s: %w", operationError, err)
}
_, err = tx.ExecContext(ctx, `
INSERT INTO events (issue_id, event_type, actor, comment)
VALUES (?, ?, ?, ?)
`, issueID, types.EventLabelAdded, actor, fmt.Sprintf("Added label: %s", label))
`, issueID, eventType, actor, eventComment)
if err != nil {
return fmt.Errorf("failed to record event: %w", err)
}
@@ -45,40 +50,28 @@ func (s *SQLiteStorage) AddLabel(ctx context.Context, issueID, label, actor stri
return tx.Commit()
}
// AddLabel adds a label to an issue
func (s *SQLiteStorage) AddLabel(ctx context.Context, issueID, label, actor string) error {
return s.executeLabelOperation(
ctx, issueID, actor,
`INSERT OR IGNORE INTO labels (issue_id, label) VALUES (?, ?)`,
[]interface{}{issueID, label},
types.EventLabelAdded,
fmt.Sprintf("Added label: %s", label),
"failed to add label",
)
}
// RemoveLabel removes a label from an issue
func (s *SQLiteStorage) RemoveLabel(ctx context.Context, issueID, label, actor string) error {
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("failed to begin transaction: %w", err)
}
defer tx.Rollback()
_, err = tx.ExecContext(ctx, `
DELETE FROM labels WHERE issue_id = ? AND label = ?
`, issueID, label)
if err != nil {
return fmt.Errorf("failed to remove label: %w", err)
}
_, err = tx.ExecContext(ctx, `
INSERT INTO events (issue_id, event_type, actor, comment)
VALUES (?, ?, ?, ?)
`, issueID, types.EventLabelRemoved, actor, fmt.Sprintf("Removed label: %s", label))
if err != nil {
return fmt.Errorf("failed to record event: %w", err)
}
// Mark issue as dirty for incremental export
_, err = tx.ExecContext(ctx, `
INSERT INTO dirty_issues (issue_id, marked_at)
VALUES (?, ?)
ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at
`, issueID, time.Now())
if err != nil {
return fmt.Errorf("failed to mark issue dirty: %w", err)
}
return tx.Commit()
return s.executeLabelOperation(
ctx, issueID, actor,
`DELETE FROM labels WHERE issue_id = ? AND label = ?`,
[]interface{}{issueID, label},
types.EventLabelRemoved,
fmt.Sprintf("Removed label: %s", label),
"failed to remove label",
)
}
// GetLabels returns all labels for an issue
internal/storage/sqlite/sqlite.go
+290 -192
View File
@@ -579,6 +579,157 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act
return nil
}
// validateBatchIssues validates all issues in a batch and sets timestamps
func validateBatchIssues(issues []*types.Issue) error {
now := time.Now()
for i, issue := range issues {
if issue == nil {
return fmt.Errorf("issue %d is nil", i)
}
issue.CreatedAt = now
issue.UpdatedAt = now
if err := issue.Validate(); err != nil {
return fmt.Errorf("validation failed for issue %d: %w", i, err)
}
}
return nil
}
// generateBatchIDs generates IDs for all issues that need them atomically
func generateBatchIDs(ctx context.Context, conn *sql.Conn, issues []*types.Issue) error {
// Count how many issues need IDs
needIDCount := 0
for _, issue := range issues {
if issue.ID == "" {
needIDCount++
}
}
if needIDCount == 0 {
return nil
}
// Get prefix from config
var prefix string
err := conn.QueryRowContext(ctx, `SELECT value FROM config WHERE key = ?`, "issue_prefix").Scan(&prefix)
if err == sql.ErrNoRows || prefix == "" {
prefix = "bd"
} else if err != nil {
return fmt.Errorf("failed to get config: %w", err)
}
// Atomically reserve ID range
var nextID int
err = conn.QueryRowContext(ctx, `
INSERT INTO issue_counters (prefix, last_id)
SELECT ?, COALESCE(MAX(CAST(substr(id, LENGTH(?) + 2) AS INTEGER)), 0) + ?
FROM issues
WHERE id LIKE ? || '-%'
AND substr(id, LENGTH(?) + 2) GLOB '[0-9]*'
ON CONFLICT(prefix) DO UPDATE SET
last_id = MAX(
last_id,
(SELECT COALESCE(MAX(CAST(substr(id, LENGTH(?) + 2) AS INTEGER)), 0)
FROM issues
WHERE id LIKE ? || '-%'
AND substr(id, LENGTH(?) + 2) GLOB '[0-9]*')
) + ?
RETURNING last_id
`, prefix, prefix, needIDCount, prefix, prefix, prefix, prefix, prefix, needIDCount).Scan(&nextID)
if err != nil {
return fmt.Errorf("failed to generate ID range: %w", err)
}
// Assign IDs sequentially from the reserved range
currentID := nextID - needIDCount + 1
for i := range issues {
if issues[i].ID == "" {
issues[i].ID = fmt.Sprintf("%s-%d", prefix, currentID)
currentID++
}
}
return nil
}
// bulkInsertIssues inserts all issues using a prepared statement
func bulkInsertIssues(ctx context.Context, conn *sql.Conn, issues []*types.Issue) error {
stmt, err := conn.PrepareContext(ctx, `
INSERT INTO issues (
id, title, description, design, acceptance_criteria, notes,
status, priority, issue_type, assignee, estimated_minutes,
created_at, updated_at, closed_at, external_ref
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`)
if err != nil {
return fmt.Errorf("failed to prepare statement: %w", err)
}
defer stmt.Close()
for _, issue := range issues {
_, err = stmt.ExecContext(ctx,
issue.ID, issue.Title, issue.Description, issue.Design,
issue.AcceptanceCriteria, issue.Notes, issue.Status,
issue.Priority, issue.IssueType, issue.Assignee,
issue.EstimatedMinutes, issue.CreatedAt, issue.UpdatedAt,
issue.ClosedAt, issue.ExternalRef,
)
if err != nil {
return fmt.Errorf("failed to insert issue %s: %w", issue.ID, err)
}
}
return nil
}
// bulkRecordEvents records creation events for all issues
func bulkRecordEvents(ctx context.Context, conn *sql.Conn, issues []*types.Issue, actor string) error {
stmt, err := conn.PrepareContext(ctx, `
INSERT INTO events (issue_id, event_type, actor, new_value)
VALUES (?, ?, ?, ?)
`)
if err != nil {
return fmt.Errorf("failed to prepare event statement: %w", err)
}
defer stmt.Close()
for _, issue := range issues {
eventData, err := json.Marshal(issue)
if err != nil {
// Fall back to minimal description if marshaling fails
eventData = []byte(fmt.Sprintf(`{"id":"%s","title":"%s"}`, issue.ID, issue.Title))
}
_, err = stmt.ExecContext(ctx, issue.ID, types.EventCreated, actor, string(eventData))
if err != nil {
return fmt.Errorf("failed to record event for %s: %w", issue.ID, err)
}
}
return nil
}
// bulkMarkDirty marks all issues as dirty for incremental export
func bulkMarkDirty(ctx context.Context, conn *sql.Conn, issues []*types.Issue) error {
stmt, err := conn.PrepareContext(ctx, `
INSERT INTO dirty_issues (issue_id, marked_at)
VALUES (?, ?)
ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at
`)
if err != nil {
return fmt.Errorf("failed to prepare dirty statement: %w", err)
}
defer stmt.Close()
dirtyTime := time.Now()
for _, issue := range issues {
_, err = stmt.ExecContext(ctx, issue.ID, dirtyTime)
if err != nil {
return fmt.Errorf("failed to mark dirty %s: %w", issue.ID, err)
}
}
return nil
}
// CreateIssues creates multiple issues atomically in a single transaction.
// This provides significant performance improvements over calling CreateIssue in a loop:
// - Single connection acquisition
@@ -630,34 +781,22 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act
// - Single issue creation (use CreateIssue for simplicity)
// - Interactive user operations (use CreateIssue)
func (s *SQLiteStorage) CreateIssues(ctx context.Context, issues []*types.Issue, actor string) error {
// Handle empty batch
if len(issues) == 0 {
return nil
}
// Phase 1: Check for nil and validate all issues first (fail-fast)
now := time.Now()
for i, issue := range issues {
if issue == nil {
return fmt.Errorf("issue %d is nil", i)
}
issue.CreatedAt = now
issue.UpdatedAt = now
if err := issue.Validate(); err != nil {
return fmt.Errorf("validation failed for issue %d: %w", i, err)
}
// Phase 1: Validate all issues first (fail-fast)
if err := validateBatchIssues(issues); err != nil {
return err
}
// Phase 2: Acquire dedicated connection and start transaction
// Phase 2: Acquire connection and start transaction
conn, err := s.db.Conn(ctx)
if err != nil {
return fmt.Errorf("failed to acquire connection: %w", err)
}
defer conn.Close()
// Begin IMMEDIATE transaction to acquire write lock early
if _, err := conn.ExecContext(ctx, "BEGIN IMMEDIATE"); err != nil {
return fmt.Errorf("failed to begin immediate transaction: %w", err)
}
@@ -665,129 +804,28 @@ func (s *SQLiteStorage) CreateIssues(ctx context.Context, issues []*types.Issue,
committed := false
defer func() {
if !committed {
conn.ExecContext(context.Background(), "ROLLBACK")
_, _ = conn.ExecContext(context.Background(), "ROLLBACK")
}
}()
// Phase 3: Batch ID generation
// Count how many issues need IDs
needIDCount := 0
for _, issue := range issues {
if issue.ID == "" {
needIDCount++
}
// Phase 3: Generate IDs for issues that need them
if err := generateBatchIDs(ctx, conn, issues); err != nil {
return err
}
// Generate ID range atomically if needed
if needIDCount > 0 {
// Get prefix from config
var prefix string
err := conn.QueryRowContext(ctx, `SELECT value FROM config WHERE key = ?`, "issue_prefix").Scan(&prefix)
if err == sql.ErrNoRows || prefix == "" {
prefix = "bd"
} else if err != nil {
return fmt.Errorf("failed to get config: %w", err)
}
// Atomically reserve ID range: [nextID-needIDCount+1, nextID]
// This is the key optimization - one counter update instead of N
var nextID int
err = conn.QueryRowContext(ctx, `
INSERT INTO issue_counters (prefix, last_id)
SELECT ?, COALESCE(MAX(CAST(substr(id, LENGTH(?) + 2) AS INTEGER)), 0) + ?
FROM issues
WHERE id LIKE ? || '-%'
AND substr(id, LENGTH(?) + 2) GLOB '[0-9]*'
ON CONFLICT(prefix) DO UPDATE SET
last_id = MAX(
last_id,
(SELECT COALESCE(MAX(CAST(substr(id, LENGTH(?) + 2) AS INTEGER)), 0)
FROM issues
WHERE id LIKE ? || '-%'
AND substr(id, LENGTH(?) + 2) GLOB '[0-9]*')
) + ?
RETURNING last_id
`, prefix, prefix, needIDCount, prefix, prefix, prefix, prefix, prefix, needIDCount).Scan(&nextID)
if err != nil {
return fmt.Errorf("failed to generate ID range: %w", err)
}
// Assign IDs sequentially from the reserved range
currentID := nextID - needIDCount + 1
for i := range issues {
if issues[i].ID == "" {
issues[i].ID = fmt.Sprintf("%s-%d", prefix, currentID)
currentID++
}
}
// Phase 4: Bulk insert issues
if err := bulkInsertIssues(ctx, conn, issues); err != nil {
return err
}
// Phase 4: Bulk insert issues using prepared statement
stmt, err := conn.PrepareContext(ctx, `
INSERT INTO issues (
id, title, description, design, acceptance_criteria, notes,
status, priority, issue_type, assignee, estimated_minutes,
created_at, updated_at, closed_at, external_ref
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`)
if err != nil {
return fmt.Errorf("failed to prepare statement: %w", err)
}
defer stmt.Close()
for _, issue := range issues {
_, err = stmt.ExecContext(ctx,
issue.ID, issue.Title, issue.Description, issue.Design,
issue.AcceptanceCriteria, issue.Notes, issue.Status,
issue.Priority, issue.IssueType, issue.Assignee,
issue.EstimatedMinutes, issue.CreatedAt, issue.UpdatedAt,
issue.ClosedAt, issue.ExternalRef,
)
if err != nil {
return fmt.Errorf("failed to insert issue %s: %w", issue.ID, err)
}
// Phase 5: Record creation events
if err := bulkRecordEvents(ctx, conn, issues, actor); err != nil {
return err
}
// Phase 5: Bulk record creation events
eventStmt, err := conn.PrepareContext(ctx, `
INSERT INTO events (issue_id, event_type, actor, new_value)
VALUES (?, ?, ?, ?)
`)
if err != nil {
return fmt.Errorf("failed to prepare event statement: %w", err)
}
defer eventStmt.Close()
for _, issue := range issues {
eventData, err := json.Marshal(issue)
if err != nil {
// Fall back to minimal description if marshaling fails
eventData = []byte(fmt.Sprintf(`{"id":"%s","title":"%s"}`, issue.ID, issue.Title))
}
_, err = eventStmt.ExecContext(ctx, issue.ID, types.EventCreated, actor, string(eventData))
if err != nil {
return fmt.Errorf("failed to record event for %s: %w", issue.ID, err)
}
}
// Phase 6: Bulk mark dirty for incremental export
dirtyStmt, err := conn.PrepareContext(ctx, `
INSERT INTO dirty_issues (issue_id, marked_at)
VALUES (?, ?)
ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at
`)
if err != nil {
return fmt.Errorf("failed to prepare dirty statement: %w", err)
}
defer dirtyStmt.Close()
dirtyTime := time.Now()
for _, issue := range issues {
_, err = dirtyStmt.ExecContext(ctx, issue.ID, dirtyTime)
if err != nil {
return fmt.Errorf("failed to mark dirty %s: %w", issue.ID, err)
}
// Phase 6: Mark issues dirty for incremental export
if err := bulkMarkDirty(ctx, conn, issues); err != nil {
return err
}
// Phase 7: Commit transaction
@@ -858,6 +896,124 @@ var allowedUpdateFields = map[string]bool{
"external_ref": true,
}
// validatePriority validates a priority value
func validatePriority(value interface{}) error {
if priority, ok := value.(int); ok {
if priority < 0 || priority > 4 {
return fmt.Errorf("priority must be between 0 and 4 (got %d)", priority)
}
}
return nil
}
// validateStatus validates a status value
func validateStatus(value interface{}) error {
if status, ok := value.(string); ok {
if !types.Status(status).IsValid() {
return fmt.Errorf("invalid status: %s", status)
}
}
return nil
}
// validateIssueType validates an issue type value
func validateIssueType(value interface{}) error {
if issueType, ok := value.(string); ok {
if !types.IssueType(issueType).IsValid() {
return fmt.Errorf("invalid issue type: %s", issueType)
}
}
return nil
}
// validateTitle validates a title value
func validateTitle(value interface{}) error {
if title, ok := value.(string); ok {
if len(title) == 0 || len(title) > 500 {
return fmt.Errorf("title must be 1-500 characters")
}
}
return nil
}
// validateEstimatedMinutes validates an estimated_minutes value
func validateEstimatedMinutes(value interface{}) error {
if mins, ok := value.(int); ok {
if mins < 0 {
return fmt.Errorf("estimated_minutes cannot be negative")
}
}
return nil
}
// fieldValidators maps field names to their validation functions
var fieldValidators = map[string]func(interface{}) error{
"priority": validatePriority,
"status": validateStatus,
"issue_type": validateIssueType,
"title": validateTitle,
"estimated_minutes": validateEstimatedMinutes,
}
// validateFieldUpdate validates a field update value
func validateFieldUpdate(key string, value interface{}) error {
if validator, ok := fieldValidators[key]; ok {
return validator(value)
}
return nil
}
// determineEventType determines the event type for an update based on old and new status
func determineEventType(oldIssue *types.Issue, updates map[string]interface{}) types.EventType {
statusVal, hasStatus := updates["status"]
if !hasStatus {
return types.EventUpdated
}
newStatus, ok := statusVal.(string)
if !ok {
return types.EventUpdated
}
if newStatus == string(types.StatusClosed) {
return types.EventClosed
}
if oldIssue.Status == types.StatusClosed {
return types.EventReopened
}
return types.EventStatusChanged
}
// manageClosedAt automatically manages the closed_at field based on status changes
func manageClosedAt(oldIssue *types.Issue, updates map[string]interface{}, setClauses []string, args []interface{}) ([]string, []interface{}) {
statusVal, hasStatus := updates["status"]
if !hasStatus {
return setClauses, args
}
newStatus, ok := statusVal.(string)
if !ok {
return setClauses, args
}
if newStatus == string(types.StatusClosed) {
// Changing to closed: ensure closed_at is set
if _, hasClosedAt := updates["closed_at"]; !hasClosedAt {
now := time.Now()
updates["closed_at"] = now
setClauses = append(setClauses, "closed_at = ?")
args = append(args, now)
}
} else if oldIssue.Status == types.StatusClosed {
// Changing from closed to something else: clear closed_at
updates["closed_at"] = nil
setClauses = append(setClauses, "closed_at = ?")
args = append(args, nil)
}
return setClauses, args
}
// UpdateIssue updates fields on an issue
func (s *SQLiteStorage) UpdateIssue(ctx context.Context, id string, updates map[string]interface{}, actor string) error {
// Get old issue for event
@@ -880,37 +1036,8 @@ func (s *SQLiteStorage) UpdateIssue(ctx context.Context, id string, updates map[
}
// Validate field values
switch key {
case "priority":
if priority, ok := value.(int); ok {
if priority < 0 || priority > 4 {
return fmt.Errorf("priority must be between 0 and 4 (got %d)", priority)
}
}
case "status":
if status, ok := value.(string); ok {
if !types.Status(status).IsValid() {
return fmt.Errorf("invalid status: %s", status)
}
}
case "issue_type":
if issueType, ok := value.(string); ok {
if !types.IssueType(issueType).IsValid() {
return fmt.Errorf("invalid issue type: %s", issueType)
}
}
case "title":
if title, ok := value.(string); ok {
if len(title) == 0 || len(title) > 500 {
return fmt.Errorf("title must be 1-500 characters")
}
}
case "estimated_minutes":
if mins, ok := value.(int); ok {
if mins < 0 {
return fmt.Errorf("estimated_minutes cannot be negative")
}
}
if err := validateFieldUpdate(key, value); err != nil {
return err
}
setClauses = append(setClauses, fmt.Sprintf("%s = ?", key))
@@ -918,26 +1045,7 @@ func (s *SQLiteStorage) UpdateIssue(ctx context.Context, id string, updates map[
}
// Auto-manage closed_at when status changes (enforce invariant)
if statusVal, ok := updates["status"]; ok {
newStatus, ok := statusVal.(string)
if !ok {
return fmt.Errorf("status must be a string")
}
if newStatus == string(types.StatusClosed) {
// Changing to closed: ensure closed_at is set
if _, hasClosedAt := updates["closed_at"]; !hasClosedAt {
now := time.Now()
updates["closed_at"] = now
setClauses = append(setClauses, "closed_at = ?")
args = append(args, now)
}
} else if oldIssue.Status == types.StatusClosed {
// Changing from closed to something else: clear closed_at
updates["closed_at"] = nil
setClauses = append(setClauses, "closed_at = ?")
args = append(args, nil)
}
}
setClauses, args = manageClosedAt(oldIssue, updates, setClauses, args)
args = append(args, id)
@@ -969,17 +1077,7 @@ func (s *SQLiteStorage) UpdateIssue(ctx context.Context, id string, updates map[
oldDataStr := string(oldData)
newDataStr := string(newData)
eventType := types.EventUpdated
if statusVal, ok := updates["status"]; ok {
if statusVal == string(types.StatusClosed) {
eventType = types.EventClosed
} else if oldIssue.Status == types.StatusClosed {
// Reopening a closed issue
eventType = types.EventReopened
} else {
eventType = types.EventStatusChanged
}
}
eventType := determineEventType(oldIssue, updates)
_, err = tx.ExecContext(ctx, `
INSERT INTO events (issue_id, event_type, actor, old_value, new_value)