From 0d8a9a925eab31c0662e0bab1f53d483465c3ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Sch=C3=A4r?= Date: Fri, 25 Oct 2024 19:38:32 +0200 Subject: [PATCH 1/3] Improve duplicate recognition --- app/Repository/Member/MemberMatch.php | 95 +++++++++++++++---- .../Repository/Member/MemberMatchTest.php | 48 +++++++++- 2 files changed, 122 insertions(+), 21 deletions(-) diff --git a/app/Repository/Member/MemberMatch.php b/app/Repository/Member/MemberMatch.php index f5c5e3e..eddcbf2 100644 --- a/app/Repository/Member/MemberMatch.php +++ b/app/Repository/Member/MemberMatch.php @@ -29,7 +29,7 @@ class MemberMatch const NO_MATCH = 0; /** - * It's an unambiguous match of exaclty one member + * It's an unambiguous match of exactly one member */ const MATCH = 1; @@ -69,7 +69,7 @@ private function __construct(int $status, array $matches) $this->status = $status; $this->matches = $matches; } - + /** * Find duplicate members in given group or its subgroups * @@ -100,38 +100,71 @@ public static function match( if (!empty($matches) && $member->firstName->getValue()) { self::selectByFirstName($matches, $member->firstName->getValue()); } - + if (count($matches)) { return self::create($matches, false); } } - + + // find by phone and return if found + if ($member->mobilePhone->getValue()) { + $query = self::buildMobilePhoneQuery($member); + $matches = self::matchByQuery($query, $rootGroups, $memberRepository); + if (!empty($matches) && $member->firstName->getValue()) { + self::selectByFirstName($matches, $member->firstName->getValue()); + } + + if (count($matches)) { + return self::create($matches, false); + } + } + // don't proceed, if we don't have first and last name if (!($member->firstName->getValue() && $member->lastName->getValue())) { return new MemberMatch(self::NO_MATCH, []); } - + // search webling by first and last name $query = self::buildNameQuery($member); $matches = self::matchByQuery($query, $rootGroups, $memberRepository); - + if (!empty($matches)) { // make sure we filter out all entries where only the beginning of the // name was identical, but the given name was no a short name self::removeWrongNameMatches($matches, $member->firstName->getValue(), $member->lastName->getValue()); } - + if (!empty($matches) && $member->zip->getValue()) { // filter out all results, where the zip didn't match self::removeWrongZipMatches($matches, $member->zip->getValue()); return self::create($matches, false); } - - return self::create($matches, true); + + // if there is more information available in the record, we create an ambiguous match + if (!empty($matches) && self::hasAdditionalInformation($member)) { + return self::create($matches, true); + } + + return new MemberMatch(self::NO_MATCH, []); } - + + /** + * Check if the member has a phone number or address assigned + * + * @param Member $member + * + * @return bool + */ + private static function hasAdditionalInformation(Member $member): bool { + return $member->mobilePhone->getValue() || + $member->landlinePhone->getValue() || + $member->workPhone->getValue() || + $member->address1->getValue() || + $member->address2->getValue(); + } + /** * Return webling query to find members by email * @@ -142,22 +175,41 @@ public static function match( private static function buildEmailQuery(Member $member): string { // as webling compares casesensitive, transform everything to lower case. - + $email1 = mb_strtolower(self::escape($member->email1->getWeblingValue())); $email2 = mb_strtolower(self::escape($member->email2->getWeblingValue())); - + $query = []; if ($member->email1->getWeblingValue()) { $query[] = "(LOWER(`{$member->email1->getWeblingKey()}`) = '$email1' OR LOWER(`{$member->email2->getWeblingKey()}`) = '$email1')"; } - + if ($member->email2->getWeblingValue()) { $query[] = "(LOWER(`{$member->email1->getWeblingKey()}`) = '$email2' OR LOWER(`{$member->email2->getWeblingKey()}`) = '$email2')"; } - + return implode(' OR ', $query); } - + + /** + * Return webling query to find members by mobile phone + * + * @param Member $member + * + * @return string + */ + private static function buildMobilePhoneQuery(Member $member): string + { + $mobilePhone = self::normalizePhoneNumber($member->mobilePhone->getWeblingValue()); + + $query = []; + if ($member->mobilePhone->getWeblingValue()) { + $query[] = "`{$member->mobilePhone->getWeblingKey()}` = '$mobilePhone'"; + } + + return implode(' OR ', $query); + } + /** * Return members that matched the given query and log exceptions that * should not occur here. @@ -342,7 +394,18 @@ private static function removeWrongZipMatches(array &$matches, string $zip) } } } - + + /** + * Normalize a phone number by removing non-digits and standardizing format. + * + * @param string $phoneNumber + * @return string + */ + private static function normalizePhoneNumber(string $phoneNumber): string + { + return preg_replace('/\D/', '', str_replace('+41', '0', $phoneNumber)); + } + /** * Escape single quotes * diff --git a/tests/Unit/Repository/Member/MemberMatchTest.php b/tests/Unit/Repository/Member/MemberMatchTest.php index f9e352e..b281c31 100644 --- a/tests/Unit/Repository/Member/MemberMatchTest.php +++ b/tests/Unit/Repository/Member/MemberMatchTest.php @@ -167,7 +167,7 @@ public function test__match() */ $member = $this->getNewMember(); $member->email1->setValue(''); - + // no match $member1 = $this->saveMember(clone $member); $member1->firstName->setValue('unknown'); @@ -176,16 +176,25 @@ public function test__match() $this->assertEmpty($match->getMatches()); $this->assertEquals(0, $match->count()); $this->memberRepo->delete($member1); - - // ambiguous match + + // no zip, no address $member1 = $this->saveMember(clone $member); $member1->zip->setValue(''); + $member1->mobilePhone->setValue(''); + $match = MemberMatch::match($member1, [$this->group], $this->memberRepo); + $this->assertEquals(MemberMatch::NO_MATCH, $match->getStatus()); + $this->assertEmpty($match->getMatches()); + $this->assertEquals(0, $match->count()); + + // ambiguous match happens only if there is additional information available + $member1->address1->setValue('123 Test Street'); + $member1->mobilePhone->setValue(''); $match = MemberMatch::match($member1, [$this->group], $this->memberRepo); $this->assertEquals(MemberMatch::AMBIGUOUS_MATCH, $match->getStatus()); $this->assertEquals($member1->id, $match->getMatches()[0]->id); $this->assertEquals(1, $match->count()); $this->memberRepo->delete($member1); - + // single match $member1 = $this->saveMember($member); $match = MemberMatch::match($member, [$this->group], $this->memberRepo); @@ -211,8 +220,37 @@ public function test__match() $this->assertEmpty($match->getMatches()); $this->assertEquals(0, $match->count()); $this->memberRepo->delete($member1); + + /** + * match by name and phone number + * phone number is only used if there is no email + */ + $member->mobilePhone->setValue('0761234567'); + + // correct phone number + $member1 = $this->saveMember(clone $member); + $member1->zip->setValue(''); + $member1->address1->setValue(''); + $member1->mobilePhone->setValue('+41 76 123 45 67'); + $match = MemberMatch::match($member1, [$this->group], $this->memberRepo); + $this->assertEquals(MemberMatch::MATCH, $match->getStatus()); + $this->assertEquals($member1->id, $match->getMatches()[0]->id); + $this->assertEquals(1, $match->count()); + $this->memberRepo->delete($member1); + + // wrong phone number + $member1 = $this->saveMember(clone $member); + $member1->zip->setValue(''); + $member1->address1->setValue(''); + $member1->mobilePhone->setValue('0797654321'); + $match = MemberMatch::match($member1, [$this->group], $this->memberRepo); + $this->assertEquals(MemberMatch::AMBIGUOUS_MATCH, $match->getStatus()); + $this->memberRepo->delete($member1); - // multiple matches + /** + * multiple matches + */ + $member->mobilePhone->setValue(''); $member1 = $this->saveMember($member); $member2 = $this->saveMember($member); $match = MemberMatch::match($member, [$this->group], $this->memberRepo); From 158fc931363a2eb5b7941f050d13c4be57e64ba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Sch=C3=A4r?= Date: Fri, 25 Oct 2024 19:41:31 +0200 Subject: [PATCH 2/3] fix typo --- app/Repository/Member/MemberMatch.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Repository/Member/MemberMatch.php b/app/Repository/Member/MemberMatch.php index eddcbf2..46ccac9 100644 --- a/app/Repository/Member/MemberMatch.php +++ b/app/Repository/Member/MemberMatch.php @@ -366,7 +366,7 @@ private static function removeWrongNameMatches(array &$matches, string $firstNam { foreach ($matches as $idx => $match) { if (!self::isShortNameOf($firstName, $match->firstName->getValue()) - && !self::isShortNameOf($match->firstName->getValue(), $lastName)) { + && !self::isShortNameOf($match->firstName->getValue(), $firstName)) { unset($matches[$idx]); } if (!self::isShortNameOf($lastName, $match->lastName->getValue()) From f0fe203a4bd36cab38032838c5730ebb98f6a076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Sch=C3=A4r?= Date: Mon, 18 Nov 2024 15:49:41 +0100 Subject: [PATCH 3/3] Fix RestApiMemberTest --- .../Http/Controllers/RestApi/RestApiMemberTest.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Http/Controllers/RestApi/RestApiMemberTest.php b/tests/Feature/Http/Controllers/RestApi/RestApiMemberTest.php index 326b50d..976c3df 100644 --- a/tests/Feature/Http/Controllers/RestApi/RestApiMemberTest.php +++ b/tests/Feature/Http/Controllers/RestApi/RestApiMemberTest.php @@ -1143,6 +1143,9 @@ public function testPostMatch_200_match_apostrophe() ], 'lastName' => [ 'value' => $member->lastName->getValue(), + ], + 'email1' => [ + 'value' => $member->email1->getValue(), ] ]; @@ -1167,6 +1170,9 @@ public function testPostMatch_200_match_ampersand() ], 'lastName' => [ 'value' => $member->lastName->getValue(), + ], + 'email1' => [ + 'value' => $member->email1->getValue(), ] ]; @@ -1182,7 +1188,8 @@ public function testPostMatch_200_match_ampersand() public function testPostMatch_200_ambiguous() { $member = $this->getMember(__METHOD__); - + $member->address1->setValue("Rue de l'Annonciade 22"); + $m = [ 'firstName' => [ 'value' => $member->firstName->getValue(), @@ -1190,6 +1197,9 @@ public function testPostMatch_200_ambiguous() 'lastName' => [ 'value' => $member->lastName->getValue(), ], + 'address1' => [ + 'value' => $member->address1->getValue(), + ] ]; // precondition: assert we dont have any records with the same name