[COMMS-934] Adjust WorkPackageAtTimestampRepresenter with target_versions

app/models/journable/timestamps.rb

@@ -93,20 +93,8 @@ def column_names_missing_in_journal
   def at_timestamp(timestamp)
     return unless journal = journals.at_timestamp(timestamp).first
 
-    attributes = journal.data.attributes.merge(
-      {
-        "id" => id,
-        "created_at" => created_at,
-        "updated_at" => journal.updated_at,
-        "timestamp" => timestamp,
-        "journal_id" => journal.id
-      }
-    )
-    self.class.column_names_missing_in_journal.each do |missing_column_name|
-      attributes[missing_column_name] = nil
-    end
-    journable = self.class.instantiate(attributes)
-    ::Journable::WithHistoricAttributes.load_custom_values(journable)
+    journable = self.class.instantiate(historic_attributes(journal, timestamp))
+    ::Journable::WithHistoricAttributes.load_journal_associations(journable)
     journable.readonly!
     journable
   end
@@ -128,4 +116,20 @@ def rollback!
 
     self.class.find(id).update! attributes.except("id", "timestamp", "journal_id", "created_at", "updated_at")
   end
+
+  private
+
+  def historic_attributes(journal, timestamp)
+    attributes = journal.data.attributes.merge(
+      "id" => id,
+      "created_at" => created_at,
+      "updated_at" => journal.updated_at,
+      "timestamp" => timestamp,
+      "journal_id" => journal.id
+    )
+    self.class.column_names_missing_in_journal.each do |missing_column_name|
+      attributes[missing_column_name] = nil
+    end
+    attributes
+  end
 end

app/models/journable/with_historic_attributes.rb

@@ -120,8 +120,8 @@ def wrap(journable_or_journables,
       end
     end
 
-    def load_custom_values(journalized)
-      Loader.new(journalized).load_custom_values
+    def load_journal_associations(journalized)
+      Loader.new(journalized).load_journal_associations
     end
 
     private
@@ -262,9 +262,36 @@ def changes_at_timestamp(timestamp)
       )
     )
 
+    merge_target_versions_changes!(changes, historic_journable)
+
     changes
   end
 
+  def merge_target_versions_changes!(changes, historic_journable)
+    return unless __getobj__.respond_to?(:target_versions)
+
+    changes.delete("version_id")
+    changes.merge!(target_versions_changes(historic_journable))
+  end
+
+  def target_versions_changes(historic_journable)
+    old_ids = sorted_target_version_ids(historic_journable)
+    new_ids = sorted_target_version_ids(__getobj__)
+
+    {}.tap do |changes|
+      changes["version_id"] = [old_ids.first, new_ids.first] if old_ids.first != new_ids.first
+      changes["target_versions"] = [old_ids, new_ids].map { joined_version_ids(it) } if old_ids != new_ids
+    end
+  end
+
+  def sorted_target_version_ids(work_package)
+    work_package.target_versions.map(&:id).sort
+  end
+
+  def joined_version_ids(ids)
+    ids.join(",").presence
+  end
+
   def changed_attributes_at_timestamp(timestamp)
     changes_at_timestamp(timestamp)&.transform_values(&:last)
   end

app/models/journable/with_historic_attributes/loader.rb

@@ -56,7 +56,7 @@ def work_package_ids_of_query_at_timestamp(query:, timestamp: nil)
       @work_package_ids_of_query_at_timestamp[query][timestamp]
     end
 
-    def load_custom_values(journalized = journables)
+    def load_journal_associations(journalized = journables)
       journal_ids = begin
         journalized.map(&:journal_id)
       rescue NoMethodError
@@ -66,16 +66,33 @@ def load_custom_values(journalized = journables)
               "ie: WorkPackage.at_timestamp(1.day.ago) or WorkPackage.find(1).at_timestamp(1.day.ago)"
       end
 
+      load_custom_value_associations(journalized, journal_ids)
+      load_target_versions_associations(journalized, journal_ids)
+
+      journalized
+    end
+
+    private
+
+    def load_custom_value_associations(journalized, journal_ids)
       customizable_journals_by_journal_id = load_customizable_journals_by_journal_id(journal_ids)
 
       journalized.each do |work_package|
         customizable_journals = Array(customizable_journals_by_journal_id[work_package.journal_id])
         set_custom_value_association_from_journal!(work_package:, customizable_journals:)
       end
-      journalized
     end
 
-    private
+    def load_target_versions_associations(journalized, journal_ids)
+      return unless journalized_class.method_defined?(:target_versions)
+
+      version_journals_by_journal_id = load_version_journals_by_journal_id(journal_ids)
+
+      journalized.each do |work_package|
+        version_journals = Array(version_journals_by_journal_id[work_package.journal_id])
+        set_target_versions_association_from_journal!(work_package:, version_journals:)
+      end
+    end
 
     def work_package_ids_of_query_at_timestamp_calculation(query, timestamp)
       query = query.dup
@@ -97,7 +114,7 @@ def currently_invisible_journables
 
     def journalized_at_timestamp(tms)
       journalized = (currently_invisible_journalized_at_timestamp(tms) + currently_visible_journalized_at_timestamp(tms))
-      load_custom_values(journalized)
+      load_journal_associations(journalized)
     end
 
     def currently_invisible_journalized_at_timestamp(timestamp)
@@ -119,6 +136,13 @@ def load_customizable_journals_by_journal_id(journal_ids)
         .group_by(&:journal_id)
     end
 
+    def load_version_journals_by_journal_id(journal_ids)
+      Journal::WorkPackageVersionJournal
+        .where(journal_id: journal_ids, kind: "target")
+        .includes(:version)
+        .group_by(&:journal_id)
+    end
+
     def set_custom_value_association_from_journal!(work_package:, customizable_journals:)
       # Build the associated customizable_journals as custom values, this way the historic work packages
       # will behave just as the normal ones. Additionally set the reverse customized association
@@ -131,6 +155,13 @@ def set_custom_value_association_from_journal!(work_package:, customizable_journ
       work_package.association(:custom_values).target = historic_custom_values
     end
 
+    def set_target_versions_association_from_journal!(work_package:, version_journals:)
+      historic_versions = version_journals.filter_map(&:version).sort_by(&:id)
+
+      work_package.association(:target_versions).loaded!
+      work_package.association(:target_versions).target = historic_versions
+    end
+
     attr_accessor :journables
   end
   private_constant :Loader

lib/api/v3/work_packages/eager_loading/historic_attributes.rb

@@ -106,10 +106,18 @@ def timestamps
 
     def override_attributes(work_package, source)
       work_package.attributes = source.attributes.except("timestamp", "journal_id")
+      override_target_versions(work_package, source)
       work_package.clear_changes_information
       work_package.readonly!
     end
 
+    def override_target_versions(work_package, source)
+      return unless work_package.respond_to?(:target_versions)
+
+      work_package.association(:target_versions).loaded!
+      work_package.association(:target_versions).target = source.target_versions.to_a
+    end
+
     def set_timestamp_attributes(work_package, source, timestamp)
       work_package.matches_filters_at_timestamp = source.matches_query_filters_at_timestamps.include?(timestamp)
       work_package.exists_at_timestamp = source.exists_at_timestamps.include?(timestamp)

lib/api/v3/work_packages/work_package_at_timestamp_representer.rb

@@ -51,6 +51,7 @@ class WorkPackageAtTimestampRepresenter < WorkPackageRepresenter
           priority
           type
           version
+          target_versions
           parent
         ].freeze
 
@@ -101,7 +102,7 @@ def rendered_properties
         # contains the lower camel-cased names.
         def rendered_properties_for_links
           @rendered_properties_for_links ||= rendered_properties.map do |property|
-            if property.starts_with?("custom_field_")
+            if property.starts_with?("custom_field_") || property == "target_versions"
               API::Utilities::PropertyNameConverter.from_ar_name(property)
             else
               property

spec/lib/api/v3/work_packages/work_package_at_timestamp_representer_rendering_spec.rb

@@ -49,6 +49,7 @@
   let(:type) { build_stubbed(:type) }
   let(:priority) { build_stubbed(:priority) }
   let(:version) { build_stubbed(:version) }
+  let(:target_versions) { [version] }
   let(:parent) do
     build_stubbed(:work_package).tap do |wp|
       allow(wp)
@@ -97,12 +98,14 @@
               .with(:wrapped?)
               .and_return(true)
       allow(wp)
-        .to receive_messages(available_custom_fields:, custom_field_values: [custom_value])
+        .to receive_messages(available_custom_fields:,
+                             custom_field_values: [custom_value],
+                             effective_target_versions: target_versions)
     end
   end
   let(:timestamp) { Timestamp.new(1.day.ago) }
 
-  let(:attributes_changed_to_baseline) { work_package.attributes.keys + ["custom_field_#{custom_field.id}"] }
+  let(:attributes_changed_to_baseline) { work_package.attributes.keys + ["custom_field_#{custom_field.id}", "target_versions"] }
   let(:exists_at_timestamp) { true }
   let(:with_query) { true }
   let(:matches_filters_at_timestamp) { true }
@@ -169,6 +172,12 @@
             "href" => api_v3_paths.version(version.id),
             "title" => version.name
           },
+          "targetVersions" => [
+            {
+              "href" => api_v3_paths.version(version.id),
+              "title" => version.name
+            }
+          ],
           "parent" => {
             "displayId" => parent.display_id.to_s,
             "href" => api_v3_paths.work_package(parent.id),
@@ -228,6 +237,124 @@
     end
   end
 
+  context "with the target versions changed together with the single-value version" do
+    let(:version_b) { build_stubbed(:version) }
+    let(:target_versions) { [version, version_b] }
+    let(:attributes_changed_to_baseline) { %w[version_id target_versions] }
+
+    let(:expected_json) do
+      {
+        "_meta" => {
+          "matchesFilters" => true,
+          "exists" => true,
+          "timestamp" => timestamp.to_s
+        },
+        "_links" => {
+          "version" => {
+            "href" => api_v3_paths.version(version.id),
+            "title" => version.name
+          },
+          "targetVersions" => [
+            {
+              "href" => api_v3_paths.version(version.id),
+              "title" => version.name
+            },
+            {
+              "href" => api_v3_paths.version(version_b.id),
+              "title" => version_b.name
+            }
+          ],
+          "self" => {
+            "href" => api_v3_paths.work_package(work_package.id, timestamps: timestamp),
+            "title" => work_package.subject
+          },
+          "schema" => {
+            "href" => api_v3_paths.work_package_schema(work_package.project_id, work_package.type_id)
+          }
+        }
+      }.to_json
+    end
+
+    it "renders as expected" do
+      expect(subject)
+        .to be_json_eql(expected_json)
+    end
+  end
+
+  context "with only the target versions changed" do
+    let(:version_b) { build_stubbed(:version) }
+    let(:target_versions) { [version, version_b] }
+    let(:attributes_changed_to_baseline) { %w[target_versions] }
+
+    let(:expected_json) do
+      {
+        "_meta" => {
+          "matchesFilters" => true,
+          "exists" => true,
+          "timestamp" => timestamp.to_s
+        },
+        "_links" => {
+          "targetVersions" => [
+            {
+              "href" => api_v3_paths.version(version.id),
+              "title" => version.name
+            },
+            {
+              "href" => api_v3_paths.version(version_b.id),
+              "title" => version_b.name
+            }
+          ],
+          "self" => {
+            "href" => api_v3_paths.work_package(work_package.id, timestamps: timestamp),
+            "title" => work_package.subject
+          },
+          "schema" => {
+            "href" => api_v3_paths.work_package_schema(work_package.project_id, work_package.type_id)
+          }
+        }
+      }.to_json
+    end
+
+    it "renders as expected" do
+      expect(subject)
+        .to be_json_eql(expected_json)
+    end
+  end
+
+  context "with all target versions removed" do
+    let(:version) { nil }
+    let(:target_versions) { [] }
+    let(:attributes_changed_to_baseline) { %w[version_id target_versions] }
+
+    let(:expected_json) do
+      {
+        "_meta" => {
+          "matchesFilters" => true,
+          "exists" => true,
+          "timestamp" => timestamp.to_s
+        },
+        "_links" => {
+          "version" => {
+            "href" => nil
+          },
+          "targetVersions" => [],
+          "self" => {
+            "href" => api_v3_paths.work_package(work_package.id, timestamps: timestamp),
+            "title" => work_package.subject
+          },
+          "schema" => {
+            "href" => api_v3_paths.work_package_schema(work_package.project_id, work_package.type_id)
+          }
+        }
+      }.to_json
+    end
+
+    it "renders as expected" do
+      expect(subject)
+        .to be_json_eql(expected_json)
+    end
+  end
+
   context "without a linked property" do
     let(:attributes_changed_to_baseline) { %w[subject start_date] }
 
@@ -431,6 +558,12 @@
             "href" => api_v3_paths.version(version.id),
             "title" => version.name
           },
+          "targetVersions" => [
+            {
+              "href" => api_v3_paths.version(version.id),
+              "title" => version.name
+            }
+          ],
           "parent" => {
             "displayId" => parent.display_id.to_s,
             "href" => api_v3_paths.work_package(parent.id),