From 19b319993cb566ceace63c1a6a558351293c5696 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Fri, 14 Jun 2024 17:17:17 +0200 Subject: [PATCH 1/3] Make AddressChangeId comparable Add `equals` method to `AddressChangeId`. This allows to compare IDs with resorting to casting to string or using the `==` (value equality) operator. --- src/Domain/Model/AddressChangeId.php | 4 ++++ tests/Unit/Domain/Model/AddressChangeIdTest.php | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Domain/Model/AddressChangeId.php b/src/Domain/Model/AddressChangeId.php index 302f6f3..b32dcfa 100644 --- a/src/Domain/Model/AddressChangeId.php +++ b/src/Domain/Model/AddressChangeId.php @@ -25,4 +25,8 @@ public function __toString(): string { return $this->identifier; } + public function equals( string|AddressChangeId $id ): bool { + return is_string( $id ) ? $this->identifier === $id : $this->identifier === $id->identifier; + } + } diff --git a/tests/Unit/Domain/Model/AddressChangeIdTest.php b/tests/Unit/Domain/Model/AddressChangeIdTest.php index 0982bf6..46e1edc 100644 --- a/tests/Unit/Domain/Model/AddressChangeIdTest.php +++ b/tests/Unit/Domain/Model/AddressChangeIdTest.php @@ -22,7 +22,7 @@ public function testConstructorAcceptValidUuids(): void { /** * @dataProvider invalidUUIDProvider */ - public function testPersonalAddressChangeThrowsExceptionsWhenUUIDIsInvalid( string $invalidUUID ): void { + public function testThrowsExceptionsWhenUUIDIsInvalid( string $invalidUUID ): void { $this->expectException( \InvalidArgumentException::class ); AddressChangeId::fromString( $invalidUUID ); } @@ -37,4 +37,19 @@ public static function invalidUUIDProvider(): \Generator { yield [ 'e-f-f-e-d' ]; yield [ 'This-is-not-a-UUID' ]; } + + public function testCanCheckEqualityWithStrings(): void { + $uuid = '72dfed91-fa40-4af0-9e80-c6010ab29cd1'; + $addressChangeId = AddressChangeId::fromString( $uuid ); + + $this->assertTrue( $addressChangeId->equals( $uuid ), 'IDs should compare equals' ); + } + + public function testCanCheckEqualityWithOtherID(): void { + $uuid = '72dfed91-fa40-4af0-9e80-c6010ab29cd1'; + $addressChangeId = AddressChangeId::fromString( $uuid ); + $addressChangeIdWithSameUUID = AddressChangeId::fromString( $uuid ); + + $this->assertTrue( $addressChangeId->equals( $addressChangeIdWithSameUUID ), 'IDs should compare equals' ); + } } From e0b7a008ec428dd6112b848866495a89459dad03 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Fri, 14 Jun 2024 17:19:06 +0200 Subject: [PATCH 2/3] Add `hasBeenUsed` method to `AddressChange` When an address change record is freshly initialized, it has the same current and previous identifier. Each change to the record will change the ID. This introduces the concept of a "used" record. This is slightly different that the less granular concept of "isModified". We'll use this in the Fundraising Application. --- src/Domain/Model/AddressChange.php | 4 ++++ tests/Unit/Domain/Model/AddressChangeTest.php | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/Domain/Model/AddressChange.php b/src/Domain/Model/AddressChange.php index 76dec1d..c93cea3 100644 --- a/src/Domain/Model/AddressChange.php +++ b/src/Domain/Model/AddressChange.php @@ -115,6 +115,10 @@ public function isModified(): bool { return $this->createdAt < $this->modifiedAt; } + public function hasBeenUsed(): bool { + return !$this->getCurrentIdentifier()->equals( $this->previousIdentifier ); + } + private function markAsModified( AddressChangeId $newIdentifier ): void { $this->previousIdentifier = $this->getCurrentIdentifier(); $this->identifier = $newIdentifier; diff --git a/tests/Unit/Domain/Model/AddressChangeTest.php b/tests/Unit/Domain/Model/AddressChangeTest.php index e24c288..cdef0b6 100644 --- a/tests/Unit/Domain/Model/AddressChangeTest.php +++ b/tests/Unit/Domain/Model/AddressChangeTest.php @@ -132,4 +132,24 @@ public function testUsedAndExportedAddressReturnsCorrectExportState(): void { $this->assertEquals( AddressChange::EXPORT_STATE_USED_EXPORTED, $addressChange->getExportState() ); } + + public function testNewAddressChangesAreUnused(): void { + $addressChange = new AddressChange( AddressType::Person, AddressChange::EXTERNAL_ID_TYPE_DONATION, self::DUMMY_DONATION_ID, $this->identifier ); + + $this->assertFalse( $addressChange->hasBeenUsed() ); + } + + public function testAddressChangesWithAddressesAreUsed(): void { + $addressChange = new AddressChange( AddressType::Person, AddressChange::EXTERNAL_ID_TYPE_DONATION, self::DUMMY_DONATION_ID, $this->identifier ); + $addressChange->performAddressChange( ValidAddress::newValidPersonalAddress(), $this->newIdentifier ); + + $this->assertTrue( $addressChange->hasBeenUsed() ); + } + + public function testAddressChangesWithOptOutAreUsed(): void { + $addressChange = new AddressChange( AddressType::Person, AddressChange::EXTERNAL_ID_TYPE_DONATION, self::DUMMY_DONATION_ID, $this->identifier ); + $addressChange->optOutOfDonationReceipt( $this->newIdentifier ); + + $this->assertTrue( $addressChange->hasBeenUsed() ); + } } From 44a541dedc4de4def50a8f675d9cde83ec228f50 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Fri, 14 Jun 2024 17:51:53 +0200 Subject: [PATCH 3/3] Keep identical address change IDs An address change record allows two changes: - setting an address - opting out of the donation receipt Each of those actions takes an `AddressChangeId` and "rotates" the current ID to the `previousIdentifier`. When doing both changes, the Id should only be rotated once, to avoid `hasBeenUsed` returning the wrong value and to be able to check in the frontend if the previous ID still returns a (used) address changed record. --- src/Domain/Model/AddressChange.php | 6 ++++-- tests/Unit/Domain/Model/AddressChangeTest.php | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Domain/Model/AddressChange.php b/src/Domain/Model/AddressChange.php index c93cea3..6e4489f 100644 --- a/src/Domain/Model/AddressChange.php +++ b/src/Domain/Model/AddressChange.php @@ -120,8 +120,10 @@ public function hasBeenUsed(): bool { } private function markAsModified( AddressChangeId $newIdentifier ): void { - $this->previousIdentifier = $this->getCurrentIdentifier(); - $this->identifier = $newIdentifier; + if ( !$this->getCurrentIdentifier()->equals( $newIdentifier ) ) { + $this->previousIdentifier = $this->getCurrentIdentifier(); + $this->identifier = $newIdentifier; + } $this->modifiedAt = new \DateTime(); $this->resetExportState(); diff --git a/tests/Unit/Domain/Model/AddressChangeTest.php b/tests/Unit/Domain/Model/AddressChangeTest.php index cdef0b6..73ee99c 100644 --- a/tests/Unit/Domain/Model/AddressChangeTest.php +++ b/tests/Unit/Domain/Model/AddressChangeTest.php @@ -152,4 +152,15 @@ public function testAddressChangesWithOptOutAreUsed(): void { $this->assertTrue( $addressChange->hasBeenUsed() ); } + + public function testMultipleModificationsWithTheSameIdentifierKeepsIdentifiers(): void { + $addressChange = $this->newPersonAddressChange(); + $initialIdentifier = $addressChange->getCurrentIdentifier(); + + $addressChange->performAddressChange( ValidAddress::newValidPersonalAddress(), $this->newIdentifier ); + $addressChange->optOutOfDonationReceipt( $this->newIdentifier ); + + $this->assertEquals( $initialIdentifier, $addressChange->getPreviousIdentifier() ); + $this->assertEquals( $this->newIdentifier, $addressChange->getCurrentIdentifier() ); + } }