diff --git a/build.gradle b/build.gradle index 44cda6ec..16f2b768 100755 --- a/build.gradle +++ b/build.gradle @@ -64,7 +64,7 @@ dependencies { implementation 'com.github.cb372:scalacache-guava_2.12:0.28.0' - implementation 'org.webjars:swagger-ui:4.11.1' + implementation 'org.webjars:swagger-ui:5.9.0' // Also update version in DocumentationRouter compileOnly "io.vertx:vertx-codegen:${vertxVersion}" diff --git a/src/main/resources/swagger.json b/src/main/resources/swagger.json index 91aaff37..aa40caab 100644 --- a/src/main/resources/swagger.json +++ b/src/main/resources/swagger.json @@ -1065,6 +1065,9 @@ "parameters": [ { "$ref": "#/parameters/value" + }, + { + "$ref": "#/parameters/forceHistoryOpt" } ], "responses": { @@ -1086,6 +1089,9 @@ "parameters": [ { "$ref": "#/parameters/value" + }, + { + "$ref": "#/parameters/forceHistoryOpt" } ], "responses": { @@ -1107,6 +1113,9 @@ "parameters": [ { "$ref": "#/parameters/value" + }, + { + "$ref": "#/parameters/forceHistoryOpt" } ], "responses": { @@ -2318,19 +2327,10 @@ ] }, "historyType": { - "description": "The type of the history entry", - "type": "string", - "enum": [ - "row", - "cell", - "comment", - "cell_flag", - "row_flag", - "row_permissions" - ] + "$ref": "#/definitions/Enum: HistoryType" }, "languageType": { - "description": "Name of the user who has triggered a change", + "description": "The language type of the column", "type": "string", "enum": [ "neutral", @@ -3582,6 +3582,18 @@ "listener" ] }, + "Enum: HistoryType": { + "type": "string", + "description": "The type of the history entry", + "enum": [ + "row", + "cell", + "comment", + "cell_flag", + "row_flag", + "row_permissions" + ] + }, "Response: Subfolder": { "type": "object", "required": [ @@ -3822,6 +3834,14 @@ } } }, + "forceHistoryOpt": { + "name": "forceHistory", + "description": "If set to true, a history entry will be created even if the value did not change.", + "in": "query", + "required": false, + "type": "boolean", + "default": false + }, "fileuuid": { "name": "fileuuid", "in": "path", @@ -3847,8 +3867,10 @@ "description": "The type of the history to filter for.", "in": "query", "type": "string", - "format": "string", - "required": false + "required": false, + "items": { + "$ref": "#/definitions/Enum: HistoryType" + } }, "groupId": { "name": "groupId", diff --git a/src/main/scala/com/campudus/tableaux/controller/TableauxController.scala b/src/main/scala/com/campudus/tableaux/controller/TableauxController.scala index e5eef03a..0143d2f8 100644 --- a/src/main/scala/com/campudus/tableaux/controller/TableauxController.scala +++ b/src/main/scala/com/campudus/tableaux/controller/TableauxController.scala @@ -458,18 +458,18 @@ class TableauxController( } yield filled } - def replaceCellValue[A](tableId: TableId, columnId: ColumnId, rowId: RowId, value: A)( + def replaceCellValue[A](tableId: TableId, columnId: ColumnId, rowId: RowId, value: A, forceHistory: Boolean = false)( implicit user: TableauxUser ): Future[Cell[_]] = { checkArguments(greaterZero(tableId), greaterZero(columnId), greaterZero(rowId)) logger.info(s"replaceCellValue $tableId $columnId $rowId $value") for { table <- repository.retrieveTable(tableId) - filled <- repository.replaceCellValue(table, columnId, rowId, value) + filled <- repository.replaceCellValue(table, columnId, rowId, value, forceHistory) } yield filled } - def updateCellValue[A](tableId: TableId, columnId: ColumnId, rowId: RowId, value: A)( + def updateCellValue[A](tableId: TableId, columnId: ColumnId, rowId: RowId, value: A, forceHistory: Boolean = false)( implicit user: TableauxUser ): Future[Cell[_]] = { checkArguments(greaterZero(tableId), greaterZero(columnId), greaterZero(rowId)) @@ -477,7 +477,7 @@ class TableauxController( for { table <- repository.retrieveTable(tableId) - updated <- repository.updateCellValue(table, columnId, rowId, value) + updated <- repository.updateCellValue(table, columnId, rowId, value, forceHistory) } yield updated } diff --git a/src/main/scala/com/campudus/tableaux/database/model/HistoryModel.scala b/src/main/scala/com/campudus/tableaux/database/model/HistoryModel.scala index 0d241579..a42a59dd 100644 --- a/src/main/scala/com/campudus/tableaux/database/model/HistoryModel.scala +++ b/src/main/scala/com/campudus/tableaux/database/model/HistoryModel.scala @@ -906,19 +906,18 @@ case class CreateHistoryModel(tableauxModel: TableauxModel, connection: Database def createCells(table: Table, rowId: RowId, values: Seq[(ColumnType[_], _)])( implicit user: TableauxUser ): Future[Unit] = { - val columns = values.map({ case (col: ColumnType[_], _) => col }) - val (_, _, _, attachments) = ColumnType.splitIntoTypes(columns) - ColumnType.splitIntoTypesWithValues(values) match { case Failure(ex) => Future.failed(ex) - case Success((simples, multis, links, _)) => + case Success((simples, multis, links, attachments)) => for { _ <- if (simples.isEmpty) Future.successful(()) else createSimple(table, rowId, simples) _ <- if (multis.isEmpty) Future.successful(()) else createTranslation(table, rowId, multis) _ <- if (links.isEmpty) Future.successful(()) else createLinks(table, rowId, links, allowRecursion = true) - _ <- if (attachments.isEmpty) Future.successful(()) else createAttachments(table, rowId, attachments) + _ <- + if (attachments.isEmpty) Future.successful(()) + else createAttachments(table, rowId, attachments.map({ case (column, _) => column })) } yield () } } diff --git a/src/main/scala/com/campudus/tableaux/database/model/TableauxModel.scala b/src/main/scala/com/campudus/tableaux/database/model/TableauxModel.scala index 2d0fee1d..3d0163d6 100644 --- a/src/main/scala/com/campudus/tableaux/database/model/TableauxModel.scala +++ b/src/main/scala/com/campudus/tableaux/database/model/TableauxModel.scala @@ -338,7 +338,7 @@ class TableauxModel( { case (future, (table, column, id)) => { future.flatMap(_ => { - updateOrReplaceValue(table, column.id, id, replacingRowId, false, t) + updateOrReplaceValue(table, column.id, id, replacingRowId, false, maybeTransaction = t) }).map(_ => ()) } } @@ -645,12 +645,15 @@ class TableauxModel( } } + private def hasCellChanged(oldCell: Cell[_], newCell: Cell[_]): Boolean = oldCell.value != newCell.value + private def updateOrReplaceValue[A]( table: Table, columnId: ColumnId, rowId: RowId, value: A, replace: Boolean = false, + forceHistory: Boolean = false, maybeTransaction: Option[DbTransaction] = None )(implicit user: TableauxUser): Future[Cell[_]] = { for { @@ -673,6 +676,7 @@ class TableauxModel( roleModel.checkAuthorization(EditCellValue, ComparisonObjects(table, column, value)) } + oldCell <- retrieveCell(column, rowId, true) _ <- createHistoryModel.createCellsInit(table, rowId, Seq((column, value))) _ <- @@ -687,21 +691,28 @@ class TableauxModel( _ <- updateRowModel.updateRow(table, rowId, Seq((column, value)), maybeTransaction) _ <- invalidateCellAndDependentColumns(column, rowId) - _ <- createHistoryModel.createCells(table, rowId, Seq((column, value))) + newCell <- retrieveCell(column, rowId, true) + + shouldCreateHistory = forceHistory || hasCellChanged(oldCell, newCell) - changedCell <- retrieveCell(column, rowId, true) - } yield changedCell + _ <- + if (shouldCreateHistory) { + createHistoryModel.createCells(table, rowId, Seq((column, value))) + } else { + Future.successful(()) + } + } yield newCell } - def updateCellValue[A](table: Table, columnId: ColumnId, rowId: RowId, value: A)( + def updateCellValue[A](table: Table, columnId: ColumnId, rowId: RowId, value: A, forceHistory: Boolean = false)( implicit user: TableauxUser ): Future[Cell[_]] = - updateOrReplaceValue(table, columnId, rowId, value) + updateOrReplaceValue(table, columnId, rowId, value, forceHistory = forceHistory) - def replaceCellValue[A](table: Table, columnId: ColumnId, rowId: RowId, value: A)( + def replaceCellValue[A](table: Table, columnId: ColumnId, rowId: RowId, value: A, forceHistory: Boolean = false)( implicit user: TableauxUser ): Future[Cell[_]] = - updateOrReplaceValue(table, columnId, rowId, value, replace = true) + updateOrReplaceValue(table, columnId, rowId, value, replace = true, forceHistory = forceHistory) def clearCellValue(table: Table, columnId: ColumnId, rowId: RowId)( implicit user: TableauxUser diff --git a/src/main/scala/com/campudus/tableaux/router/BaseRouter.scala b/src/main/scala/com/campudus/tableaux/router/BaseRouter.scala index a581933a..e2242ad1 100644 --- a/src/main/scala/com/campudus/tableaux/router/BaseRouter.scala +++ b/src/main/scala/com/campudus/tableaux/router/BaseRouter.scala @@ -158,6 +158,10 @@ trait BaseRouter extends VertxAccess { context.request().getParam(name).map(_.toBoolean) } + def getBoolQuery(name: String, context: RoutingContext): Option[Boolean] = { + context.queryParams().get(name).map(_.toBoolean) + } + def getStringParam(name: String, context: RoutingContext): Option[String] = { context.request().getParam(name) } diff --git a/src/main/scala/com/campudus/tableaux/router/DocumentationRouter.scala b/src/main/scala/com/campudus/tableaux/router/DocumentationRouter.scala index 36b6592d..841e6c68 100644 --- a/src/main/scala/com/campudus/tableaux/router/DocumentationRouter.scala +++ b/src/main/scala/com/campudus/tableaux/router/DocumentationRouter.scala @@ -19,7 +19,7 @@ object DocumentationRouter { class DocumentationRouter(override val config: TableauxConfig) extends BaseRouter { - private val swaggerUiVersion = "4.11.1" + private val swaggerUiVersion = "5.9.0" private val directory = """(?[A-Za-z0-9-_\\.]{1,60}){1}""" private val file = s"""(?[A-Za-z0-9-_\\.]{1,60}){1}""" diff --git a/src/main/scala/com/campudus/tableaux/router/TableauxRouter.scala b/src/main/scala/com/campudus/tableaux/router/TableauxRouter.scala index 095ea782..daf42139 100644 --- a/src/main/scala/com/campudus/tableaux/router/TableauxRouter.scala +++ b/src/main/scala/com/campudus/tableaux/router/TableauxRouter.scala @@ -665,6 +665,7 @@ class TableauxRouter(override val config: TableauxConfig, val controller: Tablea */ private def updateCell(context: RoutingContext): Unit = { implicit val user = TableauxUser(context) + val forceHistory = getBoolQuery("forceHistory", context).getOrElse(false) for { tableId <- getTableId(context) columnId <- getColumnId(context) @@ -675,7 +676,7 @@ class TableauxRouter(override val config: TableauxConfig, val controller: Tablea asyncGetReply { val json = getJson(context) for { - updated <- controller.updateCellValue(tableId, columnId, rowId, json.getValue("value")) + updated <- controller.updateCellValue(tableId, columnId, rowId, json.getValue("value"), forceHistory) } yield updated } ) @@ -687,6 +688,7 @@ class TableauxRouter(override val config: TableauxConfig, val controller: Tablea */ private def replaceCell(context: RoutingContext): Unit = { implicit val user = TableauxUser(context) + val forceHistory = getBoolQuery("forceHistory", context).getOrElse(false) for { tableId <- getTableId(context) columnId <- getColumnId(context) @@ -699,7 +701,7 @@ class TableauxRouter(override val config: TableauxConfig, val controller: Tablea for { updated <- if (json.containsKey("value")) { - controller.replaceCellValue(tableId, columnId, rowId, json.getValue("value")) + controller.replaceCellValue(tableId, columnId, rowId, json.getValue("value"), forceHistory) } else { Future.failed(InvalidJsonException("request must contain a value", "value_is_missing")) } diff --git a/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryChangeDetectionTest.scala b/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryChangeDetectionTest.scala new file mode 100644 index 00000000..273bc629 --- /dev/null +++ b/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryChangeDetectionTest.scala @@ -0,0 +1,215 @@ +package com.campudus.tableaux.api.content + +import com.campudus.tableaux.api.media.MediaTestBase +import com.campudus.tableaux.database.DatabaseConnection +import com.campudus.tableaux.testtools.RequestCreation.{CurrencyCol, MultiCountry, Rows} +import com.campudus.tableaux.testtools.TableauxTestBase + +import io.vertx.core.json.JsonArray +import io.vertx.ext.unit.TestContext +import io.vertx.ext.unit.junit.VertxUnitRunner +import io.vertx.scala.SQLConnection +import org.vertx.scala.core.json.{Json, JsonObject} + +import scala.concurrent.Future + +import org.junit.{Ignore, Test} +import org.junit.Assert._ +import org.junit.runner.RunWith +import org.skyscreamer.jsonassert.{JSONAssert, JSONCompareMode} + +@RunWith(classOf[VertxUnitRunner]) +class CreateHistoryChangeDetectionTest extends LinkTestBase with TestHelper { + + @Test + def changeSimpleValueTwice_changeACellOnlyOnce(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> "a fix value") + + for { + _ <- createEmptyDefaultTable() + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("POST", "/tables/1/columns/1/rows/1", newValue) + _ <- sendRequest("POST", "/tables/1/columns/1/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/1/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeSimpleValueTwice_withForceHistory(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> "a fix value") + + for { + _ <- createEmptyDefaultTable() + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("POST", "/tables/1/columns/1/rows/1", newValue) + _ <- sendRequest("POST", "/tables/1/columns/1/rows/1?forceHistory=true", newValue) + rows <- sendRequest("GET", "/tables/1/columns/1/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(2, rows.size()) + } + } + + @Test + def changeMultilanguageValueTwice_text(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> Json.obj("de-DE" -> "a fix value")) + + for { + _ <- createTableWithMultilanguageColumns("history test") + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("PATCH", "/tables/1/columns/1/rows/1", newValue) + _ <- sendRequest("PATCH", "/tables/1/columns/1/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/1/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + + } + + @Test + def changeMultilanguageValueTwice_boolean(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> Json.obj("de-DE" -> true)) + + for { + _ <- createTableWithMultilanguageColumns("history test") + _ <- sendRequest("POST", "/tables/1/rows") + // Booleans always gets a initial history entry on first change, so +1 history row + _ <- sendRequest("PATCH", "/tables/1/columns/2/rows/1", newValue) + _ <- sendRequest("PATCH", "/tables/1/columns/2/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/2/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(2, rows.size()) + } + } + + @Test + def changeMultilanguageValueTwice_numeric(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> Json.obj("de-DE" -> 42)) + + for { + _ <- createTableWithMultilanguageColumns("history test") + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("PATCH", "/tables/1/columns/3/rows/1", newValue) + _ <- sendRequest("PATCH", "/tables/1/columns/3/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/3/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeMultilanguageValueTwice_datetime(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> Json.obj("de-DE" -> "2019-01-18T00:00:00.000Z")) + + for { + _ <- createTableWithMultilanguageColumns("history test") + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("PATCH", "/tables/1/columns/7/rows/1", newValue) + _ <- sendRequest("PATCH", "/tables/1/columns/7/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/7/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeMultilanguageValueTwice_currency(implicit c: TestContext): Unit = okTest { + val newValue = Json.obj("value" -> Json.obj("DE" -> 2999.99)) + val multiCountryCurrencyColumn = MultiCountry(CurrencyCol("currency-column"), Seq("DE", "GB")) + + for { + _ <- createSimpleTableWithCell("table1", multiCountryCurrencyColumn) + _ <- sendRequest("POST", "/tables/1/rows") + _ <- sendRequest("PATCH", "/tables/1/columns/1/rows/1", newValue) + _ <- sendRequest("PATCH", "/tables/1/columns/1/rows/1", newValue) + rows <- sendRequest("GET", "/tables/1/columns/1/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeLinkValueTwice_singleLanguage(implicit c: TestContext): Unit = okTest { + val twoLinks = Json.obj("value" -> Json.obj("values" -> Json.arr(1, 2))) + + for { + linkColumnId <- setupTwoTablesWithEmptyLinks() + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1", twoLinks) + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1", twoLinks) + rows <- sendRequest("GET", s"/tables/1/columns/$linkColumnId/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeLinkValueTwice_withForceHistory(implicit c: TestContext): Unit = okTest { + val twoLinks = Json.obj("value" -> Json.obj("values" -> Json.arr(1, 2))) + + for { + linkColumnId <- setupTwoTablesWithEmptyLinks() + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1", twoLinks) + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1?forceHistory=true", twoLinks) + rows <- sendRequest("GET", s"/tables/1/columns/$linkColumnId/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(2, rows.size()) + } + } + + @Test + def changeLinkValueTwice_singleLanguageMultiIdentifiers(implicit c: TestContext): Unit = okTest { + val twoLinks = Json.obj("value" -> Json.obj("values" -> Json.arr(1, 2))) + + for { + linkColumnId <- setupTwoTablesWithEmptyLinks() + + // Change target table structure to have a second identifier + _ <- sendRequest("POST", s"/tables/2/columns/2", Json.obj("identifier" -> true)) + + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1", twoLinks) + _ <- sendRequest("PUT", s"/tables/1/columns/$linkColumnId/rows/1", twoLinks) + rows <- sendRequest("GET", s"/tables/1/columns/$linkColumnId/rows/1/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, rows.size()) + } + } + + @Test + def changeLinkValueTwice_addOne(implicit c: TestContext): Unit = okTest { + val putLink = Json.obj("value" -> Json.obj("values" -> Json.arr(5))) + + for { + _ <- setupTwoTablesWithEmptyLinks() + + _ <- sendRequest("PUT", s"/tables/1/columns/3/rows/1", putLink) + _ <- sendRequest("PUT", s"/tables/1/columns/3/rows/1", putLink) + + history <- sendRequest("GET", "/tables/1/columns/3/rows/1/history?historyType=cell").map(toRowsArray) + targetHistory <- sendRequest("GET", "/tables/2/columns/3/rows/5/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, history.size()) + assertEquals(1, targetHistory.size()) + } + } + + @Test + def changeLink_addTwoLinksAtOnce(implicit c: TestContext): Unit = okTest { + val putLink = Json.obj("value" -> Json.obj("values" -> Json.arr(4, 5))) + + for { + _ <- setupTwoTablesWithEmptyLinks() + + _ <- sendRequest("PUT", s"/tables/1/columns/3/rows/1", putLink) + _ <- sendRequest("PUT", s"/tables/1/columns/3/rows/1", putLink) + + history <- sendRequest("GET", "/tables/1/columns/3/rows/1/history?historyType=cell").map(toRowsArray) + targetHistory1 <- sendRequest("GET", "/tables/2/columns/3/rows/4/history?historyType=cell").map(toRowsArray) + targetHistory2 <- sendRequest("GET", "/tables/2/columns/3/rows/5/history?historyType=cell").map(toRowsArray) + } yield { + assertEquals(1, history.size()) + assertEquals(1, targetHistory1.size()) + assertEquals(1, targetHistory2.size()) + } + } +} diff --git a/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryTest.scala b/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryTest.scala index 35f664d4..8be35aad 100644 --- a/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryTest.scala +++ b/src/test/scala/com/campudus/tableaux/api/content/CreateHistoryTest.scala @@ -1637,7 +1637,7 @@ class CreateHistoryCompatibilityTest extends LinkTestBase with TestHelper { // Booleans always gets a initial history entry on first change val expectedInitialLinks = """{ "value": true} """ - val expectedAfterPost1 = """{ "value": true }""" + val expectedAfterPost1 = """{ "value": false }""" val booleanColumn = s"""{"columns": [{"kind": "boolean", "name": "Boolean Column", "languageType": "neutral"} ] }"""