From e9d5032bbafcb3e985cdb2cde6fc6d54aa1e31bb Mon Sep 17 00:00:00 2001 From: vimleshg Date: Fri, 6 Oct 2023 14:55:28 +0100 Subject: [PATCH 1/2] TDR-3427 - Add series name as well while updating series id for the consignment. --- .../db/repository/ConsignmentRepository.scala | 6 ++--- .../tdr/api/service/ConsignmentService.scala | 7 +++--- .../ConsignmentRepositorySpec.scala | 23 +++++++++++++++++++ .../api/service/ConsignmentServiceSpec.scala | 19 ++++++++------- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala index baf9ee95d..8423f7d80 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala @@ -88,11 +88,11 @@ class ConsignmentRepository(db: Database, timeSource: TimeSource) { db.run(query.result) } - def updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput: ConsignmentFields.UpdateConsignmentSeriesIdInput): Future[Int] = { + def updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput: ConsignmentFields.UpdateConsignmentSeriesIdInput, seriesName: Option[String]): Future[Int] = { val update = Consignment .filter(_.consignmentid === updateConsignmentSeriesIdInput.consignmentId) - .map(t => t.seriesid) - .update(Some(updateConsignmentSeriesIdInput.seriesId)) + .map(t => (t.seriesid, t.seriesname)) + .update(Some(updateConsignmentSeriesIdInput.seriesId), seriesName) db.run(update) } diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala index 857989575..48b7e4073 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala @@ -73,8 +73,6 @@ class ConsignmentService( for { sequence <- consignmentRepository.getNextConsignmentSequence body <- transferringBodyService.getBodyByCode(userBody) - series <- if (seriesId.isDefined) seriesRepository.getSeries(seriesId.get) else Future(Seq()) - seriesName = if (series.nonEmpty) Some(series.head.name) else None consignmentRef = ConsignmentReference.createConsignmentReference(yearNow, sequence) consignmentId = uuidSource.uuid consignmentRow = ConsignmentRow( @@ -86,7 +84,7 @@ class ConsignmentService( consignmentreference = consignmentRef, consignmenttype = consignmentType, bodyid = body.bodyId, - seriesname = seriesName, + seriesname = None, transferringbodyname = Some(body.name) ) descriptiveMetadataStatusRow = ConsignmentstatusRow(uuidSource.uuid, consignmentId, DescriptiveMetadata, NotEntered, timestampNow, Option(timestampNow)) @@ -125,7 +123,8 @@ class ConsignmentService( def updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput: UpdateConsignmentSeriesIdInput): Future[Int] = { for { - result <- consignmentRepository.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput) + series <- seriesRepository.getSeries(updateConsignmentSeriesIdInput.seriesId) + result <- consignmentRepository.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(series.head.name)) seriesStatus = if (result == 1) Completed else Failed _ <- consignmentStatusRepository.updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, "Series", seriesStatus, Timestamp.from(timeSource.now)) } yield result diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala index 368b5b99f..acca05b9b 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala @@ -5,6 +5,7 @@ import com.dimafeng.testcontainers.PostgreSQLContainer import org.scalatest.concurrent.ScalaFutures import org.scalatest.matchers.should.Matchers import uk.gov.nationalarchives.Tables.ConsignmentstatusRow +import uk.gov.nationalarchives.tdr.api.graphql.fields.ConsignmentFields import uk.gov.nationalarchives.tdr.api.graphql.fields.ConsignmentFields.{ConsignmentFilters, StartUploadInput, UpdateExportDataInput} import uk.gov.nationalarchives.tdr.api.service.CurrentTimeSource import uk.gov.nationalarchives.tdr.api.service.FileStatusService.{InProgress, Upload} @@ -297,6 +298,28 @@ class ConsignmentRepositorySpec extends TestContainerUtils with ScalaFutures wit consignmentReferences should equal(List("TDR-2021-B", "TDR-2021-A")) } + "updateSeriesIdAndNameOfConsignment" should "update id and name of the consignment" in withContainers { case container: PostgreSQLContainer => + val db = container.database + val consignmentRepository = new ConsignmentRepository(db, new CurrentTimeSource) + val utils = TestUtils(db) + val seriesId: UUID = UUID.fromString("20e88b3c-d063-4a6e-8b61-187d8c51d11d") + val seriesName: String = "Mock1" + val bodyId: UUID = UUID.fromString("8a72cc59-7f2f-4e55-a263-4a4cb9f677f5") + + utils.createConsignment(consignmentIdOne, userId, consignmentRef = "TDR-2021-A") + utils.addTransferringBody(bodyId, "MOCK Department", "Code123") + utils.addSeries(seriesId, bodyId, "TDR-2020-XYZ", seriesName) + + val input = ConsignmentFields.UpdateConsignmentSeriesIdInput(consignmentId = consignmentIdOne, seriesId = seriesId) + + val response = consignmentRepository.updateSeriesIdAndNameOfConsignment(input, seriesName.some).futureValue + + response should be(1) + val consignment = consignmentRepository.getConsignment(consignmentIdOne).futureValue.head + consignment.seriesid should be(seriesId.some) + consignment.seriesname should be(seriesName.some) + } + "totalConsignments" should "return total number of consignments" in withContainers { case container: PostgreSQLContainer => val db = container.database val consignmentRepository = new ConsignmentRepository(db, new CurrentTimeSource) diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala index 1ace3e4c1..870022067 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala @@ -56,7 +56,7 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc consignmentreference = consignmentReference, consignmenttype = "standard", bodyid = bodyId, - seriesname = Some(seriesName), + seriesname = None, transferringbodyname = Some(bodyName) ) @@ -93,14 +93,12 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc val result = consignmentService.addConsignment(AddConsignmentInput(Some(seriesId), "standard"), mockToken).futureValue - verify(seriesRepositoryMock).getSeries(seriesId) - result.consignmentid shouldBe consignmentId result.seriesid shouldBe Some(seriesId) result.userid shouldBe userId result.consignmentType shouldBe "standard" result.bodyId shouldBe bodyId - result.seriesName shouldBe Some(seriesName) + result.seriesName shouldBe None result.transferringBodyName shouldBe Some(bodyName) } @@ -138,7 +136,6 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc val mockToken = mock[Token] val mockBody = mock[TransferringBody] val consignmentStatusRow = mock[ConsignmentstatusRow] - when(seriesRepositoryMock.getSeries(seriesId)).thenReturn(Future.successful(Seq(mockSeries))) when(consignmentStatusRepoMock.addConsignmentStatuses(any[Seq[ConsignmentstatusRow]])).thenReturn(Future.successful(Seq(consignmentStatusRow))) when(consignmentRepoMock.getNextConsignmentSequence).thenReturn(Future.successful(consignmentSequence)) when(consignmentRepoMock.addConsignment(any[ConsignmentRow])).thenReturn(mockResponse) @@ -311,19 +308,20 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc series.description shouldBe mockSeries.head.description } - "updateSeriesIdOfConsignment" should "update the seriesId and status for a given consignment" in { + "updateSeriesIdOfConsignment" should "update the seriesId, seriesName and status for a given consignment" in { val updateConsignmentSeriesIdInput = UpdateConsignmentSeriesIdInput(consignmentId, seriesId) val statusType = "Series" val expectedSeriesStatus = Completed val expectedResult = 1 - when(consignmentRepoMock.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput)) + when(consignmentRepoMock.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) .thenReturn(Future.successful(1)) when(consignmentStatusRepoMock.updateConsignmentStatus(consignmentId, statusType, Completed, Timestamp.from(fixedTimeSource))) .thenReturn(Future.successful(1)) + when(seriesRepositoryMock.getSeries(updateConsignmentSeriesIdInput.seriesId)).thenReturn(Future.successful(Seq(mockSeries))) val result = consignmentService.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput).futureValue - verify(consignmentRepoMock).updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput) + verify(consignmentRepoMock).updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) verify(consignmentStatusRepoMock) .updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, statusType, expectedSeriesStatus, Timestamp.from(fixedTimeSource)) @@ -335,14 +333,15 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc val statusType = "Series" val expectedSeriesStatus = Failed val expectedResult = 0 - when(consignmentRepoMock.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput)) + when(consignmentRepoMock.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) .thenReturn(Future.successful(0)) when(consignmentStatusRepoMock.updateConsignmentStatus(consignmentId, statusType, Failed, Timestamp.from(fixedTimeSource))) .thenReturn(Future.successful(1)) + when(seriesRepositoryMock.getSeries(updateConsignmentSeriesIdInput.seriesId)).thenReturn(Future.successful(Seq(mockSeries))) val result = consignmentService.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput).futureValue - verify(consignmentRepoMock).updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput) + verify(consignmentRepoMock).updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) verify(consignmentStatusRepoMock) .updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, statusType, expectedSeriesStatus, Timestamp.from(fixedTimeSource)) From 8c983185e11326fc420bb878ac6b681b73221aad Mon Sep 17 00:00:00 2001 From: vimleshg Date: Tue, 10 Oct 2023 15:19:45 +0100 Subject: [PATCH 2/2] Pull request feedback --- .../db/repository/ConsignmentRepository.scala | 2 +- .../api/graphql/fields/ConsignmentFields.scala | 2 +- .../tdr/api/service/ConsignmentService.scala | 4 ++-- .../repository/ConsignmentRepositorySpec.scala | 4 ++-- .../tdr/api/service/ConsignmentServiceSpec.scala | 16 ++++++++-------- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala index 8423f7d80..24c73a160 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepository.scala @@ -88,7 +88,7 @@ class ConsignmentRepository(db: Database, timeSource: TimeSource) { db.run(query.result) } - def updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput: ConsignmentFields.UpdateConsignmentSeriesIdInput, seriesName: Option[String]): Future[Int] = { + def updateSeriesOfConsignment(updateConsignmentSeriesIdInput: ConsignmentFields.UpdateConsignmentSeriesIdInput, seriesName: Option[String]): Future[Int] = { val update = Consignment .filter(_.consignmentid === updateConsignmentSeriesIdInput.consignmentId) .map(t => (t.seriesid, t.seriesname)) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/graphql/fields/ConsignmentFields.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/graphql/fields/ConsignmentFields.scala index 6d0f14796..d8fc0c95e 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/graphql/fields/ConsignmentFields.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/graphql/fields/ConsignmentFields.scala @@ -309,7 +309,7 @@ object ConsignmentFields { "updateConsignmentSeriesId", OptionType(IntType), arguments = UpdateConsignmentSeriesIdArg :: Nil, - resolve = ctx => ctx.ctx.consignmentService.updateSeriesIdOfConsignment(ctx.arg(UpdateConsignmentSeriesIdArg)), + resolve = ctx => ctx.ctx.consignmentService.updateSeriesOfConsignment(ctx.arg(UpdateConsignmentSeriesIdArg)), tags = List(ValidateUserHasAccessToConsignment(UpdateConsignmentSeriesIdArg), ValidateUpdateConsignmentSeriesId) ) ) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala index 48b7e4073..36219e89f 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentService.scala @@ -121,10 +121,10 @@ class ConsignmentService( consignment.map(rows => rows.headOption.map(series => Series(series.seriesid, series.bodyid, series.name, series.code, series.description))) } - def updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput: UpdateConsignmentSeriesIdInput): Future[Int] = { + def updateSeriesOfConsignment(updateConsignmentSeriesIdInput: UpdateConsignmentSeriesIdInput): Future[Int] = { for { series <- seriesRepository.getSeries(updateConsignmentSeriesIdInput.seriesId) - result <- consignmentRepository.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(series.head.name)) + result <- consignmentRepository.updateSeriesOfConsignment(updateConsignmentSeriesIdInput, series.headOption.map(_.name)) seriesStatus = if (result == 1) Completed else Failed _ <- consignmentStatusRepository.updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, "Series", seriesStatus, Timestamp.from(timeSource.now)) } yield result diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala index acca05b9b..eee6b67c6 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/db/repository/ConsignmentRepositorySpec.scala @@ -298,7 +298,7 @@ class ConsignmentRepositorySpec extends TestContainerUtils with ScalaFutures wit consignmentReferences should equal(List("TDR-2021-B", "TDR-2021-A")) } - "updateSeriesIdAndNameOfConsignment" should "update id and name of the consignment" in withContainers { case container: PostgreSQLContainer => + "updateSeriesOfConsignment" should "update id and name of the consignment" in withContainers { case container: PostgreSQLContainer => val db = container.database val consignmentRepository = new ConsignmentRepository(db, new CurrentTimeSource) val utils = TestUtils(db) @@ -312,7 +312,7 @@ class ConsignmentRepositorySpec extends TestContainerUtils with ScalaFutures wit val input = ConsignmentFields.UpdateConsignmentSeriesIdInput(consignmentId = consignmentIdOne, seriesId = seriesId) - val response = consignmentRepository.updateSeriesIdAndNameOfConsignment(input, seriesName.some).futureValue + val response = consignmentRepository.updateSeriesOfConsignment(input, seriesName.some).futureValue response should be(1) val consignment = consignmentRepository.getConsignment(consignmentIdOne).futureValue.head diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala index 870022067..74ca06db3 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ConsignmentServiceSpec.scala @@ -308,40 +308,40 @@ class ConsignmentServiceSpec extends AnyFlatSpec with MockitoSugar with ResetMoc series.description shouldBe mockSeries.head.description } - "updateSeriesIdOfConsignment" should "update the seriesId, seriesName and status for a given consignment" in { + "updateSeriesOfConsignment" should "update the seriesId, seriesName and status for a given consignment" in { val updateConsignmentSeriesIdInput = UpdateConsignmentSeriesIdInput(consignmentId, seriesId) val statusType = "Series" val expectedSeriesStatus = Completed val expectedResult = 1 - when(consignmentRepoMock.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) + when(consignmentRepoMock.updateSeriesOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) .thenReturn(Future.successful(1)) when(consignmentStatusRepoMock.updateConsignmentStatus(consignmentId, statusType, Completed, Timestamp.from(fixedTimeSource))) .thenReturn(Future.successful(1)) when(seriesRepositoryMock.getSeries(updateConsignmentSeriesIdInput.seriesId)).thenReturn(Future.successful(Seq(mockSeries))) - val result = consignmentService.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput).futureValue + val result = consignmentService.updateSeriesOfConsignment(updateConsignmentSeriesIdInput).futureValue - verify(consignmentRepoMock).updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) + verify(consignmentRepoMock).updateSeriesOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) verify(consignmentStatusRepoMock) .updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, statusType, expectedSeriesStatus, Timestamp.from(fixedTimeSource)) result should equal(expectedResult) } - "updateSeriesIdOfConsignment" should "update the status with 'Failed' if seriesId update fails for a given consignment" in { + "updateSeriesOfConsignment" should "update the status with 'Failed' if seriesId update fails for a given consignment" in { val updateConsignmentSeriesIdInput = UpdateConsignmentSeriesIdInput(consignmentId, seriesId) val statusType = "Series" val expectedSeriesStatus = Failed val expectedResult = 0 - when(consignmentRepoMock.updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) + when(consignmentRepoMock.updateSeriesOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName))) .thenReturn(Future.successful(0)) when(consignmentStatusRepoMock.updateConsignmentStatus(consignmentId, statusType, Failed, Timestamp.from(fixedTimeSource))) .thenReturn(Future.successful(1)) when(seriesRepositoryMock.getSeries(updateConsignmentSeriesIdInput.seriesId)).thenReturn(Future.successful(Seq(mockSeries))) - val result = consignmentService.updateSeriesIdOfConsignment(updateConsignmentSeriesIdInput).futureValue + val result = consignmentService.updateSeriesOfConsignment(updateConsignmentSeriesIdInput).futureValue - verify(consignmentRepoMock).updateSeriesIdAndNameOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) + verify(consignmentRepoMock).updateSeriesOfConsignment(updateConsignmentSeriesIdInput, Some(seriesName)) verify(consignmentStatusRepoMock) .updateConsignmentStatus(updateConsignmentSeriesIdInput.consignmentId, statusType, expectedSeriesStatus, Timestamp.from(fixedTimeSource))