SW-6205 Make workflow history follow variable replacements (#2564)

If a variable is replaced by a newer version, we want the workflow history of the
old variable to be considered part of the history of the new variable too.

This should fix the system appearing to forget the statuses and comments of
variables when an upload of the all-variables CSV causes variables to be replaced.

src/main/kotlin/com/terraformation/backend/documentproducer/db/VariableWorkflowStore.kt

@@ -18,6 +18,7 @@ import jakarta.inject.Named
 import java.time.InstantSource
 import org.jooq.Condition
 import org.jooq.DSLContext
+import org.jooq.Field
 import org.jooq.impl.DSL
 import org.springframework.context.ApplicationEventPublisher
 
@@ -59,12 +60,19 @@ class VariableWorkflowStore(
               PROJECT_ID,
               VARIABLE_ID,
               VARIABLE_WORKFLOW_STATUS_ID,
+              currentVariableIdField,
           )
           .from(VARIABLE_WORKFLOW_HISTORY)
+          .join(VARIABLES)
+          .on(VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID.eq(VARIABLES.ID))
           .where(PROJECT_ID.eq(projectId))
-          .and(VARIABLE_ID.eq(variableId))
-          .orderBy(CREATED_TIME.desc())
-          .fetch { ExistingVariableWorkflowHistoryModel(it) }
+          .and(
+              VARIABLES.STABLE_ID.eq(
+                  DSL.select(VARIABLES.STABLE_ID)
+                      .from(VARIABLES)
+                      .where(VARIABLES.ID.eq(variableId))))
+          .orderBy(CREATED_TIME.desc(), ID.desc())
+          .fetch { ExistingVariableWorkflowHistoryModel.of(it, currentVariableIdField) }
     }
   }
 
@@ -90,7 +98,10 @@ class VariableWorkflowStore(
           fetchCurrentByCondition(
                   DSL.and(
                       PROJECT_ID.eq(projectId),
-                      VARIABLE_ID.eq(variableId),
+                      VARIABLES.STABLE_ID.eq(
+                          DSL.select(VARIABLES.STABLE_ID)
+                              .from(VARIABLES)
+                              .where(VARIABLES.ID.eq(variableId))),
                   ))
               .firstOrNull()
 
@@ -108,7 +119,7 @@ class VariableWorkflowStore(
               .set(VARIABLE_ID, variableId)
               .set(VARIABLE_WORKFLOW_STATUS_ID, status)
               .returningResult()
-              .fetchOne { ExistingVariableWorkflowHistoryModel(it) }
+              .fetchOne { ExistingVariableWorkflowHistoryModel.of(it) }
 
       if (existing == null || status != existing.status || feedback != existing.feedback) {
         // Notify a reviewable event, if changed
@@ -130,6 +141,23 @@ class VariableWorkflowStore(
     }
   }
 
+  /**
+   * Subquery field for the ID of the most recent variable that has the same stable ID as the
+   * variable referenced by [VARIABLE_WORKFLOW_HISTORY]. This is needed because old history entries
+   * might refer to variables that have subsequently been replaced, and we want to include them in
+   * the history of the latest version of the variable.
+   */
+  private val currentVariableIdField: Field<VariableId?> by lazy {
+    DSL.field(
+        DSL.select(DSL.max(VARIABLES.ID))
+            .from(VARIABLES)
+            .where(
+                VARIABLES.STABLE_ID.eq(
+                    DSL.select(VARIABLES.STABLE_ID)
+                        .from(VARIABLES)
+                        .where(VARIABLES.ID.eq(VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID)))))
+  }
+
   private fun fetchCurrentByCondition(
       condition: Condition
   ): List<ExistingVariableWorkflowHistoryModel> {
@@ -145,20 +173,15 @@ class VariableWorkflowStore(
               PROJECT_ID,
               VARIABLE_ID,
               VARIABLE_WORKFLOW_STATUS_ID,
+              currentVariableIdField,
           )
-          .distinctOn(PROJECT_ID, VARIABLE_ID)
+          .distinctOn(PROJECT_ID, VARIABLES.STABLE_ID)
           .from(VARIABLE_WORKFLOW_HISTORY)
+          .join(VARIABLES)
+          .on(VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID.eq(VARIABLES.ID))
           .where(condition)
-          .orderBy(PROJECT_ID, VARIABLE_ID, ID.desc())
-          .fetch { record ->
-            val model = ExistingVariableWorkflowHistoryModel(record)
-
-            if (currentUser().canReadInternalVariableWorkflowDetails(model.projectId)) {
-              model
-            } else {
-              model.copy(internalComment = null)
-            }
-          }
+          .orderBy(PROJECT_ID, VARIABLES.STABLE_ID, ID.desc())
+          .fetch { ExistingVariableWorkflowHistoryModel.of(it, currentVariableIdField) }
     }
   }
 }

src/main/kotlin/com/terraformation/backend/documentproducer/model/ExistingVariableWorkflowHistoryModel.kt

@@ -1,14 +1,15 @@
 package com.terraformation.backend.documentproducer.model
 
+import com.terraformation.backend.auth.currentUser
 import com.terraformation.backend.db.default_schema.ProjectId
 import com.terraformation.backend.db.default_schema.UserId
 import com.terraformation.backend.db.docprod.VariableId
 import com.terraformation.backend.db.docprod.VariableValueId
 import com.terraformation.backend.db.docprod.VariableWorkflowHistoryId
 import com.terraformation.backend.db.docprod.VariableWorkflowStatus
-import com.terraformation.backend.db.docprod.tables.pojos.VariableWorkflowHistoryRow
 import com.terraformation.backend.db.docprod.tables.references.VARIABLE_WORKFLOW_HISTORY
 import java.time.Instant
+import org.jooq.Field
 import org.jooq.Record
 
 data class ExistingVariableWorkflowHistoryModel(
@@ -18,35 +19,42 @@ data class ExistingVariableWorkflowHistoryModel(
     val id: VariableWorkflowHistoryId,
     val internalComment: String?,
     val maxVariableValueId: VariableValueId,
+    /** The ID of the variable at the time the workflow event happened. */
+    val originalVariableId: VariableId,
     val projectId: ProjectId,
     val status: VariableWorkflowStatus,
+    /**
+     * The current ID of the variable. May differ from [originalVariableId] if the variable was
+     * replaced after the workflow event happened; in that case this will be the most recent
+     * variable in the chain of replacements of the original one.
+     */
     val variableId: VariableId,
 ) {
-  constructor(
-      record: Record
-  ) : this(
-      createdBy = record[VARIABLE_WORKFLOW_HISTORY.CREATED_BY]!!,
-      createdTime = record[VARIABLE_WORKFLOW_HISTORY.CREATED_TIME]!!,
-      feedback = record[VARIABLE_WORKFLOW_HISTORY.FEEDBACK],
-      id = record[VARIABLE_WORKFLOW_HISTORY.ID]!!,
-      internalComment = record[VARIABLE_WORKFLOW_HISTORY.INTERNAL_COMMENT],
-      maxVariableValueId = record[VARIABLE_WORKFLOW_HISTORY.MAX_VARIABLE_VALUE_ID]!!,
-      projectId = record[VARIABLE_WORKFLOW_HISTORY.PROJECT_ID]!!,
-      status = record[VARIABLE_WORKFLOW_HISTORY.VARIABLE_WORKFLOW_STATUS_ID]!!,
-      variableId = record[VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID]!!,
-  )
+  companion object {
+    fun of(
+        record: Record,
+        currentVariableIdField: Field<VariableId?> = VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID,
+    ): ExistingVariableWorkflowHistoryModel {
+      val projectId = record[VARIABLE_WORKFLOW_HISTORY.PROJECT_ID]!!
+      val internalComment =
+          if (currentUser().canReadInternalVariableWorkflowDetails(projectId)) {
+            record[VARIABLE_WORKFLOW_HISTORY.INTERNAL_COMMENT]
+          } else {
+            null
+          }
 
-  constructor(
-      row: VariableWorkflowHistoryRow
-  ) : this(
-      row.createdBy!!,
-      row.createdTime!!,
-      row.feedback,
-      row.id!!,
-      row.internalComment,
-      row.maxVariableValueId!!,
-      row.projectId!!,
-      row.variableWorkflowStatusId!!,
-      row.variableId!!,
-  )
+      return ExistingVariableWorkflowHistoryModel(
+          createdBy = record[VARIABLE_WORKFLOW_HISTORY.CREATED_BY]!!,
+          createdTime = record[VARIABLE_WORKFLOW_HISTORY.CREATED_TIME]!!,
+          feedback = record[VARIABLE_WORKFLOW_HISTORY.FEEDBACK],
+          id = record[VARIABLE_WORKFLOW_HISTORY.ID]!!,
+          internalComment = internalComment,
+          maxVariableValueId = record[VARIABLE_WORKFLOW_HISTORY.MAX_VARIABLE_VALUE_ID]!!,
+          originalVariableId = record[VARIABLE_WORKFLOW_HISTORY.VARIABLE_ID]!!,
+          projectId = projectId,
+          status = record[VARIABLE_WORKFLOW_HISTORY.VARIABLE_WORKFLOW_STATUS_ID]!!,
+          variableId = record[currentVariableIdField]!!,
+      )
+    }
+  }
 }