insert multiple items #266
Conversation
| try { | ||
| positionFinal = Integer.parseInt(position); | ||
| } catch (RuntimeException e) { | ||
| return -1; |
There was a problem hiding this comment.
invalid should not return -1
| /** | ||
| * This method updates existing item list | ||
| * | ||
| * @param existingUserSet | ||
| * @return updated user set | ||
| */ | ||
| UserSet updateItemList(UserSet existingUserSet); | ||
| UserSet updateUserSetInMongo(UserSet existingUserSet); |
There was a problem hiding this comment.
should not be renamed, this method is meant to update only the list of items and the total, not the metadata. The purpose should be inline with the method name
| @@ -257,4 +261,6 @@ ItemIdsResultPage buildItemIdsResultsPage(String setId, List<String> itemIds, in | |||
|
|
|||
| void validateGallerySize(UserSet webUserSet, int newItems) throws ItemValidationException; | |||
|
|
|||
| UserSet updatePagination(UserSet userSet, UserSetConfiguration config); | |||
There was a problem hiding this comment.
this should not me public method, it is meant to be internal to the service
| @@ -482,6 +498,83 @@ protected ResponseEntity<String> insertItemIntoUserSet(HttpServletRequest reques | |||
|
|
|||
| UserSet updatedUserSet = | |||
| getUserSetService().insertItem(datasetId, localId, position, existingUserSet); | |||
| getUserSetService().updatePagination(updatedUserSet, getConfiguration()); | |||
There was a problem hiding this comment.
the update pagination should be called within the service not from the controller
| // retrieve an existing user set based on its identifier | ||
| UserSet existingUserSet = getUserSetService().getUserSetById(identifier); | ||
|
|
||
| if (existingUserSet.isOpenSet()) { |
There was a problem hiding this comment.
The validation here is dupplicated with the code from insertItemIntoUserSet, should be refactored to eliminate dupplication.
| // for entity user sets, add users with 'editor' role as contributors | ||
| addContributorForEntitySet(existingUserSet, authentication); | ||
|
|
||
| boolean itemsRemoved=false; |
There was a problem hiding this comment.
the processing of the items belongs to the business logic and must be moved to the service method
|
|
||
| //update the pagination fields of the set (used only for the serialization to the output) | ||
| @Override | ||
| public UserSet updatePagination(UserSet userSet, UserSetConfiguration config) { |
There was a problem hiding this comment.
should be protected not public
| */ | ||
| for(String newItem : items) { | ||
| int itemindex=usersetItems.indexOf(newItem); | ||
| if(itemindex>=0) { |
There was a problem hiding this comment.
use contains method instead
| /*remove all duplicate items from the set and count the number | ||
| of removed pinned items, to change the pinned field | ||
| */ | ||
| for(String newItem : items) { |
There was a problem hiding this comment.
not implemented according to the specs, we need to implement a method which computes the list of dupplicated items first and than to update the list
There was a problem hiding this comment.
It is implemented according to the specs, the method is finding the duplicates using a foor loop, not the streams, because we also need to update the pinned field if the duplicated items were pinned before, and that is all done in one foor loop.
| /*remove from the new items the ones that were pinned before | ||
| *and from the user set the ones that are duplicates | ||
| */ | ||
| for(String newItem : newItemsCopy) { |
There was a problem hiding this comment.
not implemented according to the specs, we need to implement a method which computes the list of dupplicated items first and than to update the list
There was a problem hiding this comment.
It is implemented according to the specs, similar as the comment above.
EA-3957-insert-multiple-items
No description provided.