Skip to content

Conversation

@dilberryhoundog
Copy link
Contributor

I (Claude actually) found a bug when testing the API. Cards moved directly from "not now" to "done" don't appear in the indexed_by=closed query. This bug also prevents cards from appearing in the "status = done" filter in the UI if moved from "not now".

I decided to include some tests that ensure each "from x column to done" pathway is working, these tests would've pick up this bug. It also prevents any regressions on any of the other closure contexts.

Here is the bug report Claude produced...


Bug Report: Closed cards from NOT NOW missing from indexed_by=closed filter

Date: 2025-12-21
Discovered via: API testing
Severity: Medium (data appears correctly but filtering fails)

Summary

Cards closed directly from NOT NOW state retain their not_now record, causing them to be excluded from the indexed_by=closed filter results. The cards show closed: true in individual API responses but don't appear in filtered lists.

Reproduction Steps

  1. Create a card (starts in MAYBE?/triage)
  2. Move card to NOT NOW: POST /cards/:number/not_now
  3. Close the card directly: POST /cards/:number/closure
  4. Query closed cards: GET /cards?indexed_by=closed

Expected: Card appears in closed list
Actual: Card is missing from closed list

Root Cause

The Card#close method in app/models/card/closeable.rb does not clean up the not_now record:

def close(user: Current.user)
  unless closed?
    transaction do
      create_closure! user: user
      track_event :closed, creator: user
    end
  end
end

Meanwhile, the Filter#cards method in app/models/filter.rb excludes cards with not_now records:

result = result.where.missing(:not_now) unless include_not_now_cards?

The include_not_now_cards? method only returns true when indexed_by.not_now?, so closed cards with orphaned not_now records are filtered out.

Affected Code Paths

  • API: POST /:account/cards/:number/closure
  • UI: Clicking "Mark as Done" on a card in NOT NOW column
  • Both paths call Card#close which has the bug

Verified Behavior

Other transitions properly clean up via resume:

  • send_back_to_triage → calls resume → destroys not_now
  • triage_into(column) → calls resume → destroys not_now
  • postpone → calls send_back_to_triage first

Only close skips this cleanup.

Suggested Fix

In app/models/card/closeable.rb, add not_now&.destroy to the close method:

def close(user: Current.user)
  unless closed?
    transaction do
      not_now&.destroy  # Clean up NOT NOW state
      create_closure! user: user
      track_event :closed, creator: user
    end
  end
end

Data Cleanup

Existing orphaned records can be cleaned with:

# Find cards with both closure and not_now records
Card.joins(:closure, :not_now).find_each do |card|
  card.not_now.destroy
end

Test Cases to Add

test "close card from triage column" do
  card = cards(:logo)
  assert_equal columns(:writebook_triage), card.column

  card.close
  assert card.closed?
end

test "close card from active column" do
  card = cards(:text)
  assert_equal columns(:writebook_in_progress), card.column

  card.close
  assert card.closed?
end

test "close card from NOT NOW" do
  card = cards(:logo)

  card.postpone
  assert card.postponed?
  assert card.not_now.present?

  card.close
  assert card.closed?
  assert_nil card.reload.not_now
end

Resolution

Status: Fixed
Date: 2025-12-23

Fix Applied:

  • Added not_now&.destroy to Card#close in app/models/card/closeable.rb
  • Pattern mirrors resume method in app/models/card/postponable.rb

Tests Added:

  • Three close transition tests in test/models/card/closeable_test.rb
  • Tests verify closing works from all contexts: triage, active column, and NOT NOW

Verification:

  • API tested: Card moved to NOT NOW → closed → appeared in indexed_by=closed
  • Test suite: All 6 closeable tests pass, all 21 related filter/postponable tests pass

- Update `Card::Closeable` to destroy associated "not now" state during close.
- Add new tests to verify closure behavior for different column types, including "not now".
@dilberryhoundog
Copy link
Contributor Author

dilberryhoundog commented Dec 23, 2025

Here is how to reproduce the bug in the UI:

Start with a card in 'NOT NOW'

Screenshot 2025-12-23 at 7 09 20 pm

Move to 'DONE'

Screenshot 2025-12-23 at 7 09 49 pm

Filter the board, with a status of 'Done'

Screenshot 2025-12-23 at 7 10 34 pm

No card shows in the view

Screenshot 2025-12-23 at 7 10 49 pm

After the fix is applied the card is now showing

(Move the card back to 'NOT NOW' then close (DONE) the card again, to refresh the logic)
Screenshot 2025-12-23 at 7 12 18 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant