Skip to content

Commit

Permalink
SW-6260 Ignore deletion of outdated variable values (#2607)
Browse files Browse the repository at this point in the history
If a variable value has already been deleted or replaced with a newer value, don't
attempt to delete it again. Previously, deleting an already-deleted value would
cause the request to fail with a server error.
  • Loading branch information
sgrimm authored Nov 19, 2024
1 parent 0db73a7 commit 6f46a89
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,31 @@ class VariableValueStore(
val containingRowId = fetchContainingRowId(valueId)
val listPosition = valuesRow.listPosition!!
val projectId = operation.projectId

val isOutdated =
dslContext
.selectOne()
.from(VARIABLE_VALUES)
.leftJoin(VARIABLE_VALUE_TABLE_ROWS)
.on(VARIABLE_VALUES.ID.eq(VARIABLE_VALUE_TABLE_ROWS.VARIABLE_VALUE_ID))
.where(VARIABLE_VALUES.PROJECT_ID.eq(projectId))
.and(VARIABLE_VALUES.VARIABLE_ID.eq(variableId))
.and(VARIABLE_VALUES.LIST_POSITION.eq(listPosition))
.and(
if (containingRowId != null) {
VARIABLE_VALUE_TABLE_ROWS.TABLE_ROW_VALUE_ID.eq(containingRowId)
} else {
VARIABLE_VALUE_TABLE_ROWS.TABLE_ROW_VALUE_ID.isNull
})
.and(VARIABLE_VALUES.ID.gt(valueId))
.fetch()
.isNotEmpty

if (isOutdated) {
logVariableOperation("Ignoring deletion of outdated value", projectId, variablesRow, valueId)
return
}

val maxListPosition =
fetchMaxListPosition(projectId, variableId, containingRowId)
?: throw IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,26 @@ class VariableValueStoreTest : DatabaseTest(), RunsAsUser {
"Values after append, append, delete, append, delete, append")
}

@Test
fun `deleting an already-deleted value does nothing`() {
val variableId = insertTextVariable(insertVariable(type = VariableType.Text))

val append1Result =
store.updateValues(
listOf(AppendValueOperation(NewTextValue(newValueProps(variableId), "1"))))

store.updateValues(
listOf(DeleteValueOperation(inserted.projectId, append1Result.first().id)))
val round1Values = store.listValues(inserted.documentId)

store.updateValues(
listOf(DeleteValueOperation(inserted.projectId, append1Result.first().id)))
val round2Values = store.listValues(inserted.documentId)

assertEquals(
round1Values, round2Values, "Should not have made any additional changes to values")
}

@Test
fun `can delete a table row whose values are all deleted`() {
val tableVariableId = insertTableVariable()
Expand Down

0 comments on commit 6f46a89

Please sign in to comment.