diff --git a/Gemfile.lock b/Gemfile.lock index e372f36..b7c1279 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - superthread (0.8.1) + superthread (0.9.0) activesupport faraday glamour @@ -267,7 +267,7 @@ CHECKSUMS standard-custom (1.0.2) sha256=424adc84179a074f1a2a309bb9cf7cd6bfdb2b6541f20c6bf9436c0ba22a652b standard-performance (1.9.0) sha256=49483d31be448292951d80e5e67cdcb576c2502103c7b40aec6f1b6e9c88e3f2 stringio (3.2.0) sha256=c37cb2e58b4ffbd33fe5cd948c05934af997b36e0b6ca6fdf43afa234cf222e1 - superthread (0.8.1) + superthread (0.9.0) thor (1.5.0) sha256=e3a9e55fe857e44859ce104a84675ab6e8cd59c650a49106a05f55f136425e73 tsort (0.2.0) sha256=9650a793f6859a43b6641671278f79cfead60ac714148aabe4e3f0060480089f tzinfo (2.0.6) sha256=8daf828cc77bcf7d63b0e3bdb6caa47e2272dcfaf4fbfe46f8c3a9df087a829b diff --git a/architecture-assessment.md b/architecture-assessment.md new file mode 100644 index 0000000..cd8b95f --- /dev/null +++ b/architecture-assessment.md @@ -0,0 +1,171 @@ +# Architecture Assessment — Superthread + +Generated: 2026-03-21 | Framework: Ruby gem (Thor CLI, Faraday HTTP, Shale models, Zeitwerk) + +## Summary + +Superthread is a well-structured Ruby gem with clean layered organization (CLI → Resources → Models), idiomatic framework usage, and consistent API interfaces across resources. The codebase aligns well with its stated philosophy of convention-driven development with MVC sensibilities — the CLI layer acts as a thin controller, resources handle API communication, and Shale models provide type-safe data representation. + +The issues found are real but manageable. The most significant are a layer inversion where a model depends on the CLI formatter, a dual object system (legacy hash-wrapper vs. Shale models) that creates branching dispatch, and some dead code that creates confusion for readers. No critical issues were found — this is a healthy codebase with room for incremental cleanup. + +**Finding counts:** Critical: 0 | High: 0 | Medium: 4 | Low: 6 + +## Lens Health + +| Lens | Health | +|------|--------| +| Structural Organization | Good | +| Design Patterns | Good | +| Composition & Inheritance | Needs Attention | +| Coupling & Cohesion | Needs Attention | +| Framework Alignment | Good | +| API & Interface Design | Good | +| Duplication & Reuse | Needs Attention | + +## Findings by Lens + +### Structural Organization + +**Health: Good** + +Layer-by-layer organization (`cli/`, `models/`, `resources/`, `objects/`) is clean and consistent. CLI concerns are properly co-located under `cli/concerns/`. Models group shared behavior under `models/concerns/`. The spec tree mirrors the source tree. Zeitwerk autoloading is configured correctly. + +### Design Patterns + +**Health: Good** + +Factory error pattern (`ApiError.from_response`) is well applied. The resource base class provides consistent HTTP helper methods. Shale model layer is a clean adapter. Concerns act as mixins for cross-cutting behavior. No forced or over-applied patterns detected. + +### Composition & Inheritance + +**Health: Needs Attention** + +#### Finding: Dead code — Confirmable concern superseded by inline method in Base + +- **Severity:** Medium +- **Location:** `lib/superthread/cli/concerns/confirmable.rb`, `lib/superthread/cli/base.rb:343-352` +- **What:** `Confirmable` defines a `confirming` method using `options[:force]`, but `Base` defines its own `confirming` using `options[:skip_confirm]`. The concern is never included anywhere. The option name mismatch means including it would silently shadow `Base#confirming` with different semantics. The docstring example shows `include Concerns::Confirmable` — a pattern that was planned but never executed. +- **Why it matters:** Creates confusion for any developer who reads it and assumes it's active. The dead code needs to be kept in sync with the live version or it will drift further. +- **Recommendation:** Delete `confirmable.rb`. The behavior lives correctly in `Base#confirming`. + +#### Finding: Dead code — LinkedCard model is never used + +- **Severity:** Low +- **Location:** `lib/superthread/models/card.rb:245-256` +- **What:** `LinkedCard < Card` adds a single `linked_card_type` attribute but is never referenced outside its own file. The actual linked-card path uses `LinkedCardRef` via `Card#links`. A reader sees two parallel hierarchies (`LinkedCard < Card` and `LinkedCardRef < CardRef`) and has to work out which is live. +- **Why it matters:** `LinkedCard` looks authoritative because it's a proper Shale model inheriting the full 30-attribute weight of `Card`. The real path (`LinkedCardRef`) is less obvious. This is a readability trap. +- **Recommendation:** Delete `LinkedCard`. The `LinkedCardRef` pattern already handles the use case correctly. + +#### Finding: LinkedCardRef#to_s overrides identical inherited method + +- **Severity:** Low +- **Location:** `lib/superthread/models/card.rb:316-318` vs `287-289` +- **What:** `LinkedCardRef#to_s` returns `"#{title} (#{id})"` — character-for-character identical to `CardRef#to_s` which it inherits. The override is pure noise. +- **Why it matters:** A reader assumes the override is intentional and that it does something different. +- **Recommendation:** Remove the override. + +#### Finding: Base includes 7 resolver concerns unconditionally + +- **Severity:** Low +- **Location:** `lib/superthread/cli/base.rb:26-32` +- **What:** Every CLI subclass inherits all 7 resolver concerns (`WorkspaceResolvable`, `SpaceResolvable`, `BoardResolvable`, etc.) regardless of whether it uses them. Commands like `Accounts`, `Config`, and `Search` likely use none of them. +- **Why it matters:** At this project's scale this is not a practical problem — all concerns are lazy (private, called only on demand) so there's no runtime cost. Noted for completeness. +- **Recommendation:** No action needed at current scale. Revisit if a heavyweight concern with initialization cost is added. + +### Coupling & Cohesion + +**Health: Needs Attention** + +#### Finding: Model references CLI layer constant (layer inversion) + +- **Severity:** Medium +- **Location:** `lib/superthread/models/card.rb:201`, `lib/superthread/cli/formatter.rb:54-59` +- **What:** `Models::Card#priority_name` calls `Cli::Formatter::PRIORITY_LABELS` directly. The model layer has an upward dependency on the CLI layer. Any non-CLI consumer of `Models::Card` that calls `#priority_name` will pull in the entire `Cli::Formatter` module (which requires `active_support` and `unicode/display_width`). +- **Why it matters:** Models are the innermost layer; they should have zero knowledge of the CLI. This breaks the MVC principle and makes the model untestable in isolation without loading CLI infrastructure. +- **Recommendation:** Move `PRIORITY_LABELS` into the model layer — either directly on `Models::Card` as `PRIORITY_NAMES = {4=>"urgent", 3=>"high", 2=>"medium", 1=>"low"}.freeze` or in a small constants module. Have `Cli::Formatter` reference that shared constant instead. This reverses the dependency direction without changing behavior. + +#### Finding: Dual object system with branching dispatch + +- **Severity:** Medium +- **Location:** `lib/superthread/client.rb:127-151`, `lib/superthread/objects/collection.rb:156-168` +- **What:** The client and collection both carry an `if shale_model?` branch routing to either Shale deserialization or the legacy `Superthread::Object` hash wrapper. Several endpoints (`add_related`, `add_member`, `add_tags`, search) still return untyped `Object` instances. +- **Why it matters:** Every developer must understand two object protocols — the static Shale attribute API and the dynamic `method_missing` hash-key API. The branching dispatch is duplicated in two places (Client and Collection), and callers expecting typed models from untyped endpoints get silent surprises. +- **Recommendation:** Define minimal typed models for mutation endpoints (e.g., `Models::OperationResult`) and `Models::SearchResult` for search. Migrate incrementally until the `shale_model?` branches can be removed. + +#### Finding: normalize_timestamp is a data utility living in the CLI layer + +- **Severity:** Low +- **Location:** `lib/superthread/cli/formatter.rb:79-83` +- **What:** Pure data utility (converts millisecond timestamps to seconds) called from Cards CLI, Activity CLI, and the `DateParsable` concern — always via `Formatter` delegation. +- **Why it matters:** Same layer-inversion pattern as `PRIORITY_LABELS`. Any future resource or model needing timestamp normalization must depend on the CLI layer. +- **Recommendation:** Move to a `Superthread::TimeUtils` module or as a class method on `Superthread::Model`. Have Formatter delegate to it. + +#### Finding: Dead code — workspace_path in Resources::Base + +- **Severity:** Low +- **Location:** `lib/superthread/resources/base.rb:205-208` +- **What:** Defined but never called anywhere. Every resource builds workspace-prefixed paths inline using `safe_id` directly. +- **Why it matters:** Creates a false API surface — a future developer may use it believing it's the canonical pattern. +- **Recommendation:** Delete it. + +### Framework Alignment + +**Health: Good** + +Thor is used correctly with subcommands, `class_options`, and `exit_on_failure?`. Faraday is used as a thin connection with no unnecessary middleware complexity. Shale is used declaratively as intended. Zeitwerk autoloading is correctly configured with `eager_load_namespace` for namespaces that need early registration. `ActiveSupport::Concern` is used properly for all mixins. No reinvented wheels detected. + +### API & Interface Design + +**Health: Good** + +Resource interfaces are highly consistent: all take `workspace_id` as a first positional argument, additional IDs positionally, and named params for optional data. `compact_params` and `safe_id` helpers are uniformly applied. CLI output methods (`output_item`, `output_list`, `output_success`, `output_error`) provide a coherent output API. The `--json` flag is respected throughout. + +### Duplication & Reuse + +**Health: Needs Attention** + +#### Finding: apply_date_filters duplicates DateParsable#filter_by_date + +- **Severity:** Medium +- **Location:** `lib/superthread/cli/cards.rb:541-565`, `lib/superthread/cli/concerns/date_parsable.rb:103-113` +- **What:** Cards defines its own `apply_date_filters` and `meets_date_threshold?` private methods that replicate the same logic the already-included `DateParsable` concern provides via `filter_by_date` — iterate items, normalize timestamps, gate on a threshold. +- **Why it matters:** If timestamp normalization logic changes in `filter_by_date`, `apply_date_filters` silently diverges. The concern exists specifically to avoid this duplication. +- **Recommendation:** Delete both private methods from `cards.rb`. Replace with two chained `filter_by_date` calls: `cards = filter_by_date(cards, field: :time_created, since: parse_date(options[:since]))` and `cards = filter_by_date(cards, field: :time_updated, since: parse_date(options[:updated_since]))`. + +#### Finding: Cards CLI uses inline begin/rescue instead of with_not_found + +- **Severity:** Low +- **Location:** `lib/superthread/cli/cards.rb` (5 sites: lines 110-114, 228-274, 287-291, 311-318, 350) +- **What:** Cards has 5 sites using inline `begin/rescue` blocks to catch `ForbiddenError`/`NotFoundError`, while every other CLI class uses the `with_not_found` helper consistently. +- **Why it matters:** The inline pattern is noisier (4-5 lines vs 1 wrapper call) and creates inconsistency across the CLI layer. +- **Recommendation:** Replace each inline `begin/rescue` with `with_not_found` calls, following the `pages.rb` pattern. + +#### Finding: resolve_list_with_context handles three dispatch paths inline + +- **Severity:** Low +- **Location:** `lib/superthread/cli/cards.rb:503-539` +- **What:** 36-line method with three distinct dispatch paths: explicit context, sprint context, or board context. Each builds its result differently. +- **Why it matters:** Well-commented but dense. The sprint lookup path has non-obvious list matching logic (case-insensitive name match with ID fallback). +- **Recommendation:** Extract to three private methods: `resolve_list_with_explicit_context`, `resolve_list_in_sprint_context`, `resolve_list_on_board`. Do this when next touching the move feature. + +## Prioritized Recommendations + +### Quick Wins + +1. **Delete dead code** — Remove `Confirmable` concern, `LinkedCard` model, `LinkedCardRef#to_s` override, and `workspace_path` helper. Four deletions, zero risk. (~30 minutes) +2. **Fix layer inversion for PRIORITY_LABELS** — Move the constant from `Cli::Formatter` to `Models::Card`. Update Formatter to reference the model constant. (~30 minutes) +3. **Replace apply_date_filters with DateParsable concern** — Delete the duplicate methods, use the existing concern. (~30 minutes) + +### Medium Efforts + +4. **Migrate Cards CLI to use with_not_found** — Replace 5 inline begin/rescue blocks with the helper, following pages.rb as the model. (~1-2 hours) +5. **Move normalize_timestamp to a shared utility** — Create `Superthread::TimeUtils` or add to `Model` base class. Update all call sites. (~1 hour) +6. **Extract resolve_list_with_context into three methods** — Readability improvement for the card move feature. (~1 hour) + +### Strategic Restructuring + +7. **Migrate remaining legacy Object endpoints to typed Shale models** — Define `Models::OperationResult`, `Models::SearchResult`, etc. Migrate `add_related`, `add_member`, `add_tags`, and search endpoints. Remove `shale_model?` branching from Client and Collection. (Multi-session effort, each endpoint is isolated) + +## Action Plan + +*Empty until decisions are made on which recommendations to pursue.* diff --git a/lib/superthread/cli/activity.rb b/lib/superthread/cli/activity.rb index f22e84b..4c77ae2 100644 --- a/lib/superthread/cli/activity.rb +++ b/lib/superthread/cli/activity.rb @@ -89,9 +89,9 @@ def categorize_activity(cards, since_ts) } cards.each do |card| - created_ts = normalize_timestamp(card.time_created) - updated_ts = normalize_timestamp(card.time_updated) - completed_ts = normalize_timestamp(card.completed_date) + created_ts = TimeUtils.normalize_timestamp(card.time_created) + updated_ts = TimeUtils.normalize_timestamp(card.time_updated) + completed_ts = TimeUtils.normalize_timestamp(card.completed_date) # Check for completion (takes precedence) if completed_ts && completed_ts >= since_ts @@ -113,14 +113,6 @@ def categorize_activity(cards, since_ts) activity end - # Normalize timestamp to seconds (API sometimes returns milliseconds). - # - # @param ts [Integer, nil] Timestamp - # @return [Integer, nil] Normalized timestamp in seconds - def normalize_timestamp(ts) - Formatter.normalize_timestamp(ts) - end - # Output activity as JSON. # # @param activity [Hash] the categorized activity data with :created, :updated, :completed keys diff --git a/lib/superthread/cli/cards.rb b/lib/superthread/cli/cards.rb index f28a5d4..0024000 100644 --- a/lib/superthread/cli/cards.rb +++ b/lib/superthread/cli/cards.rb @@ -41,10 +41,11 @@ def list end opts[:list_id] = resolve_list(options[:list]) if options[:list] - cards = client.cards.list(workspace_id, **opts) + cards = client.cards.list(workspace_id, **opts).to_a # Client-side date filtering (API doesn't support time-based filters) - cards = apply_date_filters(cards) + cards = filter_by_date(cards, field: :time_created, since: parse_date(options[:since])) if options[:since] + cards = filter_by_date(cards, field: :time_updated, since: parse_date(options[:updated_since])) if options[:updated_since] enrich_members(cards) output_list cards, columns: %i[id title priority list_title members], @@ -107,10 +108,8 @@ def search(term) # @return [void] def get(card_id) handle_error do - begin - card = client.cards.find(workspace_id, card_id) - rescue Superthread::ForbiddenError, Superthread::NotFoundError - raise Thor::Error, "Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards." + card = with_not_found("Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards.") do + client.cards.find(workspace_id, card_id) end enrich_members(card) @@ -224,54 +223,63 @@ def create def update(card_id) handle_error do require_space_for_sprint! + not_found_msg = "Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards." - begin - # Update content via dedicated PUT endpoint (separate from PATCH) - if options[:content] + # Update content via dedicated PUT endpoint (separate from PATCH) + if options[:content] + with_not_found(not_found_msg) do client.cards.update_content(workspace_id, card_id, content: options[:content]) end + end - # Update other fields via PATCH (skip if only content was provided) - has_patch_fields = options[:title] || options[:list] || options[:priority] || - !options[:archived].nil? || options[:epic] || options[:position] || options[:sprint] - - if has_patch_fields - # WORKAROUND: API ignores title when combined with list_id, - # so we make separate requests when both are provided. - # TODO: Remove when API is fixed (https://superthread.com/api/known-issues) - if options[:list] && (options[:title] || options[:priority] || !options[:archived].nil? || options[:epic]) - # First update non-move fields - field_opts = symbolized_options(:title, :priority, :archived) - field_opts[:epic_id] = options[:epic] if options[:epic] - client.cards.update(workspace_id, card_id, **field_opts) unless field_opts.empty? - - # Then move the card (include sprint context) - move_opts = resolve_list_with_context(options[:list], card_id) - move_opts[:position] = options[:position] if options[:position] - card = client.cards.update(workspace_id, card_id, **move_opts) - else - opts = symbolized_options(:title, :priority, :archived) - opts[:epic_id] = options[:epic] if options[:epic] - opts[:position] = options[:position] if options[:position] - - if options[:list] - opts.merge!(resolve_list_with_context(options[:list], card_id)) - elsif options[:sprint] - # Moving to a sprint without --list — default to first list - opts[:sprint_id] = sprint_id - opts[:project_id] = space_id - sprint_obj = client.sprints.find(workspace_id, sprint_id, space_id: space_id) - opts[:list_id] = sprint_obj.lists.first&.id + # Update other fields via PATCH (skip if only content was provided) + has_patch_fields = options[:title] || options[:list] || options[:priority] || + !options[:archived].nil? || options[:epic] || options[:position] || options[:sprint] + + if has_patch_fields + # WORKAROUND: API ignores title when combined with list_id, + # so we make separate requests when both are provided. + # TODO: Remove when API is fixed (https://superthread.com/api/known-issues) + if options[:list] && (options[:title] || options[:priority] || !options[:archived].nil? || options[:epic]) + # First update non-move fields + field_opts = symbolized_options(:title, :priority, :archived) + field_opts[:epic_id] = options[:epic] if options[:epic] + unless field_opts.empty? + with_not_found(not_found_msg) do + client.cards.update(workspace_id, card_id, **field_opts) end + end + + # Then move the card (include sprint context) + move_opts = resolve_list_with_context(options[:list], card_id) + move_opts[:position] = options[:position] if options[:position] + card = with_not_found(not_found_msg) do + client.cards.update(workspace_id, card_id, **move_opts) + end + else + opts = symbolized_options(:title, :priority, :archived) + opts[:epic_id] = options[:epic] if options[:epic] + opts[:position] = options[:position] if options[:position] + + if options[:list] + opts.merge!(resolve_list_with_context(options[:list], card_id)) + elsif options[:sprint] + # Moving to a sprint without --list — default to first list + opts[:sprint_id] = sprint_id + opts[:project_id] = space_id + sprint_obj = client.sprints.find(workspace_id, sprint_id, space_id: space_id) + opts[:list_id] = sprint_obj.lists.first&.id + end - card = client.cards.update(workspace_id, card_id, **opts) + card = with_not_found(not_found_msg) do + client.cards.update(workspace_id, card_id, **opts) end end + end - # If only content was updated, fetch the card for output - card ||= client.cards.find(workspace_id, card_id) - rescue Superthread::ForbiddenError, Superthread::NotFoundError - raise Thor::Error, "Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards." + # If only content was updated, fetch the card for output + card ||= with_not_found(not_found_msg) do + client.cards.find(workspace_id, card_id) end output_item card, labels: {id: "Card ID"} end @@ -284,10 +292,8 @@ def update(card_id) # @return [void] def delete(card_ref) handle_error do - begin - card = client.cards.find(workspace_id, card_ref) - rescue Superthread::ForbiddenError, Superthread::NotFoundError - raise Thor::Error, "Card not found: '#{card_ref}'. Use 'suth cards list -b BOARD' to see available cards." + card = with_not_found("Card not found: '#{card_ref}'. Use 'suth cards list -b BOARD' to see available cards.") do + client.cards.find(workspace_id, card_ref) end confirming("Delete card '#{card.title}' (#{card.id})?") do client.cards.destroy(workspace_id, card.id) @@ -308,14 +314,12 @@ def delete(card_ref) # @return [void] def duplicate(card_id) handle_error do - begin - opts = symbolized_options(:title) - opts[:project_id] = options[:project] - opts[:board_id] = board_id - opts[:list_id] = resolve_list(options[:list]) - card = client.cards.duplicate(workspace_id, card_id, **opts) - rescue Superthread::ForbiddenError, Superthread::NotFoundError - raise Thor::Error, "Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards." + opts = symbolized_options(:title) + opts[:project_id] = options[:project] + opts[:board_id] = board_id + opts[:list_id] = resolve_list(options[:list]) + card = with_not_found("Card not found: '#{card_id}'. Use 'suth cards list -b BOARD' to see available cards.") do + client.cards.duplicate(workspace_id, card_id, **opts) end output_item card, labels: {id: "Card ID"} end @@ -339,10 +343,11 @@ def assigned(user_ref) opts[:user_id] = resolve_user(user_ref) opts[:board_id] = board_id if options[:board] opts[:project_id] = options[:project] if options[:project] - cards = client.cards.assigned(workspace_id, **opts) + cards = client.cards.assigned(workspace_id, **opts).to_a # Client-side date filtering (API doesn't support time-based filters) - cards = apply_date_filters(cards) + cards = filter_by_date(cards, field: :time_created, since: parse_date(options[:since])) if options[:since] + cards = filter_by_date(cards, field: :time_updated, since: parse_date(options[:updated_since])) if options[:updated_since] enrich_members(cards) output_list cards, columns: %i[id title priority list_title members], @@ -492,76 +497,73 @@ def enrich_members(cards) # Resolve a list reference and include sprint context when needed. # # The API requires sprint_id and project_id alongside list_id when moving - # cards within a sprint. This method handles three scenarios: - # 1. --board or --sprint given: resolve list name with that context - # 2. Neither given: fetch the card to discover its sprint/board context - # 3. Card is on a board (no sprint): resolve normally + # cards within a sprint. Dispatches to one of three strategies based on + # whether explicit context (--board/--sprint) was provided. # # @param list_ref [String] the list ID or name to resolve # @param card_id [String] the card identifier to look up for context # @return [Hash] params hash with :list_id and optional :sprint_id, :project_id def resolve_list_with_context(list_ref, card_id) - result = {} - if options[:board] || options[:sprint] - result[:list_id] = resolve_list(list_ref) - if options[:sprint] - result[:sprint_id] = sprint_id - result[:project_id] = space_id - end + resolve_list_with_explicit_context(list_ref) else existing = client.cards.find(workspace_id, card_id) if existing.sprint_id - sprint_obj = client.sprints.find(workspace_id, existing.sprint_id, - space_id: existing.project_id) - list = sprint_obj.lists&.find { |l| l.title&.downcase == list_ref.downcase } - unless list || looks_like_id?(list_ref) - available = sprint_obj.lists&.map(&:title)&.join(", ") || "none" - raise Thor::Error, "List '#{list_ref}' not found in sprint. Available: #{available}" - end - result[:list_id] = list ? list.id : list_ref - result[:sprint_id] = existing.sprint_id - result[:project_id] = existing.project_id - elsif looks_like_id?(list_ref) - result[:list_id] = list_ref + resolve_list_in_sprint_context(existing, list_ref) else - board = client.boards.find(workspace_id, existing.board_id) - list = board.lists&.find { |l| l.title&.downcase == list_ref.downcase } - unless list - available = board.lists&.map(&:title)&.join(", ") || "none" - raise Thor::Error, "List '#{list_ref}' not found on board. Available: #{available}" - end - result[:list_id] = list.id + resolve_list_on_board(existing, list_ref) end end + end + # Resolve list when --board or --sprint was explicitly provided. + # + # @param list_ref [String] the list ID or name + # @return [Hash] params hash with :list_id and optional :sprint_id, :project_id + def resolve_list_with_explicit_context(list_ref) + result = {list_id: resolve_list(list_ref)} + if options[:sprint] + result[:sprint_id] = sprint_id + result[:project_id] = space_id + end result end - # Apply date filters to a collection of cards. - # The API doesn't support time-based filtering, so we filter client-side. + # Resolve list for a card that lives in a sprint. # - # @param cards [Enumerable] the cards to apply --since and --updated_since filters to - # @return [Array] Filtered cards - def apply_date_filters(cards) - since_ts = parse_date(options[:since]) if options[:since] - updated_ts = parse_date(options[:updated_since]) if options[:updated_since] - return cards.to_a unless since_ts || updated_ts - - cards.to_a.select do |card| - (since_ts.nil? || meets_date_threshold?(card.time_created, since_ts)) && - (updated_ts.nil? || meets_date_threshold?(card.time_updated, updated_ts)) + # @param existing [Card] the current card with sprint context + # @param list_ref [String] the list ID or name + # @return [Hash] params hash with :list_id, :sprint_id, :project_id + def resolve_list_in_sprint_context(existing, list_ref) + sprint_obj = client.sprints.find(workspace_id, existing.sprint_id, + space_id: existing.project_id) + list = sprint_obj.lists&.find { |l| l.title&.downcase == list_ref.downcase } + unless list || looks_like_id?(list_ref) + available = sprint_obj.lists&.map(&:title)&.join(", ") || "none" + raise Thor::Error, "List '#{list_ref}' not found in sprint. Available: #{available}" end + { + list_id: list ? list.id : list_ref, + sprint_id: existing.sprint_id, + project_id: existing.project_id + } end - # Check if a timestamp meets a minimum threshold after normalization. + # Resolve list for a card that lives on a board (no sprint). # - # @param timestamp [Object] the raw timestamp value from the model - # @param threshold [Integer] the minimum Unix timestamp - # @return [Boolean] true if the normalized timestamp meets or exceeds the threshold - def meets_date_threshold?(timestamp, threshold) - ts = Formatter.normalize_timestamp(timestamp) - ts && ts >= threshold + # @param existing [Card] the current card with board context + # @param list_ref [String] the list ID or name + # @return [Hash] params hash with :list_id + def resolve_list_on_board(existing, list_ref) + return {list_id: list_ref} if looks_like_id?(list_ref) + + board = client.boards.find(workspace_id, existing.board_id) + list = board.lists&.find { |l| l.title&.downcase == list_ref.downcase } + unless list + available = board.lists&.map(&:title)&.join(", ") || "none" + raise Thor::Error, "List '#{list_ref}' not found on board. Available: #{available}" + end + {list_id: list.id} end # Output card relationships (parent, children, links) in human-readable format. diff --git a/lib/superthread/cli/concerns/board_resolvable.rb b/lib/superthread/cli/concerns/board_resolvable.rb index 6330a9a..9e7e446 100644 --- a/lib/superthread/cli/concerns/board_resolvable.rb +++ b/lib/superthread/cli/concerns/board_resolvable.rb @@ -4,6 +4,7 @@ module Superthread module Cli + # Shared behavior modules for CLI commands. module Concerns # Resolves board references (ID or name) to board IDs. # diff --git a/lib/superthread/cli/concerns/confirmable.rb b/lib/superthread/cli/concerns/confirmable.rb deleted file mode 100644 index 24f302f..0000000 --- a/lib/superthread/cli/concerns/confirmable.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "active_support/concern" - -module Superthread - module Cli - # Shared behavior modules for CLI commands. - module Concerns - # Provides confirmation prompts for destructive actions. - # Respects the --force flag to skip confirmation. - # - # @example - # class Cards < Base - # include Concerns::Confirmable - # - # def delete(card_id) - # handle_error do - # confirming("Delete card #{card_id}?") do - # client.cards.destroy(workspace_id, card_id) - # Ui.success "Card #{card_id} deleted" - # end - # end - # end - # end - module Confirmable - extend ActiveSupport::Concern - - # Execute a block with optional confirmation prompt. - # - # Skips confirmation if --force flag is set, otherwise prompts user. - # - # @param message [String] the confirmation question to display - # @yield the block to execute if user confirms - # @return [Object, nil] the block's return value, or nil if cancelled - def confirming(message) - if options[:force] || confirm_action(message) - yield - else - Ui.muted "Cancelled." - end - end - - private - - # Prompt user for yes/no confirmation via the UI. - # - # @param message [String] the confirmation question to display - # @return [Boolean] true if user confirms, false otherwise - def confirm_action(message) - Ui.confirm(message) - end - end - end - end -end diff --git a/lib/superthread/cli/concerns/date_parsable.rb b/lib/superthread/cli/concerns/date_parsable.rb index 0412bfd..eeedb83 100644 --- a/lib/superthread/cli/concerns/date_parsable.rb +++ b/lib/superthread/cli/concerns/date_parsable.rb @@ -105,7 +105,7 @@ def filter_by_date(items, field:, since: nil, until_time: nil) ts = item.send(field) next false if ts.nil? - ts = Formatter.normalize_timestamp(ts) + ts = Superthread::TimeUtils.normalize_timestamp(ts) next false if ts.nil? (since.nil? || ts >= since) && (until_time.nil? || ts <= until_time) diff --git a/lib/superthread/cli/formatter.rb b/lib/superthread/cli/formatter.rb index d509483..662403a 100644 --- a/lib/superthread/cli/formatter.rb +++ b/lib/superthread/cli/formatter.rb @@ -50,17 +50,9 @@ module Formatter }.freeze # Human-readable labels for priority levels. + # References the canonical constant from Models::Card. # @return [Hash{Integer => String}] - PRIORITY_LABELS = { - 4 => "urgent", - 3 => "high", - 2 => "medium", - 1 => "low" - }.freeze - - # Threshold for detecting millisecond timestamps vs second timestamps. - # Timestamps above this value are assumed to be in milliseconds. - MAX_SECONDS_TIMESTAMP = 9_999_999_999 + PRIORITY_LABELS = Superthread::Models::Card::PRIORITY_NAMES module_function @@ -73,13 +65,12 @@ def strip_html(content) end # Normalizes a timestamp to seconds (API sometimes returns milliseconds). + # Delegates to {Superthread::TimeUtils.normalize_timestamp}. # # @param ts [Integer, nil] timestamp in seconds or milliseconds # @return [Integer, nil] timestamp in seconds, or nil if nil/zero def normalize_timestamp(ts) - return nil if ts.nil? || ts == 0 - - (ts > MAX_SECONDS_TIMESTAMP) ? ts / 1000 : ts + Superthread::TimeUtils.normalize_timestamp(ts) end # Truncates a string to a maximum length with an ellipsis indicator. diff --git a/lib/superthread/models/card.rb b/lib/superthread/models/card.rb index 2a26d9e..383ba40 100644 --- a/lib/superthread/models/card.rb +++ b/lib/superthread/models/card.rb @@ -22,6 +22,15 @@ class Card < Superthread::Model include Concerns::Presentable include Concerns::Timestampable + # Human-readable labels for priority levels. + # @return [Hash{Integer => String}] + PRIORITY_NAMES = { + 4 => "urgent", + 3 => "high", + 2 => "medium", + 1 => "low" + }.freeze + detail_fields :id, :title, :status, :priority, :list_title, :board_title, :time_created, :time_updated list_columns :id, :title, :status, :priority, :list_title @@ -198,7 +207,7 @@ def bookmarked? # # @return [String, nil] priority label (urgent, high, medium, low) def priority_name - Cli::Formatter::PRIORITY_LABELS[priority] + PRIORITY_NAMES[priority] end # Returns the parent card as a CardRef, or nil if none. @@ -233,28 +242,6 @@ def links_by_type end end - # Represents a linked card with relationship type. - # - # Extends Card with linked_card_type attribute to indicate how this - # card relates to another (blocks, blocked_by, related, duplicates). - # - # @example - # linked = card.linked_cards.first - # linked.relationship # => "blocks" - # linked.title # => "Other Card" - class LinkedCard < Card - # @!attribute [rw] linked_card_type - # @return [String] relationship type (blocks, blocked_by, related, duplicates) - attribute :linked_card_type, Shale::Type::String - - # Returns the relationship type between this card and the linked card. - # - # @return [String] relationship type (blocks, blocked_by, related, duplicates) - def relationship - linked_card_type - end - end - # Lightweight reference to a card (id + title only). # # Used for parent/child relationships to avoid circular model references. @@ -309,13 +296,6 @@ def initialize(data) super @relationship = data["linked_card_type"] || data[:linked_card_type] end - - # Returns a human-readable string representation. - # - # @return [String] title with ID in parentheses - def to_s - "#{title} (#{id})" - end end end end diff --git a/lib/superthread/resources/base.rb b/lib/superthread/resources/base.rb index 9116fd4..5318309 100644 --- a/lib/superthread/resources/base.rb +++ b/lib/superthread/resources/base.rb @@ -197,16 +197,6 @@ def format_mentions(workspace_id, content) MentionFormatter.new(@client, workspace_id).format(content) end - # Builds an API path prefixed with the workspace ID. - # - # @param workspace_id [String] the workspace identifier - # @param path [String] additional path segments to append - # @return [String] the full API path (e.g., "/ws-123/cards") - def workspace_path(workspace_id, path = "") - ws = safe_id("workspace_id", workspace_id) - "/#{ws}#{path}" - end - # Returns a success response object for delete operations. # # @return [Superthread::Object] a response object with success: true diff --git a/lib/superthread/time_utils.rb b/lib/superthread/time_utils.rb new file mode 100644 index 0000000..dff7e0c --- /dev/null +++ b/lib/superthread/time_utils.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Superthread + # Shared timestamp utilities used across layers. + module TimeUtils + # Threshold for detecting millisecond timestamps vs second timestamps. + # Timestamps above this value are assumed to be in milliseconds. + MAX_SECONDS_TIMESTAMP = 9_999_999_999 + + module_function + + # Normalizes a timestamp to seconds (API sometimes returns milliseconds). + # + # @param ts [Integer, nil] timestamp in seconds or milliseconds + # @return [Integer, nil] timestamp in seconds, or nil if nil/zero + def normalize_timestamp(ts) + return nil if ts.nil? || ts == 0 + + (ts > MAX_SECONDS_TIMESTAMP) ? ts / 1000 : ts + end + end +end diff --git a/lib/superthread/version.rb b/lib/superthread/version.rb index bad5827..178b56d 100644 --- a/lib/superthread/version.rb +++ b/lib/superthread/version.rb @@ -2,5 +2,5 @@ module Superthread # Current version of the Superthread gem. - VERSION = "0.8.1" + VERSION = "0.9.0" end