API: Allow updates to last_active_at (#2076)

* Allow Card#last_updated_at to be set

This is useful when doing an import from another system. I'm currently
working on a script to import our Github issues into Fizzy.

This is discussed in
https://github.com/basecamp/fizzy/pull/2056#discussion_r2609560246

* Add nil fallback and expand test coverage for last_active_at

Adds a safety fallback to Time.current if created_at is unexpectedly nil
during card creation.

Test coverage to verify:
* last_active_at defaults to created_at when not provided
* last_active_at can be updated via API on existing cards
* import workflow where last_active_at is restored after comments
* publishing doesn't overwrite explicit last_active_at values

---------

Co-authored-by: Jeremy Daer <jeremy@37signals.com>

app/controllers/cards_controller.rb

@@ -61,6 +61,6 @@ class CardsController < ApplicationController
     end
 
     def card_params
-      params.expect(card: [ :status, :title, :description, :image, :created_at, tag_ids: [] ])
+      params.expect(card: [ :status, :title, :description, :image, :created_at, :last_active_at, tag_ids: [] ])
     end
 end

app/models/card/eventable.rb

@@ -4,7 +4,7 @@ module Card::Eventable
   include ::Eventable
 
   included do
-    before_create { self.last_active_at = Time.current }
+    before_create { self.last_active_at ||= created_at || Time.current }
 
     after_save :track_title_change, if: :saved_change_to_title?
   end
@@ -12,7 +12,7 @@ module Card::Eventable
   def event_was_created(event)
     transaction do
       create_system_comment_for(event)
-      touch_last_active_at
+      touch_last_active_at unless was_just_published?
     end
   end
 

docs/API.md

@@ -496,6 +496,7 @@ Creates a new card in a board.
 | `image` | file | No | Header image for the card |
 | `tag_ids` | array | No | Array of tag IDs to apply to the card |
 | `created_at` | datetime | No | Override creation timestamp (ISO 8601 format) |
+| `last_active_at` | datetime | No | Override last activity timestamp (ISO 8601 format) |
 
 __Request:__
 
@@ -523,6 +524,7 @@ Updates a card.
 | `status` | string | No | Card status: `drafted`, `published` |
 | `image` | file | No | Header image for the card |
 | `tag_ids` | array | No | Array of tag IDs to apply to the card |
+| `last_active_at` | datetime | No | Override last activity timestamp (ISO 8601 format) |
 
 __Request:__
 

test/controllers/cards_controller_test.rb

@@ -21,8 +21,9 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
     end
 
     card = Card.last
-    assert card.drafted?
     assert_redirected_to card
+
+    assert card.drafted?
   end
 
   test "create resumes existing draft if it exists" do
@@ -30,10 +31,9 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
     assert_no_difference -> { Card.count } do
       post board_cards_path(boards(:writebook))
-    end
-
       assert_redirected_to draft
     end
+  end
 
   test "show" do
     get card_path(cards(:logo))
@@ -68,6 +68,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
     boards(:writebook).update! all_access: false
     boards(:writebook).accesses.revoke_from users(:kevin)
+
     get card_path(cards(:logo))
     assert_response :not_found
   end
@@ -75,6 +76,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
   test "admins can see delete button on any card" do
     get card_path(cards(:logo))
     assert_response :success
+
     assert_match "Delete this card", response.body
   end
 
@@ -83,6 +85,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
     get card_path(cards(:logo))
     assert_response :success
+
     assert_match "Delete this card", response.body
   end
 
@@ -91,6 +94,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
     get card_path(cards(:logo))
     assert_response :success
+
     assert_no_match "Delete this card", response.body
   end
 
@@ -124,10 +128,9 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
   test "show card with comment containing malformed remote image attachment" do
     card = cards(:logo)
-    card.comments.create!(
+    card.comments.create! \
       creator: users(:kevin),
       body: '<action-text-attachment url="image.png" content-type="image/*" presentation="gallery"></action-text-attachment>'
-    )
 
     get card_path(card)
     assert_response :success
@@ -140,6 +143,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
 
     get card_path(card), as: :json
     assert_response :success
+
     assert_equal card.title, @response.parsed_body["title"]
     assert_equal 2, @response.parsed_body["steps"].size
   end
@@ -149,12 +153,12 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
       post board_cards_path(boards(:writebook)),
         params: { card: { title: "My new card", description: "Big if true", tag_ids: [ tags(:web).id, tags(:mobile).id ] } },
         as: :json
+      assert_response :created
     end
 
-    assert_response :created
-    assert_equal card_path(Card.last, format: :json), @response.headers["Location"]
-
     card = Card.last
+    assert_equal card_path(card, format: :json), @response.headers["Location"]
+
     assert_equal "My new card", card.title
     assert_equal "Big if true", card.description.to_plain_text
     assert_equal [ tags(:mobile), tags(:web) ].sort, card.tags.sort
@@ -167,25 +171,91 @@ class CardsControllerTest < ActionDispatch::IntegrationTest
       post board_cards_path(boards(:writebook)),
         params: { card: { title: "Backdated card", created_at: custom_time } },
         as: :json
+      assert_response :created
     end
 
-    assert_response :created
     assert_equal custom_time, Card.last.created_at
   end
 
+  test "create as JSON with custom last_active_at" do
+    created_time = Time.utc(2024, 1, 15, 10, 30, 0)
+    last_active_time = Time.utc(2024, 6, 1, 12, 0, 0)
+
+    assert_difference -> { Card.count }, +1 do
+      post board_cards_path(boards(:writebook)),
+        params: { card: { title: "Card with activity", created_at: created_time, last_active_at: last_active_time } },
+        as: :json
+      assert_response :created
+    end
+
+    card = Card.last
+    assert_equal created_time, card.created_at
+    assert_equal last_active_time, card.last_active_at
+  end
+
+  test "create as JSON defaults last_active_at to created_at when not provided" do
+    created_time = Time.utc(2024, 1, 15, 10, 30, 0)
+
+    assert_difference -> { Card.count }, +1 do
+      post board_cards_path(boards(:writebook)),
+        params: { card: { title: "Backdated card without last_active_at", created_at: created_time } },
+        as: :json
+      assert_response :created
+    end
+
+    card = Card.last
+    assert_equal created_time, card.created_at
+    assert_equal created_time, card.last_active_at
+  end
+
+  test "update as JSON with custom last_active_at" do
+    card = cards(:logo)
+    custom_time = Time.utc(2024, 3, 15, 14, 0, 0)
+
+    put card_path(card, format: :json), params: { card: { last_active_at: custom_time } }
+
+    assert_response :success
+    assert_equal custom_time, card.reload.last_active_at
+  end
+
+  test "update as JSON can restore last_active_at after comments overwrite it" do
+    created_time = Time.utc(2024, 1, 15, 10, 30, 0)
+    last_active_time = Time.utc(2024, 6, 1, 12, 0, 0)
+
+    # Create a card with custom timestamps (simulating import)
+    post board_cards_path(boards(:writebook)),
+      params: { card: { title: "Imported card", created_at: created_time, last_active_at: last_active_time } },
+      as: :json
+    assert_response :created
+
+    card = Card.last
+
+    # Adding a comment overwrites last_active_at (this is expected)
+    card.comments.create!(creator: users(:kevin), body: "Imported comment")
+    assert_not_equal last_active_time, card.reload.last_active_at
+
+    # After import, restore the correct last_active_at
+    put card_path(card, format: :json), params: { card: { last_active_at: last_active_time } }
+    assert_response :success
+
+    assert_equal last_active_time, card.reload.last_active_at
+  end
+
   test "update as JSON" do
     card = cards(:logo)
-    put card_path(card, format: :json), params: { card: { title: "Update test" } }
 
+    put card_path(card, format: :json), params: { card: { title: "Update test" } }
     assert_response :success
+
     assert_equal "Update test", card.reload.title
   end
 
   test "delete as JSON" do
     card = cards(:logo)
-    delete card_path(card, format: :json)
 
+    delete card_path(card, format: :json)
     assert_response :no_content
+
     assert_not Card.exists?(card.id)
   end
 end

test/models/card/eventable_test.rb

@@ -5,11 +5,47 @@ class Card::EventableTest < ActiveSupport::TestCase
     Current.session = sessions(:david)
   end
 
-  test "new cards get the current time as the last activity time" do
+  test "new cards default last_active_at to created_at" do
     freeze_time
 
-    card = boards(:writebook).cards.create!(title: "Some card card", creator: users(:david))
-    assert_equal Time.current, card.last_active_at
+    card = boards(:writebook).cards.create!(title: "Some card", creator: users(:david))
+    assert_equal card.created_at, card.last_active_at
+  end
+
+  test "new cards with custom created_at default last_active_at to that time" do
+    custom_time = 1.week.ago.change(usec: 0)
+
+    card = boards(:writebook).cards.create!(title: "Backdated card", creator: users(:david), created_at: custom_time)
+    assert_equal custom_time, card.created_at
+    assert_equal custom_time, card.last_active_at
+  end
+
+  test "new cards preserve explicit last_active_at" do
+    created_time = 2.weeks.ago.change(usec: 0)
+    last_active_time = 1.week.ago.change(usec: 0)
+
+    card = boards(:writebook).cards.create! \
+      title: "Card with explicit timestamps",
+      creator: users(:david),
+      created_at: created_time,
+      last_active_at: last_active_time
+
+    assert_equal created_time, card.created_at
+    assert_equal last_active_time, card.last_active_at
+  end
+
+  test "publishing a card does not overwrite last_active_at" do
+    created_time = 2.weeks.ago.change(usec: 0)
+    last_active_time = 1.week.ago.change(usec: 0)
+
+    card = boards(:writebook).cards.create! \
+      title: "Published card",
+      creator: users(:david),
+      status: :published,
+      created_at: created_time,
+      last_active_at: last_active_time
+
+    assert_equal last_active_time, card.last_active_at
   end
 
   test "tracking events update the last activity time" do