feat(listsEndpoint) Insert list item in specific position (DSP-1227) (#…

…1798)

* feat(insertChildNodeInPosition) allow creation of a child node with a given position + tests

* doc(insertChildNode) documentation for creating a new child node and inserting it in a given position

* test(insertChildNode) e2e test

* test (insertNode) verify validity of the position in the payload

* refactor (insertNode) edit documentation

* style(lists): Clean up a few small things.

* style(lists): Simplify code.

* feat (insertChildNode) add test data

* test (insertChildNode) remove duplicate test, it is already in ListsMessagesADMSpec

Co-authored-by: Benjamin Geer <benjaminlewis.geer@unibas.ch>

docs/03-apis/api-admin/lists.md

@@ -224,8 +224,9 @@ There is no need to specify the project IRI because it is automatically extracte
 
   - Required permission: SystemAdmin / ProjectAdmin
   - Appends a new child node under the supplied nodeIri. If the supplied nodeIri
-    is the listIri, then a new child node is appended to the top level. Children
-    are currently only appended.
+    is the listIri, then a new child node is appended to the top level. If a position is given
+     for the new child node, the node will be created and inserted in the specified position, otherwise 
+     the node is appended to the end of parent's children.
   - POST: `/admin/lists/<parentNodeIri>`
   - BODY:
 
@@ -269,6 +270,22 @@ The response will contain the basic information of the node, `nodeinfo`, as belo
     }
 }
 ```
+The new node can be created and inserted in a specific position which must be given in the payload as shown below. If necessary, 
+according to the given position, the sibling nodes will be shifted. Note that `position` cannot have a value higher than the
+number of existing children.
+
+```json
+{   "parentNodeIri": "http://rdfh.ch/lists/0001/a-list",
+    "projectIri": "http://rdfh.ch/projects/0001",
+    "name": "Inserted new child",
+    "position": 0,
+    "labels": [{ "value": "New List Node", "language": "en"}],
+    "comments": []
+}
+```
+
+In case the new node should be appended to the list of current children, either `position: -1` must be given in the 
+payload or the `position` parameter must be left out of the payload. 
 
 ### Update node's information
 The basic information of a node such as its labels, comments, name, or all of them can be updated. The parameters that 

docs/03-apis/api-admin/lists_new-list-admin-routes_v1.md

@@ -18,7 +18,7 @@ License along with Knora.  If not, see <http://www.gnu.org/licenses/>.
 -->
 
 # Lists Endpoint
-## To use some of the routes in this endpoint the [feature toggle](../feature-toggles.md), `new-list-admin-routes:1` must
+## To use the routes in this endpoint the [feature toggle](../feature-toggles.md), `new-list-admin-routes:1` must
  be activated.
 
 ## Endpoint Overview
@@ -171,6 +171,23 @@ The response will contain the basic information of the node, `nodeinfo`, as belo
 }
 ```
 
+The new node can be created and inserted in a specific position which must be given in the payload as shown below. If necessary, 
+according to the given position, the sibling nodes will be shifted. Note that `position` cannot have a value higher than 
+number of existing children.
+
+```json
+{   "parentNodeIri": "http://rdfh.ch/lists/0001/a-list",
+    "projectIri": "http://rdfh.ch/projects/0001",
+    "name": "Inserted new child",
+    "position": 0,
+    "labels": [{ "value": "New List Node", "language": "en"}],
+    "comments": []
+}
+```
+
+In case the new node should be appended to the list of current children, either `position: -1` must be given in the 
+payload or the `position` parameter must be left out of the payload. 
+
 ### Update basic information (entire list or a node)
 The basic information of a list or a node such as its labels, comments, name, or all of them can be updated. The parameters that 
 must be updated together with the new value must be given in the JSON body of the request together with the IRI of the 

webapi/scripts/expected-client-test-data.txt

@@ -33,6 +33,8 @@ test-data/admin/lists/get-list-node-info-response.json
 test-data/admin/lists/get-list-response.json
 test-data/admin/lists/get-lists-response.json
 test-data/admin/lists/get-node-response.json
+test-data/admin/lists/insert-childNode-in-position-request.json
+test-data/admin/lists/insert-childNode-in-position-response.json
 test-data/admin/lists/toggle_new-list-admin-routes_v1/
 test-data/admin/lists/toggle_new-list-admin-routes_v1/add-second-child-to-root-request.json
 test-data/admin/lists/toggle_new-list-admin-routes_v1/add-second-child-to-root-response.json

webapi/src/main/scala/org/knora/webapi/messages/admin/responder/listsmessages/ListsMessagesADM.scala

@@ -77,22 +77,26 @@ case class CreateListApiRequestADM(id: Option[IRI] = None,
 
 /**
   * Represents an API request payload that asks the Knora API server to create a new node.
-  * If the IRI of the parent node is given, the new node is attached to the parent node as a sublist node. If other
-  * child nodes exist, the newly created list node will be appended to the end of the list of children.
+  * If the IRI of the parent node is given, the new node is attached to the parent node as a sublist node.
+  * If a specific position is given, insert the child node there. Otherwise, the newly created list node will be appended
+  * to the end of the list of children.
   * If no parent node IRI is given in the payload, a new list is created with this node as its root node.
   * At least one label needs to be supplied.
   *
   * @param id            the optional custom IRI of the list node.
   * @param parentNodeIri the optional IRI of the parent node.
   * @param projectIri    the IRI of the project.
   * @param name          the optional name of the list node.
+  * @param position      the optional position of the node.
   * @param labels        labels of the list node.
   * @param comments      comments of the list node.
+  *
   */
 case class CreateNodeApiRequestADM(id: Option[IRI] = None,
                                    parentNodeIri: Option[IRI] = None,
                                    projectIri: IRI,
                                    name: Option[String] = None,
+                                   position: Option[Int] = None,
                                    labels: Seq[StringLiteralV2],
                                    comments: Seq[StringLiteralV2])
     extends ListADMJsonProtocol {
@@ -116,6 +120,10 @@ case class CreateNodeApiRequestADM(id: Option[IRI] = None,
     throw BadRequestException(LABEL_MISSING_ERROR)
   }
 
+  if (position.exists(_ < -1)) {
+    throw BadRequestException(INVALID_POSITION)
+  }
+
   def toJsValue: JsValue = createListNodeApiRequestADMFormat.write(this)
 }
 
@@ -167,6 +175,10 @@ case class ChangeNodeInfoApiRequestADM(listIri: IRI,
     throw BadRequestException(UPDATE_REQUEST_EMPTY_LABEL_ERROR)
   }
 
+  if (position.exists(_ < -1)) {
+    throw BadRequestException(INVALID_POSITION)
+  }
+
   def toJsValue: JsValue = changeListInfoApiRequestADMFormat.write(this)
 }
 
@@ -216,6 +228,9 @@ case class ChangeNodePositionApiRequestADM(position: Int, parentIri: IRI) extend
     throw BadRequestException(s"Invalid IRI is given: $parentIri.")
   }
 
+  if (position < -1) {
+    throw BadRequestException(INVALID_POSITION)
+  }
   def toJsValue: JsValue = changeNodePositionApiRequestADMFormat.write(this)
 }
 
@@ -530,7 +545,7 @@ case class ListADM(listinfo: ListRootNodeInfoADM, children: Seq[ListChildNodeADM
   def sorted: ListADM = {
     ListADM(
       listinfo = listinfo,
-      children = children.sortBy(_.position) map (_.sorted)
+      children = children.sortBy(_.position).map(_.sorted)
     )
   }
 }
@@ -546,7 +561,7 @@ case class NodeADM(nodeinfo: ListChildNodeInfoADM, children: Seq[ListChildNodeAD
   def sorted: NodeADM = {
     NodeADM(
       nodeinfo = nodeinfo,
-      children = children.sortBy(_.position) map (_.sorted)
+      children = children.sortBy(_.position).map(_.sorted)
     )
   }
 }
@@ -771,7 +786,7 @@ case class ListRootNodeADM(id: IRI,
       name = name,
       labels = labels.sortByStringValue,
       comments = comments.sortByStringValue,
-      children = children.sortBy(_.position) map (_.sorted)
+      children = children.sortBy(_.position).map(_.sorted)
     )
   }
 
@@ -831,7 +846,7 @@ case class ListChildNodeADM(id: IRI,
       comments = comments.sortByStringValue,
       position = position,
       hasRootNode = hasRootNode,
-      children = children.sortBy(_.position) map (_.sorted)
+      children = children.sortBy(_.position).map(_.sorted)
     )
   }
 
@@ -1279,7 +1294,7 @@ trait ListADMJsonProtocol extends SprayJsonSupport with DefaultJsonProtocol with
   implicit val createListApiRequestADMFormat: RootJsonFormat[CreateListApiRequestADM] =
     jsonFormat(CreateListApiRequestADM, "id", "projectIri", "name", "labels", "comments")
   implicit val createListNodeApiRequestADMFormat: RootJsonFormat[CreateNodeApiRequestADM] =
-    jsonFormat(CreateNodeApiRequestADM, "id", "parentNodeIri", "projectIri", "name", "labels", "comments")
+    jsonFormat(CreateNodeApiRequestADM, "id", "parentNodeIri", "projectIri", "name", "position", "labels", "comments")
   implicit val changeListInfoApiRequestADMFormat: RootJsonFormat[ChangeNodeInfoApiRequestADM] = jsonFormat(
     ChangeNodeInfoApiRequestADM,
     "listIri",

webapi/src/main/scala/org/knora/webapi/messages/admin/responder/listsmessages/ListsMessagesUtilADM.scala

@@ -31,4 +31,5 @@ object ListsMessagesUtilADM {
   val LIST_NODE_CREATE_PERMISSION_ERROR = "A list node can only be created by the project or system administrator."
   val LIST_CHANGE_PERMISSION_ERROR = "A list can only be changed by the project or system administrator."
   val UPDATE_REQUEST_EMPTY_LABEL_ERROR = "List labels cannot be empty."
+  val INVALID_POSITION = "Invalid position value is given, position should be either a positive value or -1."
 }

webapi/src/main/scala/org/knora/webapi/responders/admin/ListsResponderADM.scala

@@ -808,10 +808,16 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
                          featureFactoryConfig: FeatureFactoryConfig): Future[IRI] = {
 
     def getPositionOfNewChild(children: Seq[ListChildNodeADM]): Int = {
-      val position = if (children.isEmpty) {
-        0
-      } else {
+      if (createNodeRequest.position.exists(_ > children.size)) {
+        val givenPosition = createNodeRequest.position.get
+        throw BadRequestException(
+          s"Invalid position given $givenPosition, maximum allowed position is = ${children.size}.")
+      }
+
+      val position = if (createNodeRequest.position.isEmpty || createNodeRequest.position.exists(_.equals(-1))) {
         children.size
+      } else {
+        createNodeRequest.position.get
       }
       position
     }
@@ -823,7 +829,9 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
       }
     }
 
-    def getRootNodeAndPositionOfNewChild(parentNodeIri: IRI, featureFactoryConfig: FeatureFactoryConfig) = {
+    def getRootNodeAndPositionOfNewChild(parentNodeIri: IRI,
+                                         dataNamedGraph: IRI,
+                                         featureFactoryConfig: FeatureFactoryConfig): Future[(Some[Int], Some[IRI])] = {
       for {
         /* Verify that the list node exists by retrieving the whole node including children one level deep (need for position calculation) */
         maybeParentListNode <- listNodeGetADM(
@@ -839,9 +847,28 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
           case Some(_) | None               => throw BadRequestException(s"List node '$parentNodeIri' not found.")
         }
 
-        // append child to the end
+        // get position of the new child
         position = getPositionOfNewChild(children)
 
+        // Is the node supposed to be inserted in a specific position in array of children?
+        _ <- if (position != children.size) {
+          // Yes. Shift the siblings after the given position to right in order to free the position.
+          for {
+            // shift siblings that are after given position to right
+            updatedSiblings <- shiftNodes(
+              startPos = position,
+              endPos = children.size - 1,
+              nodes = children,
+              shiftToLeft = false,
+              dataNamedGraph = dataNamedGraph,
+              featureFactoryConfig = featureFactoryConfig
+            )
+          } yield updatedSiblings
+        } else {
+          // No. new node will be appended to the end, no shifting is necessary.
+          Future.successful(children)
+        }
+
         /* get the root node, depending on the type of the parent */
         rootNodeIri = getRootNodeIri(parentListNode)
 
@@ -868,16 +895,18 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
           s"The node name ${createNodeRequest.name.get} is already used by a list inside the project ${createNodeRequest.projectIri}.")
       }
 
+      // calculate the data named graph
+      dataNamedGraph: IRI = stringFormatter.projectDataNamedGraphV2(project)
+
       // if parent node is known, find the root node of the list and the position of the new child node
-      (position, rootNodeIri) <- if (createNodeRequest.parentNodeIri.nonEmpty) {
-        getRootNodeAndPositionOfNewChild(createNodeRequest.parentNodeIri.get, featureFactoryConfig)
+      (position: Option[Int], rootNodeIri: Option[IRI]) <- if (createNodeRequest.parentNodeIri.nonEmpty) {
+        getRootNodeAndPositionOfNewChild(parentNodeIri = createNodeRequest.parentNodeIri.get,
+                                         dataNamedGraph = dataNamedGraph,
+                                         featureFactoryConfig = featureFactoryConfig)
       } else {
         Future(None, None)
       }
 
-      // calculate the data named graph
-      dataNamedGraph: IRI = stringFormatter.projectDataNamedGraphV2(project)
-
       // check the custom IRI; if not given, create an unused IRI
       customListIri: Option[SmartIri] = createNodeRequest.id.map(iri => iri.toSmartIri)
       maybeShortcode: String = project.shortcode

webapi/src/test/scala/org/knora/webapi/e2e/admin/lists/OldListsRouteADMFeatureE2ESpec.scala

@@ -927,6 +927,82 @@ class OldListsRouteADMFeatureE2ESpec
         )
       }
 
+      "insert new child in a specific position" in {
+
+        val encodedListUrl = java.net.URLEncoder.encode(newListIri.get, "utf-8")
+
+        val name = "child with position"
+        val label = "Inserted List Node Label"
+        val comment = "Inserted List Node Comment"
+
+        val insertChild =
+          s"""{
+           |    "parentNodeIri": "${newListIri.get}",
+           |    "projectIri": "${SharedTestDataADM.ANYTHING_PROJECT_IRI}",
+           |    "name": "$name",
+           |    "position": 1,
+           |    "labels": [{ "value": "$label", "language": "en"}],
+           |    "comments": [{ "value": "$comment", "language": "en"}]
+           |}""".stripMargin
+
+        clientTestDataCollector.addFile(
+          TestDataFileContent(
+            filePath = TestDataFilePath(
+              directoryPath = clientTestDataPath,
+              filename = "insert-childNode-in-position-request",
+              fileExtension = "json"
+            ),
+            text = insertChild
+          )
+        )
+        val request = Post(baseApiUrl + s"/admin/lists/" + encodedListUrl,
+                           HttpEntity(ContentTypes.`application/json`, insertChild)) ~> addCredentials(
+          anythingAdminUserCreds.basicHttpCredentials)
+        val response: HttpResponse = singleAwaitingRequest(request)
+        // println(s"response: ${response.toString}")
+        response.status should be(StatusCodes.OK)
+
+        val received: ListNodeInfoADM =
+          AkkaHttpUtils.httpResponseToJson(response).fields("nodeinfo").convertTo[ListNodeInfoADM]
+
+        // check correct node info
+        val childNodeInfo = received match {
+          case info: ListChildNodeInfoADM => info
+          case something                  => fail(s"expecting ListChildNodeInfoADM but got ${something.getClass.toString} instead.")
+        }
+
+        // check labels
+        val labels: Seq[StringLiteralV2] = childNodeInfo.labels.stringLiterals
+        labels.size should be(1)
+        labels.sorted should be(Seq(StringLiteralV2(value = label, language = Some("en"))))
+
+        // check comments
+        val comments = childNodeInfo.comments.stringLiterals
+        comments.size should be(1)
+        comments.sorted should be(Seq(StringLiteralV2(value = comment, language = Some("en"))))
+
+        // check position
+        val position = childNodeInfo.position
+        position should be(1)
+
+        // check has root node
+        val rootNode = childNodeInfo.hasRootNode
+        rootNode should be(newListIri.get)
+
+        secondChildIri.set(childNodeInfo.id)
+
+        clientTestDataCollector.addFile(
+          TestDataFileContent(
+            filePath = TestDataFilePath(
+              directoryPath = clientTestDataPath,
+              filename = "insert-childNode-in-position-response",
+              fileExtension = "json"
+            ),
+            text = responseToString(response)
+          )
+        )
+      }
+
       "add child to second child node" in {
 
         val encodedListUrl = java.net.URLEncoder.encode(secondChildIri.get, "utf-8")