Keep the times when an issue is copied

Redmine's issue copy carries attributes but not associations, so a copy
of an issue with times silently became all-day, although the design doc
promised otherwise. A copied issue now inherits the original's sidecar
row as the source for the sync, so the times of day survive and are
re-anchored on the copy's (possibly changed) dates (#17).

Also drop the dead `return true` callback idiom; callbacks halt only
via throw :abort (#20).

Author: dkastl

lib/redmine_issue_datetime/issue_extension.rb

@@ -26,10 +26,10 @@ def due_time=(value)
       @due_time_input = value
     end
 
-    # "All day" is the absence of times rather than a stored flag, so the setter
-    # is sugar for clearing both. It is applied after the time setters regardless
-    # of parameter order, so ticking the box always wins over whatever the
-    # (disabled, and therefore possibly stale) time inputs submitted.
+    # "All day" means "no times"; there is no stored flag. Setting it to
+    # true clears both times. The flag is applied during the save, not
+    # here, so it wins regardless of the order the attributes were
+    # assigned in.
     def all_day=(value)
       @all_day_input = ActiveRecord::Type::Boolean.new.cast(value)
     end
@@ -53,24 +53,29 @@ def due_time
 
     def prepare_issue_datetime_sync
       @issue_datetime_pending = nil
-      return true unless RedmineIssueDatetime.enabled_for?(tracker_id)
+      return unless RedmineIssueDatetime.enabled_for?(tracker_id)
 
-      # Ticking "all day" clears both times whatever the (disabled) time inputs
-      # submitted, so it cannot be defeated by a stale value in the form.
+      # Ticking "all day" clears both times, even if the (disabled) time
+      # inputs still submitted stale values.
       if @all_day_input
         @start_time_input = ''
         @due_time_input = ''
       end
 
-      record = issue_datetime
-      return true if record.nil? && @start_time_input.blank? && @due_time_input.blank?
+      # A copied issue has no sidecar row of its own yet, so use the
+      # original's row as the source. This is what makes a copy keep its
+      # times. Core exposes the original only as the @copied_from ivar
+      # (its public API is just copy?).
+      source = issue_datetime
+      source ||= @copied_from.issue_datetime if new_record? && copy?
+      return if source.nil? && @start_time_input.blank? && @due_time_input.blank?
 
       touched = !@start_time_input.nil? || !@due_time_input.nil? ||
                 will_save_change_to_start_date? || will_save_change_to_due_date?
-      return true unless touched
+      return unless touched
 
-      old_starts = record&.starts_at
-      old_ends = record&.ends_at
+      old_starts = source&.starts_at
+      old_ends = source&.ends_at
       new_starts = RedmineIssueDatetime.combine(start_date, @start_time_input, old_starts)
       new_ends = RedmineIssueDatetime.combine(due_date, @due_time_input, old_ends)
 
@@ -83,7 +88,6 @@ def prepare_issue_datetime_sync
       journalize_issue_datetime('due_time', old_ends, new_ends)
 
       @issue_datetime_pending = {starts_at: new_starts, ends_at: new_ends}
-      true
     end
 
     def persist_issue_datetime

test/unit/issue_copy_test.rb

@@ -0,0 +1,65 @@
+require File.expand_path('../test_helper', __dir__)
+
+class IssueCopyTest < ActiveSupport::TestCase
+  include IssueDatetimeTestHelper
+
+  fixtures :projects, :users, :email_addresses, :trackers, :projects_trackers,
+           :issue_statuses, :issues, :enumerations, :enabled_modules,
+           :members, :member_roles, :roles
+
+  def setup
+    @issue = Issue.find(1)
+    enable_issue_datetime(@issue.tracker_id)
+    @issue.update!(start_date: Date.new(2026, 8, 3), start_time: '09:15',
+                   due_date: Date.new(2026, 8, 4), due_time: '17:00')
+    @issue.reload
+  end
+
+  # The sidecar row is an association, so Redmine's issue copy does not carry
+  # it over by itself; the copy support in IssueExtension does.
+  test 'copying an issue keeps its times' do
+    copy = @issue.copy
+    assert copy.save
+
+    record = copy.reload.issue_datetime
+    assert_not_nil record, 'the copy must get its own sidecar row'
+    assert_equal Time.utc(2026, 8, 3, 9, 15), record.starts_at
+    assert_equal Time.utc(2026, 8, 4, 17, 0), record.ends_at
+  end
+
+  test 'a copy with changed dates keeps the times of day' do
+    copy = @issue.copy
+    copy.start_date = Date.new(2026, 9, 1)
+    copy.due_date = Date.new(2026, 9, 2)
+    assert copy.save
+
+    record = copy.reload.issue_datetime
+    assert_equal Time.utc(2026, 9, 1, 9, 15), record.starts_at
+    assert_equal Time.utc(2026, 9, 2, 17, 0), record.ends_at
+  end
+
+  test 'copying an all-day issue stays all day' do
+    @issue.all_day = '1'
+    assert @issue.save
+
+    copy = @issue.reload.copy
+    assert copy.save
+    assert_nil copy.reload.issue_datetime
+  end
+
+  test 'times set while copying win over the inherited ones' do
+    copy = @issue.copy
+    copy.start_time = '10:30'
+    assert copy.save
+
+    assert_equal Time.utc(2026, 8, 3, 10, 30), copy.reload.issue_datetime.starts_at
+  end
+
+  test 'the original keeps its own row untouched' do
+    copy = @issue.copy
+    assert copy.save
+
+    assert_not_equal @issue.reload.issue_datetime.id, copy.reload.issue_datetime.id
+    assert_equal Time.utc(2026, 8, 3, 9, 15), @issue.issue_datetime.starts_at
+  end
+end