From 336948ba1cc7ef951d7122a7164a499e5f5e617d Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Jan 2024 12:26:52 +0100 Subject: [PATCH] fixes from Code-Review --- .../HsOfficeCoopAssetsTransactionEntity.java | 1 + .../HsOfficeCoopSharesTransactionEntity.java | 1 + .../changelog/273-hs-office-debitor-rbac.sql | 6 ++-- ...sTransactionRepositoryIntegrationTest.java | 32 +++++++++---------- .../HsOfficeDebitorEntityPatcherUnitTest.java | 12 ++----- .../hs/office/migration/ImportOfficeData.java | 6 ++-- ...OfficePartnerControllerAcceptanceTest.java | 2 +- .../RbacRoleControllerAcceptanceTest.java | 16 ---------- .../RbacUserControllerAcceptanceTest.java | 9 ------ 9 files changed, 27 insertions(+), 58 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java index 7b374e3b..a9ffa347 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java @@ -34,6 +34,7 @@ public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, HasUu .withProp(HsOfficeCoopAssetsTransactionEntity::getTransactionType) .withProp(HsOfficeCoopAssetsTransactionEntity::getAssetValue) .withProp(HsOfficeCoopAssetsTransactionEntity::getReference) + .withProp(HsOfficeCoopAssetsTransactionEntity::getComment) .withSeparator(", ") .quotedValues(false); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java index c2fbf208..883bef77 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java @@ -29,6 +29,7 @@ public class HsOfficeCoopSharesTransactionEntity implements Stringifyable, HasUu .withProp(HsOfficeCoopSharesTransactionEntity::getTransactionType) .withProp(HsOfficeCoopSharesTransactionEntity::getShareCount) .withProp(HsOfficeCoopSharesTransactionEntity::getReference) + .withProp(HsOfficeCoopSharesTransactionEntity::getComment) .withSeparator(", ") .quotedValues(false); diff --git a/src/main/resources/db/changelog/273-hs-office-debitor-rbac.sql b/src/main/resources/db/changelog/273-hs-office-debitor-rbac.sql index fe81e497..ab70aa27 100644 --- a/src/main/resources/db/changelog/273-hs-office-debitor-rbac.sql +++ b/src/main/resources/db/changelog/273-hs-office-debitor-rbac.sql @@ -185,16 +185,16 @@ call generateRbacIdentityView('hs_office_debitor', $idName$ -- ---------------------------------------------------------------------------- call generateRbacRestrictedView('hs_office_debitor', 'target.debitorNumberSuffix', $updates$ - partnerUuid = new.partnerUuid, // TODO: remove? should never do anything + partnerUuid = new.partnerUuid, -- TODO: remove? should never do anything billable = new.billable, billingContactUuid = new.billingContactUuid, - debitorNumberSuffix = new.debitorNumberSuffix, // TODO: Should it be allowed to updated this value? + debitorNumberSuffix = new.debitorNumberSuffix, -- TODO: Should it be allowed to updated this value? refundBankAccountUuid = new.refundBankAccountUuid, vatId = new.vatId, vatCountryCode = new.vatCountryCode, vatBusiness = new.vatBusiness, vatreversecharge = new.vatreversecharge, - defaultPrefix = new.defaultPrefix // TODO: Should it be allowed to updated this value? + defaultPrefix = new.defaultPrefix -- TODO: Should it be allowed to updated this value? $updates$); --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java index 851ccb69..54c5bbb3 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java @@ -144,17 +144,17 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase // then allTheseCoopAssetsTransactionsAreReturned( result, - "CoopAssetsTransaction(10001, 2010-03-15, DEPOSIT, 320.00, ref 10001-1)", - "CoopAssetsTransaction(10001, 2021-09-01, DISBURSAL, -128.00, ref 10001-2)", - "CoopAssetsTransaction(10001, 2022-10-20, ADJUSTMENT, 128.00, ref 10001-3)", + "CoopAssetsTransaction(10001, 2010-03-15, DEPOSIT, 320.00, ref 10001-1, initial deposit)", + "CoopAssetsTransaction(10001, 2021-09-01, DISBURSAL, -128.00, ref 10001-2, partial disbursal)", + "CoopAssetsTransaction(10001, 2022-10-20, ADJUSTMENT, 128.00, ref 10001-3, some adjustment)", - "CoopAssetsTransaction(10002, 2010-03-15, DEPOSIT, 320.00, ref 10002-1)", - "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2)", - "CoopAssetsTransaction(10002, 2022-10-20, ADJUSTMENT, 128.00, ref 10002-3)", + "CoopAssetsTransaction(10002, 2010-03-15, DEPOSIT, 320.00, ref 10002-1, initial deposit)", + "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2, partial disbursal)", + "CoopAssetsTransaction(10002, 2022-10-20, ADJUSTMENT, 128.00, ref 10002-3, some adjustment)", - "CoopAssetsTransaction(10003, 2010-03-15, DEPOSIT, 320.00, ref 10003-1)", - "CoopAssetsTransaction(10003, 2021-09-01, DISBURSAL, -128.00, ref 10003-2)", - "CoopAssetsTransaction(10003, 2022-10-20, ADJUSTMENT, 128.00, ref 10003-3)"); + "CoopAssetsTransaction(10003, 2010-03-15, DEPOSIT, 320.00, ref 10003-1, initial deposit)", + "CoopAssetsTransaction(10003, 2021-09-01, DISBURSAL, -128.00, ref 10003-2, partial disbursal)", + "CoopAssetsTransaction(10003, 2022-10-20, ADJUSTMENT, 128.00, ref 10003-3, some adjustment)"); } @Test @@ -173,9 +173,9 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase // then allTheseCoopAssetsTransactionsAreReturned( result, - "CoopAssetsTransaction(10002, 2010-03-15, DEPOSIT, 320.00, ref 10002-1)", - "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2)", - "CoopAssetsTransaction(10002, 2022-10-20, ADJUSTMENT, 128.00, ref 10002-3)"); + "CoopAssetsTransaction(10002, 2010-03-15, DEPOSIT, 320.00, ref 10002-1, initial deposit)", + "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2, partial disbursal)", + "CoopAssetsTransaction(10002, 2022-10-20, ADJUSTMENT, 128.00, ref 10002-3, some adjustment)"); } @Test @@ -194,7 +194,7 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase // then allTheseCoopAssetsTransactionsAreReturned( result, - "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2)"); + "CoopAssetsTransaction(10002, 2021-09-01, DISBURSAL, -128.00, ref 10002-2, partial disbursal)"); } @Test @@ -211,9 +211,9 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase // then: exactlyTheseCoopAssetsTransactionsAreReturned( result, - "CoopAssetsTransaction(10001, 2010-03-15, DEPOSIT, 320.00, ref 10001-1)", - "CoopAssetsTransaction(10001, 2021-09-01, DISBURSAL, -128.00, ref 10001-2)", - "CoopAssetsTransaction(10001, 2022-10-20, ADJUSTMENT, 128.00, ref 10001-3)"); + "CoopAssetsTransaction(10001, 2010-03-15, DEPOSIT, 320.00, ref 10001-1, initial deposit)", + "CoopAssetsTransaction(10001, 2021-09-01, DISBURSAL, -128.00, ref 10001-2, partial disbursal)", + "CoopAssetsTransaction(10001, 2022-10-20, ADJUSTMENT, 128.00, ref 10001-3, some adjustment)"); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityPatcherUnitTest.java index 1983df59..01ea5777 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityPatcherUnitTest.java @@ -105,7 +105,7 @@ class HsOfficeDebitorEntityPatcherUnitTest extends PatchUnitTestBase< newBillingContact(PATCHED_CONTACT_UUID)) .notNullable(), new SimpleProperty<>( - "personType", + "billable", HsOfficeDebitorPatchResource::setBillable, PATCHED_BILLABLE, HsOfficeDebitorEntity::setBillable) @@ -121,19 +121,13 @@ class HsOfficeDebitorEntityPatcherUnitTest extends PatchUnitTestBase< PATCHED_VAT_COUNTRY_CODE, HsOfficeDebitorEntity::setVatCountryCode), new SimpleProperty<>( - "vatReverseCharge", - HsOfficeDebitorPatchResource::setVatReverseCharge, - PATCHED_VAT_REVERSE_CHARGE, - HsOfficeDebitorEntity::setVatReverseCharge) - .notNullable(), - new SimpleProperty<>( - "personType", + "vatBusiness", HsOfficeDebitorPatchResource::setVatBusiness, PATCHED_VAT_BUSINESS, HsOfficeDebitorEntity::setVatBusiness) .notNullable(), new SimpleProperty<>( - "personType", + "vatReverseCharge", HsOfficeDebitorPatchResource::setVatReverseCharge, PATCHED_BILLABLE, HsOfficeDebitorEntity::setVatReverseCharge) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java b/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java index b7770284..bb08f2e7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/migration/ImportOfficeData.java @@ -297,8 +297,6 @@ public class ImportOfficeData extends ContextBasedTest { @Test @Order(1040) void importCoopShares() { - assumeThat(postgresAdminUser).isEqualTo("admin"); - try (Reader reader = resourceReader("migration/share-transactions.csv")) { final var lines = readAllLines(reader); importCoopShares(justHeader(lines), withoutHeader(lines)); @@ -312,7 +310,7 @@ public class ImportOfficeData extends ContextBasedTest { void verifyCoopShares() { assumeThat(postgresAdminUser).isEqualTo("admin"); - assertThat(coopShares.toString()).isEqualToIgnoringWhitespace(""" + assertThat(toFormattedString(coopShares)).isEqualToIgnoringWhitespace(""" { 33443=CoopShareTransaction(10017, 2000-12-06, SUBSCRIPTION, 20, initial share subscription), 33451=CoopShareTransaction(10020, 2000-12-06, SUBSCRIPTION, 2, initial share subscription), @@ -339,7 +337,7 @@ public class ImportOfficeData extends ContextBasedTest { void verifyCoopAssets() { assumeThat(postgresAdminUser).isEqualTo("admin"); - assertThat(coopAssets.toString()).isEqualToIgnoringWhitespace(""" + assertThat(toFormattedString(coopAssets)).isEqualToIgnoringWhitespace(""" { 30000=CoopAssetsTransaction(10017, 2000-12-06, DEPOSIT, 1280.00, for subscription A), 31000=CoopAssetsTransaction(10020, 2000-12-06, DEPOSIT, 128.00, for subscription B), diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java index 0ac1eb2d..359c078d 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java @@ -297,7 +297,7 @@ class HsOfficePartnerControllerAcceptanceTest { .contentType(ContentType.JSON) .body(""" { - "debitorNumerPrefix": "12345", + "debitorNumberPrefix": "12345", "contactUuid": "%s", "personUuid": "%s", "details": { diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleControllerAcceptanceTest.java index 389b0f12..5de93348 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacrole/RbacRoleControllerAcceptanceTest.java @@ -7,14 +7,9 @@ import net.hostsharing.hsadminng.rbac.rbacuser.RbacUserRepository; import net.hostsharing.test.Accepts; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.server.LocalServerPort; -import jakarta.persistence.EntityManager; -import jakarta.persistence.PersistenceContext; - -import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.*; @SpringBootTest( @@ -27,9 +22,6 @@ class RbacRoleControllerAcceptanceTest { @LocalServerPort private Integer port; - @PersistenceContext - EntityManager em; - @Autowired Context context; @@ -39,14 +31,6 @@ class RbacRoleControllerAcceptanceTest { @Autowired RbacRoleRepository rbacRoleRepository; - @Value("${HSADMINNG_POSTGRES_RESTRICTED_USERNAME}") - String restrictedUser; - - @Test - void testEnv() { - assertThat(restrictedUser).isEqualTo("restricted"); - } - @Test @Accepts({ "ROL:L(List)" }) void globalAdmin_withoutAssumedRole_canViewAllRoles() { diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java index 9f2fd68e..76d738ad 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/rbacuser/RbacUserControllerAcceptanceTest.java @@ -9,7 +9,6 @@ import net.hostsharing.test.JpaAttempt; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.server.LocalServerPort; import org.springframework.transaction.annotation.Transactional; @@ -43,14 +42,6 @@ class RbacUserControllerAcceptanceTest { @Autowired RbacUserRepository rbacUserRepository; - @Value("${HSADMINNG_POSTGRES_RESTRICTED_USERNAME}") - String restrictedUser; - - @Test - void testEnv() { - assertThat(restrictedUser).isEqualTo("restricted"); - } - @Nested class CreateRbacUser {