Skip to content

Commit

Permalink
DSP-740 Update List Name (#1727)
Browse files Browse the repository at this point in the history
* feature (updateListName) listName can be changed

* fix (updateListName) add check for uniqueness of the names and fix the failing test

* refactor (updateListName) remove the redundant route

* feature (updateList) make comments and labels in payload of update request optional

* refactor (updateList) clear error message for empty labels or comments in payload

* feature (updateListName) add test data for updating list name

* refactor (updateListName) checks for empty optional labels and comments

* docs (updateListName) fix generate test data documentation

* feature (updateListName) update `expected-client-test-data.txt`
  • Loading branch information
SepidehAlassi committed Oct 9, 2020
1 parent 743cd7e commit 6c2e903
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 72 deletions.
18 changes: 16 additions & 2 deletions docs/03-apis/api-admin/lists.md
Expand Up @@ -26,10 +26,10 @@ License along with Knora. If not, see <http://www.gnu.org/licenses/>.
- `GET: /admin/lists[?projectIri=<projectIri>]` : return all lists optionally filtered by project
- `GET: /admin/lists/<listIri>` : return complete list with children
- `POST: /admin/lists` : create new list
- `PUT: /admin/lists/<listIri>` : update list information
- `POST: /admin/lists/<nodeIri>` : create new child node under the supplied parent node IRI
- NOT IMPLEMENTED: `DELETE: /admin/lists/<listIri>` : delete list including children if not used
- `GET: /admin/lists/infos/<listIri>` : return list information (without children)
- `PUT: /admin/lists/infos/<listIri>` : update list information

**List Node operations**

Expand Down Expand Up @@ -103,21 +103,35 @@ Additionally, each list can have an optional custom IRI (of [Knora IRI](../api-v
- GET: `/admin/lists/infos/<listIri>`

### Update list's information
The basic information of a list 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
list and the IRI of the project it belongs to.

- Required permission: none
- Update list information
- PUT: `/admin/lists/infos/<listIri>`
- PUT: `/admin/lists/<listIri>`
- BODY:

```json
{
"listIri": "listIri",
"projectIri": "someprojectiri",
"name": "a new name",
"labels": [{ "value": "Neue geönderte Liste", "language": "de"}, { "value": "Changed list", "language": "en"}],
"comments": [{ "value": "Neuer Kommentar", "language": "de"}, { "value": "New comment", "language": "en"}]
}
```

If only name of the list must be updated, it can be given as below in the body of the request:

```json
{
"listIri": "listIri",
"projectIri": "someprojectiri",
"name": "another name"
}
```

## List Node Operations

### Get List Node Information
Expand Down
4 changes: 2 additions & 2 deletions docs/05-internals/development/generating-client-test-data.md
Expand Up @@ -39,13 +39,13 @@ with the list in `webapi/scripts/expected-client-test-data.txt`.
To generate client test data, type:

```
make test-e2e
make client-test-data
```

When the tests have finished running, you will find the file
`client-test-data.zip` in the current directory.

If generated client test data changes, run `make test-e2e`, then run
If generated client test data changes, run `make client-test-data`, then run
this script to update the list of expected test data files:

```
Expand Down
2 changes: 2 additions & 0 deletions webapi/scripts/expected-client-test-data.txt
Expand Up @@ -32,6 +32,8 @@ test-data/admin/lists/update-list-info-comment-label-multiple-languages-request.
test-data/admin/lists/update-list-info-comment-label-multiple-languages-response.json
test-data/admin/lists/update-list-info-request.json
test-data/admin/lists/update-list-info-response.json
test-data/admin/lists/update-list-name-request.json
test-data/admin/lists/update-list-name-response.json
test-data/admin/permissions/
test-data/admin/permissions/create-administrative-permission-request.json
test-data/admin/permissions/create-administrative-permission-response.json
Expand Down
Expand Up @@ -129,8 +129,9 @@ case class CreateChildNodeApiRequestADM(parentNodeIri: IRI,
*/
case class ChangeListInfoApiRequestADM(listIri: IRI,
projectIri: IRI,
labels: Seq[StringLiteralV2],
comments: Seq[StringLiteralV2]) extends ListADMJsonProtocol {
name: Option[String] = None,
labels: Option[Seq[StringLiteralV2]] = None,
comments: Option[Seq[StringLiteralV2]] = None) extends ListADMJsonProtocol {

private val stringFormatter = StringFormatter.getInstanceForConstantOntologies

Expand All @@ -150,10 +151,14 @@ case class ChangeListInfoApiRequestADM(listIri: IRI,
throw BadRequestException(PROJECT_IRI_INVALID_ERROR)
}

if (labels.isEmpty && comments.isEmpty) {
throw BadRequestException(REQUEST_NOT_CHANGING_DATA_ERROR)
// If payload containes label or comments they should not be empty
if (labels.exists(_.isEmpty)) {
throw BadRequestException(UPDATE_REQUEST_EMPTY_LABEL_OR_COMMENT_ERROR)
}

if (comments.exists(_.isEmpty)) {
throw BadRequestException(UPDATE_REQUEST_EMPTY_LABEL_OR_COMMENT_ERROR)
}
def toJsValue: JsValue = changeListInfoApiRequestADMFormat.write(this)
}

Expand Down Expand Up @@ -963,7 +968,7 @@ trait ListADMJsonProtocol extends SprayJsonSupport with DefaultJsonProtocol with

implicit val createListApiRequestADMFormat: RootJsonFormat[CreateListApiRequestADM] = jsonFormat(CreateListApiRequestADM, "id", "projectIri", "name", "labels", "comments")
implicit val createListNodeApiRequestADMFormat: RootJsonFormat[CreateChildNodeApiRequestADM] = jsonFormat(CreateChildNodeApiRequestADM, "parentNodeIri", "projectIri", "name", "labels", "comments")
implicit val changeListInfoApiRequestADMFormat: RootJsonFormat[ChangeListInfoApiRequestADM] = jsonFormat(ChangeListInfoApiRequestADM, "listIri", "projectIri", "labels", "comments")
implicit val changeListInfoApiRequestADMFormat: RootJsonFormat[ChangeListInfoApiRequestADM] = jsonFormat(ChangeListInfoApiRequestADM, "listIri", "projectIri", "name", "labels", "comments")
implicit val nodePathGetResponseADMFormat: RootJsonFormat[NodePathGetResponseADM] = jsonFormat(NodePathGetResponseADM, "elements")
implicit val listsGetResponseADMFormat: RootJsonFormat[ListsGetResponseADM] = jsonFormat(ListsGetResponseADM, "lists")
implicit val listGetResponseADMFormat: RootJsonFormat[ListGetResponseADM] = jsonFormat(ListGetResponseADM, "list")
Expand Down
Expand Up @@ -29,5 +29,5 @@ object ListsMessagesUtilADM {
val LABEL_MISSING_ERROR = "At least one label needs to be supplied."
val LIST_CREATE_PERMISSION_ERROR = "A list 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 REQUEST_NOT_CHANGING_DATA_ERROR = "No data would be changed."
val UPDATE_REQUEST_EMPTY_LABEL_OR_COMMENT_ERROR = "List labels and comments cannot be empty."
}
Expand Up @@ -675,18 +675,17 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
* The actual task run with an IRI lock.
*/
def listInfoChangeTask(listIri: IRI, changeListRequest: ChangeListInfoApiRequestADM, requestingUser: UserADM, apiRequestID: UUID): Future[ListInfoGetResponseADM] = for {
// check if required information is supplied
_ <- Future(if (changeListRequest.labels.isEmpty && changeListRequest.comments.isEmpty) throw BadRequestException(REQUEST_NOT_CHANGING_DATA_ERROR))
_ = if (!listIri.equals(changeListRequest.listIri)) throw BadRequestException("List IRI in path and payload don't match.")
// check if listIRI in path and payload match
_ <- Future(
if (!listIri.equals(changeListRequest.listIri)) throw BadRequestException("List IRI in path and payload don't match.")
)

// check if the requesting user is allowed to perform operation
_ <- Future(
if (!requestingUser.permissions.isProjectAdmin(changeListRequest.projectIri) && !requestingUser.permissions.isSystemAdmin) {
_ = if (!requestingUser.permissions.isProjectAdmin(changeListRequest.projectIri) && !requestingUser.permissions.isSystemAdmin) {
// not project or a system admin
// log.debug("same user: {}, system admin: {}", userProfile.userData.user_id.contains(userIri), userProfile.permissionData.isSystemAdmin)
throw ForbiddenException(LIST_CHANGE_PERMISSION_ERROR)
}
)

/* Verify that the list exists. */
maybeList <- listGetADM(rootNodeIri = listIri, requestingUser = KnoraSystemInstances.Users.SystemUser)
Expand All @@ -702,6 +701,14 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
case None => throw BadRequestException(s"Project '${list.listinfo.projectIri}' not found.")
}

/* verify that the list name is unique for the project */
nodeNameUnique: Boolean <- listNodeNameIsProjectUnique(changeListRequest.projectIri, changeListRequest.name)
_ = if (!nodeNameUnique) {
throw DuplicateValueException(s"The name ${changeListRequest.name.get} is already used by a list inside the project ${changeListRequest.projectIri}.")
}

hasOldName: Boolean = list.listinfo.name.nonEmpty

// get the data graph of the project.
dataNamedGraph = stringFormatter.projectDataNamedGraphV2(project)

Expand All @@ -710,6 +717,8 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde
dataNamedGraph = dataNamedGraph,
triplestore = settings.triplestoreType,
listIri = listIri,
hasOldName = hasOldName,
maybeName = changeListRequest.name,
projectIri = project.id,
listClassIri = OntologyConstants.KnoraBase.ListNode,
maybeLabels = changeListRequest.labels,
Expand All @@ -725,15 +734,19 @@ class ListsResponderADM(responderData: ResponderData) extends Responder(responde


_ = if (changeListRequest.labels.nonEmpty) {
if (updatedList.listinfo.labels.stringLiterals.diff(changeListRequest.labels).nonEmpty) throw UpdateNotPerformedException("Lists's 'labels' where not updated. Please report this as a possible bug.")
if (updatedList.listinfo.labels.stringLiterals.diff(changeListRequest.labels.get).nonEmpty) throw UpdateNotPerformedException("Lists's 'labels' where not updated. Please report this as a possible bug.")
}

_ = if (changeListRequest.comments.nonEmpty) {
if (updatedList.listinfo.comments.stringLiterals.diff(changeListRequest.comments).nonEmpty)
if (updatedList.listinfo.comments.stringLiterals.diff(changeListRequest.comments.get).nonEmpty)

throw UpdateNotPerformedException("List's 'comments' was not updated. Please report this as a possible bug.")
}

_ = if (changeListRequest.name.nonEmpty) {
if (updatedList.listinfo.name.get != changeListRequest.name.get)
throw UpdateNotPerformedException("List's 'name' was not updated. Please report this as a possible bug.")
}
// _ = log.debug(s"listInfoChangeRequest - updatedList: {}", updatedList)

} yield ListInfoGetResponseADM(listinfo = updatedList.listinfo)
Expand Down
Expand Up @@ -48,8 +48,9 @@ class ListsRouteADM(routeData: KnoraRouteData) extends KnoraRoute(routeData) wit
/**
* Returns the route.
*/

override def knoraApiPath: Route = getLists ~ createList ~ getList ~ updateList ~ createListChildNode ~ deleteListNode ~
getListInfo ~ updateListInfo ~ getListNodeInfo
getListInfo ~ getListNodeInfo

/* return all lists optionally filtered by project */
@ApiOperation(value = "Get lists", nickname = "getlists", httpMethod = "GET", response = classOf[ListsGetResponseADM])
Expand Down Expand Up @@ -245,33 +246,6 @@ class ListsRouteADM(routeData: KnoraRouteData) extends KnoraRoute(routeData) wit
}
}

def updateListInfo: Route = path(ListsBasePath / "infos" / Segment) { iri =>
put {
/* update list info */
entity(as[ChangeListInfoApiRequestADM]) { apiRequest =>
requestContext =>
val listIri = stringFormatter.validateAndEscapeIri(iri, throw BadRequestException(s"Invalid param list IRI: $iri"))

val requestMessage: Future[ListInfoChangeRequestADM] = for {
requestingUser <- getUserADM(requestContext)
} yield ListInfoChangeRequestADM(
listIri = listIri,
changeListRequest = apiRequest,
requestingUser = requestingUser,
apiRequestID = UUID.randomUUID()
)

RouteUtilADM.runJsonRoute(
requestMessage,
requestContext,
settings,
responderManager,
log
)
}
}
}

def getListNodeInfo: Route = path(ListsBasePath / "nodes" / Segment) { iri =>
get {
/* return information about a single node (without children) */
Expand Down
Expand Up @@ -29,6 +29,8 @@
* @param listIri the IRI of the list we want to update.
* @param projectIri the IRI of the list's project.
* @param listClassIri the IRI of the OWL class that the list should belong to.
* @param hasOldName the old name of the list.
* @param maybeName the new name of the list.
* @param maybelabels the new optional label values.
* @param maybeComments the new optional comment values.
*@
Expand All @@ -37,8 +39,10 @@
listIri: IRI,
projectIri: IRI,
listClassIri: IRI,
maybeLabels: Seq[StringLiteralV2],
maybeComments: Seq[StringLiteralV2])
hasOldName: Boolean,
maybeName : Option[String],
maybeLabels: Option[Seq[StringLiteralV2]],
maybeComments: Option[Seq[StringLiteralV2]])

PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#>
Expand All @@ -54,6 +58,10 @@ DELETE {
?listIri rdfs:label ?currentLabels .
}

@if(hasOldName && maybeName.nonEmpty) {
?listIri knora-base:listNodeName ?currentListName .
}

@if(maybeComments.nonEmpty) {
?listIri rdfs:comment ?currentComments .
}
Expand All @@ -62,8 +70,12 @@ DELETE {

@* Add the new values. *@

@if(maybeName.nonEmpty) {
?listIri knora-base:listNodeName "@maybeName.get"^^xsd:string .
}

@if(maybeLabels.nonEmpty) {
@for(label <- maybeLabels) {
@for(label <- maybeLabels.get) {
@if(label.language.nonEmpty) {
?listIri rdfs:label """@label.value"""@@@{label.language.get} .
} else {
Expand All @@ -73,7 +85,7 @@ DELETE {
}

@if(maybeComments.nonEmpty) {
@for(comment <- maybeComments) {
@for(comment <- maybeComments.get) {
@if(comment.language.nonEmpty) {
?listIri rdfs:comment """@comment.value"""@@@{comment.language.get} .
} else {
Expand Down Expand Up @@ -109,6 +121,8 @@ WHERE {

?listIri knora-base:attachedToProject ?projectIri .

optional {?listIri knora-base:listNodeName ?currentListName .}

optional {?listIri rdfs:label ?currentLabels .}

optional {?listIri rdfs:comment ?currentComments .}
Expand Down

0 comments on commit 6c2e903

Please sign in to comment.