From f11230290209d73d36f2c29546e0b59e0c741cdd Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 16 Aug 2023 13:39:11 +0300 Subject: [PATCH 01/13] HAI-1844: Create hankekayttaja instance for Hanke perustaja Add functionality to include Hanke perustaja in hanke kayttaja listing. Hanke perustaja is now mandatory information in Hanke creation. A hanke generated from a cable report is a special case. In these cases perustaja is generated from the ordered person in application contacts. --- .../haitaton/hanke/HankeControllerITests.kt | 11 -- .../haitaton/hanke/HankeRepositoryITests.kt | 2 + .../hel/haitaton/hanke/HankeServiceITests.kt | 100 +++++----- .../application/ApplicationServiceITest.kt | 27 ++- .../permissions/HankeKayttajaServiceITest.kt | 182 ++++++++++++------ .../permissions/PermissionServiceITest.kt | 53 ++--- .../expectedHankeWithPoints.json.mustache | 7 +- .../expectedHankeWithPolygon.json.mustache | 7 +- .../fi/hel/haitaton/hanke/HankeController.kt | 8 +- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 40 +++- .../fi/hel/haitaton/hanke/Persistence.kt | 2 +- .../fi/hel/haitaton/hanke/PerustajaEntity.kt | 2 +- .../hanke/application/ApplicationData.kt | 3 + .../fi/hel/haitaton/hanke/domain/Hanke.kt | 2 +- .../fi/hel/haitaton/hanke/domain/Perustaja.kt | 9 +- .../hanke/permissions/HankeKayttajaService.kt | 30 ++- .../hanke/permissions/KayttajaTunniste.kt | 4 +- .../hanke/permissions/PermissionService.kt | 4 +- .../hanke/validation/HankeValidator.kt | 60 +++--- ...3-change-hanke-perustaja-non-nullable.yaml | 21 ++ .../db/changelog/db.changelog-master.yaml | 2 + .../hel/haitaton/hanke/HankeControllerTest.kt | 127 ++++++------ .../application/ApplicationServiceTest.kt | 24 +-- .../haitaton/hanke/application/ContactTest.kt | 44 +++++ .../haitaton/hanke/factory/HankeFactory.kt | 48 +++-- 25 files changed, 498 insertions(+), 321 deletions(-) create mode 100644 services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt index 201e86ab2..dd0a53060 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt @@ -13,7 +13,6 @@ import fi.hel.haitaton.hanke.geometria.Geometriat import fi.hel.haitaton.hanke.logging.DisclosureLogService import fi.hel.haitaton.hanke.permissions.PermissionCode import fi.hel.haitaton.hanke.permissions.PermissionService -import fi.hel.haitaton.hanke.permissions.Role import io.mockk.checkUnnecessaryStub import io.mockk.clearAllMocks import io.mockk.confirmVerified @@ -322,7 +321,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll createdAt = getCurrentTimeUTC(), ) every { hankeService.createHanke(any()) }.returns(createdHanke) - justRun { permissionService.setPermission(hankeId, USERNAME, Role.KAIKKI_OIKEUDET) } post(url, hankeToBeCreated) .andExpect(status().isOk) @@ -332,7 +330,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll verifySequence { hankeService.createHanke(any()) - permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) disclosureLogService.saveDisclosureLogsForHanke(createdHanke, USERNAME) } } @@ -341,7 +338,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll fun `When perustaja is provided return provided information`() { val hanke = HankeFactory.create().withPerustaja() every { hankeService.createHanke(any()) } returns hanke - justRun { permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) } post(url, hanke) .andExpect(status().isOk) @@ -350,7 +346,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll verifySequence { hankeService.createHanke(any()) - permissionService.setPermission(any(), any(), any()) disclosureLogService.saveDisclosureLogsForHanke(any(), any()) } } @@ -360,13 +355,11 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll val hanke = HankeFactory.create().apply { generated = true } every { hankeService.createHanke(hanke.copy(id = null, generated = false)) } returns hanke.copy(generated = false) - justRun { permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) } post(url, hanke).andExpect(status().isOk) verifySequence { hankeService.createHanke(any()) - permissionService.setPermission(any(), any(), any()) disclosureLogService.saveDisclosureLogsForHanke(any(), any()) } } @@ -393,7 +386,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll id = 12 omistajat[0].id = 3 } - justRun { permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) } every { hankeService.createHanke(any()) }.returns(expectedHanke) val expectedContent = expectedHanke.toJsonString() @@ -405,7 +397,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll verifySequence { hankeService.createHanke(any()) - permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) disclosureLogService.saveDisclosureLogsForHanke(expectedHanke, USERNAME) } } @@ -457,7 +448,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll expectedDateLoppu.toJsonString().substringAfter('"').substringBeforeLast('"') // faking the service call every { hankeService.createHanke(any()) }.returns(expectedHanke) - justRun { permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) } // Call it and check results post(url, hankeToBeMocked) @@ -472,7 +462,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll verifySequence { hankeService.createHanke(any()) - permissionService.setPermission(any(), any(), Role.KAIKKI_OIKEUDET) disclosureLogService.saveDisclosureLogsForHanke(expectedHanke, USERNAME) } } diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt index 5baa4146c..4a1c77c0e 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt @@ -1,6 +1,7 @@ package fi.hel.haitaton.hanke import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YRITYS +import fi.hel.haitaton.hanke.factory.HankeFactory import java.time.LocalDateTime import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test @@ -168,6 +169,7 @@ internal class HankeRepositoryITests : DatabaseTest() { createdAt = null, modifiedByUserId = null, modifiedAt = null, + perustaja = HankeFactory.defaultPerustaja.toEntity(), ) /* Keeping just seconds so that database truncation does not affect testing. */ diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 66dd6d7ba..2213351da 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -41,6 +41,8 @@ import fi.hel.haitaton.hanke.logging.Status import fi.hel.haitaton.hanke.logging.UserRole import fi.hel.haitaton.hanke.permissions.HankeKayttajaRepository import fi.hel.haitaton.hanke.permissions.KayttajaTunnisteRepository +import fi.hel.haitaton.hanke.permissions.PermissionCode +import fi.hel.haitaton.hanke.permissions.PermissionService import fi.hel.haitaton.hanke.test.Asserts.isRecent import fi.hel.haitaton.hanke.test.TestUtils import fi.hel.haitaton.hanke.test.TestUtils.nextYear @@ -100,6 +102,7 @@ class HankeServiceITests : DatabaseTest() { @Autowired private lateinit var hankeKayttajaRepository: HankeKayttajaRepository @Autowired private lateinit var kayttajaTunnisteRepository: KayttajaTunnisteRepository @Autowired private lateinit var jdbcTemplate: JdbcTemplate + @Autowired private lateinit var permissionService: PermissionService @BeforeEach fun clearMocks() { @@ -125,6 +128,10 @@ class HankeServiceITests : DatabaseTest() { // Call create and get the return object: val returnedHanke = hankeService.createHanke(hanke) + // Verify privileges + PermissionCode.values().forEach { + assertThat(permissionService.hasPermission(returnedHanke.id!!, USER_NAME, it)).isTrue() + } // Check the return object in general: assertThat(returnedHanke).isNotNull assertThat(returnedHanke).isNotSameAs(hanke) @@ -203,8 +210,8 @@ class HankeServiceITests : DatabaseTest() { assertThat(rakennuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(rakennuttaja.id) - assertThat(hankeKayttajaRepository.findAll()).hasSize(4) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) + assertThat(hankeKayttajaRepository.findAll()).hasSize(5) + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(5) } @Test @@ -1376,11 +1383,10 @@ class HankeServiceITests : DatabaseTest() { ) val templateData = TemplateData( - updatedHanke.id!!, - updatedHanke.hankeTunnus!!, - updatedHanke.perustaja != null, - updatedHanke.alueet[0].id, - updatedHanke.alueet[0].geometriat?.id, + hankeId = updatedHanke.id!!, + hankeTunnus = updatedHanke.hankeTunnus!!, + alueId = updatedHanke.alueet[0].id, + geometriaId = updatedHanke.alueet[0].geometriat?.id, hankeVersion = 1, geometriaVersion = 1, tormaystarkasteluTulos = true, @@ -1445,7 +1451,8 @@ class HankeServiceITests : DatabaseTest() { } private fun initHankeWithHakemus(alluId: Int): HankeEntity { - val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI23-1")) + val hanke = + hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = "HAI23-1")) val application = applicationRepository.save( AlluDataFactory.createApplicationEntity( @@ -1459,36 +1466,33 @@ class HankeServiceITests : DatabaseTest() { } private fun HankeEntity.toDomainObject(): Hanke = - with(this) { - Hanke( - id, - hankeTunnus, - onYKTHanke, - nimi, - kuvaus, - vaihe, - suunnitteluVaihe, - version, - createdByUserId ?: "", - createdAt?.atZone(TZ_UTC), - modifiedByUserId, - modifiedAt?.atZone(TZ_UTC), - this.status - ) - } + Hanke( + id = id, + hankeTunnus = hankeTunnus, + onYKTHanke = onYKTHanke, + nimi = nimi, + kuvaus = kuvaus, + vaihe = vaihe, + suunnitteluVaihe = suunnitteluVaihe, + version = version, + createdBy = createdByUserId ?: "", + createdAt = createdAt?.atZone(TZ_UTC), + modifiedBy = modifiedByUserId, + modifiedAt = modifiedAt?.atZone(TZ_UTC), + status = status, + perustaja = HankeFactory.defaultPerustaja + ) private fun ApplicationEntity.toDomainObject(): Application = - with(this) { - Application( - id, - alluid, - alluStatus, - applicationIdentifier, - applicationType, - applicationData, - hanke.hankeTunnus ?: "" - ) - } + Application( + id = id, + alluid = alluid, + alluStatus = alluStatus, + applicationIdentifier = applicationIdentifier, + applicationType = applicationType, + applicationData = applicationData, + hankeTunnus = hanke.hankeTunnus ?: "" + ) private fun assertFeaturePropertiesIsReset(hanke: Hanke, propertiesWanted: Map) { assertThat(hanke.alueet).isNotEmpty @@ -1504,7 +1508,6 @@ class HankeServiceITests : DatabaseTest() { data class TemplateData( val hankeId: Int, val hankeTunnus: String, - val hankePerustaja: Boolean = false, val alueId: Int? = null, val geometriaId: Int? = null, val geometriaVersion: Int = 0, @@ -1532,18 +1535,17 @@ class HankeServiceITests : DatabaseTest() { ): String { val templateData = TemplateData( - hanke.id!!, - hanke.hankeTunnus!!, - hanke.perustaja != null, - alue?.id, - alue?.geometriat?.id, - geometriaVersion, - hankeVersion, - nextYear(), - tormaystarkasteluTulos, - alue?.nimi, - alkuPvm, - loppuPvm, + hankeId = hanke.id!!, + hankeTunnus = hanke.hankeTunnus!!, + alueId = alue?.id, + geometriaId = alue?.geometriat?.id, + geometriaVersion = geometriaVersion, + hankeVersion = hankeVersion, + nextYear = nextYear(), + tormaystarkasteluTulos = tormaystarkasteluTulos, + alueNimi = alue?.nimi, + alkuPvm = alkuPvm, + loppuPvm = loppuPvm, ) return Template.parse( "/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache".getResourceAsText() diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt index d430b3452..10bf7dd26 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt @@ -121,13 +121,10 @@ class ApplicationServiceITest : DatabaseTest() { confirmVerified(cableReportServiceAllu) } - fun createHanke(): Hanke { - return hankeService.createHanke(HankeFactory.create()) - } + fun createHanke(hanke: Hanke = HankeFactory.create()): Hanke = hankeService.createHanke(hanke) - fun createHankeEntity(): HankeEntity { - return hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) - } + fun createHankeEntity(id: Int? = null, hankeTunnus: String? = "HAI-1234"): HankeEntity = + hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = hankeTunnus)) @Test fun `create creates an audit log entry for created application`() { @@ -343,8 +340,8 @@ class ApplicationServiceITest : DatabaseTest() { fun `getAllApplicationsForUser returns applications for the correct user`() { assertThat(applicationRepository.findAll()).isEmpty() val otherUser = "otherUser" - val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) - val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235")) + val hanke = createHankeEntity(hankeTunnus = "HAI-1234") + val hanke2 = createHankeEntity(hankeTunnus = "HAI-1235") permissionService.setPermission(hanke.id!!, USERNAME, Role.HAKEMUSASIOINTI) permissionService.setPermission(hanke2.id!!, "otherUser", Role.HAKEMUSASIOINTI) @@ -373,9 +370,9 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `getAllApplicationsForUser returns applications for user hankkeet`() { assertThat(applicationRepository.findAll()).isEmpty() - val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) - val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235")) - val hanke3 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1236")) + val hanke = createHankeEntity(hankeTunnus = "HAI-1234") + val hanke2 = createHankeEntity(hankeTunnus = "HAI-1235") + val hanke3 = createHankeEntity(hankeTunnus = "HAI-1236") permissionService.setPermission(hanke.id!!, USERNAME, Role.HAKEMUSASIOINTI) permissionService.setPermission(hanke2.id!!, USERNAME, Role.HAKEMUSASIOINTI) val application1 = alluDataFactory.saveApplicationEntity(username = USERNAME, hanke = hanke) @@ -400,7 +397,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `getApplicationById returns correct application`() { - val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) + val hanke = createHankeEntity(hankeTunnus = "HAI-1234") val applications = alluDataFactory.saveApplicationEntities(3, USERNAME, hanke = hanke) val selectedId = applications[1].id!! assertThat(applicationRepository.findAll()).hasSize(3) @@ -510,7 +507,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `create throws exception when application area is outside hankealue`() { - val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) + val hanke = createHanke(HankeFactory.create().withHankealue()) val cableReportApplicationData = AlluDataFactory.createCableReportApplicationData(areas = listOf(havisAmanda)) val newApplication = @@ -881,7 +878,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `updateApplicationData throws exception when application area is outside hankealue`() { - val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) + val hanke = createHanke(HankeFactory.create().withHankealue()) val hankeEntity = hankeRepository.getReferenceById(hanke.id!!) val application = alluDataFactory.saveApplicationEntity(USERNAME, hanke = hankeEntity) { it.alluid = 21 } @@ -1142,7 +1139,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `Throws an exception when application area is outside hankealue`() { - val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) + val hanke = createHanke(HankeFactory.create().withHankealue()) val hankeEntity = hankeRepository.getReferenceById(hanke.id!!) val application = alluDataFactory.saveApplicationEntity(USERNAME, hanke = hankeEntity) { diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt index 40909fdb1..308206004 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt @@ -2,7 +2,6 @@ package fi.hel.haitaton.hanke.permissions import assertk.Assert import assertk.assertThat -import assertk.assertions.containsExactly import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.each import assertk.assertions.hasSize @@ -17,9 +16,10 @@ import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.factory.AlluDataFactory import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts import fi.hel.haitaton.hanke.factory.HankeFactory -import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankeYhteystietoFactory +import fi.hel.haitaton.hanke.permissions.Role.KAIKKI_OIKEUDET +import fi.hel.haitaton.hanke.permissions.Role.KATSELUOIKEUS import fi.hel.haitaton.hanke.test.Asserts.isRecent import java.time.OffsetDateTime import org.junit.jupiter.api.Test @@ -47,6 +47,8 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Autowired private lateinit var permissionRepository: PermissionRepository @Autowired private lateinit var roleRepository: RoleRepository + private val perustaja = HankeFactory.defaultPerustaja + @Test fun `getKayttajatByHankeId should return users from correct hanke only`() { val hankeToFind = hankeFactory.save(HankeFactory.create().withYhteystiedot()) @@ -55,12 +57,13 @@ class HankeKayttajaServiceITest : DatabaseTest() { val result: List = hankeKayttajaService.getKayttajatByHankeId(hankeToFind.id!!) - assertThat(result).hasSize(4) + val yhteystiedotPlusPerustaja = 4 + 1 + assertThat(result).hasSize(yhteystiedotPlusPerustaja) } @Test fun `getKayttajatByHankeId should return data matching to the saved entity`() { - val hanke = hankeFactory.save(HankeFactory.create().withGeneratedOmistaja(1)) + val hanke = hankeFactory.save(HankeFactory.create()) val result: List = hankeKayttajaService.getKayttajatByHankeId(hanke.id!!) @@ -72,7 +75,35 @@ class HankeKayttajaServiceITest : DatabaseTest() { assertThat(nimi).isEqualTo(entity.nimi) assertThat(sahkoposti).isEqualTo(entity.sahkoposti) assertThat(rooli).isEqualTo(entity.kayttajaTunniste!!.role) - assertThat(tunnistautunut).isEqualTo(false) + assertThat(tunnistautunut).isEqualTo(true) // hanke perustaja + } + } + + @Test + fun `createToken saves kayttaja and tunniste with correct permission and other data`() { + val hankeEntity = hankeFactory.saveEntity(HankeFactory.createNewEntity(id = null)) + val savedHankeId = hankeEntity.id!! + val savedPermission = savePermission(savedHankeId, USERNAME, KAIKKI_OIKEUDET) + + hankeKayttajaService.createToken(savedHankeId, perustaja.toUserContact(), savedPermission) + + val kayttajaEntity = + hankeKayttajaRepository.findAll().also { assertThat(it).hasSize(1) }.first() + val tunnisteEntity = + kayttajaTunnisteRepository.findAll().also { assertThat(it).hasSize(1) }.first() + with(kayttajaEntity) { + assertThat(id).isNotNull() + assertThat(hankeId).isEqualTo(savedHankeId) + assertThat(permission!!).isOfEqualDataTo(savedPermission) + assertThat(sahkoposti).isEqualTo(perustaja.email) + assertThat(nimi).isEqualTo(perustaja.nimi) + } + with(tunnisteEntity) { + assertThat(id).isNotNull() + assertThat(role).isEqualTo(KAIKKI_OIKEUDET) + assertThat(tunniste).matches(Regex(kayttajaTunnistePattern)) + assertThat(sentAt).isNull() + assertThat(createdAt).isRecent() } } @@ -89,11 +120,12 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) + val initialKayttajatSize = 1 // hanke perustaja hankeKayttajaService.saveNewTokensFromApplication(application, 1) - assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() - assertThat(hankeKayttajaRepository.findAll()).isEmpty() + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(initialKayttajatSize) + assertThat(hankeKayttajaRepository.findAll()).hasSize(initialKayttajatSize) } @Test @@ -122,10 +154,10 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - val tunnisteet = kayttajaTunnisteRepository.findAll() + val tunnisteet = nonPerustajaTunnisteet() assertThat(tunnisteet).hasSize(4) assertThat(tunnisteet).areValid() - val kayttajat = hankeKayttajaRepository.findAll() + val kayttajat = nonPerustajaKayttajat() assertThat(kayttajat).hasSize(4) assertThat(kayttajat).each { kayttaja -> kayttaja.transform { it.nimi }.isEqualTo("Teppo Testihenkilö") @@ -172,8 +204,8 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(2) - val kayttajat = hankeKayttajaRepository.findAll() + assertThat(nonPerustajaTunnisteet()).hasSize(2) + val kayttajat = nonPerustajaKayttajat() assertThat(kayttajat).hasSize(2) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -207,12 +239,12 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(2) + assertThat(nonPerustajaTunnisteet()).hasSize(2) hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) - val kayttajat = hankeKayttajaRepository.findAll() + assertThat(nonPerustajaTunnisteet()).hasSize(4) + val kayttajat = nonPerustajaKayttajat() assertThat(kayttajat).hasSize(4) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -248,16 +280,16 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() + assertThat(nonPerustajaTunnisteet()).isEmpty() hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - val tunnisteet = kayttajaTunnisteRepository.findAll() + val tunnisteet = nonPerustajaTunnisteet() assertThat(tunnisteet).hasSize(2) assertThat(tunnisteet).each { tunniste -> tunniste.transform { it.hankeKayttaja?.sahkoposti }.isIn("email2", "email3") } - val kayttajat = hankeKayttajaRepository.findAll() + val kayttajat = nonPerustajaKayttajat() assertThat(kayttajat).hasSize(4) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -270,10 +302,12 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Test fun `saveNewTokensFromHanke does nothing if hanke has no contacts`() { - hankeKayttajaService.saveNewTokensFromHanke(HankeFactory.create()) + val hanke = hankeFactory.save(HankeFactory.create()) + + hankeKayttajaService.saveNewTokensFromHanke(hanke) - assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() - assertThat(hankeKayttajaRepository.findAll()).isEmpty() + assertThat(nonPerustajaTunnisteet()).isEmpty() + assertThat(nonPerustajaKayttajat()).isEmpty() } @Test @@ -293,8 +327,8 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromHanke(hanke) - val tunnisteet: List = kayttajaTunnisteRepository.findAll() - val kayttajat: List = hankeKayttajaRepository.findAll() + val tunnisteet: List = nonPerustajaTunnisteet() + val kayttajat: List = nonPerustajaKayttajat() assertThat(tunnisteet).hasSize(4) // 4 yhteyshenkilo subcontacts. assertThat(kayttajat).hasSize(4) assertThat(tunnisteet).areValid() @@ -303,27 +337,43 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Test fun `saveNewTokensFromHanke with pre-existing permissions does not create duplicate`() { - val hanke = hankeFactory.save() - saveUserAndPermission(hanke.id!!, "Existing User One", "ali.kontakti@meili.com") + val hankeEntity = hankeFactory.saveEntity() + val hanke = + HankeFactory.create(id = hankeEntity.id, hankeTunnus = hankeEntity.hankeTunnus).apply { + omistajat.add(HankeYhteystietoFactory.create()) + } + saveUserAndToken(hankeEntity, "Existing User One", "ali.kontakti@meili.com") - hankeKayttajaService.saveNewTokensFromHanke( - hanke.apply { this.omistajat.add(HankeYhteystietoFactory.create()) } - ) + hankeKayttajaService.saveNewTokensFromHanke(hanke) - val tunnisteet = kayttajaTunnisteRepository.findAll() - assertThat(tunnisteet).isEmpty() - assertThat(tunnisteet).areValid() - val kayttajat = hankeKayttajaRepository.findAll() + val tunnisteet = nonPerustajaTunnisteet() + val kayttajat = nonPerustajaKayttajat() + assertThat(tunnisteet).hasSize(1) assertThat(kayttajat).hasSize(1) - assertThat(kayttajat.map { it.sahkoposti }) - .containsExactly( - "ali.kontakti@meili.com", - ) + val kayttaja = kayttajat.first() + assertThat(kayttaja.nimi).isEqualTo("Existing User One") + assertThat(kayttaja.sahkoposti).isEqualTo("ali.kontakti@meili.com") } + private val expectedNames = + arrayOf( + "yhteys-etu1 yhteys-suku1", + "yhteys-etu2 yhteys-suku2", + "yhteys-etu3 yhteys-suku3", + "yhteys-etu4 yhteys-suku4", + ) + + private val expectedEmails = + arrayOf( + "yhteys-email1", + "yhteys-email2", + "yhteys-email3", + "yhteys-email4", + ) + private fun Assert>.areValid() = each { t -> t.transform { it.id }.isNotNull() - t.transform { it.role }.isEqualTo(Role.KATSELUOIKEUS) + t.transform { it.role }.isEqualTo(KATSELUOIKEUS) t.transform { it.createdAt }.isRecent() t.transform { it.sentAt }.isNull() t.transform { it.tunniste }.matches(Regex(kayttajaTunnistePattern)) @@ -339,21 +389,33 @@ class HankeKayttajaServiceITest : DatabaseTest() { k.transform { it.kayttajaTunniste }.isNotNull() } - private val expectedNames = - arrayOf( - "yhteys-etu1 yhteys-suku1", - "yhteys-etu2 yhteys-suku2", - "yhteys-etu3 yhteys-suku3", - "yhteys-etu4 yhteys-suku4", - ) + private fun Assert.isOfEqualDataTo(other: PermissionEntity) = + given { actual -> + assertThat(actual.id).isEqualTo(other.id) + assertThat(actual.hankeId).isEqualTo(other.hankeId) + assertThat(actual.userId).isEqualTo(other.userId) + assertThat(actual.role).isOfEqualDataTo(other.role) + } - private val expectedEmails = - arrayOf( - "yhteys-email1", - "yhteys-email2", - "yhteys-email3", - "yhteys-email4", - ) + private fun Assert.isOfEqualDataTo(other: RoleEntity) = given { actual -> + assertThat(actual.id).isEqualTo(other.id) + assertThat(actual.role).isEqualTo(other.role) + assertThat(actual.permissionCode).isEqualTo(other.permissionCode) + } + + /** + * Creating a Hanke through HankeService adds HankeKayttajaEntity for perustaja. This is a + * helper function to filter out perustaja. + */ + private fun nonPerustajaKayttajat() = + hankeKayttajaRepository.findAll().filter { it.nimi != perustaja.nimi } + + /** + * Creating a Hanke through HankeService adds KayttajaTunnisteEntity for perustaja. This is a + * helper function to filter out perustaja. + */ + private fun nonPerustajaTunnisteet() = + kayttajaTunnisteRepository.findAll().filter { it.role != KAIKKI_OIKEUDET } private fun saveUserAndToken( hanke: HankeEntity, @@ -366,7 +428,7 @@ class HankeKayttajaServiceITest : DatabaseTest() { tunniste = "existing", createdAt = OffsetDateTime.parse("2023-03-31T15:41:21Z"), sentAt = null, - role = Role.KATSELUOIKEUS, + role = KATSELUOIKEUS, hankeKayttaja = null, ) ) @@ -386,14 +448,7 @@ class HankeKayttajaServiceITest : DatabaseTest() { nimi: String, sahkoposti: String ): HankeKayttajaEntity { - val permissionEntity = - permissionRepository.save( - PermissionEntity( - userId = "fake id", - hankeId = hankeId, - role = roleRepository.findOneByRole(Role.KATSELUOIKEUS), - ) - ) + val permissionEntity = savePermission(hankeId, "fake id", KATSELUOIKEUS) return hankeKayttajaRepository.save( HankeKayttajaEntity( @@ -405,4 +460,13 @@ class HankeKayttajaServiceITest : DatabaseTest() { ) ) } + + private fun savePermission(hankeId: Int, userId: String, role: Role) = + permissionRepository.save( + PermissionEntity( + userId = userId, + hankeId = hankeId, + role = roleRepository.findOneByRole(role) + ) + ) } diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt index f97d62b1e..233f85554 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt @@ -5,6 +5,7 @@ import assertk.assertions.hasSize import assertk.assertions.isFalse import assertk.assertions.isTrue import fi.hel.haitaton.hanke.DatabaseTest +import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.HankeService import fi.hel.haitaton.hanke.factory.HankeFactory import org.junit.jupiter.api.Assertions.assertEquals @@ -20,18 +21,20 @@ import org.springframework.security.test.context.support.WithMockUser import org.springframework.test.context.ActiveProfiles import org.testcontainers.junit.jupiter.Testcontainers +private const val HANKE_TUNNUS = HankeFactory.defaultHankeTunnus +private const val USERNAME = "user" + @Testcontainers @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @ActiveProfiles("default") @WithMockUser(username = "test7358") class PermissionServiceITest : DatabaseTest() { - val username = "user" - @Autowired lateinit var permissionService: PermissionService @Autowired lateinit var permissionRepository: PermissionRepository @Autowired lateinit var roleRepository: RoleRepository @Autowired lateinit var hankeService: HankeService + @Autowired lateinit var hankeRepository: HankeRepository companion object { @JvmStatic @@ -86,7 +89,7 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `getAllowedHankeIds without permissions returns empty list`() { - val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) assertEquals(listOf(), response) } @@ -94,20 +97,20 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `getAllowedHankeIds with permissions returns list of IDs`() { val kaikkiOikeudet = roleRepository.findOneByRole(Role.KAIKKI_OIKEUDET) - val hankkeet = saveSeveralHanke(3) + val hankkeet = saveSeveralHanke(listOf(HANKE_TUNNUS, "HAI23-1", "HAI23-2")) hankkeet .map { it.id!! } .forEach { permissionRepository.save( PermissionEntity( - userId = username, + userId = USERNAME, hankeId = it, role = kaikkiOikeudet, ) ) } - val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) assertEquals(hankkeet.map { it.id }, response) } @@ -118,56 +121,57 @@ class PermissionServiceITest : DatabaseTest() { val hankemuokkaus = roleRepository.findOneByRole(Role.HANKEMUOKKAUS) val hakemusasiointi = roleRepository.findOneByRole(Role.HAKEMUSASIOINTI) val katseluoikeus = roleRepository.findOneByRole(Role.KATSELUOIKEUS) - val hankkeet = saveSeveralHanke(4) + val hankkeet = saveSeveralHanke(listOf(HANKE_TUNNUS, "HAI23-1", "HAI23-2", "HAI23-3")) listOf(kaikkiOikeudet, hankemuokkaus, hakemusasiointi, katseluoikeus).zip(hankkeet) { role, hanke -> permissionRepository.save( PermissionEntity( - userId = username, + userId = USERNAME, hankeId = hanke.id!!, role = role, ) ) } - val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) assertEquals(listOf(hankkeet[0].id, hankkeet[1].id), response) } @Test fun `hasPermission returns false without permissions`() { - assertFalse(permissionService.hasPermission(2, username, PermissionCode.EDIT)) + assertFalse(permissionService.hasPermission(2, USERNAME, PermissionCode.EDIT)) } @Test fun `hasPermission with correct permission`() { val kaikkiOikeudet = roleRepository.findOneByRole(Role.KAIKKI_OIKEUDET) - val hankeId = saveSeveralHanke(1)[0].id!! + val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! + permissionRepository.save( - PermissionEntity(userId = username, hankeId = hankeId, role = kaikkiOikeudet) + PermissionEntity(userId = USERNAME, hankeId = hankeId, role = kaikkiOikeudet) ) - assertTrue(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT)) + assertTrue(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT)) } @Test fun `hasPermission with insufficient permissions`() { val hakemusasiointi = roleRepository.findOneByRole(Role.HAKEMUSASIOINTI) - val hankeId = saveSeveralHanke(1)[0].id!! + val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! permissionRepository.save( - PermissionEntity(userId = username, hankeId = hankeId, role = hakemusasiointi) + PermissionEntity(userId = USERNAME, hankeId = hankeId, role = hakemusasiointi) ) - assertFalse(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT)) + assertFalse(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT)) } @Test fun `setPermission creates a new permission`() { - val hankeId = saveSeveralHanke(1)[0].id!! + val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! - permissionService.setPermission(hankeId, username, Role.KATSELUOIKEUS) + permissionService.setPermission(hankeId, USERNAME, Role.KATSELUOIKEUS) val permissions = permissionRepository.findAll() assertThat(permissions).hasSize(1) @@ -177,13 +181,13 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `setPermission updates an existing permission`() { - val hankeId = saveSeveralHanke(1)[0].id!! + val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! val role = roleRepository.findOneByRole(Role.KATSELUOIKEUS) permissionRepository.save( - PermissionEntity(userId = username, hankeId = hankeId, role = role) + PermissionEntity(userId = USERNAME, hankeId = hankeId, role = role) ) - permissionService.setPermission(hankeId, username, Role.HAKEMUSASIOINTI) + permissionService.setPermission(hankeId, USERNAME, Role.HAKEMUSASIOINTI) val permissions = permissionRepository.findAll() assertThat(permissions).hasSize(1) @@ -191,8 +195,9 @@ class PermissionServiceITest : DatabaseTest() { assertEquals(Role.HAKEMUSASIOINTI, permissions[0].role.role) } - private fun saveSeveralHanke(n: Int) = - createSeveralHanke(n).map { hankeService.createHanke(it) } + private fun saveSeveralHanke(hankeTunnusList: List) = + createSeveralHanke(hankeTunnusList).map { hankeRepository.save(it) } - private fun createSeveralHanke(n: Int) = (1..n).map { HankeFactory.create(id = null) } + private fun createSeveralHanke(hankeTunnusList: List) = + hankeTunnusList.map { HankeFactory.createNewEntity(id = null, hankeTunnus = it) } } diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache index 631805893..5e27516a1 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache @@ -22,15 +22,10 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", - {{#hankkeenPerustaja}} "perustaja": { "nimi": "Pertti Perustaja", - "sahkoposti": "foo@bar.com" + "email": "foo@bar.com" }, - {{/hankkeenPerustaja}} - {{^hankkeenPerustaja}} - "perustaja": null, - {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache index 452f886fa..86ff7aa67 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache @@ -22,15 +22,10 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", - {{#hankkeenPerustaja}} "perustaja": { "nimi": "Pertti Perustaja", - "sahkoposti": "foo@bar.com" + "email": "foo@bar.com" }, - {{/hankkeenPerustaja}} - {{^hankkeenPerustaja}} - "perustaja": null, - {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt index 581859622..39df4430c 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt @@ -7,7 +7,6 @@ import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.logging.DisclosureLogService import fi.hel.haitaton.hanke.permissions.PermissionCode import fi.hel.haitaton.hanke.permissions.PermissionService -import fi.hel.haitaton.hanke.permissions.Role import fi.hel.haitaton.hanke.validation.ValidHanke import io.swagger.v3.oas.annotations.Hidden import io.swagger.v3.oas.annotations.Operation @@ -188,11 +187,6 @@ class HankeController( val createdHanke = hankeService.createHanke(sanitizedHanke) - permissionService.setPermission( - createdHanke.id!!, - currentUserId(), - Role.KAIKKI_OIKEUDET, - ) disclosureLogService.saveDisclosureLogsForHanke(createdHanke, userId) return createdHanke } @@ -319,7 +313,7 @@ class HankeController( if (hankeTunnusFromPath != updatedHanke.hankeTunnus) { throw HankeArgumentException("Hanketunnus not given or doesn't match the hanke data") } - if (perustaja != null && perustaja != updatedHanke.perustaja) { + if (perustaja != updatedHanke.perustaja) { throw HankeArgumentException("Updating perustaja not allowed.") } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index dd1dcca77..04a176b71 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -6,13 +6,14 @@ import fi.hel.haitaton.hanke.ContactType.RAKENNUTTAJA import fi.hel.haitaton.hanke.ContactType.TOTEUTTAJA import fi.hel.haitaton.hanke.application.Application import fi.hel.haitaton.hanke.application.ApplicationService +import fi.hel.haitaton.hanke.application.CableReportApplicationData import fi.hel.haitaton.hanke.application.CableReportWithoutHanke import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.HankeWithApplications import fi.hel.haitaton.hanke.domain.HankeYhteystieto import fi.hel.haitaton.hanke.domain.Hankealue import fi.hel.haitaton.hanke.domain.HasId -import fi.hel.haitaton.hanke.domain.toEntity +import fi.hel.haitaton.hanke.domain.Perustaja import fi.hel.haitaton.hanke.geometria.GeometriatService import fi.hel.haitaton.hanke.logging.AuditLogService import fi.hel.haitaton.hanke.logging.HankeLoggingService @@ -118,7 +119,7 @@ open class HankeServiceImpl( val userId = currentUserId() - val entity = HankeEntity() + val entity = HankeEntity(perustaja = hanke.perustaja.toEntity()) val loggingEntryHolder = prepareLogging(entity) // Create a new hanketunnus for it and save it: @@ -151,12 +152,20 @@ open class HankeServiceImpl( postProcessAndSaveLogging(loggingEntryHolder, savedHankeEntity, userId) - return createHankeDomainObjectFromEntity(entity).also { - hankeKayttajaService.saveNewTokensFromHanke(it) + return createHankeDomainObjectFromEntity(savedHankeEntity).also { + initAccessForCreatedHanke(it, userId) hankeLoggingService.logCreate(it, userId) } } + private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { + val hankeId = hanke.id!! + val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) + logger.info { "Saving token for Hanke perustaja." } + hankeKayttajaService.createToken(hankeId, hanke.perustaja.toUserContact(), permissionAll) + hankeKayttajaService.saveNewTokensFromHanke(hanke) + } + /** * Create application when no existing hanke. Autogenerates hanke and applies application to it. */ @@ -169,7 +178,6 @@ open class HankeServiceImpl( val tunnus = hanke.hankeTunnus ?: throw HankeArgumentException("Hanke must have tunnus") val createdApplication = applicationService.create(cableReport.toNewApplication(tunnus), userId) - permissionService.setPermission(hanke.id!!, userId, Role.KAIKKI_OIKEUDET) return HankeWithApplications(hanke, listOf(createdApplication)) } @@ -335,7 +343,7 @@ open class HankeServiceImpl( if (hankeEntity.modifiedAt != null) ZonedDateTime.of(hankeEntity.modifiedAt, TZ_UTC) else null, hankeEntity.status, - hankeEntity.perustaja?.toDomainObject(), + hankeEntity.perustaja.toDomainObject(), hankeEntity.generated, ) @@ -562,7 +570,6 @@ open class HankeServiceImpl( hanke.nimi?.let { entity.nimi = hanke.nimi } hanke.kuvaus?.let { entity.kuvaus = hanke.kuvaus } - hanke.perustaja?.let { entity.perustaja = it.toEntity() } entity.generated = hanke.generated hanke.vaihe?.let { entity.vaihe = hanke.vaihe } hanke.suunnitteluVaihe?.let { entity.suunnitteluVaihe = hanke.suunnitteluVaihe } @@ -927,7 +934,12 @@ open class HankeServiceImpl( loggingEntryHolder.saveLogEntries(auditLogService) } - /** Autogenerated hanke based on application data. Generated flag true. */ + /** + * Autogenerated hanke based on application data. + * - Generated flag true. + * - Hanke name is same as application name. + * - Perustaja generated from application data orderer. + */ private fun generateHankeFrom(cableReport: CableReportWithoutHanke): Hanke = createHanke( Hanke( @@ -943,9 +955,17 @@ open class HankeServiceImpl( createdAt = null, modifiedBy = null, modifiedAt = null, - HankeStatus.DRAFT, - perustaja = null, + status = HankeStatus.DRAFT, + perustaja = generatePerustajaFrom(cableReport.applicationData), generated = true, ) ) + + private fun generatePerustajaFrom(cableReport: CableReportApplicationData): Perustaja = + with(cableReport.findOrderer()) { + if (this == null || fullName().isNullOrBlank() || email.isNullOrBlank()) { + throw HankeArgumentException("Invalid orderer $this for Hanke perustaja") + } + Perustaja(fullName()!!, email) + } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt index fa962d5e2..4012dc31e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt @@ -136,7 +136,7 @@ class HankeEntity( var createdAt: LocalDateTime? = null, var modifiedByUserId: String? = null, var modifiedAt: LocalDateTime? = null, - @Embedded var perustaja: PerustajaEntity? = null, + @Embedded var perustaja: PerustajaEntity, var generated: Boolean = false, // NOTE: using IDENTITY (i.e. db does auto-increments, Hibernate reads the result back) // can be a performance problem if there is a need to do bulk inserts. diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt index daf45d301..d0d37825e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt @@ -6,7 +6,7 @@ import jakarta.persistence.Embeddable @Embeddable data class PerustajaEntity( - @Column(name = "perustajanimi") var nimi: String?, + @Column(name = "perustajanimi") var nimi: String, @Column(name = "perustajaemail") var email: String ) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/ApplicationData.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/ApplicationData.kt index 578eb9b71..db72b7506 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/ApplicationData.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/ApplicationData.kt @@ -76,6 +76,9 @@ data class CableReportApplicationData( propertyDeveloperWithContacts, representativeWithContacts ) + + fun findOrderer(): Contact? = + customersWithContacts().flatMap { it.contacts }.find { it.orderer } } class AlluDataException(path: String, error: AlluDataError) : diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt index b9b78a062..3b88fa470 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt @@ -80,7 +80,7 @@ data class Hanke( // @JsonView(ChangeLogView::class) @field:Schema(description = "Hanke founder contact information") - var perustaja: Perustaja? = null, + var perustaja: Perustaja, // @JsonView(ChangeLogView::class) @field:Schema(description = "Indicates whether the Hanke data is generated, set by the service") diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt index f1a47608c..aa3530e7d 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt @@ -1,12 +1,15 @@ package fi.hel.haitaton.hanke.domain import fi.hel.haitaton.hanke.PerustajaEntity +import fi.hel.haitaton.hanke.permissions.UserContact import io.swagger.v3.oas.annotations.media.Schema @Schema(description = "Founder information") data class Perustaja( - @field:Schema(description = "Name") val nimi: String?, + @field:Schema(description = "Name") val nimi: String, @field:Schema(description = "Email address") val email: String -) +) { + fun toEntity(): PerustajaEntity = PerustajaEntity(nimi, email) -fun Perustaja.toEntity(): PerustajaEntity = PerustajaEntity(nimi, email) + fun toUserContact(): UserContact = UserContact(nimi, email) +} diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index 3f4b5f4b6..5ac61d082 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -48,11 +48,16 @@ class HankeKayttajaService( filterNewContacts(hankeId, contacts).forEach { contact -> createToken(hankeId, contact) } } - private fun createToken(hankeId: Int, contact: UserContact) { + @Transactional + fun createToken( + hankeId: Int, + contact: UserContact, + permission: PermissionEntity? = null, + ) { logger.info { "Creating a new user token, hankeId=$hankeId" } - val token = KayttajaTunnisteEntity.create() - val kayttajaTunnisteEntity = kayttajaTunnisteRepository.save(token) - logger.info { "Saved the new user token, id=${kayttajaTunnisteEntity.id}" } + val token: KayttajaTunnisteEntity = tunnisteFrom(permission) + val savedTunniste: KayttajaTunnisteEntity = kayttajaTunnisteRepository.save(token) + logger.info { "Saved the new user token, id=${savedTunniste.id}" } val user = hankeKayttajaRepository.save( @@ -60,13 +65,26 @@ class HankeKayttajaService( hankeId = hankeId, nimi = contact.name, sahkoposti = contact.email, - permission = null, - kayttajaTunniste = kayttajaTunnisteEntity + permission = permission, + kayttajaTunniste = savedTunniste ) ) logger.info { "Saved the user information, id=${user.id}" } } + /** + * Creates [KayttajaTunnisteEntity] based on permission. Use cases: + * - Perustaja has an existing permission, and it is used, role is KAIKKI_OIKEUDET + * - Regular kayttaja does not yet have a permission, role is defaulted to KATSELUOIKEUS + */ + private fun tunnisteFrom(permission: PermissionEntity?): KayttajaTunnisteEntity { + return if (permission != null) { + KayttajaTunnisteEntity.create(role = permission.role.role) + } else { + KayttajaTunnisteEntity.create() + } + } + private fun userContactOrNull(name: String?, email: String?): UserContact? { return when { name.isNullOrBlank() || email.isNullOrBlank() -> null diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt index c9513d5da..2ffb43d32 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt @@ -31,12 +31,12 @@ class KayttajaTunnisteEntity( private val charPool: List = ('a'..'z') + ('A'..'Z') + ('0'..'9') private val secureRandom: SecureRandom = SecureRandom() - fun create() = + fun create(role: Role = Role.KATSELUOIKEUS) = KayttajaTunnisteEntity( tunniste = randomToken(), createdAt = getCurrentTimeUTC().toOffsetDateTime(), sentAt = null, - role = Role.KATSELUOIKEUS, + role = role, hankeKayttaja = null ) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/PermissionService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/PermissionService.kt index 677cef467..1ede300db 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/PermissionService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/PermissionService.kt @@ -19,14 +19,14 @@ class PermissionService( return hasPermission(role, permission) } - fun setPermission(hankeId: Int, userId: String, role: Role) { + fun setPermission(hankeId: Int, userId: String, role: Role): PermissionEntity { val roleEntity = roleRepository.findOneByRole(role) val entity = permissionRepository.findOneByHankeIdAndUserId(hankeId, userId)?.apply { this.role = roleEntity } ?: PermissionEntity(userId = userId, hankeId = hankeId, role = roleEntity) - permissionRepository.save(entity) + return permissionRepository.save(entity) } fun verifyHankeUserAuthorization(userId: String, hanke: Hanke, permissionCode: PermissionCode) { diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt index 9ae0cfc92..b6c0b7819 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt @@ -21,30 +21,22 @@ class HankeValidator : ConstraintValidator { var ok = true if (hanke.nimi.isNullOrBlank()) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) - .addPropertyNode("nimi") - .addConstraintViolation() + context.addViolation("nimi") ok = false } if (hanke.vaihe == null) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) - .addPropertyNode("vaihe") - .addConstraintViolation() + context.addViolation("vaihe") ok = false } else if (hanke.vaihe!! == Vaihe.SUUNNITTELU && hanke.suunnitteluVaihe == null) { // if vaihe = SUUNNITTELU then suunnitteluVaihe must have value - context - .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) - .addPropertyNode("suunnitteluVaihe") - .addConstraintViolation() + context.addViolation("suunnitteluVaihe") ok = false } ok = ok && checkHankealueet(hanke, context) ok = ok && checkTyomaaTiedot(hanke, context) + ok = ok && checkPerustaja(hanke, context) return ok } @@ -57,10 +49,7 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaAlkuPvm == null || hankealue.haittaAlkuPvm!!.isAfter(MAXIMUM_DATE) ) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) - .addPropertyNode("haittaAlkuPvm") - .addConstraintViolation() + context.addViolation("haittaAlkuPvm") return false } // Must be from the and earlier than some relevant maximum date, @@ -70,10 +59,7 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaLoppuPvm == null || hankealue.haittaLoppuPvm!!.isAfter(MAXIMUM_DATE) ) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) - .addPropertyNode("haittaLoppuPvm") - .addConstraintViolation() + context.addViolation("haittaLoppuPvm") return false } if ( @@ -81,10 +67,7 @@ class HankeValidator : ConstraintValidator { hankealue.haittaLoppuPvm != null && hankealue.haittaLoppuPvm!!.isBefore(hankealue.haittaAlkuPvm) ) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) - .addPropertyNode("haittaLoppuPvm") - .addConstraintViolation() + context.addViolation("haittaLoppuPvm") return false } } @@ -98,13 +81,34 @@ class HankeValidator : ConstraintValidator { hanke.tyomaaKatuosoite != null && hanke.tyomaaKatuosoite!!.length > MAXIMUM_TYOMAAKATUOSOITE_LENGTH ) { - context - .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) - .addPropertyNode("tyomaaKatuosoite") - .addConstraintViolation() + context.addViolation("tyomaaKatuosoite") ok = false } return ok } + + private fun checkPerustaja(hanke: Hanke, context: ConstraintValidatorContext): Boolean { + var ok = true + + with(hanke.perustaja) { + if (nimi.isBlank()) { + context.addViolation("perustaja.nimi") + ok = false + } + + if (email.isBlank()) { + context.addViolation("perustaja.email") + ok = false + } + } + + return ok + } + + private fun ConstraintValidatorContext.addViolation(propertyNode: String) { + buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + .addPropertyNode(propertyNode) + .addConstraintViolation() + } } diff --git a/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml b/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml new file mode 100644 index 000000000..4495fd917 --- /dev/null +++ b/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml @@ -0,0 +1,21 @@ +databaseChangeLog: + - changeSet: + id: 043-add-not-null-constraint-for-hanke-perustaja + author: Niko Pitkonen + changes: + - addDefaultValue: + tableName: hanke + columnName: perustajanimi + defaultValue: "" + - addDefaultValue: + tableName: hanke + columnName: perustajaemail + defaultValue: "" + - addNotNullConstraint: + tableName: hanke + columnName: perustajanimi + defaultNullValue: "" + - addNotNullConstraint: + tableName: hanke + columnName: perustajaemail + defaultNullValue: "" diff --git a/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml b/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml index 793fb0e47..241f38d9e 100644 --- a/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml @@ -113,3 +113,5 @@ databaseChangeLog: file: db/changelog/changesets/041-drop-column-organisaatioid-from-yhteystiedot.yml - include: file: db/changelog/changesets/042-add-indices-to-hanke-kayttaja.yml + - include: + file: db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt index 63f28020a..8c10c582a 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt @@ -9,7 +9,6 @@ import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.logging.DisclosureLogService import fi.hel.haitaton.hanke.permissions.PermissionCode import fi.hel.haitaton.hanke.permissions.PermissionService -import fi.hel.haitaton.hanke.permissions.Role import io.mockk.Called import io.mockk.clearAllMocks import io.mockk.mockk @@ -29,11 +28,11 @@ import org.springframework.security.test.context.support.WithMockUser import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.validation.beanvalidation.MethodValidationPostProcessor -private const val username = "testuser" +private const val USERNAME = "testuser" @ExtendWith(SpringExtension::class) @Import(HankeControllerTest.TestConfiguration::class) -@WithMockUser(username) +@WithMockUser(USERNAME) class HankeControllerTest { @Configuration @@ -78,24 +77,25 @@ class HankeControllerTest { fun `test that the getHankeByTunnus returns ok`() { val hankeId = 1234 - Mockito.`when`(permissionService.hasPermission(hankeId, username, PermissionCode.VIEW)) + Mockito.`when`(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.VIEW)) .thenReturn(true) Mockito.`when`(hankeService.loadHanke(mockedHankeTunnus)) .thenReturn( Hanke( - hankeId, - mockedHankeTunnus, - true, - "Mannerheimintien remontti remonttinen", - "Lorem ipsum dolor sit amet...", - Vaihe.OHJELMOINTI, - null, - 1, - "Risto", - getCurrentTimeUTC(), - null, - null, - HankeStatus.DRAFT + id = hankeId, + hankeTunnus = mockedHankeTunnus, + onYKTHanke = true, + nimi = "Mannerheimintien remontti remonttinen", + kuvaus = "Lorem ipsum dolor sit amet...", + vaihe = Vaihe.OHJELMOINTI, + suunnitteluVaihe = null, + version = 1, + createdBy = "Risto", + createdAt = getCurrentTimeUTC(), + modifiedBy = null, + modifiedAt = null, + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) ) @@ -103,7 +103,7 @@ class HankeControllerTest { assertThat(response).isNotNull assertThat(response.nimi).isNotEmpty - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } @Test @@ -111,37 +111,39 @@ class HankeControllerTest { val listOfHanke = listOf( Hanke( - 1234, - mockedHankeTunnus, - true, - "Mannerheimintien remontti remonttinen", - "Lorem ipsum dolor sit amet...", - Vaihe.OHJELMOINTI, - null, - 1, - "Risto", - getCurrentTimeUTC(), - null, - null, - HankeStatus.DRAFT + id = 1234, + hankeTunnus = mockedHankeTunnus, + onYKTHanke = true, + nimi = "Mannerheimintien remontti remonttinen", + kuvaus = "Lorem ipsum dolor sit amet...", + vaihe = Vaihe.OHJELMOINTI, + suunnitteluVaihe = null, + version = 1, + createdBy = "Risto", + createdAt = getCurrentTimeUTC(), + modifiedBy = null, + modifiedAt = null, + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ), Hanke( - 50, - "HAME50", - true, - "Hämeenlinnanväylän uudistus", - "Lorem ipsum dolor sit amet...", - Vaihe.SUUNNITTELU, - SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, - 1, - "Paavo", - getCurrentTimeUTC(), - null, - null, - HankeStatus.DRAFT + id = 50, + hankeTunnus = "HAME50", + onYKTHanke = true, + nimi = "Hämeenlinnanväylän uudistus", + kuvaus = "Lorem ipsum dolor sit amet...", + vaihe = Vaihe.SUUNNITTELU, + suunnitteluVaihe = SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, + version = 1, + createdBy = "Paavo", + createdAt = getCurrentTimeUTC(), + modifiedBy = null, + modifiedAt = null, + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) ) - Mockito.`when`(permissionService.getAllowedHankeIds(username, PermissionCode.VIEW)) + Mockito.`when`(permissionService.getAllowedHankeIds(USERNAME, PermissionCode.VIEW)) .thenReturn(listOf(1234, 50)) Mockito.`when`(hankeService.loadHankkeetByIds(listOf(1234, 50))).thenReturn(listOfHanke) @@ -151,7 +153,7 @@ class HankeControllerTest { assertThat(hankeList[1].nimi).isEqualTo("Hämeenlinnanväylän uudistus") assertThat(hankeList[0].alueidenGeometriat()).isEmpty() assertThat(hankeList[1].alueidenGeometriat()).isEmpty() - verify { disclosureLogService.saveDisclosureLogsForHankkeet(any(), eq(username)) } + verify { disclosureLogService.saveDisclosureLogsForHankkeet(any(), eq(USERNAME)) } } @Test @@ -170,15 +172,16 @@ class HankeControllerTest { createdAt = getCurrentTimeUTC(), modifiedBy = null, modifiedAt = null, - status = HankeStatus.DRAFT + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) // mock HankeService response Mockito.`when`(hankeService.updateHanke(partialHanke)) - .thenReturn(partialHanke.copy(modifiedBy = username, modifiedAt = getCurrentTimeUTC())) + .thenReturn(partialHanke.copy(modifiedBy = USERNAME, modifiedAt = getCurrentTimeUTC())) Mockito.`when`(hankeService.loadHanke("id123")) .thenReturn(HankeFactory.create(hankeTunnus = "id123")) - Mockito.`when`(permissionService.hasPermission(123, username, PermissionCode.EDIT)) + Mockito.`when`(permissionService.hasPermission(123, USERNAME, PermissionCode.EDIT)) .thenReturn(true) // Actual call @@ -186,7 +189,7 @@ class HankeControllerTest { assertThat(response).isNotNull assertThat(response.nimi).isEqualTo("hankkeen nimi") - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } @Test @@ -205,7 +208,8 @@ class HankeControllerTest { createdAt = null, modifiedBy = null, modifiedAt = null, - status = HankeStatus.DRAFT + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) Mockito.`when`(hankeService.loadHanke("id123")).thenReturn(HankeFactory.create()) @@ -219,7 +223,7 @@ class HankeControllerTest { verify { disclosureLogService wasNot Called } } - // sending of sub types + // sending of subtypes @Test fun `test that create with listOfOmistaja can be sent to controller and is responded with 200`() { val hanke = @@ -237,6 +241,7 @@ class HankeControllerTest { modifiedBy = null, modifiedAt = null, status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) hanke.omistajat = @@ -253,10 +258,10 @@ class HankeControllerTest { alikontaktit = listOf( Yhteyshenkilo( - "Ali", - "Kontakti", - "ali.kontakti@meili.com", - "050-3789354" + etunimi = "Ali", + sukunimi = "Kontakti", + email = "ali.kontakti@meili.com", + puhelinnumero = "050-3789354" ) ), ) @@ -282,7 +287,7 @@ class HankeControllerTest { assertThat(response.omistajat).hasSize(1) assertThat(response.omistajat[0].id).isEqualTo(1) assertThat(response.omistajat[0].nimi).isEqualTo("Pekka Pekkanen") - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } @Test @@ -302,6 +307,7 @@ class HankeControllerTest { modifiedBy = null, modifiedAt = null, status = null, + perustaja = HankeFactory.defaultPerustaja ) // mock HankeService response Mockito.`when`(hankeService.updateHanke(partialHanke)).thenReturn(partialHanke) @@ -330,19 +336,18 @@ class HankeControllerTest { createdAt = getCurrentTimeUTC(), modifiedBy = null, modifiedAt = null, - status = HankeStatus.DRAFT + status = HankeStatus.DRAFT, + perustaja = HankeFactory.defaultPerustaja, ) val mockedHanke = hanke.copy() mockedHanke.id = 12 mockedHanke.hankeTunnus = "JOKU12" - Mockito.`when`(hankeService.createHanke(hanke)).thenReturn(mockedHanke) val response: Hanke = hankeController.createHanke(hanke) assertThat(response).isNotNull - Mockito.verify(permissionService).setPermission(12, username, Role.KAIKKI_OIKEUDET) - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt index 1ac3b2731..e82bfdecc 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt @@ -9,7 +9,6 @@ import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isNotEmpty import assertk.assertions.isNotNull -import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.allu.AlluException import fi.hel.haitaton.hanke.allu.AlluLoginException @@ -25,6 +24,7 @@ import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withCustomer import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withHanke import fi.hel.haitaton.hanke.factory.ApplicationHistoryFactory +import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.geometria.GeometriatDao import fi.hel.haitaton.hanke.logging.ApplicationLoggingService import fi.hel.haitaton.hanke.logging.DisclosureLogService @@ -130,7 +130,7 @@ class ApplicationServiceTest { val application: ApplicationEntity = firstArg() application.copy(id = 1) } - val hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS) + val hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS) every { hankeRepository.findByHankeTunnus(HANKE_TUNNUS) } returns hanke every { geometriatDao.validateGeometriat(any()) } returns null every { geometriatDao.isInsideHankeAlueet(1, any()) } returns true @@ -176,7 +176,7 @@ class ApplicationServiceTest { @Test fun `updateApplicationData saves disclosure logs when updating Allu data`() { - val hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS) + val hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS) val applicationEntity = AlluDataFactory.createApplicationEntity( id = 3, @@ -223,7 +223,7 @@ class ApplicationServiceTest { alluid = 42, userId = USERNAME, applicationData = applicationData, - hanke = HankeEntity(hankeTunnus = HANKE_TUNNUS), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.validateGeometriat(any()) } returns @@ -256,7 +256,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { applicationRepo.save(any()) } answers { firstArg() } @@ -295,7 +295,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.calculateCombinedArea(any()) } returns 100f @@ -329,7 +329,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeEntity(hankeTunnus = HANKE_TUNNUS, id = 1), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) assertThat(applicationEntity.applicationData.areas).isNotNull().isNotEmpty() every { applicationRepo.findOneById(3) } returns applicationEntity @@ -363,7 +363,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData.copy(rockExcavation = rockExcavation), - hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { applicationRepo.save(any()) } answers { firstArg() } @@ -410,7 +410,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeFactory.createNewEntity(id = 1, HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.isInsideHankeAlueet(1, any()) } returns true @@ -463,7 +463,6 @@ class ApplicationServiceTest { inner class HandleApplicationUpdates { private val alluid = 42 private val applicationId = 13L - private val hankeTunnus = "HAI23-1" private val receiver = AlluDataFactory.teppoEmail private val updateTime = OffsetDateTime.parse("2022-10-09T06:36:51Z") private val identifier = ApplicationHistoryFactory.defaultApplicationIdentifier @@ -553,7 +552,8 @@ class ApplicationServiceTest { @Test fun `logs error if hanketunnus is null`(output: CapturedOutput) { every { applicationRepo.getOneByAlluid(42) } returns - applicationEntity().withHanke(HankeEntity(id = 1, hankeTunnus = null)) + applicationEntity() + .withHanke(HankeFactory.createNewEntity(id = 1, hankeTunnus = null)) every { applicationRepo.save(any()) } answers { firstArg() } every { statusRepo.getReferenceById(1) } returns AlluStatus(1, updateTime) every { statusRepo.save(any()) } answers { firstArg() } @@ -604,7 +604,7 @@ class ApplicationServiceTest { alluid = alluid, applicationIdentifier = identifier, userId = "user", - hanke = HankeEntity(id = 1, hankeTunnus = hankeTunnus), + hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) .withCustomer(AlluDataFactory.createCompanyCustomerWithOrderer()) diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt index cd1c3dbce..19d5aaa16 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt @@ -1,8 +1,12 @@ package fi.hel.haitaton.hanke.application import assertk.assertThat +import assertk.assertions.hasSize import assertk.assertions.isEqualTo import assertk.assertions.isNull +import fi.hel.haitaton.hanke.factory.AlluDataFactory +import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.createContact +import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.CsvSource @@ -48,4 +52,44 @@ class ContactTest { Contact(firstName = null, lastName = null, email = DUMMY_EMAIL, phone = DUMMY_PHONE) assertThat(contact.fullName()).isNull() } + + @Test + fun `findOrderer when orderer contact exists should return it`() { + val applicationData = + AlluDataFactory.createCableReportApplicationData( + representativeWithContacts = + AlluDataFactory.createCompanyCustomer().withContacts(createContact()), + propertyDeveloperWithContacts = + AlluDataFactory.createCompanyCustomer().withContacts(createContact()), + ) + + val result = applicationData.findOrderer() + + val allContacts = applicationData.customersWithContacts().flatMap { it.contacts } + assertThat(allContacts).hasSize(4) + val expectedResult = + Contact( + firstName = "Teppo", + lastName = "Testihenkilö", + email = "teppo@example.test", + phone = "04012345678", + orderer = true, + ) + assertThat(result).isEqualTo(expectedResult) + } + + @Test + fun `findOrderer when no orderer contact exists should return null`() { + val applicationData = + AlluDataFactory.createCableReportApplicationData( + customerWithContacts = + AlluDataFactory.createCompanyCustomer().withContacts(createContact()) + ) + + val result = applicationData.findOrderer() + + val allContacts = applicationData.customersWithContacts().flatMap { it.contacts } + assertThat(allContacts).hasSize(2) + assertThat(result).isNull() + } } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt index 788499894..c9e4686b2 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt @@ -4,6 +4,7 @@ import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.HankeService import fi.hel.haitaton.hanke.HankeStatus +import fi.hel.haitaton.hanke.PerustajaEntity import fi.hel.haitaton.hanke.SuunnitteluVaihe import fi.hel.haitaton.hanke.TyomaaTyyppi import fi.hel.haitaton.hanke.Vaihe @@ -39,6 +40,8 @@ class HankeFactory( ) ) + fun save(hanke: Hanke) = hankeService.createHanke(hanke) + /** * Save a new hanke using HankeService. Then get it as an entity. * @@ -54,7 +57,7 @@ class HankeFactory( return hankeRepository.getReferenceById(hanke.id!!) } - fun save(hanke: Hanke) = hankeService.createHanke(hanke) + fun saveEntity(entity: HankeEntity) = hankeRepository.save(entity) companion object { @@ -62,6 +65,7 @@ class HankeFactory( const val defaultNimi = "Hämeentien perusparannus ja katuvalot" const val defaultId = 123 const val defaultUser = "Risto" + val defaultPerustaja = Perustaja("Pertti Perustaja", "foo@bar.com") /** * Create a simple Hanke with test values. The default values can be overridden with named @@ -82,29 +86,39 @@ class HankeFactory( createdBy: String? = defaultUser, createdAt: ZonedDateTime? = DateFactory.getStartDatetime(), hankeStatus: HankeStatus = HankeStatus.DRAFT, + perustaja: Perustaja = defaultPerustaja, ): Hanke = Hanke( - id, - hankeTunnus, - true, - nimi, - "lorem ipsum dolor sit amet...", - vaihe, - suunnitteluVaihe, - version, - createdBy, - createdAt, - null, - null, - hankeStatus, + id = id, + hankeTunnus = hankeTunnus, + onYKTHanke = true, + nimi = nimi, + kuvaus = "lorem ipsum dolor sit amet...", + vaihe = vaihe, + suunnitteluVaihe = suunnitteluVaihe, + version = version, + createdBy = createdBy, + createdAt = createdAt, + modifiedBy = null, + modifiedAt = null, + status = hankeStatus, + perustaja = perustaja, ) + /** Create minimal Entity with identifier fields and mandatory fields. */ + fun createNewEntity( + id: Int? = defaultId, + hankeTunnus: String? = defaultHankeTunnus, + perustaja: PerustajaEntity = defaultPerustaja.toEntity() + ) = HankeEntity(id = id, hankeTunnus = hankeTunnus, perustaja = perustaja) + /** * Add a hankealue with haitat to a test Hanke. * * Example: * ``` * HankeFactory.create().withHankealue() + * * ``` */ fun Hanke.withHankealue( @@ -176,9 +190,9 @@ class HankeFactory( return this } - fun Hanke.withPerustaja( - newPerustaja: Perustaja? = Perustaja("Pertti Perustaja", "foo@bar.com") - ): Hanke = apply { perustaja = newPerustaja } + fun Hanke.withPerustaja(newPerustaja: Perustaja = defaultPerustaja): Hanke = apply { + perustaja = newPerustaja + } /** * Add a number of omistaja to a hanke. Generates the yhteystiedot with From 015f7defdafbd3b093197478eebba9d2eacd4863 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Mon, 21 Aug 2023 14:16:36 +0300 Subject: [PATCH 02/13] HAI-1844 Don't add kayttajatunniste for hanke founder --- .../hel/haitaton/hanke/HankeServiceITests.kt | 2 +- .../permissions/HankeKayttajaServiceITest.kt | 23 +++---- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 3 +- .../hanke/permissions/HankeKayttajaService.kt | 68 ++++++++++--------- 4 files changed, 46 insertions(+), 50 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 2213351da..7c8602e62 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -211,7 +211,7 @@ class HankeServiceITests : DatabaseTest() { assertThat(toteuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(rakennuttaja.id) assertThat(hankeKayttajaRepository.findAll()).hasSize(5) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(5) + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) // Hanke perustaja not included } @Test diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt index 308206004..227c2ca87 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt @@ -74,23 +74,21 @@ class HankeKayttajaServiceITest : DatabaseTest() { assertThat(id).isEqualTo(entity.id) assertThat(nimi).isEqualTo(entity.nimi) assertThat(sahkoposti).isEqualTo(entity.sahkoposti) - assertThat(rooli).isEqualTo(entity.kayttajaTunniste!!.role) + assertThat(rooli).isEqualTo(entity.permission!!.role.role) assertThat(tunnistautunut).isEqualTo(true) // hanke perustaja } } @Test - fun `createToken saves kayttaja and tunniste with correct permission and other data`() { + fun `addHankeFounder saves kayttaja with correct permission and other data`() { val hankeEntity = hankeFactory.saveEntity(HankeFactory.createNewEntity(id = null)) val savedHankeId = hankeEntity.id!! val savedPermission = savePermission(savedHankeId, USERNAME, KAIKKI_OIKEUDET) - hankeKayttajaService.createToken(savedHankeId, perustaja.toUserContact(), savedPermission) + hankeKayttajaService.addHankeFounder(savedHankeId, perustaja, savedPermission) val kayttajaEntity = hankeKayttajaRepository.findAll().also { assertThat(it).hasSize(1) }.first() - val tunnisteEntity = - kayttajaTunnisteRepository.findAll().also { assertThat(it).hasSize(1) }.first() with(kayttajaEntity) { assertThat(id).isNotNull() assertThat(hankeId).isEqualTo(savedHankeId) @@ -98,13 +96,6 @@ class HankeKayttajaServiceITest : DatabaseTest() { assertThat(sahkoposti).isEqualTo(perustaja.email) assertThat(nimi).isEqualTo(perustaja.nimi) } - with(tunnisteEntity) { - assertThat(id).isNotNull() - assertThat(role).isEqualTo(KAIKKI_OIKEUDET) - assertThat(tunniste).matches(Regex(kayttajaTunnistePattern)) - assertThat(sentAt).isNull() - assertThat(createdAt).isRecent() - } } @Test @@ -120,12 +111,14 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - val initialKayttajatSize = 1 // hanke perustaja + val initialUserSize = 1 // hanke perustaja hankeKayttajaService.saveNewTokensFromApplication(application, 1) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(initialKayttajatSize) - assertThat(hankeKayttajaRepository.findAll()).hasSize(initialKayttajatSize) + val invitationTokens = kayttajaTunnisteRepository.findAll() + val hankeUsers = hankeKayttajaRepository.findAll() + assertThat(invitationTokens).isEmpty() // Hanke perustaja not included + assertThat(hankeUsers).hasSize(initialUserSize) } @Test diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index 04a176b71..cc959fe8e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -161,8 +161,7 @@ open class HankeServiceImpl( private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { val hankeId = hanke.id!! val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - logger.info { "Saving token for Hanke perustaja." } - hankeKayttajaService.createToken(hankeId, hanke.perustaja.toUserContact(), permissionAll) + hankeKayttajaService.addHankeFounder(hankeId, hanke.perustaja, permissionAll) hankeKayttajaService.saveNewTokensFromHanke(hanke) } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index 5ac61d082..f5d14b39e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -3,6 +3,7 @@ package fi.hel.haitaton.hanke.permissions import fi.hel.haitaton.hanke.HankeArgumentException import fi.hel.haitaton.hanke.application.ApplicationEntity import fi.hel.haitaton.hanke.domain.Hanke +import fi.hel.haitaton.hanke.domain.Perustaja import mu.KotlinLogging import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional @@ -30,7 +31,9 @@ class HankeKayttajaService( .flatMap { it.contacts } .mapNotNull { userContactOrNull(it.fullName(), it.email) } - filterNewContacts(hankeId, contacts).forEach { contact -> createToken(hankeId, contact) } + filterNewContacts(hankeId, contacts).forEach { contact -> + createInvitationToken(hankeId, contact) + } } @Transactional @@ -45,44 +48,45 @@ class HankeKayttajaService( .flatMap { it.alikontaktit } .mapNotNull { userContactOrNull(it.fullName(), it.email) } - filterNewContacts(hankeId, contacts).forEach { contact -> createToken(hankeId, contact) } + filterNewContacts(hankeId, contacts).forEach { contact -> + createInvitationToken(hankeId, contact) + } } @Transactional - fun createToken( - hankeId: Int, - contact: UserContact, - permission: PermissionEntity? = null, - ) { + fun addHankeFounder(hankeId: Int, founder: Perustaja, permissionEntity: PermissionEntity) { + logger.info { "Saving token for Hanke perustaja." } + saveUser( + HankeKayttajaEntity( + hankeId = hankeId, + nimi = founder.nimi, + sahkoposti = founder.email, + permission = permissionEntity, + kayttajaTunniste = null, + ) + ) + } + + private fun createInvitationToken(hankeId: Int, contact: UserContact) { logger.info { "Creating a new user token, hankeId=$hankeId" } - val token: KayttajaTunnisteEntity = tunnisteFrom(permission) - val savedTunniste: KayttajaTunnisteEntity = kayttajaTunnisteRepository.save(token) - logger.info { "Saved the new user token, id=${savedTunniste.id}" } - - val user = - hankeKayttajaRepository.save( - HankeKayttajaEntity( - hankeId = hankeId, - nimi = contact.name, - sahkoposti = contact.email, - permission = permission, - kayttajaTunniste = savedTunniste - ) + val token = KayttajaTunnisteEntity.create() + val kayttajaTunnisteEntity = kayttajaTunnisteRepository.save(token) + logger.info { "Saved the new user token, id=${kayttajaTunnisteEntity.id}" } + + saveUser( + HankeKayttajaEntity( + hankeId = hankeId, + nimi = contact.name, + sahkoposti = contact.email, + permission = null, + kayttajaTunniste = kayttajaTunnisteEntity ) - logger.info { "Saved the user information, id=${user.id}" } + ) } - /** - * Creates [KayttajaTunnisteEntity] based on permission. Use cases: - * - Perustaja has an existing permission, and it is used, role is KAIKKI_OIKEUDET - * - Regular kayttaja does not yet have a permission, role is defaulted to KATSELUOIKEUS - */ - private fun tunnisteFrom(permission: PermissionEntity?): KayttajaTunnisteEntity { - return if (permission != null) { - KayttajaTunnisteEntity.create(role = permission.role.role) - } else { - KayttajaTunnisteEntity.create() - } + private fun saveUser(hankeKayttajaEntity: HankeKayttajaEntity) { + val user = hankeKayttajaRepository.save(hankeKayttajaEntity) + logger.info { "Saved the user information, id=${user.id}" } } private fun userContactOrNull(name: String?, email: String?): UserContact? { From 4e9252b5052e23249a8e86d357801ea3b5cf90cb Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Mon, 21 Aug 2023 16:12:44 +0300 Subject: [PATCH 03/13] HAI-1844 Change hanke perustaja back to nullable, use validator to validate create request --- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 35 +++--- .../fi/hel/haitaton/hanke/Persistence.kt | 2 +- .../fi/hel/haitaton/hanke/PerustajaEntity.kt | 8 +- .../hanke/application/CustomerWithContacts.kt | 15 +++ .../fi/hel/haitaton/hanke/domain/Hanke.kt | 4 +- .../fi/hel/haitaton/hanke/domain/Perustaja.kt | 5 +- .../hanke/permissions/HankeKayttajaService.kt | 2 +- .../hanke/validation/HankeValidator.kt | 8 +- ...3-change-hanke-perustaja-non-nullable.yaml | 21 ---- .../db/changelog/db.changelog-master.yaml | 2 - .../hel/haitaton/hanke/HankeControllerTest.kt | 117 +++++++++++------- .../haitaton/hanke/application/ContactTest.kt | 30 +++++ 12 files changed, 154 insertions(+), 95 deletions(-) delete mode 100644 services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index cc959fe8e..3f307c802 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -119,7 +119,7 @@ open class HankeServiceImpl( val userId = currentUserId() - val entity = HankeEntity(perustaja = hanke.perustaja.toEntity()) + val entity = HankeEntity() val loggingEntryHolder = prepareLogging(entity) // Create a new hanketunnus for it and save it: @@ -158,13 +158,6 @@ open class HankeServiceImpl( } } - private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { - val hankeId = hanke.id!! - val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - hankeKayttajaService.addHankeFounder(hankeId, hanke.perustaja, permissionAll) - hankeKayttajaService.saveNewTokensFromHanke(hanke) - } - /** * Create application when no existing hanke. Autogenerates hanke and applies application to it. */ @@ -242,6 +235,15 @@ open class HankeServiceImpl( hankeLoggingService.logDelete(hanke, userId) } + private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { + val hankeId = hanke.id!! + val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) + val perustaja = + hanke.perustaja ?: throw HankeArgumentException("Missing perustaja information") + hankeKayttajaService.addHankeFounder(hankeId, perustaja, permissionAll) + hankeKayttajaService.saveNewTokensFromHanke(hanke) + } + private fun anyHakemusProcessingInAllu(hakemukset: List): Boolean = hakemukset.any { logger.info { "Hakemus ${it.id} has alluStatus ${it.alluStatus}" } @@ -342,7 +344,7 @@ open class HankeServiceImpl( if (hankeEntity.modifiedAt != null) ZonedDateTime.of(hankeEntity.modifiedAt, TZ_UTC) else null, hankeEntity.status, - hankeEntity.perustaja.toDomainObject(), + hankeEntity.perustaja?.toDomainObject(), hankeEntity.generated, ) @@ -569,6 +571,7 @@ open class HankeServiceImpl( hanke.nimi?.let { entity.nimi = hanke.nimi } hanke.kuvaus?.let { entity.kuvaus = hanke.kuvaus } + hanke.perustaja?.let { entity.perustaja = it.toEntity() } entity.generated = hanke.generated hanke.vaihe?.let { entity.vaihe = hanke.vaihe } hanke.suunnitteluVaihe?.let { entity.suunnitteluVaihe = hanke.suunnitteluVaihe } @@ -960,11 +963,11 @@ open class HankeServiceImpl( ) ) - private fun generatePerustajaFrom(cableReport: CableReportApplicationData): Perustaja = - with(cableReport.findOrderer()) { - if (this == null || fullName().isNullOrBlank() || email.isNullOrBlank()) { - throw HankeArgumentException("Invalid orderer $this for Hanke perustaja") - } - Perustaja(fullName()!!, email) - } + private fun generatePerustajaFrom(cableReport: CableReportApplicationData): Perustaja { + val orderer = + cableReport.findOrderer() + ?: throw HankeArgumentException("Orderer not found for Hanke perustaja") + + return orderer.toHankePerustaja() + } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt index 4012dc31e..fa962d5e2 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/Persistence.kt @@ -136,7 +136,7 @@ class HankeEntity( var createdAt: LocalDateTime? = null, var modifiedByUserId: String? = null, var modifiedAt: LocalDateTime? = null, - @Embedded var perustaja: PerustajaEntity, + @Embedded var perustaja: PerustajaEntity? = null, var generated: Boolean = false, // NOTE: using IDENTITY (i.e. db does auto-increments, Hibernate reads the result back) // can be a performance problem if there is a need to do bulk inserts. diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt index d0d37825e..fe41bf274 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PerustajaEntity.kt @@ -6,8 +6,8 @@ import jakarta.persistence.Embeddable @Embeddable data class PerustajaEntity( - @Column(name = "perustajanimi") var nimi: String, + @Column(name = "perustajanimi") var nimi: String?, @Column(name = "perustajaemail") var email: String -) - -fun PerustajaEntity.toDomainObject(): Perustaja = Perustaja(nimi, email) +) { + fun toDomainObject(): Perustaja = Perustaja(nimi, email) +} diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/CustomerWithContacts.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/CustomerWithContacts.kt index 1e250815a..98ebf6a43 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/CustomerWithContacts.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/application/CustomerWithContacts.kt @@ -4,6 +4,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonIgnoreProperties import com.fasterxml.jackson.annotation.JsonView import fi.hel.haitaton.hanke.ChangeLogView +import fi.hel.haitaton.hanke.HankeArgumentException import fi.hel.haitaton.hanke.allu.Contact as AlluContact import fi.hel.haitaton.hanke.allu.Customer as AlluCustomer import fi.hel.haitaton.hanke.allu.CustomerType @@ -11,6 +12,7 @@ import fi.hel.haitaton.hanke.allu.CustomerWithContacts as AlluCustomerWithContac import fi.hel.haitaton.hanke.allu.PostalAddress as AlluPostalAddress import fi.hel.haitaton.hanke.allu.StreetAddress as AlluStreetAddress import fi.hel.haitaton.hanke.domain.BusinessId +import fi.hel.haitaton.hanke.domain.Perustaja @JsonIgnoreProperties(ignoreUnknown = true) data class CustomerWithContacts(val customer: Customer, val contacts: List) { @@ -44,6 +46,19 @@ data class Contact( } return names.filter { !it.isNullOrBlank() }.joinToString(" ") } + + /** + * It is possible to create a cable report without an existing Hanke. In these cases an + * application contact (orderer) is used as Hanke perustaja. + */ + fun toHankePerustaja(): Perustaja { + val name = fullName() + if (name.isNullOrBlank() || email.isNullOrBlank()) { + throw HankeArgumentException("Invalid orderer $this for Hanke perustaja") + } + + return Perustaja(name, email) + } } @JsonIgnoreProperties(ignoreUnknown = true) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt index 3b88fa470..db6da98e3 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt @@ -79,8 +79,8 @@ data class Hanke( var status: HankeStatus? = HankeStatus.DRAFT, // @JsonView(ChangeLogView::class) - @field:Schema(description = "Hanke founder contact information") - var perustaja: Perustaja, + @field:Schema(description = "Hanke founder contact information", required = true) + var perustaja: Perustaja? = null, // @JsonView(ChangeLogView::class) @field:Schema(description = "Indicates whether the Hanke data is generated, set by the service") diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt index aa3530e7d..a2c73ef4b 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Perustaja.kt @@ -1,15 +1,12 @@ package fi.hel.haitaton.hanke.domain import fi.hel.haitaton.hanke.PerustajaEntity -import fi.hel.haitaton.hanke.permissions.UserContact import io.swagger.v3.oas.annotations.media.Schema @Schema(description = "Founder information") data class Perustaja( - @field:Schema(description = "Name") val nimi: String, + @field:Schema(description = "Name") val nimi: String?, @field:Schema(description = "Email address") val email: String ) { fun toEntity(): PerustajaEntity = PerustajaEntity(nimi, email) - - fun toUserContact(): UserContact = UserContact(nimi, email) } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index f5d14b39e..81c9be923 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -59,7 +59,7 @@ class HankeKayttajaService( saveUser( HankeKayttajaEntity( hankeId = hankeId, - nimi = founder.nimi, + nimi = founder.nimi!!, sahkoposti = founder.email, permission = permissionEntity, kayttajaTunniste = null, diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt index b6c0b7819..a41b1667e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt @@ -92,7 +92,13 @@ class HankeValidator : ConstraintValidator { var ok = true with(hanke.perustaja) { - if (nimi.isBlank()) { + if (this == null) { + context.addViolation("perustaja") + ok = false + return false + } + + if (nimi.isNullOrBlank()) { context.addViolation("perustaja.nimi") ok = false } diff --git a/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml b/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml deleted file mode 100644 index 4495fd917..000000000 --- a/services/hanke-service/src/main/resources/db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml +++ /dev/null @@ -1,21 +0,0 @@ -databaseChangeLog: - - changeSet: - id: 043-add-not-null-constraint-for-hanke-perustaja - author: Niko Pitkonen - changes: - - addDefaultValue: - tableName: hanke - columnName: perustajanimi - defaultValue: "" - - addDefaultValue: - tableName: hanke - columnName: perustajaemail - defaultValue: "" - - addNotNullConstraint: - tableName: hanke - columnName: perustajanimi - defaultNullValue: "" - - addNotNullConstraint: - tableName: hanke - columnName: perustajaemail - defaultNullValue: "" diff --git a/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml b/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml index 241f38d9e..793fb0e47 100644 --- a/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/services/hanke-service/src/main/resources/db/changelog/db.changelog-master.yaml @@ -113,5 +113,3 @@ databaseChangeLog: file: db/changelog/changesets/041-drop-column-organisaatioid-from-yhteystiedot.yml - include: file: db/changelog/changesets/042-add-indices-to-hanke-kayttaja.yml - - include: - file: db/changelog/changesets/043-change-hanke-perustaja-non-nullable.yaml diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt index 8c10c582a..54bfa9b46 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt @@ -4,6 +4,7 @@ import fi.hel.haitaton.hanke.configuration.Feature import fi.hel.haitaton.hanke.configuration.FeatureFlags import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.HankeYhteystieto +import fi.hel.haitaton.hanke.domain.Perustaja import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YKSITYISHENKILO import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.logging.DisclosureLogService @@ -14,11 +15,15 @@ import io.mockk.clearAllMocks import io.mockk.mockk import io.mockk.verify import jakarta.validation.ConstraintViolationException +import java.util.stream.Stream import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatExceptionOfType import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource import org.mockito.Mockito import org.springframework.beans.factory.annotation.Autowired import org.springframework.context.annotation.Bean @@ -29,6 +34,9 @@ import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.validation.beanvalidation.MethodValidationPostProcessor private const val USERNAME = "testuser" +private const val HANKE_NAME_MANNERHEIMINTIE = "Mannerheimintien remontti remonttinen" +private const val HANKE_NAME_HAMEENLINNANVAYLA = "Hämeenlinnanväylän uudistus" +private const val HANKE_MOCK_KUVAUS = "Lorem ipsum dolor sit amet..." @ExtendWith(SpringExtension::class) @Import(HankeControllerTest.TestConfiguration::class) @@ -85,8 +93,8 @@ class HankeControllerTest { id = hankeId, hankeTunnus = mockedHankeTunnus, onYKTHanke = true, - nimi = "Mannerheimintien remontti remonttinen", - kuvaus = "Lorem ipsum dolor sit amet...", + nimi = HANKE_NAME_MANNERHEIMINTIE, + kuvaus = HANKE_MOCK_KUVAUS, vaihe = Vaihe.OHJELMOINTI, suunnitteluVaihe = null, version = 1, @@ -106,6 +114,60 @@ class HankeControllerTest { verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } + @ParameterizedTest + @MethodSource("invalidPerustajaArguments") + fun `test that the createHanke will give validation errors from invalid hanke data for perustaja`( + perustaja: Perustaja?, + errorMessage: String, + ) { + val partialHanke = + Hanke( + id = 0, + hankeTunnus = null, + nimi = HANKE_NAME_MANNERHEIMINTIE, + kuvaus = HANKE_MOCK_KUVAUS, + onYKTHanke = false, + vaihe = Vaihe.OHJELMOINTI, + suunnitteluVaihe = null, + version = 1, + createdBy = "", + createdAt = null, + modifiedBy = null, + modifiedAt = null, + status = HankeStatus.DRAFT, + perustaja = perustaja, + ) + + assertThatExceptionOfType(ConstraintViolationException::class.java) + .isThrownBy { hankeController.createHanke(partialHanke) } + .withMessageContaining(errorMessage) + + verify { disclosureLogService wasNot Called } + } + + companion object { + @JvmStatic + private fun invalidPerustajaArguments(): Stream = + Stream.of( + Arguments.of( + Perustaja(nimi = "Test Person", email = ""), + expectedErrorMessage(field = "perustaja.email") + ), + Arguments.of( + Perustaja(nimi = "", email = "test.mail@mail.com"), + expectedErrorMessage(field = "perustaja.nimi") + ), + Arguments.of( + Perustaja(nimi = null, email = "test.mail@mail.com"), + expectedErrorMessage(field = "perustaja.nimi") + ), + Arguments.of(null, expectedErrorMessage(field = "perustaja")) + ) + + private fun expectedErrorMessage(field: String) = + "createHanke.hanke.$field: ${HankeError.HAI1002}" + } + @Test fun `test when called without parameters then getHankeList returns ok and two items without geometry`() { val listOfHanke = @@ -114,8 +176,8 @@ class HankeControllerTest { id = 1234, hankeTunnus = mockedHankeTunnus, onYKTHanke = true, - nimi = "Mannerheimintien remontti remonttinen", - kuvaus = "Lorem ipsum dolor sit amet...", + nimi = HANKE_NAME_MANNERHEIMINTIE, + kuvaus = HANKE_MOCK_KUVAUS, vaihe = Vaihe.OHJELMOINTI, suunnitteluVaihe = null, version = 1, @@ -130,8 +192,8 @@ class HankeControllerTest { id = 50, hankeTunnus = "HAME50", onYKTHanke = true, - nimi = "Hämeenlinnanväylän uudistus", - kuvaus = "Lorem ipsum dolor sit amet...", + nimi = HANKE_NAME_HAMEENLINNANVAYLA, + kuvaus = HANKE_MOCK_KUVAUS, vaihe = Vaihe.SUUNNITTELU, suunnitteluVaihe = SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, version = 1, @@ -149,8 +211,8 @@ class HankeControllerTest { val hankeList = hankeController.getHankeList(false) - assertThat(hankeList[0].nimi).isEqualTo("Mannerheimintien remontti remonttinen") - assertThat(hankeList[1].nimi).isEqualTo("Hämeenlinnanväylän uudistus") + assertThat(hankeList[0].nimi).isEqualTo(HANKE_NAME_MANNERHEIMINTIE) + assertThat(hankeList[1].nimi).isEqualTo(HANKE_NAME_HAMEENLINNANVAYLA) assertThat(hankeList[0].alueidenGeometriat()).isEmpty() assertThat(hankeList[1].alueidenGeometriat()).isEmpty() verify { disclosureLogService.saveDisclosureLogsForHankkeet(any(), eq(USERNAME)) } @@ -162,8 +224,8 @@ class HankeControllerTest { Hanke( id = 123, hankeTunnus = "id123", - nimi = "hankkeen nimi", - kuvaus = "lorem ipsum dolor sit amet...", + nimi = HANKE_NAME_MANNERHEIMINTIE, + kuvaus = HANKE_MOCK_KUVAUS, onYKTHanke = false, vaihe = Vaihe.SUUNNITTELU, suunnitteluVaihe = SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, @@ -188,7 +250,7 @@ class HankeControllerTest { val response: Hanke = hankeController.updateHanke(partialHanke, "id123") assertThat(response).isNotNull - assertThat(response.nimi).isEqualTo("hankkeen nimi") + assertThat(response.nimi).isEqualTo(HANKE_NAME_MANNERHEIMINTIE) verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } } @@ -231,7 +293,7 @@ class HankeControllerTest { id = null, hankeTunnus = null, nimi = "hankkeen nimi", - kuvaus = "lorem ipsum dolor sit amet...", + kuvaus = HANKE_MOCK_KUVAUS, onYKTHanke = false, vaihe = Vaihe.OHJELMOINTI, suunnitteluVaihe = null, @@ -319,35 +381,4 @@ class HankeControllerTest { verify { disclosureLogService wasNot Called } } - - @Test - fun `test that creating a Hanke also adds owner permissions for creating user`() { - val hanke = - Hanke( - id = null, - hankeTunnus = null, - nimi = "hankkeen nimi", - kuvaus = "lorem ipsum dolor sit amet...", - onYKTHanke = false, - vaihe = Vaihe.OHJELMOINTI, - suunnitteluVaihe = null, - version = 1, - createdBy = "Tiina", - createdAt = getCurrentTimeUTC(), - modifiedBy = null, - modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, - ) - - val mockedHanke = hanke.copy() - mockedHanke.id = 12 - mockedHanke.hankeTunnus = "JOKU12" - Mockito.`when`(hankeService.createHanke(hanke)).thenReturn(mockedHanke) - - val response: Hanke = hankeController.createHanke(hanke) - - assertThat(response).isNotNull - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } - } } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt index 19d5aaa16..bbdd15076 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ContactTest.kt @@ -4,12 +4,16 @@ import assertk.assertThat import assertk.assertions.hasSize import assertk.assertions.isEqualTo import assertk.assertions.isNull +import fi.hel.haitaton.hanke.HankeArgumentException import fi.hel.haitaton.hanke.factory.AlluDataFactory import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.createContact import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts +import java.util.stream.Stream import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.CsvSource +import org.junit.jupiter.params.provider.MethodSource private const val DUMMY_EMAIL = "dummymail@mail.com" private const val DUMMY_PHONE = "04012345678" @@ -92,4 +96,30 @@ class ContactTest { assertThat(allContacts).hasSize(2) assertThat(result).isNull() } + + @ParameterizedTest + @MethodSource("invalidContacts") + fun `toHankePerustaja when invalid contact input should throw`(contact: Contact) { + assertThrows { contact.toHankePerustaja() } + } + + companion object { + @JvmStatic + fun invalidContacts(): Stream { + val contact = + Contact( + firstName = "Firstname", + lastName = "Lastname", + email = "test@email.com", + phone = "04012345678", + orderer = true + ) + return Stream.of( + contact.copy(firstName = null, lastName = null), + contact.copy(firstName = "", lastName = ""), + contact.copy(email = null), + contact.copy(email = "") + ) + } + } } From 7d1fb48d48c75bc5e36fd38b88afa84ba8f2da81 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Tue, 22 Aug 2023 10:03:20 +0300 Subject: [PATCH 04/13] HAI-1844 Fix hankevalidator, use correct error code --- .../hanke/validation/HankeValidator.kt | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt index a41b1667e..bd28c199e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt @@ -21,16 +21,16 @@ class HankeValidator : ConstraintValidator { var ok = true if (hanke.nimi.isNullOrBlank()) { - context.addViolation("nimi") + context.addViolation("nimi", HankeError.HAI1002) ok = false } if (hanke.vaihe == null) { - context.addViolation("vaihe") + context.addViolation("vaihe", HankeError.HAI1002) ok = false } else if (hanke.vaihe!! == Vaihe.SUUNNITTELU && hanke.suunnitteluVaihe == null) { // if vaihe = SUUNNITTELU then suunnitteluVaihe must have value - context.addViolation("suunnitteluVaihe") + context.addViolation("suunnitteluVaihe", HankeError.HAI1002) ok = false } @@ -49,7 +49,7 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaAlkuPvm == null || hankealue.haittaAlkuPvm!!.isAfter(MAXIMUM_DATE) ) { - context.addViolation("haittaAlkuPvm") + context.addViolation("haittaAlkuPvm", HankeError.HAI1032) return false } // Must be from the and earlier than some relevant maximum date, @@ -59,7 +59,7 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaLoppuPvm == null || hankealue.haittaLoppuPvm!!.isAfter(MAXIMUM_DATE) ) { - context.addViolation("haittaLoppuPvm") + context.addViolation("haittaLoppuPvm", HankeError.HAI1032) return false } if ( @@ -67,7 +67,7 @@ class HankeValidator : ConstraintValidator { hankealue.haittaLoppuPvm != null && hankealue.haittaLoppuPvm!!.isBefore(hankealue.haittaAlkuPvm) ) { - context.addViolation("haittaLoppuPvm") + context.addViolation("haittaLoppuPvm", HankeError.HAI1032) return false } } @@ -81,7 +81,7 @@ class HankeValidator : ConstraintValidator { hanke.tyomaaKatuosoite != null && hanke.tyomaaKatuosoite!!.length > MAXIMUM_TYOMAAKATUOSOITE_LENGTH ) { - context.addViolation("tyomaaKatuosoite") + context.addViolation("tyomaaKatuosoite", HankeError.HAI1002) ok = false } @@ -93,18 +93,17 @@ class HankeValidator : ConstraintValidator { with(hanke.perustaja) { if (this == null) { - context.addViolation("perustaja") - ok = false + context.addViolation("perustaja", HankeError.HAI1002) return false } if (nimi.isNullOrBlank()) { - context.addViolation("perustaja.nimi") + context.addViolation("perustaja.nimi", HankeError.HAI1002) ok = false } if (email.isBlank()) { - context.addViolation("perustaja.email") + context.addViolation("perustaja.email", HankeError.HAI1002) ok = false } } @@ -112,8 +111,8 @@ class HankeValidator : ConstraintValidator { return ok } - private fun ConstraintValidatorContext.addViolation(propertyNode: String) { - buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + private fun ConstraintValidatorContext.addViolation(propertyNode: String, error: HankeError) { + buildConstraintViolationWithTemplate(error.toString()) .addPropertyNode(propertyNode) .addConstraintViolation() } From 6d2b476555716f1fda1e95ea2ade6806aa9212f1 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 23 Aug 2023 15:37:31 +0300 Subject: [PATCH 05/13] HAI-1844 Redo so that this is implemented only for johtoselvitys hanke generation --- .../haitaton/hanke/HankeControllerITests.kt | 29 --- .../haitaton/hanke/HankeRepositoryITests.kt | 2 - .../hel/haitaton/hanke/HankeServiceITests.kt | 104 +++++----- .../application/ApplicationServiceITest.kt | 27 +-- .../permissions/HankeKayttajaServiceITest.kt | 130 +++++------- .../permissions/PermissionServiceITest.kt | 10 +- .../expectedHankeWithPoints.json.mustache | 5 + .../expectedHankeWithPolygon.json.mustache | 5 + .../fi/hel/haitaton/hanke/HankeController.kt | 4 +- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 24 ++- .../fi/hel/haitaton/hanke/domain/Hanke.kt | 2 +- .../hanke/permissions/HankeKayttajaService.kt | 24 +-- .../hanke/validation/HankeValidator.kt | 65 +++--- .../hel/haitaton/hanke/HankeControllerTest.kt | 193 ++++++------------ .../application/ApplicationServiceTest.kt | 24 +-- .../haitaton/hanke/factory/HankeFactory.kt | 43 ++-- 16 files changed, 285 insertions(+), 406 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt index dd0a53060..a28c985ec 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt @@ -1,6 +1,5 @@ package fi.hel.haitaton.hanke -import com.fasterxml.jackson.databind.node.ObjectNode import fi.hel.haitaton.hanke.application.ApplicationsResponse import fi.hel.haitaton.hanke.domain.HankeWithApplications import fi.hel.haitaton.hanke.domain.Hankealue @@ -8,7 +7,6 @@ import fi.hel.haitaton.hanke.factory.AlluDataFactory import fi.hel.haitaton.hanke.factory.DateFactory import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaja -import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withPerustaja import fi.hel.haitaton.hanke.geometria.Geometriat import fi.hel.haitaton.hanke.logging.DisclosureLogService import fi.hel.haitaton.hanke.permissions.PermissionCode @@ -334,22 +332,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll } } - @Test - fun `When perustaja is provided return provided information`() { - val hanke = HankeFactory.create().withPerustaja() - every { hankeService.createHanke(any()) } returns hanke - - post(url, hanke) - .andExpect(status().isOk) - .andExpect(jsonPath("$.perustaja.nimi").value("Pertti Perustaja")) - .andExpect(jsonPath("$.perustaja.email").value("foo@bar.com")) - - verifySequence { - hankeService.createHanke(any()) - disclosureLogService.saveDisclosureLogsForHanke(any(), any()) - } - } - @Test fun `Sanitize hanke input and return 200`() { val hanke = HankeFactory.create().apply { generated = true } @@ -364,17 +346,6 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll } } - @Test - fun `With perustaja without sahkoposti returns 400`() { - val hakemus = HankeFactory.create().withPerustaja() - val content: ObjectNode = OBJECT_MAPPER.valueToTree(hakemus) - (content.get("perustaja") as ObjectNode).remove("email") - - postRaw(url, content.toJsonString()) - .andExpect(status().isBadRequest) - .andExpect(hankeError(HankeError.HAI0003)) - } - @Test fun `Add Hanke and HankeYhteystiedot and return it with newly created hankeTunnus (POST)`() { val hankeToBeMocked = diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt index 4a1c77c0e..5baa4146c 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeRepositoryITests.kt @@ -1,7 +1,6 @@ package fi.hel.haitaton.hanke import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YRITYS -import fi.hel.haitaton.hanke.factory.HankeFactory import java.time.LocalDateTime import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test @@ -169,7 +168,6 @@ internal class HankeRepositoryITests : DatabaseTest() { createdAt = null, modifiedByUserId = null, modifiedAt = null, - perustaja = HankeFactory.defaultPerustaja.toEntity(), ) /* Keeping just seconds so that database truncation does not affect testing. */ diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 7c8602e62..140be0eaf 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -29,7 +29,6 @@ import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaj import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistajat import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedRakennuttaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withHankealue -import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withPerustaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankealueFactory import fi.hel.haitaton.hanke.geometria.Geometriat @@ -96,13 +95,13 @@ class HankeServiceITests : DatabaseTest() { @MockkBean private lateinit var cableReportService: CableReportService @Autowired private lateinit var hankeService: HankeService + @Autowired private lateinit var permissionService: PermissionService @Autowired private lateinit var auditLogRepository: AuditLogRepository @Autowired private lateinit var applicationRepository: ApplicationRepository @Autowired private lateinit var hankeRepository: HankeRepository @Autowired private lateinit var hankeKayttajaRepository: HankeKayttajaRepository @Autowired private lateinit var kayttajaTunnisteRepository: KayttajaTunnisteRepository @Autowired private lateinit var jdbcTemplate: JdbcTemplate - @Autowired private lateinit var permissionService: PermissionService @BeforeEach fun clearMocks() { @@ -117,8 +116,7 @@ class HankeServiceITests : DatabaseTest() { @Test fun `create Hanke with full data set succeeds and returns a new domain object with the correct values`() { - val hanke: Hanke = - getATestHanke().withYhteystiedot { it.id = null }.withPerustaja().withHankealue() + val hanke: Hanke = getATestHanke().withYhteystiedot { it.id = null }.withHankealue() val datetimeAlku = hanke.alueet[0].haittaAlkuPvm // nextyear.2.20 23:45:56Z val datetimeLoppu = hanke.alueet[0].haittaLoppuPvm // nextyear.2.21 0:12:34Z @@ -149,7 +147,7 @@ class HankeServiceITests : DatabaseTest() { assertThat(returnedHanke.loppuPvm).isEqualTo(expectedDateLoppu) assertThat(returnedHanke.vaihe).isEqualTo(Vaihe.SUUNNITTELU) assertThat(returnedHanke.suunnitteluVaihe).isEqualTo(SuunnitteluVaihe.RAKENNUS_TAI_TOTEUTUS) - assertThat(returnedHanke.perustaja).isEqualTo(Perustaja("Pertti Perustaja", "foo@bar.com")) + assertThat(returnedHanke.perustaja).isNull() assertThat(returnedHanke.tyomaaKatuosoite).isEqualTo("Testikatu 1") assertThat(returnedHanke.tyomaaTyyppi).contains(TyomaaTyyppi.VESI, TyomaaTyyppi.MUU) assertThat(returnedHanke.alueet[0].kaistaHaitta) @@ -210,8 +208,8 @@ class HankeServiceITests : DatabaseTest() { assertThat(rakennuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(rakennuttaja.id) - assertThat(hankeKayttajaRepository.findAll()).hasSize(5) - assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) // Hanke perustaja not included + assertThat(hankeKayttajaRepository.findAll()).hasSize(4) + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) } @Test @@ -942,9 +940,10 @@ class HankeServiceITests : DatabaseTest() { with(result) { val application = applications.first() - assertThat(hanke.hankeTunnus).isEqualTo(application.hankeTunnus) assertThat(hanke.nimi).isEqualTo(application.applicationData.name) + assertThat(hanke.perustaja) + .isEqualTo(Perustaja("Teppo Testihenkilö", "teppo@example.test")) assertThat(application.applicationData.name) .isEqualTo(inputApplication.applicationData.name) } @@ -1383,10 +1382,11 @@ class HankeServiceITests : DatabaseTest() { ) val templateData = TemplateData( - hankeId = updatedHanke.id!!, - hankeTunnus = updatedHanke.hankeTunnus!!, - alueId = updatedHanke.alueet[0].id, - geometriaId = updatedHanke.alueet[0].geometriat?.id, + updatedHanke.id!!, + updatedHanke.hankeTunnus!!, + updatedHanke.perustaja != null, + updatedHanke.alueet[0].id, + updatedHanke.alueet[0].geometriat?.id, hankeVersion = 1, geometriaVersion = 1, tormaystarkasteluTulos = true, @@ -1451,8 +1451,7 @@ class HankeServiceITests : DatabaseTest() { } private fun initHankeWithHakemus(alluId: Int): HankeEntity { - val hanke = - hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = "HAI23-1")) + val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI23-1")) val application = applicationRepository.save( AlluDataFactory.createApplicationEntity( @@ -1466,33 +1465,36 @@ class HankeServiceITests : DatabaseTest() { } private fun HankeEntity.toDomainObject(): Hanke = - Hanke( - id = id, - hankeTunnus = hankeTunnus, - onYKTHanke = onYKTHanke, - nimi = nimi, - kuvaus = kuvaus, - vaihe = vaihe, - suunnitteluVaihe = suunnitteluVaihe, - version = version, - createdBy = createdByUserId ?: "", - createdAt = createdAt?.atZone(TZ_UTC), - modifiedBy = modifiedByUserId, - modifiedAt = modifiedAt?.atZone(TZ_UTC), - status = status, - perustaja = HankeFactory.defaultPerustaja - ) + with(this) { + Hanke( + id, + hankeTunnus, + onYKTHanke, + nimi, + kuvaus, + vaihe, + suunnitteluVaihe, + version, + createdByUserId ?: "", + createdAt?.atZone(TZ_UTC), + modifiedByUserId, + modifiedAt?.atZone(TZ_UTC), + this.status + ) + } private fun ApplicationEntity.toDomainObject(): Application = - Application( - id = id, - alluid = alluid, - alluStatus = alluStatus, - applicationIdentifier = applicationIdentifier, - applicationType = applicationType, - applicationData = applicationData, - hankeTunnus = hanke.hankeTunnus ?: "" - ) + with(this) { + Application( + id, + alluid, + alluStatus, + applicationIdentifier, + applicationType, + applicationData, + hanke.hankeTunnus ?: "" + ) + } private fun assertFeaturePropertiesIsReset(hanke: Hanke, propertiesWanted: Map) { assertThat(hanke.alueet).isNotEmpty @@ -1508,6 +1510,7 @@ class HankeServiceITests : DatabaseTest() { data class TemplateData( val hankeId: Int, val hankeTunnus: String, + val hankePerustaja: Boolean = false, val alueId: Int? = null, val geometriaId: Int? = null, val geometriaVersion: Int = 0, @@ -1535,17 +1538,18 @@ class HankeServiceITests : DatabaseTest() { ): String { val templateData = TemplateData( - hankeId = hanke.id!!, - hankeTunnus = hanke.hankeTunnus!!, - alueId = alue?.id, - geometriaId = alue?.geometriat?.id, - geometriaVersion = geometriaVersion, - hankeVersion = hankeVersion, - nextYear = nextYear(), - tormaystarkasteluTulos = tormaystarkasteluTulos, - alueNimi = alue?.nimi, - alkuPvm = alkuPvm, - loppuPvm = loppuPvm, + hanke.id!!, + hanke.hankeTunnus!!, + hanke.perustaja != null, + alue?.id, + alue?.geometriat?.id, + geometriaVersion, + hankeVersion, + nextYear(), + tormaystarkasteluTulos, + alue?.nimi, + alkuPvm, + loppuPvm, ) return Template.parse( "/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache".getResourceAsText() diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt index 10bf7dd26..d430b3452 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceITest.kt @@ -121,10 +121,13 @@ class ApplicationServiceITest : DatabaseTest() { confirmVerified(cableReportServiceAllu) } - fun createHanke(hanke: Hanke = HankeFactory.create()): Hanke = hankeService.createHanke(hanke) + fun createHanke(): Hanke { + return hankeService.createHanke(HankeFactory.create()) + } - fun createHankeEntity(id: Int? = null, hankeTunnus: String? = "HAI-1234"): HankeEntity = - hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = hankeTunnus)) + fun createHankeEntity(): HankeEntity { + return hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) + } @Test fun `create creates an audit log entry for created application`() { @@ -340,8 +343,8 @@ class ApplicationServiceITest : DatabaseTest() { fun `getAllApplicationsForUser returns applications for the correct user`() { assertThat(applicationRepository.findAll()).isEmpty() val otherUser = "otherUser" - val hanke = createHankeEntity(hankeTunnus = "HAI-1234") - val hanke2 = createHankeEntity(hankeTunnus = "HAI-1235") + val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) + val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235")) permissionService.setPermission(hanke.id!!, USERNAME, Role.HAKEMUSASIOINTI) permissionService.setPermission(hanke2.id!!, "otherUser", Role.HAKEMUSASIOINTI) @@ -370,9 +373,9 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `getAllApplicationsForUser returns applications for user hankkeet`() { assertThat(applicationRepository.findAll()).isEmpty() - val hanke = createHankeEntity(hankeTunnus = "HAI-1234") - val hanke2 = createHankeEntity(hankeTunnus = "HAI-1235") - val hanke3 = createHankeEntity(hankeTunnus = "HAI-1236") + val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) + val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235")) + val hanke3 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1236")) permissionService.setPermission(hanke.id!!, USERNAME, Role.HAKEMUSASIOINTI) permissionService.setPermission(hanke2.id!!, USERNAME, Role.HAKEMUSASIOINTI) val application1 = alluDataFactory.saveApplicationEntity(username = USERNAME, hanke = hanke) @@ -397,7 +400,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `getApplicationById returns correct application`() { - val hanke = createHankeEntity(hankeTunnus = "HAI-1234") + val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234")) val applications = alluDataFactory.saveApplicationEntities(3, USERNAME, hanke = hanke) val selectedId = applications[1].id!! assertThat(applicationRepository.findAll()).hasSize(3) @@ -507,7 +510,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `create throws exception when application area is outside hankealue`() { - val hanke = createHanke(HankeFactory.create().withHankealue()) + val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) val cableReportApplicationData = AlluDataFactory.createCableReportApplicationData(areas = listOf(havisAmanda)) val newApplication = @@ -878,7 +881,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `updateApplicationData throws exception when application area is outside hankealue`() { - val hanke = createHanke(HankeFactory.create().withHankealue()) + val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) val hankeEntity = hankeRepository.getReferenceById(hanke.id!!) val application = alluDataFactory.saveApplicationEntity(USERNAME, hanke = hankeEntity) { it.alluid = 21 } @@ -1139,7 +1142,7 @@ class ApplicationServiceITest : DatabaseTest() { @Test fun `Throws an exception when application area is outside hankealue`() { - val hanke = createHanke(HankeFactory.create().withHankealue()) + val hanke = hankeService.createHanke(HankeFactory.create().withHankealue()) val hankeEntity = hankeRepository.getReferenceById(hanke.id!!) val application = alluDataFactory.saveApplicationEntity(USERNAME, hanke = hankeEntity) { diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt index 227c2ca87..a2bcb354b 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt @@ -2,6 +2,7 @@ package fi.hel.haitaton.hanke.permissions import assertk.Assert import assertk.assertThat +import assertk.assertions.containsExactly import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.each import assertk.assertions.hasSize @@ -16,10 +17,9 @@ import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.factory.AlluDataFactory import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts import fi.hel.haitaton.hanke.factory.HankeFactory +import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankeYhteystietoFactory -import fi.hel.haitaton.hanke.permissions.Role.KAIKKI_OIKEUDET -import fi.hel.haitaton.hanke.permissions.Role.KATSELUOIKEUS import fi.hel.haitaton.hanke.test.Asserts.isRecent import java.time.OffsetDateTime import org.junit.jupiter.api.Test @@ -57,13 +57,12 @@ class HankeKayttajaServiceITest : DatabaseTest() { val result: List = hankeKayttajaService.getKayttajatByHankeId(hankeToFind.id!!) - val yhteystiedotPlusPerustaja = 4 + 1 - assertThat(result).hasSize(yhteystiedotPlusPerustaja) + assertThat(result).hasSize(4) } @Test fun `getKayttajatByHankeId should return data matching to the saved entity`() { - val hanke = hankeFactory.save(HankeFactory.create()) + val hanke = hankeFactory.save(HankeFactory.create().withGeneratedOmistaja(1)) val result: List = hankeKayttajaService.getKayttajatByHankeId(hanke.id!!) @@ -74,16 +73,16 @@ class HankeKayttajaServiceITest : DatabaseTest() { assertThat(id).isEqualTo(entity.id) assertThat(nimi).isEqualTo(entity.nimi) assertThat(sahkoposti).isEqualTo(entity.sahkoposti) - assertThat(rooli).isEqualTo(entity.permission!!.role.role) - assertThat(tunnistautunut).isEqualTo(true) // hanke perustaja + assertThat(rooli).isEqualTo(entity.kayttajaTunniste!!.role) + assertThat(tunnistautunut).isEqualTo(false) } } @Test fun `addHankeFounder saves kayttaja with correct permission and other data`() { - val hankeEntity = hankeFactory.saveEntity(HankeFactory.createNewEntity(id = null)) + val hankeEntity = hankeFactory.saveEntity() val savedHankeId = hankeEntity.id!! - val savedPermission = savePermission(savedHankeId, USERNAME, KAIKKI_OIKEUDET) + val savedPermission = savePermission(savedHankeId, USERNAME, Role.KAIKKI_OIKEUDET) hankeKayttajaService.addHankeFounder(savedHankeId, perustaja, savedPermission) @@ -111,14 +110,11 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - val initialUserSize = 1 // hanke perustaja hankeKayttajaService.saveNewTokensFromApplication(application, 1) - val invitationTokens = kayttajaTunnisteRepository.findAll() - val hankeUsers = hankeKayttajaRepository.findAll() - assertThat(invitationTokens).isEmpty() // Hanke perustaja not included - assertThat(hankeUsers).hasSize(initialUserSize) + assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() + assertThat(hankeKayttajaRepository.findAll()).isEmpty() } @Test @@ -147,10 +143,10 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - val tunnisteet = nonPerustajaTunnisteet() + val tunnisteet = kayttajaTunnisteRepository.findAll() assertThat(tunnisteet).hasSize(4) assertThat(tunnisteet).areValid() - val kayttajat = nonPerustajaKayttajat() + val kayttajat = hankeKayttajaRepository.findAll() assertThat(kayttajat).hasSize(4) assertThat(kayttajat).each { kayttaja -> kayttaja.transform { it.nimi }.isEqualTo("Teppo Testihenkilö") @@ -197,8 +193,8 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - assertThat(nonPerustajaTunnisteet()).hasSize(2) - val kayttajat = nonPerustajaKayttajat() + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(2) + val kayttajat = hankeKayttajaRepository.findAll() assertThat(kayttajat).hasSize(2) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -232,12 +228,12 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - assertThat(nonPerustajaTunnisteet()).hasSize(2) + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(2) hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - assertThat(nonPerustajaTunnisteet()).hasSize(4) - val kayttajat = nonPerustajaKayttajat() + assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) + val kayttajat = hankeKayttajaRepository.findAll() assertThat(kayttajat).hasSize(4) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -273,16 +269,16 @@ class HankeKayttajaServiceITest : DatabaseTest() { applicationData = applicationData, hanke = hanke ) - assertThat(nonPerustajaTunnisteet()).isEmpty() + assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!) - val tunnisteet = nonPerustajaTunnisteet() + val tunnisteet = kayttajaTunnisteRepository.findAll() assertThat(tunnisteet).hasSize(2) assertThat(tunnisteet).each { tunniste -> tunniste.transform { it.hankeKayttaja?.sahkoposti }.isIn("email2", "email3") } - val kayttajat = nonPerustajaKayttajat() + val kayttajat = hankeKayttajaRepository.findAll() assertThat(kayttajat).hasSize(4) assertThat(kayttajat.map { it.sahkoposti }) .containsExactlyInAnyOrder( @@ -295,12 +291,10 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Test fun `saveNewTokensFromHanke does nothing if hanke has no contacts`() { - val hanke = hankeFactory.save(HankeFactory.create()) - - hankeKayttajaService.saveNewTokensFromHanke(hanke) + hankeKayttajaService.saveNewTokensFromHanke(HankeFactory.create()) - assertThat(nonPerustajaTunnisteet()).isEmpty() - assertThat(nonPerustajaKayttajat()).isEmpty() + assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() + assertThat(hankeKayttajaRepository.findAll()).isEmpty() } @Test @@ -320,8 +314,8 @@ class HankeKayttajaServiceITest : DatabaseTest() { hankeKayttajaService.saveNewTokensFromHanke(hanke) - val tunnisteet: List = nonPerustajaTunnisteet() - val kayttajat: List = nonPerustajaKayttajat() + val tunnisteet: List = kayttajaTunnisteRepository.findAll() + val kayttajat: List = hankeKayttajaRepository.findAll() assertThat(tunnisteet).hasSize(4) // 4 yhteyshenkilo subcontacts. assertThat(kayttajat).hasSize(4) assertThat(tunnisteet).areValid() @@ -330,43 +324,27 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Test fun `saveNewTokensFromHanke with pre-existing permissions does not create duplicate`() { - val hankeEntity = hankeFactory.saveEntity() - val hanke = - HankeFactory.create(id = hankeEntity.id, hankeTunnus = hankeEntity.hankeTunnus).apply { - omistajat.add(HankeYhteystietoFactory.create()) - } - saveUserAndToken(hankeEntity, "Existing User One", "ali.kontakti@meili.com") + val hanke = hankeFactory.save() + saveUserAndPermission(hanke.id!!, "Existing User One", "ali.kontakti@meili.com") - hankeKayttajaService.saveNewTokensFromHanke(hanke) + hankeKayttajaService.saveNewTokensFromHanke( + hanke.apply { this.omistajat.add(HankeYhteystietoFactory.create()) } + ) - val tunnisteet = nonPerustajaTunnisteet() - val kayttajat = nonPerustajaKayttajat() - assertThat(tunnisteet).hasSize(1) + val tunnisteet = kayttajaTunnisteRepository.findAll() + assertThat(tunnisteet).isEmpty() + assertThat(tunnisteet).areValid() + val kayttajat = hankeKayttajaRepository.findAll() assertThat(kayttajat).hasSize(1) - val kayttaja = kayttajat.first() - assertThat(kayttaja.nimi).isEqualTo("Existing User One") - assertThat(kayttaja.sahkoposti).isEqualTo("ali.kontakti@meili.com") + assertThat(kayttajat.map { it.sahkoposti }) + .containsExactly( + "ali.kontakti@meili.com", + ) } - private val expectedNames = - arrayOf( - "yhteys-etu1 yhteys-suku1", - "yhteys-etu2 yhteys-suku2", - "yhteys-etu3 yhteys-suku3", - "yhteys-etu4 yhteys-suku4", - ) - - private val expectedEmails = - arrayOf( - "yhteys-email1", - "yhteys-email2", - "yhteys-email3", - "yhteys-email4", - ) - private fun Assert>.areValid() = each { t -> t.transform { it.id }.isNotNull() - t.transform { it.role }.isEqualTo(KATSELUOIKEUS) + t.transform { it.role }.isEqualTo(Role.KATSELUOIKEUS) t.transform { it.createdAt }.isRecent() t.transform { it.sentAt }.isNull() t.transform { it.tunniste }.matches(Regex(kayttajaTunnistePattern)) @@ -396,19 +374,21 @@ class HankeKayttajaServiceITest : DatabaseTest() { assertThat(actual.permissionCode).isEqualTo(other.permissionCode) } - /** - * Creating a Hanke through HankeService adds HankeKayttajaEntity for perustaja. This is a - * helper function to filter out perustaja. - */ - private fun nonPerustajaKayttajat() = - hankeKayttajaRepository.findAll().filter { it.nimi != perustaja.nimi } + private val expectedNames = + arrayOf( + "yhteys-etu1 yhteys-suku1", + "yhteys-etu2 yhteys-suku2", + "yhteys-etu3 yhteys-suku3", + "yhteys-etu4 yhteys-suku4", + ) - /** - * Creating a Hanke through HankeService adds KayttajaTunnisteEntity for perustaja. This is a - * helper function to filter out perustaja. - */ - private fun nonPerustajaTunnisteet() = - kayttajaTunnisteRepository.findAll().filter { it.role != KAIKKI_OIKEUDET } + private val expectedEmails = + arrayOf( + "yhteys-email1", + "yhteys-email2", + "yhteys-email3", + "yhteys-email4", + ) private fun saveUserAndToken( hanke: HankeEntity, @@ -421,7 +401,7 @@ class HankeKayttajaServiceITest : DatabaseTest() { tunniste = "existing", createdAt = OffsetDateTime.parse("2023-03-31T15:41:21Z"), sentAt = null, - role = KATSELUOIKEUS, + role = Role.KATSELUOIKEUS, hankeKayttaja = null, ) ) @@ -441,7 +421,7 @@ class HankeKayttajaServiceITest : DatabaseTest() { nimi: String, sahkoposti: String ): HankeKayttajaEntity { - val permissionEntity = savePermission(hankeId, "fake id", KATSELUOIKEUS) + val permissionEntity = savePermission(hankeId, "fake id", Role.KATSELUOIKEUS) return hankeKayttajaRepository.save( HankeKayttajaEntity( diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt index 233f85554..992d40d87 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt @@ -33,8 +33,8 @@ class PermissionServiceITest : DatabaseTest() { @Autowired lateinit var permissionService: PermissionService @Autowired lateinit var permissionRepository: PermissionRepository @Autowired lateinit var roleRepository: RoleRepository - @Autowired lateinit var hankeService: HankeService @Autowired lateinit var hankeRepository: HankeRepository + @Autowired lateinit var hankeService: HankeService companion object { @JvmStatic @@ -148,7 +148,6 @@ class PermissionServiceITest : DatabaseTest() { fun `hasPermission with correct permission`() { val kaikkiOikeudet = roleRepository.findOneByRole(Role.KAIKKI_OIKEUDET) val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! - permissionRepository.save( PermissionEntity(userId = USERNAME, hankeId = hankeId, role = kaikkiOikeudet) ) @@ -196,8 +195,7 @@ class PermissionServiceITest : DatabaseTest() { } private fun saveSeveralHanke(hankeTunnusList: List) = - createSeveralHanke(hankeTunnusList).map { hankeRepository.save(it) } - - private fun createSeveralHanke(hankeTunnusList: List) = - hankeTunnusList.map { HankeFactory.createNewEntity(id = null, hankeTunnus = it) } + hankeTunnusList.map { + hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = it)) + } } diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache index 5e27516a1..a4c9c750a 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache @@ -22,10 +22,15 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", + {{#hankkeenPerustaja}} "perustaja": { "nimi": "Pertti Perustaja", "email": "foo@bar.com" }, + {{/hankkeenPerustaja}} + {{^hankkeenPerustaja}} + "perustaja": null, + {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache index 86ff7aa67..46c019ec5 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache @@ -22,10 +22,15 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", + {{#hankkeenPerustaja}} "perustaja": { "nimi": "Pertti Perustaja", "email": "foo@bar.com" }, + {{/hankkeenPerustaja}} + {{^hankkeenPerustaja}} + "perustaja": null, + {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt index 39df4430c..ea6be220e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt @@ -180,7 +180,7 @@ class HankeController( if (hanke == null) { throw HankeArgumentException("No hanke given when creating hanke") } - val sanitizedHanke = hanke.copy(id = null, generated = false) + val sanitizedHanke = hanke.copy(id = null, generated = false, perustaja = null) val userId = currentUserId() logger.info { "Creating Hanke for user $userId: ${hanke.toLogString()} " } @@ -313,7 +313,7 @@ class HankeController( if (hankeTunnusFromPath != updatedHanke.hankeTunnus) { throw HankeArgumentException("Hanketunnus not given or doesn't match the hanke data") } - if (perustaja != updatedHanke.perustaja) { + if (perustaja != null && perustaja != updatedHanke.perustaja) { throw HankeArgumentException("Updating perustaja not allowed.") } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index 3f307c802..519813d84 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -8,6 +8,7 @@ import fi.hel.haitaton.hanke.application.Application import fi.hel.haitaton.hanke.application.ApplicationService import fi.hel.haitaton.hanke.application.CableReportApplicationData import fi.hel.haitaton.hanke.application.CableReportWithoutHanke +import fi.hel.haitaton.hanke.application.Contact import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.HankeWithApplications import fi.hel.haitaton.hanke.domain.HankeYhteystieto @@ -152,7 +153,7 @@ open class HankeServiceImpl( postProcessAndSaveLogging(loggingEntryHolder, savedHankeEntity, userId) - return createHankeDomainObjectFromEntity(savedHankeEntity).also { + return createHankeDomainObjectFromEntity(entity).also { initAccessForCreatedHanke(it, userId) hankeLoggingService.logCreate(it, userId) } @@ -235,21 +236,22 @@ open class HankeServiceImpl( hankeLoggingService.logDelete(hanke, userId) } - private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { - val hankeId = hanke.id!! - val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - val perustaja = - hanke.perustaja ?: throw HankeArgumentException("Missing perustaja information") - hankeKayttajaService.addHankeFounder(hankeId, perustaja, permissionAll) - hankeKayttajaService.saveNewTokensFromHanke(hanke) - } - private fun anyHakemusProcessingInAllu(hakemukset: List): Boolean = hakemukset.any { logger.info { "Hakemus ${it.id} has alluStatus ${it.alluStatus}" } !applicationService.isStillPending(it) } + private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { + val hankeId = hanke.id!! + val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) + val perustaja = hanke.perustaja + if (perustaja != null) { + hankeKayttajaService.addHankeFounder(hankeId, perustaja, permissionAll) + } + hankeKayttajaService.saveNewTokensFromHanke(hanke) + } + // TODO: functions to remove and invalidate Hanke's tormaystarkastelu-data // At least invalidation can be done purely working on the particular // tormaystarkasteluTulosEntity and -Repository. @@ -964,7 +966,7 @@ open class HankeServiceImpl( ) private fun generatePerustajaFrom(cableReport: CableReportApplicationData): Perustaja { - val orderer = + val orderer: Contact = cableReport.findOrderer() ?: throw HankeArgumentException("Orderer not found for Hanke perustaja") diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt index db6da98e3..b9b78a062 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt @@ -79,7 +79,7 @@ data class Hanke( var status: HankeStatus? = HankeStatus.DRAFT, // @JsonView(ChangeLogView::class) - @field:Schema(description = "Hanke founder contact information", required = true) + @field:Schema(description = "Hanke founder contact information") var perustaja: Perustaja? = null, // @JsonView(ChangeLogView::class) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index 81c9be923..982396423 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -31,9 +31,7 @@ class HankeKayttajaService( .flatMap { it.contacts } .mapNotNull { userContactOrNull(it.fullName(), it.email) } - filterNewContacts(hankeId, contacts).forEach { contact -> - createInvitationToken(hankeId, contact) - } + filterNewContacts(hankeId, contacts).forEach { contact -> createToken(hankeId, contact) } } @Transactional @@ -48,9 +46,7 @@ class HankeKayttajaService( .flatMap { it.alikontaktit } .mapNotNull { userContactOrNull(it.fullName(), it.email) } - filterNewContacts(hankeId, contacts).forEach { contact -> - createInvitationToken(hankeId, contact) - } + filterNewContacts(hankeId, contacts).forEach { contact -> createToken(hankeId, contact) } } @Transactional @@ -67,19 +63,21 @@ class HankeKayttajaService( ) } - private fun createInvitationToken(hankeId: Int, contact: UserContact) { + private fun createToken(hankeId: Int, contact: UserContact) { logger.info { "Creating a new user token, hankeId=$hankeId" } val token = KayttajaTunnisteEntity.create() val kayttajaTunnisteEntity = kayttajaTunnisteRepository.save(token) logger.info { "Saved the new user token, id=${kayttajaTunnisteEntity.id}" } saveUser( - HankeKayttajaEntity( - hankeId = hankeId, - nimi = contact.name, - sahkoposti = contact.email, - permission = null, - kayttajaTunniste = kayttajaTunnisteEntity + hankeKayttajaRepository.save( + HankeKayttajaEntity( + hankeId = hankeId, + nimi = contact.name, + sahkoposti = contact.email, + permission = null, + kayttajaTunniste = kayttajaTunnisteEntity + ) ) ) } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt index bd28c199e..9ae0cfc92 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/validation/HankeValidator.kt @@ -21,22 +21,30 @@ class HankeValidator : ConstraintValidator { var ok = true if (hanke.nimi.isNullOrBlank()) { - context.addViolation("nimi", HankeError.HAI1002) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + .addPropertyNode("nimi") + .addConstraintViolation() ok = false } if (hanke.vaihe == null) { - context.addViolation("vaihe", HankeError.HAI1002) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + .addPropertyNode("vaihe") + .addConstraintViolation() ok = false } else if (hanke.vaihe!! == Vaihe.SUUNNITTELU && hanke.suunnitteluVaihe == null) { // if vaihe = SUUNNITTELU then suunnitteluVaihe must have value - context.addViolation("suunnitteluVaihe", HankeError.HAI1002) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + .addPropertyNode("suunnitteluVaihe") + .addConstraintViolation() ok = false } ok = ok && checkHankealueet(hanke, context) ok = ok && checkTyomaaTiedot(hanke, context) - ok = ok && checkPerustaja(hanke, context) return ok } @@ -49,7 +57,10 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaAlkuPvm == null || hankealue.haittaAlkuPvm!!.isAfter(MAXIMUM_DATE) ) { - context.addViolation("haittaAlkuPvm", HankeError.HAI1032) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) + .addPropertyNode("haittaAlkuPvm") + .addConstraintViolation() return false } // Must be from the and earlier than some relevant maximum date, @@ -59,7 +70,10 @@ class HankeValidator : ConstraintValidator { if ( hankealue.haittaLoppuPvm == null || hankealue.haittaLoppuPvm!!.isAfter(MAXIMUM_DATE) ) { - context.addViolation("haittaLoppuPvm", HankeError.HAI1032) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) + .addPropertyNode("haittaLoppuPvm") + .addConstraintViolation() return false } if ( @@ -67,7 +81,10 @@ class HankeValidator : ConstraintValidator { hankealue.haittaLoppuPvm != null && hankealue.haittaLoppuPvm!!.isBefore(hankealue.haittaAlkuPvm) ) { - context.addViolation("haittaLoppuPvm", HankeError.HAI1032) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1032.toString()) + .addPropertyNode("haittaLoppuPvm") + .addConstraintViolation() return false } } @@ -81,39 +98,13 @@ class HankeValidator : ConstraintValidator { hanke.tyomaaKatuosoite != null && hanke.tyomaaKatuosoite!!.length > MAXIMUM_TYOMAAKATUOSOITE_LENGTH ) { - context.addViolation("tyomaaKatuosoite", HankeError.HAI1002) + context + .buildConstraintViolationWithTemplate(HankeError.HAI1002.toString()) + .addPropertyNode("tyomaaKatuosoite") + .addConstraintViolation() ok = false } return ok } - - private fun checkPerustaja(hanke: Hanke, context: ConstraintValidatorContext): Boolean { - var ok = true - - with(hanke.perustaja) { - if (this == null) { - context.addViolation("perustaja", HankeError.HAI1002) - return false - } - - if (nimi.isNullOrBlank()) { - context.addViolation("perustaja.nimi", HankeError.HAI1002) - ok = false - } - - if (email.isBlank()) { - context.addViolation("perustaja.email", HankeError.HAI1002) - ok = false - } - } - - return ok - } - - private fun ConstraintValidatorContext.addViolation(propertyNode: String, error: HankeError) { - buildConstraintViolationWithTemplate(error.toString()) - .addPropertyNode(propertyNode) - .addConstraintViolation() - } } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt index 54bfa9b46..76c7e36d4 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt @@ -4,7 +4,6 @@ import fi.hel.haitaton.hanke.configuration.Feature import fi.hel.haitaton.hanke.configuration.FeatureFlags import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.HankeYhteystieto -import fi.hel.haitaton.hanke.domain.Perustaja import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YKSITYISHENKILO import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.logging.DisclosureLogService @@ -15,15 +14,11 @@ import io.mockk.clearAllMocks import io.mockk.mockk import io.mockk.verify import jakarta.validation.ConstraintViolationException -import java.util.stream.Stream import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatExceptionOfType import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.Arguments -import org.junit.jupiter.params.provider.MethodSource import org.mockito.Mockito import org.springframework.beans.factory.annotation.Autowired import org.springframework.context.annotation.Bean @@ -33,14 +28,11 @@ import org.springframework.security.test.context.support.WithMockUser import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.validation.beanvalidation.MethodValidationPostProcessor -private const val USERNAME = "testuser" -private const val HANKE_NAME_MANNERHEIMINTIE = "Mannerheimintien remontti remonttinen" -private const val HANKE_NAME_HAMEENLINNANVAYLA = "Hämeenlinnanväylän uudistus" -private const val HANKE_MOCK_KUVAUS = "Lorem ipsum dolor sit amet..." +private const val username = "testuser" @ExtendWith(SpringExtension::class) @Import(HankeControllerTest.TestConfiguration::class) -@WithMockUser(USERNAME) +@WithMockUser(username) class HankeControllerTest { @Configuration @@ -85,25 +77,24 @@ class HankeControllerTest { fun `test that the getHankeByTunnus returns ok`() { val hankeId = 1234 - Mockito.`when`(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.VIEW)) + Mockito.`when`(permissionService.hasPermission(hankeId, username, PermissionCode.VIEW)) .thenReturn(true) Mockito.`when`(hankeService.loadHanke(mockedHankeTunnus)) .thenReturn( Hanke( - id = hankeId, - hankeTunnus = mockedHankeTunnus, - onYKTHanke = true, - nimi = HANKE_NAME_MANNERHEIMINTIE, - kuvaus = HANKE_MOCK_KUVAUS, - vaihe = Vaihe.OHJELMOINTI, - suunnitteluVaihe = null, - version = 1, - createdBy = "Risto", - createdAt = getCurrentTimeUTC(), - modifiedBy = null, - modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, + hankeId, + mockedHankeTunnus, + true, + "Mannerheimintien remontti remonttinen", + "Lorem ipsum dolor sit amet...", + Vaihe.OHJELMOINTI, + null, + 1, + "Risto", + getCurrentTimeUTC(), + null, + null, + HankeStatus.DRAFT ) ) @@ -111,61 +102,7 @@ class HankeControllerTest { assertThat(response).isNotNull assertThat(response.nimi).isNotEmpty - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } - } - - @ParameterizedTest - @MethodSource("invalidPerustajaArguments") - fun `test that the createHanke will give validation errors from invalid hanke data for perustaja`( - perustaja: Perustaja?, - errorMessage: String, - ) { - val partialHanke = - Hanke( - id = 0, - hankeTunnus = null, - nimi = HANKE_NAME_MANNERHEIMINTIE, - kuvaus = HANKE_MOCK_KUVAUS, - onYKTHanke = false, - vaihe = Vaihe.OHJELMOINTI, - suunnitteluVaihe = null, - version = 1, - createdBy = "", - createdAt = null, - modifiedBy = null, - modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = perustaja, - ) - - assertThatExceptionOfType(ConstraintViolationException::class.java) - .isThrownBy { hankeController.createHanke(partialHanke) } - .withMessageContaining(errorMessage) - - verify { disclosureLogService wasNot Called } - } - - companion object { - @JvmStatic - private fun invalidPerustajaArguments(): Stream = - Stream.of( - Arguments.of( - Perustaja(nimi = "Test Person", email = ""), - expectedErrorMessage(field = "perustaja.email") - ), - Arguments.of( - Perustaja(nimi = "", email = "test.mail@mail.com"), - expectedErrorMessage(field = "perustaja.nimi") - ), - Arguments.of( - Perustaja(nimi = null, email = "test.mail@mail.com"), - expectedErrorMessage(field = "perustaja.nimi") - ), - Arguments.of(null, expectedErrorMessage(field = "perustaja")) - ) - - private fun expectedErrorMessage(field: String) = - "createHanke.hanke.$field: ${HankeError.HAI1002}" + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } } @Test @@ -173,49 +110,47 @@ class HankeControllerTest { val listOfHanke = listOf( Hanke( - id = 1234, - hankeTunnus = mockedHankeTunnus, - onYKTHanke = true, - nimi = HANKE_NAME_MANNERHEIMINTIE, - kuvaus = HANKE_MOCK_KUVAUS, - vaihe = Vaihe.OHJELMOINTI, - suunnitteluVaihe = null, - version = 1, - createdBy = "Risto", - createdAt = getCurrentTimeUTC(), - modifiedBy = null, - modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, + 1234, + mockedHankeTunnus, + true, + "Mannerheimintien remontti remonttinen", + "Lorem ipsum dolor sit amet...", + Vaihe.OHJELMOINTI, + null, + 1, + "Risto", + getCurrentTimeUTC(), + null, + null, + HankeStatus.DRAFT ), Hanke( - id = 50, - hankeTunnus = "HAME50", - onYKTHanke = true, - nimi = HANKE_NAME_HAMEENLINNANVAYLA, - kuvaus = HANKE_MOCK_KUVAUS, - vaihe = Vaihe.SUUNNITTELU, - suunnitteluVaihe = SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, - version = 1, - createdBy = "Paavo", - createdAt = getCurrentTimeUTC(), - modifiedBy = null, - modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, + 50, + "HAME50", + true, + "Hämeenlinnanväylän uudistus", + "Lorem ipsum dolor sit amet...", + Vaihe.SUUNNITTELU, + SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, + 1, + "Paavo", + getCurrentTimeUTC(), + null, + null, + HankeStatus.DRAFT ) ) - Mockito.`when`(permissionService.getAllowedHankeIds(USERNAME, PermissionCode.VIEW)) + Mockito.`when`(permissionService.getAllowedHankeIds(username, PermissionCode.VIEW)) .thenReturn(listOf(1234, 50)) Mockito.`when`(hankeService.loadHankkeetByIds(listOf(1234, 50))).thenReturn(listOfHanke) val hankeList = hankeController.getHankeList(false) - assertThat(hankeList[0].nimi).isEqualTo(HANKE_NAME_MANNERHEIMINTIE) - assertThat(hankeList[1].nimi).isEqualTo(HANKE_NAME_HAMEENLINNANVAYLA) + assertThat(hankeList[0].nimi).isEqualTo("Mannerheimintien remontti remonttinen") + assertThat(hankeList[1].nimi).isEqualTo("Hämeenlinnanväylän uudistus") assertThat(hankeList[0].alueidenGeometriat()).isEmpty() assertThat(hankeList[1].alueidenGeometriat()).isEmpty() - verify { disclosureLogService.saveDisclosureLogsForHankkeet(any(), eq(USERNAME)) } + verify { disclosureLogService.saveDisclosureLogsForHankkeet(any(), eq(username)) } } @Test @@ -224,8 +159,8 @@ class HankeControllerTest { Hanke( id = 123, hankeTunnus = "id123", - nimi = HANKE_NAME_MANNERHEIMINTIE, - kuvaus = HANKE_MOCK_KUVAUS, + nimi = "hankkeen nimi", + kuvaus = "lorem ipsum dolor sit amet...", onYKTHanke = false, vaihe = Vaihe.SUUNNITTELU, suunnitteluVaihe = SuunnitteluVaihe.KATUSUUNNITTELU_TAI_ALUEVARAUS, @@ -234,24 +169,23 @@ class HankeControllerTest { createdAt = getCurrentTimeUTC(), modifiedBy = null, modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, + status = HankeStatus.DRAFT ) // mock HankeService response Mockito.`when`(hankeService.updateHanke(partialHanke)) - .thenReturn(partialHanke.copy(modifiedBy = USERNAME, modifiedAt = getCurrentTimeUTC())) + .thenReturn(partialHanke.copy(modifiedBy = username, modifiedAt = getCurrentTimeUTC())) Mockito.`when`(hankeService.loadHanke("id123")) .thenReturn(HankeFactory.create(hankeTunnus = "id123")) - Mockito.`when`(permissionService.hasPermission(123, USERNAME, PermissionCode.EDIT)) + Mockito.`when`(permissionService.hasPermission(123, username, PermissionCode.EDIT)) .thenReturn(true) // Actual call val response: Hanke = hankeController.updateHanke(partialHanke, "id123") assertThat(response).isNotNull - assertThat(response.nimi).isEqualTo(HANKE_NAME_MANNERHEIMINTIE) - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } + assertThat(response.nimi).isEqualTo("hankkeen nimi") + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } } @Test @@ -270,8 +204,7 @@ class HankeControllerTest { createdAt = null, modifiedBy = null, modifiedAt = null, - status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, + status = HankeStatus.DRAFT ) Mockito.`when`(hankeService.loadHanke("id123")).thenReturn(HankeFactory.create()) @@ -285,7 +218,7 @@ class HankeControllerTest { verify { disclosureLogService wasNot Called } } - // sending of subtypes + // sending of sub types @Test fun `test that create with listOfOmistaja can be sent to controller and is responded with 200`() { val hanke = @@ -293,7 +226,7 @@ class HankeControllerTest { id = null, hankeTunnus = null, nimi = "hankkeen nimi", - kuvaus = HANKE_MOCK_KUVAUS, + kuvaus = "lorem ipsum dolor sit amet...", onYKTHanke = false, vaihe = Vaihe.OHJELMOINTI, suunnitteluVaihe = null, @@ -303,7 +236,6 @@ class HankeControllerTest { modifiedBy = null, modifiedAt = null, status = HankeStatus.DRAFT, - perustaja = HankeFactory.defaultPerustaja, ) hanke.omistajat = @@ -320,10 +252,10 @@ class HankeControllerTest { alikontaktit = listOf( Yhteyshenkilo( - etunimi = "Ali", - sukunimi = "Kontakti", - email = "ali.kontakti@meili.com", - puhelinnumero = "050-3789354" + "Ali", + "Kontakti", + "ali.kontakti@meili.com", + "050-3789354" ) ), ) @@ -349,7 +281,7 @@ class HankeControllerTest { assertThat(response.omistajat).hasSize(1) assertThat(response.omistajat[0].id).isEqualTo(1) assertThat(response.omistajat[0].nimi).isEqualTo("Pekka Pekkanen") - verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(USERNAME)) } + verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } } @Test @@ -369,7 +301,6 @@ class HankeControllerTest { modifiedBy = null, modifiedAt = null, status = null, - perustaja = HankeFactory.defaultPerustaja ) // mock HankeService response Mockito.`when`(hankeService.updateHanke(partialHanke)).thenReturn(partialHanke) diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt index e82bfdecc..1ac3b2731 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/application/ApplicationServiceTest.kt @@ -9,6 +9,7 @@ import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isNotEmpty import assertk.assertions.isNotNull +import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.allu.AlluException import fi.hel.haitaton.hanke.allu.AlluLoginException @@ -24,7 +25,6 @@ import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withContacts import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withCustomer import fi.hel.haitaton.hanke.factory.AlluDataFactory.Companion.withHanke import fi.hel.haitaton.hanke.factory.ApplicationHistoryFactory -import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.geometria.GeometriatDao import fi.hel.haitaton.hanke.logging.ApplicationLoggingService import fi.hel.haitaton.hanke.logging.DisclosureLogService @@ -130,7 +130,7 @@ class ApplicationServiceTest { val application: ApplicationEntity = firstArg() application.copy(id = 1) } - val hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS) + val hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS) every { hankeRepository.findByHankeTunnus(HANKE_TUNNUS) } returns hanke every { geometriatDao.validateGeometriat(any()) } returns null every { geometriatDao.isInsideHankeAlueet(1, any()) } returns true @@ -176,7 +176,7 @@ class ApplicationServiceTest { @Test fun `updateApplicationData saves disclosure logs when updating Allu data`() { - val hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS) + val hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS) val applicationEntity = AlluDataFactory.createApplicationEntity( id = 3, @@ -223,7 +223,7 @@ class ApplicationServiceTest { alluid = 42, userId = USERNAME, applicationData = applicationData, - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.validateGeometriat(any()) } returns @@ -256,7 +256,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { applicationRepo.save(any()) } answers { firstArg() } @@ -295,7 +295,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.calculateCombinedArea(any()) } returns 100f @@ -329,7 +329,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(hankeTunnus = HANKE_TUNNUS, id = 1), ) assertThat(applicationEntity.applicationData.areas).isNotNull().isNotEmpty() every { applicationRepo.findOneById(3) } returns applicationEntity @@ -363,7 +363,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData.copy(rockExcavation = rockExcavation), - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { applicationRepo.save(any()) } answers { firstArg() } @@ -410,7 +410,7 @@ class ApplicationServiceTest { alluid = null, userId = USERNAME, applicationData = applicationData, - hanke = HankeFactory.createNewEntity(id = 1, HANKE_TUNNUS), + hanke = HankeEntity(id = 1, hankeTunnus = HANKE_TUNNUS), ) every { applicationRepo.findOneById(3) } returns applicationEntity every { geometriatDao.isInsideHankeAlueet(1, any()) } returns true @@ -463,6 +463,7 @@ class ApplicationServiceTest { inner class HandleApplicationUpdates { private val alluid = 42 private val applicationId = 13L + private val hankeTunnus = "HAI23-1" private val receiver = AlluDataFactory.teppoEmail private val updateTime = OffsetDateTime.parse("2022-10-09T06:36:51Z") private val identifier = ApplicationHistoryFactory.defaultApplicationIdentifier @@ -552,8 +553,7 @@ class ApplicationServiceTest { @Test fun `logs error if hanketunnus is null`(output: CapturedOutput) { every { applicationRepo.getOneByAlluid(42) } returns - applicationEntity() - .withHanke(HankeFactory.createNewEntity(id = 1, hankeTunnus = null)) + applicationEntity().withHanke(HankeEntity(id = 1, hankeTunnus = null)) every { applicationRepo.save(any()) } answers { firstArg() } every { statusRepo.getReferenceById(1) } returns AlluStatus(1, updateTime) every { statusRepo.save(any()) } answers { firstArg() } @@ -604,7 +604,7 @@ class ApplicationServiceTest { alluid = alluid, applicationIdentifier = identifier, userId = "user", - hanke = HankeFactory.createNewEntity(id = 1, hankeTunnus = HANKE_TUNNUS), + hanke = HankeEntity(id = 1, hankeTunnus = hankeTunnus), ) .withCustomer(AlluDataFactory.createCompanyCustomerWithOrderer()) diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt index c9e4686b2..a48a04ca7 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt @@ -4,7 +4,6 @@ import fi.hel.haitaton.hanke.HankeEntity import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.HankeService import fi.hel.haitaton.hanke.HankeStatus -import fi.hel.haitaton.hanke.PerustajaEntity import fi.hel.haitaton.hanke.SuunnitteluVaihe import fi.hel.haitaton.hanke.TyomaaTyyppi import fi.hel.haitaton.hanke.Vaihe @@ -40,8 +39,6 @@ class HankeFactory( ) ) - fun save(hanke: Hanke) = hankeService.createHanke(hanke) - /** * Save a new hanke using HankeService. Then get it as an entity. * @@ -57,7 +54,7 @@ class HankeFactory( return hankeRepository.getReferenceById(hanke.id!!) } - fun saveEntity(entity: HankeEntity) = hankeRepository.save(entity) + fun save(hanke: Hanke) = hankeService.createHanke(hanke) companion object { @@ -86,31 +83,28 @@ class HankeFactory( createdBy: String? = defaultUser, createdAt: ZonedDateTime? = DateFactory.getStartDatetime(), hankeStatus: HankeStatus = HankeStatus.DRAFT, - perustaja: Perustaja = defaultPerustaja, ): Hanke = Hanke( - id = id, - hankeTunnus = hankeTunnus, - onYKTHanke = true, - nimi = nimi, - kuvaus = "lorem ipsum dolor sit amet...", - vaihe = vaihe, - suunnitteluVaihe = suunnitteluVaihe, - version = version, - createdBy = createdBy, - createdAt = createdAt, - modifiedBy = null, - modifiedAt = null, - status = hankeStatus, - perustaja = perustaja, + id, + hankeTunnus, + true, + nimi, + "lorem ipsum dolor sit amet...", + vaihe, + suunnitteluVaihe, + version, + createdBy, + createdAt, + null, + null, + hankeStatus, ) /** Create minimal Entity with identifier fields and mandatory fields. */ fun createNewEntity( id: Int? = defaultId, hankeTunnus: String? = defaultHankeTunnus, - perustaja: PerustajaEntity = defaultPerustaja.toEntity() - ) = HankeEntity(id = id, hankeTunnus = hankeTunnus, perustaja = perustaja) + ) = HankeEntity(id = id, hankeTunnus = hankeTunnus) /** * Add a hankealue with haitat to a test Hanke. @@ -118,7 +112,6 @@ class HankeFactory( * Example: * ``` * HankeFactory.create().withHankealue() - * * ``` */ fun Hanke.withHankealue( @@ -190,9 +183,9 @@ class HankeFactory( return this } - fun Hanke.withPerustaja(newPerustaja: Perustaja = defaultPerustaja): Hanke = apply { - perustaja = newPerustaja - } + fun Hanke.withPerustaja( + newPerustaja: Perustaja? = Perustaja("Pertti Perustaja", "foo@bar.com") + ): Hanke = apply { perustaja = newPerustaja } /** * Add a number of omistaja to a hanke. Generates the yhteystiedot with From 09bbd3d0a62428f579d467c0d2332287b74b970a Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 09:37:14 +0300 Subject: [PATCH 06/13] HAI-1844 Rename factory method, use let instead of if --- .../haitaton/hanke/permissions/PermissionServiceITest.kt | 2 +- .../main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt | 7 +++---- .../kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt index 992d40d87..4de9b81f3 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt @@ -196,6 +196,6 @@ class PermissionServiceITest : DatabaseTest() { private fun saveSeveralHanke(hankeTunnusList: List) = hankeTunnusList.map { - hankeRepository.save(HankeFactory.createNewEntity(id = null, hankeTunnus = it)) + hankeRepository.save(HankeFactory.createMinimalEntity(id = null, hankeTunnus = it)) } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index 519813d84..8371e3b82 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -245,10 +245,9 @@ open class HankeServiceImpl( private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { val hankeId = hanke.id!! val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - val perustaja = hanke.perustaja - if (perustaja != null) { - hankeKayttajaService.addHankeFounder(hankeId, perustaja, permissionAll) - } + + hanke.perustaja?.let { hankeKayttajaService.addHankeFounder(hankeId, it, permissionAll) } + hankeKayttajaService.saveNewTokensFromHanke(hanke) } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt index a48a04ca7..a0ee6c1da 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt @@ -101,7 +101,7 @@ class HankeFactory( ) /** Create minimal Entity with identifier fields and mandatory fields. */ - fun createNewEntity( + fun createMinimalEntity( id: Int? = defaultId, hankeTunnus: String? = defaultHankeTunnus, ) = HankeEntity(id = id, hankeTunnus = hankeTunnus) From 7b631a985570a16914459362cd12d181a672c9bb Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 09:47:46 +0300 Subject: [PATCH 07/13] HAI-1844 Include perustaja in hanke creation test --- .../kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 140be0eaf..9229ab461 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -29,6 +29,7 @@ import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaj import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistajat import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedRakennuttaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withHankealue +import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withPerustaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankealueFactory import fi.hel.haitaton.hanke.geometria.Geometriat @@ -116,7 +117,8 @@ class HankeServiceITests : DatabaseTest() { @Test fun `create Hanke with full data set succeeds and returns a new domain object with the correct values`() { - val hanke: Hanke = getATestHanke().withYhteystiedot { it.id = null }.withHankealue() + val hanke: Hanke = + getATestHanke().withYhteystiedot { it.id = null }.withPerustaja().withHankealue() val datetimeAlku = hanke.alueet[0].haittaAlkuPvm // nextyear.2.20 23:45:56Z val datetimeLoppu = hanke.alueet[0].haittaLoppuPvm // nextyear.2.21 0:12:34Z @@ -147,7 +149,7 @@ class HankeServiceITests : DatabaseTest() { assertThat(returnedHanke.loppuPvm).isEqualTo(expectedDateLoppu) assertThat(returnedHanke.vaihe).isEqualTo(Vaihe.SUUNNITTELU) assertThat(returnedHanke.suunnitteluVaihe).isEqualTo(SuunnitteluVaihe.RAKENNUS_TAI_TOTEUTUS) - assertThat(returnedHanke.perustaja).isNull() + assertThat(returnedHanke.perustaja).isEqualTo(Perustaja("Pertti Perustaja", "foo@bar.com")) assertThat(returnedHanke.tyomaaKatuosoite).isEqualTo("Testikatu 1") assertThat(returnedHanke.tyomaaTyyppi).contains(TyomaaTyyppi.VESI, TyomaaTyyppi.MUU) assertThat(returnedHanke.alueet[0].kaistaHaitta) @@ -208,7 +210,7 @@ class HankeServiceITests : DatabaseTest() { assertThat(rakennuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(rakennuttaja.id) - assertThat(hankeKayttajaRepository.findAll()).hasSize(4) + assertThat(hankeKayttajaRepository.findAll()).hasSize(5) // contacts + Hanke perustaja assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) } From 8e5d2798c04d0733c0f1b50f8709fa607af35164 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 10:02:50 +0300 Subject: [PATCH 08/13] HAI-1844 Add test to verify no hanke kayttajas are created if no perustaja or contacts --- .../kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 9229ab461..8e1061ef5 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -214,6 +214,14 @@ class HankeServiceITests : DatabaseTest() { assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) } + @Test + fun `create Hanke with without perustaja and contacts does not create hanke users`() { + hankeService.createHanke(getATestHanke()) + + assertThat(hankeKayttajaRepository.findAll()).isEmpty() + assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() + } + @Test fun `create Hanke with partial data set and update with full data set give correct status`() { // Setup Hanke (without any yhteystieto): From 2175c0ef3ed73a45a4c4243ba5e95227b7eb53d7 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 10:23:43 +0300 Subject: [PATCH 09/13] HAI-1844 Fix save operation in token creation --- .../hanke/permissions/HankeKayttajaService.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index 982396423..5dff5d073 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -70,14 +70,12 @@ class HankeKayttajaService( logger.info { "Saved the new user token, id=${kayttajaTunnisteEntity.id}" } saveUser( - hankeKayttajaRepository.save( - HankeKayttajaEntity( - hankeId = hankeId, - nimi = contact.name, - sahkoposti = contact.email, - permission = null, - kayttajaTunniste = kayttajaTunnisteEntity - ) + HankeKayttajaEntity( + hankeId = hankeId, + nimi = contact.name, + sahkoposti = contact.email, + permission = null, + kayttajaTunniste = kayttajaTunnisteEntity ) ) } From 86bfa2e790ef7626d65aeff77c524c0c76e516aa Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 14:07:53 +0300 Subject: [PATCH 10/13] HAI-1844 Simplify fixes for permissionserviceitest --- .../permissions/PermissionServiceITest.kt | 55 +++++++++---------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt index 4de9b81f3..e5070ff5e 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/PermissionServiceITest.kt @@ -5,7 +5,6 @@ import assertk.assertions.hasSize import assertk.assertions.isFalse import assertk.assertions.isTrue import fi.hel.haitaton.hanke.DatabaseTest -import fi.hel.haitaton.hanke.HankeRepository import fi.hel.haitaton.hanke.HankeService import fi.hel.haitaton.hanke.factory.HankeFactory import org.junit.jupiter.api.Assertions.assertEquals @@ -21,19 +20,17 @@ import org.springframework.security.test.context.support.WithMockUser import org.springframework.test.context.ActiveProfiles import org.testcontainers.junit.jupiter.Testcontainers -private const val HANKE_TUNNUS = HankeFactory.defaultHankeTunnus -private const val USERNAME = "user" - @Testcontainers @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @ActiveProfiles("default") @WithMockUser(username = "test7358") class PermissionServiceITest : DatabaseTest() { + val username = "user" + @Autowired lateinit var permissionService: PermissionService @Autowired lateinit var permissionRepository: PermissionRepository @Autowired lateinit var roleRepository: RoleRepository - @Autowired lateinit var hankeRepository: HankeRepository @Autowired lateinit var hankeService: HankeService companion object { @@ -89,7 +86,7 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `getAllowedHankeIds without permissions returns empty list`() { - val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) assertEquals(listOf(), response) } @@ -97,20 +94,20 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `getAllowedHankeIds with permissions returns list of IDs`() { val kaikkiOikeudet = roleRepository.findOneByRole(Role.KAIKKI_OIKEUDET) - val hankkeet = saveSeveralHanke(listOf(HANKE_TUNNUS, "HAI23-1", "HAI23-2")) + val hankkeet = saveSeveralHanke(3) hankkeet .map { it.id!! } .forEach { permissionRepository.save( PermissionEntity( - userId = USERNAME, + userId = username, hankeId = it, role = kaikkiOikeudet, ) ) } - val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) assertEquals(hankkeet.map { it.id }, response) } @@ -121,56 +118,57 @@ class PermissionServiceITest : DatabaseTest() { val hankemuokkaus = roleRepository.findOneByRole(Role.HANKEMUOKKAUS) val hakemusasiointi = roleRepository.findOneByRole(Role.HAKEMUSASIOINTI) val katseluoikeus = roleRepository.findOneByRole(Role.KATSELUOIKEUS) - val hankkeet = saveSeveralHanke(listOf(HANKE_TUNNUS, "HAI23-1", "HAI23-2", "HAI23-3")) + val hankkeet = saveSeveralHanke(4) listOf(kaikkiOikeudet, hankemuokkaus, hakemusasiointi, katseluoikeus).zip(hankkeet) { role, hanke -> permissionRepository.save( PermissionEntity( - userId = USERNAME, + userId = username, hankeId = hanke.id!!, role = role, ) ) } - val response = permissionService.getAllowedHankeIds(USERNAME, PermissionCode.EDIT) + val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT) assertEquals(listOf(hankkeet[0].id, hankkeet[1].id), response) } @Test fun `hasPermission returns false without permissions`() { - assertFalse(permissionService.hasPermission(2, USERNAME, PermissionCode.EDIT)) + assertFalse(permissionService.hasPermission(2, username, PermissionCode.EDIT)) } @Test fun `hasPermission with correct permission`() { val kaikkiOikeudet = roleRepository.findOneByRole(Role.KAIKKI_OIKEUDET) - val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! + val hankeId = saveSeveralHanke(1)[0].id!! permissionRepository.save( - PermissionEntity(userId = USERNAME, hankeId = hankeId, role = kaikkiOikeudet) + PermissionEntity(userId = username, hankeId = hankeId, role = kaikkiOikeudet) ) - assertTrue(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT)) + assertTrue(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT)) } @Test fun `hasPermission with insufficient permissions`() { val hakemusasiointi = roleRepository.findOneByRole(Role.HAKEMUSASIOINTI) - val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! + val hankeId = saveSeveralHanke(1)[0].id!! permissionRepository.save( - PermissionEntity(userId = USERNAME, hankeId = hankeId, role = hakemusasiointi) + PermissionEntity(userId = username, hankeId = hankeId, role = hakemusasiointi) ) - assertFalse(permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT)) + assertFalse(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT)) } @Test fun `setPermission creates a new permission`() { - val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! + val hankeId = saveSeveralHanke(1)[0].id!! + permissionRepository.deleteAll() // remove permission created in hanke creation - permissionService.setPermission(hankeId, USERNAME, Role.KATSELUOIKEUS) + permissionService.setPermission(hankeId, username, Role.KATSELUOIKEUS) val permissions = permissionRepository.findAll() assertThat(permissions).hasSize(1) @@ -180,13 +178,14 @@ class PermissionServiceITest : DatabaseTest() { @Test fun `setPermission updates an existing permission`() { - val hankeId = saveSeveralHanke(listOf(HANKE_TUNNUS))[0].id!! + val hankeId = saveSeveralHanke(1)[0].id!! + permissionRepository.deleteAll() // remove permission created in hanke creation val role = roleRepository.findOneByRole(Role.KATSELUOIKEUS) permissionRepository.save( - PermissionEntity(userId = USERNAME, hankeId = hankeId, role = role) + PermissionEntity(userId = username, hankeId = hankeId, role = role) ) - permissionService.setPermission(hankeId, USERNAME, Role.HAKEMUSASIOINTI) + permissionService.setPermission(hankeId, username, Role.HAKEMUSASIOINTI) val permissions = permissionRepository.findAll() assertThat(permissions).hasSize(1) @@ -194,8 +193,8 @@ class PermissionServiceITest : DatabaseTest() { assertEquals(Role.HAKEMUSASIOINTI, permissions[0].role.role) } - private fun saveSeveralHanke(hankeTunnusList: List) = - hankeTunnusList.map { - hankeRepository.save(HankeFactory.createMinimalEntity(id = null, hankeTunnus = it)) - } + private fun saveSeveralHanke(n: Int) = + createSeveralHanke(n).map { hankeService.createHanke(it) } + + private fun createSeveralHanke(n: Int) = (1..n).map { HankeFactory.create(id = null) } } From 9c1a8b5098362d113608a49f173554cab52b7fb7 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 15:29:59 +0300 Subject: [PATCH 11/13] HAI-1844 Remove perustaja from domain object and change implementation accordingly, improve authorisation check in controller --- .../haitaton/hanke/HankeControllerITests.kt | 12 +++---- .../hel/haitaton/hanke/HankeServiceITests.kt | 20 +++++------ .../expectedHankeWithPoints.json.mustache | 9 ----- .../expectedHankeWithPolygon.json.mustache | 9 ----- .../fi/hel/haitaton/hanke/HankeController.kt | 27 ++++++++------- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 33 ++++++++++--------- .../fi/hel/haitaton/hanke/domain/Hanke.kt | 4 --- .../hel/haitaton/hanke/HankeControllerTest.kt | 2 +- .../haitaton/hanke/factory/HankeFactory.kt | 4 --- 9 files changed, 46 insertions(+), 74 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt index a28c985ec..d736170d6 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeControllerITests.kt @@ -455,14 +455,14 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll @Test fun `Without permission returns 404`() { val hanke = HankeFactory.create(version = null) - every { hankeService.loadHanke(HANKE_TUNNUS) } returns hanke + every { hankeService.findHankeOrThrow(HANKE_TUNNUS) } returns hanke every { permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT) } .returns(false) put(url, hanke).andExpect(status().isNotFound).andExpect(hankeError(HankeError.HAI1001)) verifySequence { - hankeService.loadHanke(HANKE_TUNNUS) + hankeService.findHankeOrThrow(HANKE_TUNNUS) permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT) } } @@ -476,7 +476,7 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll modifiedBy = USERNAME status = HankeStatus.PUBLIC } - every { hankeService.loadHanke(HANKE_TUNNUS) } returns HankeFactory.create() + every { hankeService.findHankeOrThrow(HANKE_TUNNUS) } returns HankeFactory.create() every { permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT) } .returns(true) every { hankeService.updateHanke(any()) }.returns(updatedHanke) @@ -488,7 +488,7 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll .andExpect(jsonPath("$.status").value(HankeStatus.PUBLIC.name)) verifySequence { - hankeService.loadHanke(HANKE_TUNNUS) + hankeService.findHankeOrThrow(HANKE_TUNNUS) permissionService.hasPermission(hankeId, USERNAME, PermissionCode.EDIT) hankeService.updateHanke(any()) disclosureLogService.saveDisclosureLogsForHanke(updatedHanke, USERNAME) @@ -525,7 +525,7 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll expectedHanke.alueet[0].haittaAlkuPvm = expectedDateAlku expectedHanke.alueet[0].haittaLoppuPvm = expectedDateLoppu val expectedContent = expectedHanke.toJsonString() - every { hankeService.loadHanke(HANKE_TUNNUS) } returns hankeToBeUpdated + every { hankeService.findHankeOrThrow(HANKE_TUNNUS) } returns hankeToBeUpdated every { permissionService.hasPermission(expectedHanke.id!!, USERNAME, PermissionCode.EDIT) } returns true @@ -543,7 +543,7 @@ class HankeControllerITests(@Autowired override val mockMvc: MockMvc) : Controll ) // Note, here as string, not the enum. verifySequence { - hankeService.loadHanke(HANKE_TUNNUS) + hankeService.findHankeOrThrow(HANKE_TUNNUS) permissionService.hasPermission(expectedHanke.id!!, USERNAME, PermissionCode.EDIT) hankeService.updateHanke(any()) disclosureLogService.saveDisclosureLogsForHanke(expectedHanke, USERNAME) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt index 8e1061ef5..736c3aebd 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/HankeServiceITests.kt @@ -19,7 +19,6 @@ import fi.hel.haitaton.hanke.application.CableReportWithoutHanke import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.HankeYhteystieto import fi.hel.haitaton.hanke.domain.Hankealue -import fi.hel.haitaton.hanke.domain.Perustaja import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YHTEISO import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YKSITYISHENKILO import fi.hel.haitaton.hanke.domain.YhteystietoTyyppi.YRITYS @@ -29,7 +28,6 @@ import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaj import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistajat import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedRakennuttaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withHankealue -import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withPerustaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankealueFactory import fi.hel.haitaton.hanke.geometria.Geometriat @@ -117,8 +115,7 @@ class HankeServiceITests : DatabaseTest() { @Test fun `create Hanke with full data set succeeds and returns a new domain object with the correct values`() { - val hanke: Hanke = - getATestHanke().withYhteystiedot { it.id = null }.withPerustaja().withHankealue() + val hanke: Hanke = getATestHanke().withYhteystiedot { it.id = null }.withHankealue() val datetimeAlku = hanke.alueet[0].haittaAlkuPvm // nextyear.2.20 23:45:56Z val datetimeLoppu = hanke.alueet[0].haittaLoppuPvm // nextyear.2.21 0:12:34Z @@ -149,7 +146,6 @@ class HankeServiceITests : DatabaseTest() { assertThat(returnedHanke.loppuPvm).isEqualTo(expectedDateLoppu) assertThat(returnedHanke.vaihe).isEqualTo(Vaihe.SUUNNITTELU) assertThat(returnedHanke.suunnitteluVaihe).isEqualTo(SuunnitteluVaihe.RAKENNUS_TAI_TOTEUTUS) - assertThat(returnedHanke.perustaja).isEqualTo(Perustaja("Pertti Perustaja", "foo@bar.com")) assertThat(returnedHanke.tyomaaKatuosoite).isEqualTo("Testikatu 1") assertThat(returnedHanke.tyomaaTyyppi).contains(TyomaaTyyppi.VESI, TyomaaTyyppi.MUU) assertThat(returnedHanke.alueet[0].kaistaHaitta) @@ -210,14 +206,16 @@ class HankeServiceITests : DatabaseTest() { assertThat(rakennuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(firstId) assertThat(toteuttaja.id).isNotEqualTo(rakennuttaja.id) - assertThat(hankeKayttajaRepository.findAll()).hasSize(5) // contacts + Hanke perustaja + assertThat(hankeKayttajaRepository.findAll()).hasSize(4) assertThat(kayttajaTunnisteRepository.findAll()).hasSize(4) } @Test fun `create Hanke with without perustaja and contacts does not create hanke users`() { - hankeService.createHanke(getATestHanke()) + val hanke = hankeService.createHanke(getATestHanke()) + val hankeEntity = hankeRepository.findByHankeTunnus(hanke.hankeTunnus!!)!! + assertThat(hankeEntity.perustaja).isNull() assertThat(hankeKayttajaRepository.findAll()).isEmpty() assertThat(kayttajaTunnisteRepository.findAll()).isEmpty() } @@ -952,10 +950,11 @@ class HankeServiceITests : DatabaseTest() { val application = applications.first() assertThat(hanke.hankeTunnus).isEqualTo(application.hankeTunnus) assertThat(hanke.nimi).isEqualTo(application.applicationData.name) - assertThat(hanke.perustaja) - .isEqualTo(Perustaja("Teppo Testihenkilö", "teppo@example.test")) assertThat(application.applicationData.name) .isEqualTo(inputApplication.applicationData.name) + val hankePerustaja = hankeRepository.findByHankeTunnus(hanke.hankeTunnus!!)?.perustaja + assertThat(hankePerustaja?.nimi).isEqualTo("Teppo Testihenkilö") + assertThat(hankePerustaja?.email).isEqualTo("teppo@example.test") } } @@ -1394,7 +1393,6 @@ class HankeServiceITests : DatabaseTest() { TemplateData( updatedHanke.id!!, updatedHanke.hankeTunnus!!, - updatedHanke.perustaja != null, updatedHanke.alueet[0].id, updatedHanke.alueet[0].geometriat?.id, hankeVersion = 1, @@ -1520,7 +1518,6 @@ class HankeServiceITests : DatabaseTest() { data class TemplateData( val hankeId: Int, val hankeTunnus: String, - val hankePerustaja: Boolean = false, val alueId: Int? = null, val geometriaId: Int? = null, val geometriaVersion: Int = 0, @@ -1550,7 +1547,6 @@ class HankeServiceITests : DatabaseTest() { TemplateData( hanke.id!!, hanke.hankeTunnus!!, - hanke.perustaja != null, alue?.id, alue?.geometriat?.id, geometriaVersion, diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache index a4c9c750a..eeb5e304a 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPoints.json.mustache @@ -22,15 +22,6 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", - {{#hankkeenPerustaja}} - "perustaja": { - "nimi": "Pertti Perustaja", - "email": "foo@bar.com" - }, - {{/hankkeenPerustaja}} - {{^hankkeenPerustaja}} - "perustaja": null, - {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache index 46c019ec5..e44c3a772 100644 --- a/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache +++ b/services/hanke-service/src/integrationTest/resources/fi/hel/haitaton/hanke/logging/expectedHankeWithPolygon.json.mustache @@ -22,15 +22,6 @@ "version": {{hankeVersion}}, "generated": false, "tyomaaKatuosoite": "Testikatu 1", - {{#hankkeenPerustaja}} - "perustaja": { - "nimi": "Pertti Perustaja", - "email": "foo@bar.com" - }, - {{/hankkeenPerustaja}} - {{^hankkeenPerustaja}} - "perustaja": null, - {{/hankkeenPerustaja}} "tyomaaTyyppi": [ "MUU", "VESI" diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt index ea6be220e..d5363b913 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt @@ -180,7 +180,7 @@ class HankeController( if (hanke == null) { throw HankeArgumentException("No hanke given when creating hanke") } - val sanitizedHanke = hanke.copy(id = null, generated = false, perustaja = null) + val sanitizedHanke = hanke.copy(id = null, generated = false) val userId = currentUserId() logger.info { "Creating Hanke for user $userId: ${hanke.toLogString()} " } @@ -235,13 +235,9 @@ class HankeController( logger.info { "Updating Hanke: ${hanke.toLogString()}" } - val existingHanke = - hankeService.loadHanke(hankeTunnus)?.also { - it.verifyUserAuthorization(currentUserId(), PermissionCode.EDIT) - } - ?: throw HankeNotFoundException(hankeTunnus) - - existingHanke.validateUpdatable(hanke, hankeTunnus) + val existingHanke = hankeService.findHankeOrThrow(hankeTunnus) + validateUpdatable(existing = existingHanke, updated = hanke, hankeTunnus) + existingHanke.verifyUserAuthorization(currentUserId(), PermissionCode.EDIT) val updatedHanke = hankeService.updateHanke(hanke) logger.info { "Updated hanke ${updatedHanke.hankeTunnus}." } @@ -309,12 +305,15 @@ class HankeController( } } - private fun Hanke.validateUpdatable(updatedHanke: Hanke, hankeTunnusFromPath: String) { - if (hankeTunnusFromPath != updatedHanke.hankeTunnus) { - throw HankeArgumentException("Hanketunnus not given or doesn't match the hanke data") - } - if (perustaja != null && perustaja != updatedHanke.perustaja) { - throw HankeArgumentException("Updating perustaja not allowed.") + private fun validateUpdatable(existing: Hanke, updated: Hanke, hankeTunnusFromPath: String) { + val tunnusMatch = + listOf(existing.hankeTunnus, updated.hankeTunnus).all { it == hankeTunnusFromPath } + + if (!tunnusMatch) { + throw HankeArgumentException( + "Hanketunnus mismatch. (Existing=${existing.hankeTunnus}, Updated=${updated.hankeTunnus}, " + + "Path=$hankeTunnusFromPath)" + ) } } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index 8371e3b82..88c92c36b 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -113,11 +113,11 @@ open class HankeServiceImpl( override fun loadHankkeetByIds(ids: List) = hankeRepository.findAllById(ids).map { createHankeDomainObjectFromEntity(it) } - /** @return a new Hanke instance with the added and possibly modified values. */ @Transactional - override fun createHanke(hanke: Hanke): Hanke { - // TODO: Only create that hanke-tunnus if a specific set of fields are non-empty/set. + override fun createHanke(hanke: Hanke): Hanke = + createHankeInternal(hanke = hanke, perustaja = null) + private fun createHankeInternal(hanke: Hanke, perustaja: Perustaja?): Hanke { val userId = currentUserId() val entity = HankeEntity() @@ -128,6 +128,8 @@ open class HankeServiceImpl( entity.hankeTunnus = hanketunnus hanke.hankeTunnus = hanketunnus + entity.perustaja = perustaja?.toEntity() + // Copy values from the incoming domain object, and set some internal fields: copyNonNullHankeFieldsToEntity(hanke, entity) @@ -153,8 +155,8 @@ open class HankeServiceImpl( postProcessAndSaveLogging(loggingEntryHolder, savedHankeEntity, userId) - return createHankeDomainObjectFromEntity(entity).also { - initAccessForCreatedHanke(it, userId) + return createHankeDomainObjectFromEntity(savedHankeEntity).also { + initAccessForCreatedHanke(it, savedHankeEntity.perustaja, userId) hankeLoggingService.logCreate(it, userId) } } @@ -242,12 +244,16 @@ open class HankeServiceImpl( !applicationService.isStillPending(it) } - private fun initAccessForCreatedHanke(hanke: Hanke, userId: String) { + private fun initAccessForCreatedHanke( + hanke: Hanke, + perustaja: PerustajaEntity?, + userId: String + ) { val hankeId = hanke.id!! val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - - hanke.perustaja?.let { hankeKayttajaService.addHankeFounder(hankeId, it, permissionAll) } - + perustaja?.let { + hankeKayttajaService.addHankeFounder(hankeId, it.toDomainObject(), permissionAll) + } hankeKayttajaService.saveNewTokensFromHanke(hanke) } @@ -345,7 +351,6 @@ open class HankeServiceImpl( if (hankeEntity.modifiedAt != null) ZonedDateTime.of(hankeEntity.modifiedAt, TZ_UTC) else null, hankeEntity.status, - hankeEntity.perustaja?.toDomainObject(), hankeEntity.generated, ) @@ -571,8 +576,6 @@ open class HankeServiceImpl( hanke.onYKTHanke?.let { entity.onYKTHanke = hanke.onYKTHanke } hanke.nimi?.let { entity.nimi = hanke.nimi } hanke.kuvaus?.let { entity.kuvaus = hanke.kuvaus } - - hanke.perustaja?.let { entity.perustaja = it.toEntity() } entity.generated = hanke.generated hanke.vaihe?.let { entity.vaihe = hanke.vaihe } hanke.suunnitteluVaihe?.let { entity.suunnitteluVaihe = hanke.suunnitteluVaihe } @@ -944,7 +947,7 @@ open class HankeServiceImpl( * - Perustaja generated from application data orderer. */ private fun generateHankeFrom(cableReport: CableReportWithoutHanke): Hanke = - createHanke( + createHankeInternal( Hanke( id = null, hankeTunnus = null, @@ -959,9 +962,9 @@ open class HankeServiceImpl( modifiedBy = null, modifiedAt = null, status = HankeStatus.DRAFT, - perustaja = generatePerustajaFrom(cableReport.applicationData), generated = true, - ) + ), + perustaja = generatePerustajaFrom(cableReport.applicationData), ) private fun generatePerustajaFrom(cableReport: CableReportApplicationData): Perustaja { diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt index b9b78a062..dce4cb99f 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/domain/Hanke.kt @@ -79,10 +79,6 @@ data class Hanke( var status: HankeStatus? = HankeStatus.DRAFT, // @JsonView(ChangeLogView::class) - @field:Schema(description = "Hanke founder contact information") - var perustaja: Perustaja? = null, - // - @JsonView(ChangeLogView::class) @field:Schema(description = "Indicates whether the Hanke data is generated, set by the service") var generated: Boolean = false, ) : HasId { diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt index 76c7e36d4..32e349ff0 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt @@ -175,7 +175,7 @@ class HankeControllerTest { // mock HankeService response Mockito.`when`(hankeService.updateHanke(partialHanke)) .thenReturn(partialHanke.copy(modifiedBy = username, modifiedAt = getCurrentTimeUTC())) - Mockito.`when`(hankeService.loadHanke("id123")) + Mockito.`when`(hankeService.findHankeOrThrow("id123")) .thenReturn(HankeFactory.create(hankeTunnus = "id123")) Mockito.`when`(permissionService.hasPermission(123, username, PermissionCode.EDIT)) .thenReturn(true) diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt index a0ee6c1da..606e61e1d 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/HankeFactory.kt @@ -183,10 +183,6 @@ class HankeFactory( return this } - fun Hanke.withPerustaja( - newPerustaja: Perustaja? = Perustaja("Pertti Perustaja", "foo@bar.com") - ): Hanke = apply { perustaja = newPerustaja } - /** * Add a number of omistaja to a hanke. Generates the yhteystiedot with * [HankeYhteystietoFactory.createDifferentiated] using the given ints for differentiating From 125a9d035359e567c609a90a0e17f3207abd3844 Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 19:26:02 +0300 Subject: [PATCH 12/13] HAI-1844 Streamline perustaja user creation, fix logging, remove redundant check in update validation --- .../kotlin/fi/hel/haitaton/hanke/HankeController.kt | 13 ++++--------- .../fi/hel/haitaton/hanke/HankeServiceImpl.kt | 12 +++--------- .../hanke/permissions/HankeKayttajaService.kt | 2 +- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt index d5363b913..804786851 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt @@ -234,10 +234,9 @@ class HankeController( featureFlags.ensureEnabled(Feature.HANKE_EDITING) logger.info { "Updating Hanke: ${hanke.toLogString()}" } - val existingHanke = hankeService.findHankeOrThrow(hankeTunnus) - validateUpdatable(existing = existingHanke, updated = hanke, hankeTunnus) existingHanke.verifyUserAuthorization(currentUserId(), PermissionCode.EDIT) + validateUpdatable(hanke, hankeTunnus) val updatedHanke = hankeService.updateHanke(hanke) logger.info { "Updated hanke ${updatedHanke.hankeTunnus}." } @@ -305,14 +304,10 @@ class HankeController( } } - private fun validateUpdatable(existing: Hanke, updated: Hanke, hankeTunnusFromPath: String) { - val tunnusMatch = - listOf(existing.hankeTunnus, updated.hankeTunnus).all { it == hankeTunnusFromPath } - - if (!tunnusMatch) { + private fun validateUpdatable(hankeUpdate: Hanke, hankeTunnusFromPath: String) { + if (hankeUpdate.hankeTunnus != hankeTunnusFromPath) { throw HankeArgumentException( - "Hanketunnus mismatch. (Existing=${existing.hankeTunnus}, Updated=${updated.hankeTunnus}, " + - "Path=$hankeTunnusFromPath)" + "Hanketunnus mismatch. (In payload=${hankeUpdate.hankeTunnus}, In path=$hankeTunnusFromPath)" ) } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt index 88c92c36b..0304d2208 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeServiceImpl.kt @@ -156,7 +156,7 @@ open class HankeServiceImpl( postProcessAndSaveLogging(loggingEntryHolder, savedHankeEntity, userId) return createHankeDomainObjectFromEntity(savedHankeEntity).also { - initAccessForCreatedHanke(it, savedHankeEntity.perustaja, userId) + initAccessForCreatedHanke(it, perustaja, userId) hankeLoggingService.logCreate(it, userId) } } @@ -244,16 +244,10 @@ open class HankeServiceImpl( !applicationService.isStillPending(it) } - private fun initAccessForCreatedHanke( - hanke: Hanke, - perustaja: PerustajaEntity?, - userId: String - ) { + private fun initAccessForCreatedHanke(hanke: Hanke, perustaja: Perustaja?, userId: String) { val hankeId = hanke.id!! val permissionAll = permissionService.setPermission(hankeId, userId, Role.KAIKKI_OIKEUDET) - perustaja?.let { - hankeKayttajaService.addHankeFounder(hankeId, it.toDomainObject(), permissionAll) - } + perustaja?.let { hankeKayttajaService.addHankeFounder(hankeId, it, permissionAll) } hankeKayttajaService.saveNewTokensFromHanke(hanke) } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index 5dff5d073..1576604a3 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -51,7 +51,7 @@ class HankeKayttajaService( @Transactional fun addHankeFounder(hankeId: Int, founder: Perustaja, permissionEntity: PermissionEntity) { - logger.info { "Saving token for Hanke perustaja." } + logger.info { "Saving user for Hanke perustaja." } saveUser( HankeKayttajaEntity( hankeId = hankeId, From 9b89c028338816a3c72a92a635e6703a88c0e63b Mon Sep 17 00:00:00 2001 From: Niko Pitkonen Date: Wed, 30 Aug 2023 19:56:33 +0300 Subject: [PATCH 13/13] HAI-1844 Add test for hanke tunnus check --- .../hel/haitaton/hanke/HankeControllerTest.kt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt index 32e349ff0..3a1051144 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/HankeControllerTest.kt @@ -188,6 +188,23 @@ class HankeControllerTest { verify { disclosureLogService.saveDisclosureLogsForHanke(any(), eq(username)) } } + @Test + fun `test that the updateHanke will throw if mismatch in hanke tunnus payload vs path`() { + val hankeUpdate = HankeFactory.create() + val existingHanke = HankeFactory.create(hankeTunnus = "wrong") + Mockito.`when`(hankeService.findHankeOrThrow("wrong")).thenReturn(existingHanke) + Mockito.`when`( + permissionService.hasPermission(existingHanke.id!!, username, PermissionCode.EDIT) + ) + .thenReturn(true) + + assertThatExceptionOfType(HankeArgumentException::class.java) + .isThrownBy { hankeController.updateHanke(hankeUpdate, "wrong") } + .withMessageContaining( + "Hanketunnus mismatch. (In payload=${hankeUpdate.hankeTunnus}, In path=wrong)" + ) + } + @Test fun `test that the updateHanke will give validation errors from invalid hanke data for name`() { val partialHanke =