Skip to content

Commit eadb511

Browse files
gunnarvellegunnarvelle
committed
Add sorting after updateFolder to ensure correct order if move
1 parent 4742d32 commit eadb511

File tree

2 files changed

+205
-3
lines changed

2 files changed

+205
-3
lines changed

‎myndla-api/src/main/scala/no/ndla/myndlaapi/service/FolderWriteService.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,9 @@ class FolderWriteService(using
295295
_ <- validateUpdatedFolder(converted.name, converted.parentId, maybeSiblings, converted)
296296
updated <- folderRepository.updateFolder(id, feideId, converted)
297297
crumbs <- folderReadService.getBreadcrumbs(updated)(using ReadOnlyAutoSession)
298+
siblingsToSort <- getFolderWithDirectChildren(converted.parentId, feideId)
299+
sortRequest = FolderSortRequestDTO(sortedIds = siblingsToSort.childrenFolders.map(_.id))
300+
_ <- performSort(siblingsToSort.childrenFolders, sortRequest, feideId, sharedFolderSort = false)
298301
feideUser <- userRepository.userWithFeideId(feideId)
299302
api <- folderConverterService.toApiFolder(updated, crumbs, feideUser, isOwner = true)
300303
} yield api

‎myndla-api/src/test/scala/no/ndla/myndlaapi/service/FolderWriteServiceTest.scala‎

Lines changed: 202 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ package no.ndla.myndlaapi.service
1010

1111
import no.ndla.common.errors.{AccessDeniedException, ValidationException}
1212
import no.ndla.common.model.NDLADate
13-
import no.ndla.common.model.api.Missing
13+
import no.ndla.common.model.api.{Missing, UpdateWith}
1414
import no.ndla.common.model.domain.ResourceType
1515
import no.ndla.common.model.domain.myndla.{FolderStatus, UserRole}
1616
import no.ndla.myndlaapi.TestData.{emptyDomainFolder, emptyDomainResource, emptyMyNDLAUser}
@@ -806,17 +806,216 @@ class FolderWriteServiceTest extends UnitTestSuite with TestEnvironment {
806806
when(folderReadService.getBreadcrumbs(any)(using any)).thenReturn(Success(List.empty))
807807
when(folderRepository.getConnections(any)(using any)).thenReturn(Success(List.empty))
808808
when(folderRepository.foldersWithFeideAndParentID(eqTo(Some(parentId)), eqTo(feideId))(using any)).thenReturn(
809-
Success(List(siblingFolder))
809+
Success(List(existingFolder, siblingFolder)),
810+
Success(List(existingFolder, siblingFolder)),
810811
)
811812
when(folderRepository.folderWithId(eqTo(folderId))(using any)).thenReturn(Success(existingFolder))
812813
when(folderRepository.updateFolder(any, any, any)(using any)).thenReturn(Success(mergedFolder))
814+
when(folderRepository.setFolderRank(any[UUID], any[Int], any)(using any)).thenReturn(Success(()))
815+
when(folderRepository.withTx(any[DBSession => Try[Unit]]())).thenAnswer((i: InvocationOnMock) => {
816+
val func = i.getArgument[DBSession => Try[Unit]](0)
817+
func(mock[DBSession])
818+
})
813819
when(userRepository.userWithFeideId(any)(using any[DBSession])).thenReturn(Success(None))
814820

815-
service.updateFolder(folderId, updateFolder, Some(feideId)) should be(Success(expectedFolder))
821+
val result = service.updateFolder(folderId, updateFolder, Some(feideId))
822+
result should be(Success(expectedFolder))
816823

817824
verify(folderRepository, times(1)).updateFolder(any, any, any)(using any)
818825
}
819826

827+
test("that updating parentId of a folder sorts siblings with unique ranks") {
828+
val created = clock.now()
829+
val feideId = "FEIDE"
830+
val folderId = UUID.randomUUID()
831+
val oldParentId = UUID.randomUUID()
832+
val newParentId = UUID.randomUUID()
833+
834+
val sibling1Id = UUID.randomUUID()
835+
val sibling2Id = UUID.randomUUID()
836+
837+
val updateFolder = api.UpdatedFolderDTO(
838+
parentId = UpdateWith[String](newParentId.toString),
839+
name = Some("updated"),
840+
status = None,
841+
description = None,
842+
)
843+
844+
val movedFolder = domain.Folder(
845+
id = folderId,
846+
feideId = feideId,
847+
parentId = Some(newParentId),
848+
name = "updated",
849+
status = FolderStatus.PRIVATE,
850+
subfolders = List.empty,
851+
resources = List.empty,
852+
rank = 2,
853+
created = created,
854+
updated = created,
855+
shared = None,
856+
description = None,
857+
user = None,
858+
)
859+
860+
val sibling1 = domain.Folder(
861+
id = sibling1Id,
862+
feideId = feideId,
863+
parentId = Some(newParentId),
864+
name = "sibling1",
865+
status = FolderStatus.PRIVATE,
866+
subfolders = List.empty,
867+
resources = List.empty,
868+
rank = 1,
869+
created = created,
870+
updated = created,
871+
shared = None,
872+
description = None,
873+
user = None,
874+
)
875+
876+
val sibling2 = domain.Folder(
877+
id = sibling2Id,
878+
feideId = feideId,
879+
parentId = Some(newParentId),
880+
name = "sibling2",
881+
status = FolderStatus.PRIVATE,
882+
subfolders = List.empty,
883+
resources = List.empty,
884+
rank = 3,
885+
created = created,
886+
updated = created,
887+
shared = None,
888+
description = None,
889+
user = None,
890+
)
891+
892+
val siblings = List(sibling1, movedFolder, sibling2)
893+
val expectedRanks = siblings.map(_.rank).toSet
894+
895+
doReturn(Success(newParentId))
896+
.when(folderConverterService)
897+
.toUUIDValidated(eqTo(Some(newParentId.toString)), eqTo("parentId"))
898+
when(feideApiClient.getFeideID(any)).thenReturn(Success(feideId))
899+
when(userService.getOrCreateMyNDLAUserIfNotExist(any, any)(using any)).thenReturn(Success(emptyMyNDLAUser))
900+
when(folderRepository.folderWithId(eqTo(folderId))(using any)).thenReturn(
901+
Success(movedFolder.copy(parentId = Some(oldParentId), rank = 1, name = "updated"))
902+
)
903+
when(folderRepository.folderWithFeideId(eqTo(newParentId), eqTo(feideId))(using any)).thenReturn(
904+
Success(emptyDomainFolder)
905+
)
906+
when(folderRepository.getFoldersDepth(eqTo(newParentId))(using any)).thenReturn(Success(1L))
907+
when(folderRepository.foldersWithFeideAndParentID(eqTo(Some(newParentId)), eqTo(feideId))(using any)).thenReturn(
908+
Success(siblings)
909+
)
910+
when(folderRepository.updateFolder(any, any, any)(using any)).thenReturn(Success(movedFolder))
911+
when(folderReadService.getBreadcrumbs(any)(using any)).thenReturn(Success(List.empty))
912+
when(folderRepository.getConnections(any)(using any)).thenReturn(Success(List.empty))
913+
when(userRepository.userWithFeideId(any)(using any)).thenReturn(Success(None))
914+
when(folderRepository.setFolderRank(any, any, any)(using any)).thenReturn(Success(()))
915+
when(folderRepository.withTx(any[DBSession => Try[Unit]]())).thenAnswer((i: InvocationOnMock) => {
916+
val func = i.getArgument[DBSession => Try[Unit]](0)
917+
func(mock[DBSession])
918+
})
919+
920+
val result = service.updateFolder(folderId, updateFolder, Some(feideId))
921+
result.isSuccess should be(true)
922+
923+
// After sorting, ranks should be unique and sequential
924+
val sortedSiblings = siblings.sortBy(_.rank)
925+
sortedSiblings.map(_.rank).distinct.size should be(siblings.size)
926+
sortedSiblings.map(_.rank) should contain theSameElementsAs expectedRanks
927+
}
928+
929+
test("that updating parentId of a folder fails if sibling with same name exists") {
930+
val created = clock.now()
931+
val feideId = "FEIDE"
932+
val folderId = UUID.randomUUID()
933+
val oldParentId = UUID.randomUUID()
934+
val newParentId = UUID.randomUUID()
935+
936+
val sibling1Id = UUID.randomUUID()
937+
val sibling2Id = UUID.randomUUID()
938+
939+
val updateFolder = api.UpdatedFolderDTO(
940+
parentId = UpdateWith[String](newParentId.toString),
941+
name = Some("duplicateName"),
942+
status = None,
943+
description = None,
944+
)
945+
946+
val movedFolder = domain.Folder(
947+
id = folderId,
948+
feideId = feideId,
949+
parentId = Some(newParentId),
950+
name = "duplicateName",
951+
status = FolderStatus.PRIVATE,
952+
subfolders = List.empty,
953+
resources = List.empty,
954+
rank = 2,
955+
created = created,
956+
updated = created,
957+
shared = None,
958+
description = None,
959+
user = None,
960+
)
961+
962+
val sibling1 = domain.Folder(
963+
id = sibling1Id,
964+
feideId = feideId,
965+
parentId = Some(newParentId),
966+
name = "duplicateName", // Same name as movedFolder
967+
status = FolderStatus.PRIVATE,
968+
subfolders = List.empty,
969+
resources = List.empty,
970+
rank = 1,
971+
created = created,
972+
updated = created,
973+
shared = None,
974+
description = None,
975+
user = None,
976+
)
977+
978+
val sibling2 = domain.Folder(
979+
id = sibling2Id,
980+
feideId = feideId,
981+
parentId = Some(newParentId),
982+
name = "sibling2",
983+
status = FolderStatus.PRIVATE,
984+
subfolders = List.empty,
985+
resources = List.empty,
986+
rank = 3,
987+
created = created,
988+
updated = created,
989+
shared = None,
990+
description = None,
991+
user = None,
992+
)
993+
994+
val siblings = List(sibling1, movedFolder, sibling2)
995+
996+
doReturn(Success(newParentId))
997+
.when(folderConverterService)
998+
.toUUIDValidated(eqTo(Some(newParentId.toString)), eqTo("parentId"))
999+
when(feideApiClient.getFeideID(any)).thenReturn(Success(feideId))
1000+
when(userService.getOrCreateMyNDLAUserIfNotExist(any, any)(using any)).thenReturn(Success(emptyMyNDLAUser))
1001+
when(folderRepository.folderWithId(eqTo(folderId))(using any)).thenReturn(
1002+
Success(movedFolder.copy(parentId = Some(oldParentId), rank = 1, name = "duplicateName"))
1003+
)
1004+
when(folderRepository.folderWithFeideId(eqTo(newParentId), eqTo(feideId))(using any)).thenReturn(
1005+
Success(emptyDomainFolder)
1006+
)
1007+
when(folderRepository.getFoldersDepth(eqTo(newParentId))(using any)).thenReturn(Success(1L))
1008+
when(folderRepository.foldersWithFeideAndParentID(eqTo(Some(newParentId)), eqTo(feideId))(using any)).thenReturn(
1009+
Success(siblings)
1010+
)
1011+
when(folderReadService.getBreadcrumbs(any)(using any)).thenReturn(Success(List.empty))
1012+
when(userRepository.userWithFeideId(any)(using any)).thenReturn(Success(None))
1013+
when(folderRepository.getConnections(any)(using any)).thenReturn(Success(List.empty))
1014+
1015+
val result = service.updateFolder(folderId, updateFolder, Some(feideId))
1016+
result should be(Failure(ValidationException("name", "The folder name must be unique within its parent.")))
1017+
}
1018+
8201019
test("That deleteAllUserData works as expected") {
8211020
val feideId = "feide"
8221021

0 commit comments

Comments
 (0)