fix fixme's

This commit is contained in:
Michael Hoennig 2024-03-27 18:17:10 +01:00
parent 8bc3c17b89
commit d236a7aca4
4 changed files with 81 additions and 69 deletions

View File

@ -9,6 +9,7 @@ import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity;
import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationRepository;
import net.hostsharing.hsadminng.mapper.Mapper;
import org.apache.commons.lang3.Validate;
import org.hibernate.Hibernate;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
@ -150,6 +151,7 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi {
new HsOfficeDebitorEntityPatcher(em, current).apply(body);
final var saved = debitorRepo.save(current);
Hibernate.initialize(saved);
final var mapped = mapper.map(saved, HsOfficeDebitorResource.class);
return ResponseEntity.ok(mapped);
}

View File

@ -545,7 +545,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu
class PatchDebitor {
@Test
void globalAdmin_withoutAssumedRole_canPatchAllPropertiesOfArbitraryDebitor() {
void globalAdmin_withoutAssumedRole_canPatchArbitraryDebitor() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = givenSomeTemporaryDebitor();
@ -567,17 +567,48 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu
.port(port)
.when()
.patch("http://localhost/api/hs/office/debitors/" + givenDebitor.getUuid())
.then().assertThat()
.then().log().all().assertThat()
.statusCode(200)
.contentType(ContentType.JSON)
.body("uuid", isUuidValid())
.body("vatId", is("VAT222222"))
.body("vatCountryCode", is("AA"))
.body("vatBusiness", is(true))
.body("defaultPrefix", is("for"))
// FIXME .body("billingContact.label", is(givenContact.getLabel()))
// FIXME .body("partner.partnerRel.holder.tradeName", is(givenDebitor.getPartner().getPartnerRel().getHolder().getTradeName()))
;
.body("", lenientlyEquals("""
{
"debitorRel": {
"anchor": { "tradeName": "Fourth eG" },
"holder": { "tradeName": "Fourth eG" },
"type": "DEBITOR",
"mark": null,
"contact": { "label": "fourth contact" }
},
"debitorNumber": 10004${debitorNumberSuffix},
"debitorNumberSuffix": ${debitorNumberSuffix},
"partner": {
"partnerNumber": 10004,
"partnerRel": {
"anchor": { "tradeName": "Hostsharing eG" },
"holder": { "tradeName": "Fourth eG" },
"type": "PARTNER",
"mark": null,
"contact": { "label": "fourth contact" }
},
"details": {
"registrationOffice": "Hamburg",
"registrationNumber": "RegNo123456789",
"birthName": null,
"birthPlace": null,
"birthday": null,
"dateOfDeath": null
}
},
"billable": true,
"vatId": "VAT222222",
"vatCountryCode": "AA",
"vatBusiness": true,
"vatReverseCharge": false,
"defaultPrefix": "for"
}
"""
.replace("${debitorNumberSuffix}", givenDebitor.getDebitorNumberSuffix().toString()))
);
// @formatter:on
// finally, the debitor is actually updated
@ -595,48 +626,31 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu
}
@Test
void globalAdmin_withoutAssumedRole_canPatchPartialPropertiesOfArbitraryDebitor() {
void theContactOwner_canNotPatchARelatedDebitor() {
context.define("superuser-alex@hostsharing.net");
final var givenDebitor = givenSomeTemporaryDebitor();
final var newBillingContact = contactRepo.findContactByOptionalLabelLike("sixth").get(0);
final var location = RestAssured // @formatter:off
.given()
// @formatter:on
RestAssured // @formatter:off
.given()
.header("current-user", "superuser-alex@hostsharing.net")
.header("assumed-roles", "hs_office_contact#fourthcontact.admin")
.contentType(ContentType.JSON)
.body("""
{
"billingContactUuid": "%s",
"vatId": "VAT999999"
}
""".formatted(newBillingContact.getUuid()))
{
"vatId": "VAT999999"
}
""")
.port(port)
.when()
.when()
.patch("http://localhost/api/hs/office/debitors/" + givenDebitor.getUuid())
.then().assertThat()
.statusCode(200)
.contentType(ContentType.JSON)
.body("uuid", isUuidValid())
// FIXME .body("billingContact.label", is("sixth contact"))
.body("vatId", is("VAT999999"))
.body("vatCountryCode", is(givenDebitor.getVatCountryCode()))
.body("vatBusiness", is(givenDebitor.isVatBusiness()));
// @formatter:on
.then().log().all().assertThat()
.statusCode(403)
.body("message", containsString("ERROR: [403] Subject"))
.body("message", containsString("is not allowed to update hs_office_debitor uuid "));
// finally, the debitor is actually updated
assertThat(debitorRepo.findByUuid(givenDebitor.getUuid())).isPresent().get()
.matches(partner -> {
assertThat(partner.getDebitorRel().getHolder().getTradeName())
.isEqualTo(givenDebitor.getDebitorRel().getHolder().getTradeName());
// FIXME assertThat(partner.getDebitorRel().getContact().getLabel()).isEqualTo("sixth contact");
assertThat(partner.getVatId()).isEqualTo("VAT999999");
assertThat(partner.getVatCountryCode()).isEqualTo(givenDebitor.getVatCountryCode());
assertThat(partner.isVatBusiness()).isEqualTo(givenDebitor.isVatBusiness());
return true;
});
}
}
@Nested

View File

@ -13,6 +13,7 @@ import net.hostsharing.hsadminng.rbac.rbacgrant.RbacGrantsDiagramService;
import net.hostsharing.hsadminng.rbac.rbacrole.RawRbacRoleRepository;
import net.hostsharing.test.Array;
import net.hostsharing.test.JpaAttempt;
import org.hibernate.Hibernate;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
@ -316,7 +317,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
assertThatDebitorIsVisibleForUserWithRole(
givenDebitor,
"hs_office_relation#FourtheG-with-DEBITOR-FourtheG.admin");
"hs_office_relation#FourtheG-with-DEBITOR-FourtheG.admin", true);
final var givenNewPartnerPerson = one(personRepo.findPersonByOptionalNameLike("First"));
final var givenNewBillingPerson = one(personRepo.findPersonByOptionalNameLike("Firby"));
final var givenNewContact = one(contactRepo.findContactByOptionalLabelLike("sixth contact"));
@ -345,7 +346,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
result.assertSuccessful();
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"global#global.admin");
"global#global.admin", true);
// ... partner role was reassigned:
assertThatDebitorIsNotVisibleForUserWithRole(
@ -353,7 +354,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
"hs_office_relation#FourtheG-with-DEBITOR-FourtheG.admin");
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan.agent");
"hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan.agent", true);
// ... contact role was reassigned:
assertThatDebitorIsNotVisibleForUserWithRole(
@ -361,7 +362,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
"hs_office_contact#fifthcontact.admin");
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"hs_office_contact#sixthcontact.admin");
"hs_office_contact#sixthcontact.admin", false);
// ... bank-account role was reassigned:
assertThatDebitorIsNotVisibleForUserWithRole(
@ -369,7 +370,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
"hs_office_bankaccount#DE02200505501015871393.admin");
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"hs_office_bankaccount#DE02120300000000202051.admin");
"hs_office_bankaccount#DE02120300000000202051.admin", true);
}
@Test
@ -379,8 +380,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", null, "fig");
assertThatDebitorIsVisibleForUserWithRole(
givenDebitor,
"hs_office_relation#FourtheG-with-DEBITOR-FourtheG.admin");
assertThatDebitorActuallyInDatabase(givenDebitor);
"hs_office_relation#FourtheG-with-DEBITOR-FourtheG.admin", true);
assertThatDebitorActuallyInDatabase(givenDebitor, true);
final var givenNewBankAccount = one(bankAccountRepo.findByOptionalHolderLike("first"));
// when
@ -394,12 +395,12 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
result.assertSuccessful();
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"global#global.admin");
"global#global.admin", true);
// ... bank-account role was assigned:
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"hs_office_bankaccount#DE02120300000000202051.admin");
"hs_office_bankaccount#DE02120300000000202051.admin", true);
}
@Test
@ -409,8 +410,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", "Fourth", "fih");
assertThatDebitorIsVisibleForUserWithRole(
givenDebitor,
"hs_office_relation#HostsharingeG-with-PARTNER-FourtheG.agent");
assertThatDebitorActuallyInDatabase(givenDebitor);
"hs_office_relation#HostsharingeG-with-PARTNER-FourtheG.agent", true);
assertThatDebitorActuallyInDatabase(givenDebitor, true);
// when
final var result = jpaAttempt.transacted(() -> {
@ -423,7 +424,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
result.assertSuccessful();
assertThatDebitorIsVisibleForUserWithRole(
result.returnedValue(),
"global#global.admin");
"global#global.admin", true);
// ... bank-account role was removed from previous bank-account admin:
assertThatDebitorIsNotVisibleForUserWithRole(
@ -438,8 +439,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "eighth", "Fourth", "eig");
assertThatDebitorIsVisibleForUserWithRole(
givenDebitor,
"hs_office_relation#HostsharingeG-with-PARTNER-FourtheG.agent");
assertThatDebitorActuallyInDatabase(givenDebitor);
"hs_office_relation#HostsharingeG-with-PARTNER-FourtheG.agent", true);
assertThatDebitorActuallyInDatabase(givenDebitor, true);
// when
final var result = jpaAttempt.transacted(() -> {
@ -458,10 +459,10 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
// given
context("superuser-alex@hostsharing.net");
final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "ninth", "Fourth", "nin");
assertThatDebitorActuallyInDatabase(givenDebitor, true);
assertThatDebitorIsVisibleForUserWithRole(
givenDebitor,
"hs_office_contact#ninthcontact.admin");
assertThatDebitorActuallyInDatabase(givenDebitor);
"hs_office_contact#ninthcontact.admin", false);
// when
final var result = jpaAttempt.transacted(() -> {
@ -471,21 +472,20 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
});
// then
// FIXME: This error message would be better:
// result.assertExceptionWithRootCauseMessage(JpaSystemException.class,
// "[403] Subject ", " is not allowed to update hs_office_debitor uuid");
result.assertExceptionWithRootCauseMessage(
JpaObjectRetrievalFailureException.class,
// this technical error message gets translated to a [403] error at the controller level
"Unable to find net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity with id ");
}
private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved) {
private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved, final boolean withPartner) {
final var found = debitorRepo.findByUuid(saved.getUuid());
assertThat(found).isNotEmpty();
found.ifPresent(foundEntity -> {
em.refresh(foundEntity);
Hibernate.initialize(foundEntity);
assertThat(foundEntity).isNotSameAs(saved);
if ( saved.getPartner() != null) { // FIXME: check, why there is no partner for the updated contact
if (withPartner) {
assertThat(foundEntity.getPartner()).isNotNull();
}
assertThat(foundEntity.getDebitorRel()).extracting(HsOfficeRelationEntity::toString)
@ -495,10 +495,11 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean
private void assertThatDebitorIsVisibleForUserWithRole(
final HsOfficeDebitorEntity entity,
final String assumedRoles) {
final String assumedRoles,
final boolean withPartner) {
jpaAttempt.transacted(() -> {
context("superuser-alex@hostsharing.net", assumedRoles);
assertThatDebitorActuallyInDatabase(entity);
assertThatDebitorActuallyInDatabase(entity, withPartner);
}).assertSuccessful();
}

View File

@ -1171,11 +1171,6 @@ class Record {
return value == null || value.isBlank();
}
Byte getByte(final String columnName) {
final String value = getString(columnName);
return isNotBlank(value) ? Byte.valueOf(value.trim()) : 0;
}
boolean getBoolean(final String columnName) {
final String value = getString(columnName);
return isNotBlank(value) &&