From cb16bda200be7457d55226d784eee0523479a29b Mon Sep 17 00:00:00 2001 From: Elias Kohout Date: Tue, 31 Mar 2026 23:10:56 +0200 Subject: [PATCH] refactor: simplify service interface to use tags/rels for all node properties Co-Authored-By: Claude Sonnet 4.6 --- cmd/add.go | 35 +++++--- cmd/list.go | 29 +++++-- cmd/update.go | 28 ++++--- service/node_service.go | 55 +++++-------- service/node_service_impl.go | 151 ++++++++++++++++------------------- 5 files changed, 151 insertions(+), 147 deletions(-) diff --git a/cmd/add.go b/cmd/add.go index 1fa6a06..a515b35 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -1,6 +1,7 @@ package cmd import ( + "axolotl/models" "axolotl/output" "axolotl/service" "fmt" @@ -9,7 +10,7 @@ import ( "github.com/spf13/cobra" ) -var cDue, cContent, cStatus, cPrio, cType, cNamespace, cAssignee string +var cDue, cContent, cType, cStatus, cPrio, cNamespace, cAssignee string var cTags, cRels []string var addCmd = &cobra.Command{ @@ -22,15 +23,27 @@ var addCmd = &cobra.Command{ } input := service.AddInput{ - Title: args[0], - Content: cContent, - DueDate: cDue, - Type: cType, - Status: cStatus, - Priority: cPrio, - Namespace: cNamespace, - Assignee: cAssignee, - Tags: cTags, + Title: args[0], + Content: cContent, + DueDate: cDue, + Tags: append([]string{}, cTags...), + } + + // Shorthand flags expand to tags or rels. + if cType != "" { + input.Tags = append(input.Tags, "_type::"+cType) + } + if cStatus != "" { + input.Tags = append(input.Tags, "_status::"+cStatus) + } + if cPrio != "" { + input.Tags = append(input.Tags, "_prio::"+cPrio) + } + if cNamespace != "" { + input.Rels = append(input.Rels, service.RelInput{Type: models.RelInNamespace, Target: cNamespace}) + } + if cAssignee != "" { + input.Rels = append(input.Rels, service.RelInput{Type: models.RelAssignee, Target: cAssignee}) } for _, r := range cRels { @@ -55,7 +68,7 @@ var addCmd = &cobra.Command{ func init() { rootCmd.AddCommand(addCmd) f := addCmd.Flags() - f.StringVar(&cType, "type", "issue", "node type (issue, note, …)") + f.StringVar(&cType, "type", "", "node type (issue, note, …)") f.StringVar(&cStatus, "status", "", "initial status (open, done)") f.StringVar(&cPrio, "prio", "", "priority (high, medium, low)") f.StringVar(&cNamespace, "namespace", "", "namespace name or ID") diff --git a/cmd/list.go b/cmd/list.go index cc0ae6d..1b47b07 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -1,6 +1,7 @@ package cmd import ( + "axolotl/models" "axolotl/output" "axolotl/service" "fmt" @@ -22,13 +23,27 @@ var listCmd = &cobra.Command{ } filter := service.ListFilter{ - Tags: lTags, - Status: lStatus, - Priority: lPrio, - Type: lType, - Namespace: lNamespace, - Assignee: lAssignee, - Mention: lMention, + Tags: append([]string{}, lTags...), + } + + // Shorthand flags expand to tag prefixes or rel filters. + if lStatus != "" { + filter.Tags = append(filter.Tags, "_status::"+lStatus) + } + if lPrio != "" { + filter.Tags = append(filter.Tags, "_prio::"+lPrio) + } + if lType != "" { + filter.Tags = append(filter.Tags, "_type::"+lType) + } + if lNamespace != "" { + filter.Rels = append(filter.Rels, service.RelInput{Type: models.RelInNamespace, Target: lNamespace}) + } + if lAssignee != "" { + filter.Rels = append(filter.Rels, service.RelInput{Type: models.RelAssignee, Target: lAssignee}) + } + if lMention != "" { + filter.Rels = append(filter.Rels, service.RelInput{Type: models.RelMentions, Target: lMention}) } for _, r := range lRels { diff --git a/cmd/update.go b/cmd/update.go index e86879d..81ab40f 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -1,6 +1,7 @@ package cmd import ( + "axolotl/models" "axolotl/output" "axolotl/service" "fmt" @@ -10,10 +11,11 @@ import ( ) var ( - uTitle, uContent, uDue string - uClearDue bool - uStatus, uPrio, uType, uNamespace, uAssignee string - uAddTags, uRmTags, uAddRels, uRmRels []string + uTitle, uContent, uDue string + uClearDue bool + uStatus, uPrio, uType string + uNamespace, uAssignee string + uAddTags, uRmTags, uAddRels, uRmRels []string ) var updateCmd = &cobra.Command{ @@ -26,7 +28,7 @@ var updateCmd = &cobra.Command{ } input := service.UpdateInput{ - AddTags: uAddTags, + AddTags: append([]string{}, uAddTags...), RemoveTags: uRmTags, } @@ -43,20 +45,22 @@ var updateCmd = &cobra.Command{ empty := "" input.DueDate = &empty } + + // Shorthand flags expand to tags or rels. + if cmd.Flags().Changed("type") { + input.AddTags = append(input.AddTags, "_type::"+uType) + } if cmd.Flags().Changed("status") { - input.Status = &uStatus + input.AddTags = append(input.AddTags, "_status::"+uStatus) } if cmd.Flags().Changed("prio") { - input.Priority = &uPrio - } - if cmd.Flags().Changed("type") { - input.Type = &uType + input.AddTags = append(input.AddTags, "_prio::"+uPrio) } if cmd.Flags().Changed("namespace") { - input.Namespace = &uNamespace + input.AddRels = append(input.AddRels, service.RelInput{Type: models.RelInNamespace, Target: uNamespace}) } if cmd.Flags().Changed("assignee") { - input.Assignee = &uAssignee + input.AddRels = append(input.AddRels, service.RelInput{Type: models.RelAssignee, Target: uAssignee}) } for _, r := range uAddRels { diff --git a/service/node_service.go b/service/node_service.go index 4de3e82..24af545 100644 --- a/service/node_service.go +++ b/service/node_service.go @@ -24,56 +24,37 @@ type NodeService interface { } // AddInput describes a new node to create. +// Tags may include special property tags (_type::, _status::, _prio::); the +// service applies defaults (type=issue, status=open for issues) and validates. +// Rels may include assignee, in_namespace, blocks, subtask, related, etc.; +// user and namespace targets are auto-created as needed. type AddInput struct { - Title string - Content string - DueDate string - Type string // default: "issue" - Status string // default: "open" when Type is "issue" - Priority string - // Namespace is a namespace name or node ID. Defaults to the current user. - Namespace string - // Assignee is a username or node ID. - Assignee string - // Tags are arbitrary user-defined labels (not system properties). - Tags []string - // Rels are additional typed edges (e.g. blocks, subtask, related). - Rels []RelInput + Title string + Content string + DueDate string + Tags []string + Rels []RelInput } // UpdateInput describes changes to apply to an existing node. // Nil pointer fields mean "no change". +// Setting _status::done in AddTags is rejected when the node has open blockers. +// Adding assignee or in_namespace rels replaces the previous single target. type UpdateInput struct { - Title *string - Content *string - DueDate *string // nil = no change; pointer to "" = clear due date - // Status "done" is rejected when the node has open blockers. - Status *string - Priority *string - Type *string - // Namespace replaces the current namespace. - Namespace *string - // Assignee replaces the current assignee. - Assignee *string + Title *string + Content *string + DueDate *string // nil = no change; pointer to "" = clear due date AddTags []string RemoveTags []string AddRels []RelInput RemoveRels []RelInput } -// ListFilter specifies which nodes to return. Empty fields are ignored. +// ListFilter specifies which nodes to return. Empty slices are ignored. +// Tags are matched as exact tag values or prefixes (e.g. "_status::open"). +// Rels are resolved to node IDs; a missing target returns no results. type ListFilter struct { - Tags []string - Status string - Priority string - Type string - // Namespace filters by namespace name or node ID. - Namespace string - // Assignee filters by username or node ID. - Assignee string - // Mention filters to nodes that mention the given username or node ID. - Mention string - // Rels are additional relation filters (e.g. blocks:someID). + Tags []string Rels []RelInput } diff --git a/service/node_service_impl.go b/service/node_service_impl.go index 5518407..41a7bcd 100644 --- a/service/node_service_impl.go +++ b/service/node_service_impl.go @@ -7,6 +7,7 @@ import ( "maps" "regexp" "slices" + "strings" "time" ) @@ -25,6 +26,43 @@ func mentions(t string) []string { return slices.Collect(maps.Keys(seen)) } +// --- Validation --- + +var ( + validTypes = map[string]bool{"issue": true, "note": true, "user": true, "namespace": true} + validStatuses = map[string]bool{"open": true, "done": true} + validPrios = map[string]bool{"high": true, "medium": true, "low": true} +) + +func validateTags(tags []string) error { + for _, t := range tags { + if v, ok := strings.CutPrefix(t, "_type::"); ok { + if !validTypes[v] { + return fmt.Errorf("invalid type %q: must be one of issue, note, user, namespace", v) + } + } else if v, ok := strings.CutPrefix(t, "_status::"); ok { + if !validStatuses[v] { + return fmt.Errorf("invalid status %q: must be one of open, done", v) + } + } else if v, ok := strings.CutPrefix(t, "_prio::"); ok { + if !validPrios[v] { + return fmt.Errorf("invalid priority %q: must be one of high, medium, low", v) + } + } + } + return nil +} + +// tagValue returns the value of the first tag with the given prefix, or "". +func tagValue(tags []string, prefix string) string { + for _, t := range tags { + if v, ok := strings.CutPrefix(t, prefix); ok { + return v + } + } + return "" +} + // --- Query --- func (s *nodeServiceImpl) GetByID(id string) (*models.Node, error) { @@ -32,86 +70,51 @@ func (s *nodeServiceImpl) GetByID(id string) (*models.Node, error) { } func (s *nodeServiceImpl) List(filter ListFilter) ([]*models.Node, error) { - // Build tag prefixes from both semantic fields and raw tags. - tagPrefixes := append([]string{}, filter.Tags...) - if filter.Status != "" { - tagPrefixes = append(tagPrefixes, "_status::"+filter.Status) - } - if filter.Priority != "" { - tagPrefixes = append(tagPrefixes, "_prio::"+filter.Priority) - } - if filter.Type != "" { - tagPrefixes = append(tagPrefixes, "_type::"+filter.Type) - } - - // Build rel filters, resolving names to node IDs (read-only, no auto-creation). - type relEntry struct { - relType models.RelType - name string - } - namedRels := []relEntry{ - {models.RelAssignee, filter.Assignee}, - {models.RelInNamespace, filter.Namespace}, - {models.RelMentions, filter.Mention}, - } var relFilters []*models.Rel - for _, e := range namedRels { - if e.name == "" { - continue - } - id, ok := s.lookupRelTarget(e.relType, e.name) - if !ok { - return nil, nil // named target doesn't exist; no nodes can match - } - relFilters = append(relFilters, &models.Rel{Type: e.relType, Target: id}) - } for _, ri := range filter.Rels { id, ok := s.lookupRelTarget(ri.Type, ri.Target) if !ok { - return nil, nil + return nil, nil // named target doesn't exist; no nodes can match } relFilters = append(relFilters, &models.Rel{Type: ri.Type, Target: id}) } - - return s.store.FindNodes(tagPrefixes, relFilters) + return s.store.FindNodes(filter.Tags, relFilters) } // --- Lifecycle --- func (s *nodeServiceImpl) Add(input AddInput) (*models.Node, error) { + // Copy tags so we can extend without mutating the input. + tags := make([]string, len(input.Tags)) + copy(tags, input.Tags) + // Apply defaults. - nodeType := input.Type + nodeType := tagValue(tags, "_type::") if nodeType == "" { nodeType = "issue" + tags = append(tags, "_type::issue") } - status := input.Status - if status == "" && nodeType == "issue" { - status = "open" + if nodeType == "issue" && tagValue(tags, "_status::") == "" { + tags = append(tags, "_status::open") } - // Build initial tag set from semantic fields. - tags := []string{"_type::" + nodeType} - if status != "" { - tags = append(tags, "_status::"+status) + // Validate special tags. + if err := validateTags(tags); err != nil { + return nil, err } - if input.Priority != "" { - tags = append(tags, "_prio::"+input.Priority) - } - tags = append(tags, input.Tags...) - // Build initial relation map from semantic fields. + // Build initial relation map from rels input. rels := make(map[models.RelType][]string) - if input.Namespace != "" { - rels[models.RelInNamespace] = []string{input.Namespace} - } else { - rels[models.RelInNamespace] = []string{s.userID} // default: creator's namespace - } - if input.Assignee != "" { - rels[models.RelAssignee] = []string{input.Assignee} - } + hasNamespace := false for _, ri := range input.Rels { + if ri.Type == models.RelInNamespace { + hasNamespace = true + } rels[ri.Type] = append(rels[ri.Type], ri.Target) } + if !hasNamespace { + rels[models.RelInNamespace] = []string{s.userID} + } id, err := s.store.GenerateID() if err != nil { @@ -176,12 +179,20 @@ func (s *nodeServiceImpl) Add(input AddInput) (*models.Node, error) { func (s *nodeServiceImpl) Update(id string, input UpdateInput) (*models.Node, error) { // Enforce blocking constraint before allowing status=done. - if input.Status != nil && *input.Status == "done" { - if err := s.checkBlockers(id); err != nil { - return nil, err + for _, t := range input.AddTags { + if t == "_status::done" { + if err := s.checkBlockers(id); err != nil { + return nil, err + } + break } } + // Validate tags being added. + if err := validateTags(input.AddTags); err != nil { + return nil, err + } + err := s.store.Transaction(func(st store.Store) error { current, err := st.GetNode(id) if err != nil { @@ -211,15 +222,6 @@ func (s *nodeServiceImpl) Update(id string, input UpdateInput) (*models.Node, er for _, t := range current.Tags() { tmp.AddTag(t) } - if input.Type != nil { - tmp.AddTag("_type::" + *input.Type) - } - if input.Status != nil { - tmp.AddTag("_status::" + *input.Status) - } - if input.Priority != nil { - tmp.AddTag("_prio::" + *input.Priority) - } for _, t := range input.AddTags { tmp.AddTag(t) } @@ -251,18 +253,8 @@ func (s *nodeServiceImpl) Update(id string, input UpdateInput) (*models.Node, er } } - // Build relation additions, including structured fields. - var addRels []RelInput - if input.Namespace != nil { - addRels = append(addRels, RelInput{Type: models.RelInNamespace, Target: *input.Namespace}) - } - if input.Assignee != nil { - addRels = append(addRels, RelInput{Type: models.RelAssignee, Target: *input.Assignee}) - } - addRels = append(addRels, input.AddRels...) - currentRels := current.Relations() - for _, ri := range addRels { + for _, ri := range input.AddRels { resolved, err := s.resolveRelTarget(st, ri) if err != nil { return err @@ -485,4 +477,3 @@ func (s *nodeServiceImpl) ensureNamespace(st store.Store, name string) (string, } return id, nil } -