When variables are upgraded, also replace injections/references to th…

…e old variable within section text for documents (#2546)

When a variable is upgraded (because its definition is changed within the all variables CSV and uploaded), there may still be old injections/references to this variable within the section text for documents. This causes a few issues, the most obvious being that the frontend receives multiple versions of a variable. The frontend receives the old version of the variable, because it exists within the value of a variable present (the section variable), and it receives the new variable because that is the current version of a variable that can be injected/referenced within the document.

This PR fixes the issue by adding an additional step to the "Upgrade Old Variable Values" which goes through and upgrades old injections/references to "old" variables.

src/main/kotlin/com/terraformation/backend/documentproducer/VariableService.kt

@@ -33,6 +33,7 @@ class VariableService(
           }
 
           variableValueStore.upgradeSectionDefaultValues(result.replacements)
+          variableValueStore.upgradeSectionValueVariables(result.replacements)
         }
       }
 
@@ -75,6 +76,7 @@ class VariableService(
               }
 
               variableValueStore.upgradeSectionDefaultValues(replacements)
+              variableValueStore.upgradeSectionValueVariables(replacements)
             }
       }
     }

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

@@ -224,6 +224,24 @@ class VariableValueStore(
         ?.value1() ?: 0
   }
 
+  // Get the VariableValue for the injected variable within a section variable
+  private fun fetchSectionValuesUsingVariable(variableId: VariableId): List<ExistingSectionValue> {
+    val variableValues2 = VARIABLE_VALUES.`as`("variable_values_2")
+    val conditions =
+        listOf(
+            VARIABLE_SECTION_VALUES.USED_VARIABLE_ID.eq(variableId),
+            // Don't return values that were later superseded by other values
+            DSL.notExists(
+                DSL.selectOne()
+                    .from(variableValues2)
+                    .where(variableValues2.VARIABLE_ID.eq(VARIABLE_VALUES.VARIABLE_ID))
+                    .and(variableValues2.PROJECT_ID.eq(VARIABLE_VALUES.PROJECT_ID))
+                    .and(variableValues2.LIST_POSITION.eq(VARIABLE_VALUES.LIST_POSITION))
+                    .and(variableValues2.ID.gt(VARIABLE_VALUES.ID))))
+
+    return fetchByConditions(conditions, false).filterIsInstance<ExistingSectionValue>()
+  }
+
   private fun fetchByConditions(
       conditions: List<Condition>,
       includeDeletedValues: Boolean
@@ -438,6 +456,31 @@ class VariableValueStore(
     }
   }
 
+  fun upgradeSectionValueVariables(replacements: Map<VariableId, VariableId>) {
+    val sectionOperations =
+        replacements.flatMap { (oldVariableId, newVariableId) ->
+          fetchSectionValuesUsingVariable(oldVariableId).mapNotNull { sectionValue ->
+            val valueVariable = sectionValue.value as? SectionValueVariable
+            valueVariable?.let { sectionValueVariable ->
+              UpdateValueOperation(
+                  ExistingSectionValue(
+                      BaseVariableValueProperties(
+                          sectionValue.id,
+                          sectionValue.projectId,
+                          sectionValue.listPosition,
+                          sectionValue.variableId,
+                          sectionValue.citation),
+                      SectionValueVariable(
+                          newVariableId,
+                          sectionValueVariable.usageType,
+                          sectionValueVariable.displayStyle)))
+            }
+          }
+        }
+
+    updateValues(sectionOperations, triggerWorkflows = false)
+  }
+
   /**
    * Updates the values in a document by applying a list of operations.
    *

src/test/kotlin/com/terraformation/backend/documentproducer/db/VariableValueStoreTest.kt

@@ -7,7 +7,9 @@ import com.terraformation.backend.accelerator.event.VariableValueUpdatedEvent
 import com.terraformation.backend.db.DatabaseTest
 import com.terraformation.backend.db.default_schema.ProjectId
 import com.terraformation.backend.db.docprod.VariableId
+import com.terraformation.backend.db.docprod.VariableInjectionDisplayStyle
 import com.terraformation.backend.db.docprod.VariableType
+import com.terraformation.backend.db.docprod.VariableUsageType
 import com.terraformation.backend.db.docprod.VariableValueId
 import com.terraformation.backend.db.docprod.VariableWorkflowStatus
 import com.terraformation.backend.documentproducer.event.CompletedSectionVariableUpdatedEvent
@@ -22,6 +24,7 @@ import com.terraformation.backend.documentproducer.model.ImageValueDetails
 import com.terraformation.backend.documentproducer.model.NewImageValue
 import com.terraformation.backend.documentproducer.model.NewTableValue
 import com.terraformation.backend.documentproducer.model.NewTextValue
+import com.terraformation.backend.documentproducer.model.SectionValueVariable
 import com.terraformation.backend.mockUser
 import io.mockk.every
 import org.junit.jupiter.api.Assertions.*
@@ -564,6 +567,115 @@ class VariableValueStoreTest : DatabaseTest(), RunsAsUser {
     }
   }
 
+  @Nested
+  inner class UpgradeSectionValueVariables {
+    @Test
+    fun `updates variable references in sections to use new variable IDs`() {
+      val projectId = inserted.projectId
+      insertDocumentTemplate()
+      insertVariableManifest()
+      insertDocument()
+
+      val oldVariableId = insertTextVariable()
+      insertValue(textValue = "referenced text", variableId = oldVariableId)
+      val sectionVariableId = insertVariableManifestEntry(insertSectionVariable())
+
+      insertSectionValue(listPosition = 0, textValue = "some text", variableId = sectionVariableId)
+      val oldReferringSectionValueId =
+          insertSectionValue(
+              displayStyle = VariableInjectionDisplayStyle.Block,
+              listPosition = 1,
+              usageType = VariableUsageType.Injection,
+              usedVariableId = oldVariableId,
+              variableId = sectionVariableId)
+
+      val newVariableId =
+          insertTextVariable(
+              insertVariable(replacesVariableId = oldVariableId, type = VariableType.Text))
+
+      every { user.canUpdateInternalVariableWorkflowDetails(projectId) } returns true
+
+      val oldValues =
+          store.listValues(projectId = projectId, variableIds = listOf(sectionVariableId))
+      val oldTextSectionValue = oldValues.single { it.listPosition == 0 }
+
+      store.upgradeSectionValueVariables(mapOf(oldVariableId to newVariableId))
+
+      val newValues =
+          store.listValues(projectId = projectId, variableIds = listOf(sectionVariableId))
+      val newTextSectionValue = newValues.single { it.listPosition == 0 }
+      val newReferringSectionValue = newValues.single { it.listPosition == 1 }
+
+      assertEquals(
+          oldTextSectionValue, newTextSectionValue, "Text value should not have been modified")
+      assertNotEquals(
+          oldReferringSectionValueId,
+          newReferringSectionValue.id,
+          "Value with variable reference should have been replaced")
+      assertEquals(
+          SectionValueVariable(
+              newVariableId, VariableUsageType.Injection, VariableInjectionDisplayStyle.Block),
+          newReferringSectionValue.value,
+          "Section should refer to new variable")
+    }
+
+    @Test
+    fun `does not modify sections with up-to-date variable references`() {
+      val projectId = inserted.projectId
+      insertDocumentTemplate()
+      insertVariableManifest()
+      insertDocument()
+      val sectionVariableId = insertVariableManifestEntry(insertSectionVariable())
+
+      val oldVariableId = insertTextVariable()
+
+      // The first version of the section references the outdated variable, there is no project
+      // value for this variable yet
+      insertSectionValue(listPosition = 0, textValue = "some text", variableId = sectionVariableId)
+      insertSectionValue(
+          displayStyle = VariableInjectionDisplayStyle.Block,
+          listPosition = 1,
+          usageType = VariableUsageType.Injection,
+          usedVariableId = oldVariableId,
+          variableId = sectionVariableId)
+
+      // The variable is upgraded at some point
+      val newVariableId =
+          insertTextVariable(
+              insertVariable(replacesVariableId = oldVariableId, type = VariableType.Text))
+
+      // The section has already been updated to use the new variable ID
+      insertSectionValue(
+          listPosition = 0, textValue = "some updated text", variableId = sectionVariableId)
+      insertSectionValue(
+          displayStyle = VariableInjectionDisplayStyle.Block,
+          listPosition = 1,
+          usageType = VariableUsageType.Injection,
+          usedVariableId = newVariableId,
+          variableId = sectionVariableId)
+
+      // A project value is added to the new variable
+      insertValue(textValue = "referenced text", variableId = newVariableId)
+
+      every { user.canUpdateInternalVariableWorkflowDetails(projectId) } returns true
+
+      val oldValues =
+          store.listValues(projectId = projectId, variableIds = listOf(sectionVariableId))
+      val oldReferringSectionValue = oldValues.single { it.listPosition == 1 }
+
+      store.upgradeSectionValueVariables(mapOf(oldVariableId to newVariableId))
+
+      val newValues =
+          store.listValues(projectId = projectId, variableIds = listOf(sectionVariableId))
+      val newReferringSectionValue = newValues.single { it.listPosition == 1 }
+
+      assertEquals(
+          oldReferringSectionValue.id,
+          newReferringSectionValue.id,
+          "Value with variable reference should not have been replaced")
+    }
+  }
+
   private fun existingValueProps(
       valueId: VariableValueId,
       variableId: VariableId,