From 7347b2a5fd5984e7f3410b60fb471fe479f52651 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 30 Apr 2024 12:05:42 +0200 Subject: [PATCH] amendmends according to code-review --- .../hs-office/hs-office-contact-schemas.yaml | 15 +++++++++- .../501-contact/5010-hs-office-contact.sql | 2 +- .../5018-hs-office-contact-test-data.sql | 2 +- ...OfficeContactControllerAcceptanceTest.java | 16 +++++----- .../HsOfficeContactEntityPatcherUnitTest.java | 12 ++++---- ...OfficeDebitorControllerAcceptanceTest.java | 10 +++---- .../hs/office/migration/ImportOfficeData.java | 30 +++++++++---------- 7 files changed, 50 insertions(+), 37 deletions(-) diff --git a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml index 6d9f2a46..5905bdf4 100644 --- a/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml +++ b/src/main/resources/api-definition/hs-office/hs-office-contact-schemas.yaml @@ -56,4 +56,17 @@ components: # forces generating a java.lang.Object containing a Map, instead of class HsOfficeContactEmailAddresses anyOf: - type: object - additionalProperties: true + properties: + phone_office: + type: string + nullable: true + phone_private: + type: string + nullable: true + phone_mobile: + type: string + nullable: true + fax: + type: string + nullable: true + additionalProperties: false diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql index 8e45f583..ca875a89 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5010-hs-office-contact.sql @@ -11,7 +11,7 @@ create table if not exists hs_office_contact label varchar(128) not null, postalAddress text, emailAddresses jsonb not null, - phoneNumbers text + phoneNumbers jsonb not null ); --// diff --git a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql index 2a989593..e9e7a9e0 100644 --- a/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/501-contact/5018-hs-office-contact-test-data.sql @@ -32,7 +32,7 @@ begin contLabel, postalAddr, ('{ "main": "' || emailAddr || '" }')::jsonb, - ('{ "office": "+49 123 1234567" }')::jsonb + ('{ "phone_office": "+49 123 1234567" }')::jsonb ); end; $$; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java index 0b53d0b1..1b209737 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java @@ -188,7 +188,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "main": "contact-admin@firstcontact.example.com" }, "phoneNumbers": { - "office": "+49 123 1234567" + "phone_office": "+49 123 1234567" } } """)); // @formatter:on @@ -216,7 +216,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "postalAddress": "Patched Address", "phoneNumbers": { - "office": "+01 100 123456" + "phone_office": "+01 100 123456" } } """) @@ -230,7 +230,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is("Temp patched contact")) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is("Patched Address")) - .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); + .body("phoneNumbers", is(Map.of("phone_office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated @@ -240,7 +240,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(person.getLabel()).isEqualTo("Temp patched contact"); assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo("Patched Address"); - assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("phone_office", "+01 100 123456")); return true; }); } @@ -261,7 +261,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu "main": "patched@example.org" }, "phoneNumbers": { - "office": "+01 100 123456" + "phone_office": "+01 100 123456" } } """) @@ -275,7 +275,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .body("label", is(givenContact.getLabel())) .body("emailAddresses", is(Map.of("main", "patched@example.org"))) .body("postalAddress", is(givenContact.getPostalAddress())) - .body("phoneNumbers", is(Map.of("office", "+01 100 123456"))); + .body("phoneNumbers", is(Map.of("phone_office", "+01 100 123456"))); // @formatter:on // finally, the contact is actually updated @@ -284,7 +284,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu assertThat(person.getLabel()).isEqualTo(givenContact.getLabel()); assertThat(person.getEmailAddresses()).containsExactlyEntriesOf(Map.of("main", "patched@example.org")); assertThat(person.getPostalAddress()).isEqualTo(givenContact.getPostalAddress()); - assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("office", "+01 100 123456")); + assertThat(person.getPhoneNumbers()).containsExactlyEntriesOf(Map.of("phone_office", "+01 100 123456")); return true; }); } @@ -357,7 +357,7 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .label("Temp from " + Context.getCallerMethodNameFromStackFrame(1) ) .emailAddresses(Map.of("main", RandomStringUtils.randomAlphabetic(10) + "@example.org")) .postalAddress("Postal Address " + RandomStringUtils.randomAlphabetic(10)) - .phoneNumbers(Map.of("office", "+01 200 " + RandomStringUtils.randomNumeric(8))) + .phoneNumbers(Map.of("phone_office", "+01 200 " + RandomStringUtils.randomNumeric(8))) .build(); return contactRepo.save(newContact); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java index ce565cc7..a9c20958 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntityPatcherUnitTest.java @@ -31,13 +31,13 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< ); private static final Map PATCH_PHONE_NUMBERS = patchMap( - entry("mobile", null), - entry("private", "+49 40 987654321"), + entry("phone_mobile", null), + entry("phone_private", "+49 40 987654321"), entry("fax", "+49 40 12345-99") ); private static final Map PATCHED_PHONE_NUMBERS = patchMap( - entry("office", "+49 40 12345-00"), - entry("private", "+49 40 987654321"), + entry("phone_office", "+49 40 12345-00"), + entry("phone_private", "+49 40 987654321"), entry("fax", "+49 40 12345-99") ); @@ -51,8 +51,8 @@ class HsOfficeContactEntityPatcherUnitTest extends PatchUnitTestBase< entry("paul", "paul@example.com"), entry("mila", "mila@example.com"))); entity.putPhoneNumbers(Map.ofEntries( - entry("office", "+49 40 12345-00"), - entry("mobile", "+49 1555 1234567"), + entry("phone_office", "+49 40 12345-00"), + entry("phone_mobile", "+49 1555 1234567"), entry("fax", "+49 40 12345-90"))); entity.setPostalAddress("Initialstraße 50\n20000 Hamburg"); return entity; diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 71505f38..9bda7ec4 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -108,7 +108,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -133,7 +133,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "contact": { "label": "first contact", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "details": { @@ -231,7 +231,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu } @Test - void globalAdmin_withoutAssumedRoles_canFindDebitorDebitorByDebitorNumber() throws JSONException { + void globalAdmin_withoutAssumedRoles_canFindDebitorDebitorByDebitorNumber() { RestAssured // @formatter:off .given() @@ -466,7 +466,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "debitorNumber": 1000111, @@ -482,7 +482,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "label": "first contact", "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "office": "+49 123 1234567" } + "phoneNumbers": { "phone_office": "+49 123 1234567" } } }, "details": { 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 5b3ee744..9bb44f3d 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 @@ -390,16 +390,16 @@ public class ImportOfficeData extends ContextBasedTest { assertThat(toFormattedString(coopAssets)).isEqualToIgnoringWhitespace(""" { - 30000=CoopAssetsTransaction(M-1001700: 2000-12-06, DEPOSIT, 1280.00, legacy data import, for subscription A), - 31000=CoopAssetsTransaction(M-1002000: 2000-12-06, DEPOSIT, 128.00, legacy data import, for subscription B), - 32000=CoopAssetsTransaction(M-1001700: 2005-01-10, DEPOSIT, 2560.00, legacy data import, for subscription C), - 33001=CoopAssetsTransaction(M-1001700: 2005-01-10, TRANSFER, -512.00, legacy data import, for transfer to 10), - 33002=CoopAssetsTransaction(M-1002000: 2005-01-10, ADOPTION, 512.00, legacy data import, for transfer from 7), - 34001=CoopAssetsTransaction(M-1002000: 2016-12-31, CLEARING, -8.00, legacy data import, for cancellation D), - 34002=CoopAssetsTransaction(M-1002000: 2016-12-31, DISBURSAL, -100.00, legacy data import, for cancellation D), - 34003=CoopAssetsTransaction(M-1002000: 2016-12-31, LOSS, -20.00, legacy data import, for cancellation D), - 35001=CoopAssetsTransaction(M-1909000: 2024-01-15, DEPOSIT, 128.00, legacy data import, for subscription E), - 35002=CoopAssetsTransaction(M-1909000: 2024-01-20, ADJUSTMENT, -128.00, legacy data import, chargeback for subscription E, M-1909000:DEP:+128.00) + 30000=CoopAssetsTransaction(M-1001700: 2000-12-06, DEPOSIT, 1280.00, 1001700, for subscription A), + 31000=CoopAssetsTransaction(M-1002000: 2000-12-06, DEPOSIT, 128.00, 1002000, for subscription B), + 32000=CoopAssetsTransaction(M-1001700: 2005-01-10, DEPOSIT, 2560.00, 1001700, for subscription C), + 33001=CoopAssetsTransaction(M-1001700: 2005-01-10, TRANSFER, -512.00, 1001700, for transfer to 10), + 33002=CoopAssetsTransaction(M-1002000: 2005-01-10, ADOPTION, 512.00, 1002000, for transfer from 7), + 34001=CoopAssetsTransaction(M-1002000: 2016-12-31, CLEARING, -8.00, 1002000, for cancellation D), + 34002=CoopAssetsTransaction(M-1002000: 2016-12-31, DISBURSAL, -100.00, 1002000, for cancellation D), + 34003=CoopAssetsTransaction(M-1002000: 2016-12-31, LOSS, -20.00, 1002000, for cancellation D), + 35001=CoopAssetsTransaction(M-1909000: 2024-01-15, DEPOSIT, 128.00, 1909000, for subscription E), + 35002=CoopAssetsTransaction(M-1909000: 2024-01-20, ADJUSTMENT, -128.00, 1909000, chargeback for subscription E, M-1909000:DEP:+128.00) } """); } @@ -867,7 +867,7 @@ public class ImportOfficeData extends ContextBasedTest { .transactionType(assetTypeMapping.get(rec.getString("action"))) .assetValue(rec.getBigDecimal("amount")) .comment(rec.getString("comment")) - .reference("legacy data import") // TODO.: Kundennummer + .reference(member.getMemberNumber().toString()) .build(); if (assetTransaction.getTransactionType() == HsOfficeCoopAssetsTransactionType.ADJUSTMENT) { @@ -1123,13 +1123,13 @@ public class ImportOfficeData extends ContextBasedTest { private Map toPhoneNumbers(final Record rec) { final var phoneNumbers = new LinkedHashMap(); if (isNotBlank(rec.getString("phone_private"))) - phoneNumbers.put("privat", rec.getString("phone_private")); + phoneNumbers.put("phone_private", rec.getString("phone_private")); if (isNotBlank(rec.getString("phone_office"))) - phoneNumbers.put("geschäftlich", rec.getString("phone_office")); + phoneNumbers.put("phone_office", rec.getString("phone_office")); if (isNotBlank(rec.getString("phone_mobile"))) - phoneNumbers.put("mobil", rec.getString("phone_mobile")); + phoneNumbers.put("phone_mobile", rec.getString("phone_mobile")); if (isNotBlank(rec.getString("fax"))) - phoneNumbers.put("Fax", rec.getString("fax")); + phoneNumbers.put("fax", rec.getString("fax")); return phoneNumbers; }