From e335fa8f78a69ae80f6a8ecc84e56cf53ec612f7 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Tue, 12 Nov 2024 17:12:42 -0500 Subject: [PATCH 01/10] integration_tests: updated test_user_callerid with expectations related to new phone number resource --- .../suite/base/test_user_callerid.py | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/integration_tests/suite/base/test_user_callerid.py b/integration_tests/suite/base/test_user_callerid.py index 5f72639fd..3ef6432be 100644 --- a/integration_tests/suite/base/test_user_callerid.py +++ b/integration_tests/suite/base/test_user_callerid.py @@ -25,39 +25,31 @@ def test_list_when_no_incall(user): assert_that(response.total, equal_to(1)) -@fixtures.extension(exten='5555551234', context=INCALL_CONTEXT) -@fixtures.incall() @fixtures.extension(exten='5555556789', context=INCALL_CONTEXT) @fixtures.incall() @fixtures.user() -def test_list_with_associated_type(extension1, incall1, extension2, incall2, user): +def test_list_with_associated_type(extension, incall, user): destination = {'type': 'user', 'user_id': user['id']} - confd.incalls(incall1['id']).put(destination=destination).assert_updated() - confd.incalls(incall2['id']).put(destination=destination).assert_updated() + confd.incalls(incall['id']).put(destination=destination).assert_updated() - with a.incall_extension(incall1, extension1): - with a.incall_extension(incall2, extension2): - response = confd.users(user['uuid']).callerids.outgoing.get() + with a.incall_extension(incall, extension): + response = confd.users(user['uuid']).callerids.outgoing.get() expected = [ - {'type': 'main', 'number': '5555551234'}, - {'type': 'associated', 'number': '5555551234'}, {'type': 'associated', 'number': '5555556789'}, {'type': 'anonymous'}, ] assert_that(response.items, contains_inanyorder(*expected)) - assert_that(response.total, equal_to(4)) + assert_that(response.total, equal_to(2)) -@fixtures.extension(exten='5555551234', context=INCALL_CONTEXT) -@fixtures.incall(destination={'type': 'custom', 'command': 'Playback(Welcome)'}) +@fixtures.phone_number(main=True, number='5555551234') @fixtures.extension(exten='5555556789', context=INCALL_CONTEXT) @fixtures.incall(destination={'type': 'custom', 'command': 'Playback(IGNORED)'}) @fixtures.user() -def test_list_with_main_type(extension1, incall1, extension2, incall2, user): - with a.incall_extension(incall1, extension1): - with a.incall_extension(incall2, extension2): - response = confd.users(user['uuid']).callerids.outgoing.get() +def test_list_with_main_type(phone_number, extension, incall, user): + with a.incall_extension(incall, extension): + response = confd.users(user['uuid']).callerids.outgoing.get() # The first created is the main and other are ignored expected = [ @@ -68,26 +60,40 @@ def test_list_with_main_type(extension1, incall1, extension2, incall2, user): assert_that(response.total, equal_to(2)) -@fixtures.extension(exten='5555551234', context=INCALL_CONTEXT) -@fixtures.incall(destination={'type': 'custom', 'command': 'Playback(Welcome)'}) +@fixtures.phone_number(shared=True, number='5555551234') +@fixtures.user() +def test_list_with_shared(phone_number, user): + response = confd.users(user['uuid']).callerids.outgoing.get() + + # The first created is the main and other are ignored + expected = [ + {'type': 'shared', 'number': '5555551234'}, + {'type': 'anonymous'}, + ] + assert_that(response.items, contains_inanyorder(*expected)) + assert_that(response.total, equal_to(2)) + + +@fixtures.phone_number(main=True, number='5555551234') +@fixtures.phone_number(shared=True, number='5555551235') @fixtures.extension(exten='5555556789', context=INCALL_CONTEXT) @fixtures.incall() @fixtures.user() -def test_list_with_all_type(main_extension, main_incall, extension, incall, user): +def test_list_with_all_type(main_number, shared_number, extension, incall, user): destination = {'type': 'user', 'user_id': user['id']} confd.incalls(incall['id']).put(destination=destination).assert_updated() - with a.incall_extension(main_incall, main_extension): - with a.incall_extension(incall, extension): - response = confd.users(user['uuid']).callerids.outgoing.get() + with a.incall_extension(incall, extension): + response = confd.users(user['uuid']).callerids.outgoing.get() expected = [ {'type': 'main', 'number': '5555551234'}, + {'type': 'shared', 'number': '5555551235'}, {'type': 'associated', 'number': '5555556789'}, {'type': 'anonymous'}, ] assert_that(response.items, contains_inanyorder(*expected)) - assert_that(response.total, equal_to(3)) + assert_that(response.total, equal_to(4)) @fixtures.user(wazo_tenant=MAIN_TENANT) From 7a6b4023a52f1139963e2573adbdbd9644345bf4 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Tue, 12 Nov 2024 17:13:46 -0500 Subject: [PATCH 02/10] user_callerid: updated service to use phone_number for main, and added 'shared' caller id type --- wazo_confd/plugins/user_callerid/service.py | 30 ++++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/wazo_confd/plugins/user_callerid/service.py b/wazo_confd/plugins/user_callerid/service.py index b1f16fda2..6a6c90917 100644 --- a/wazo_confd/plugins/user_callerid/service.py +++ b/wazo_confd/plugins/user_callerid/service.py @@ -3,25 +3,47 @@ from xivo_dao.resources.user import dao as user_dao from xivo_dao.resources.incall import dao as incall_dao +from xivo_dao.resources.phone_number import dao as phone_number_dao +from dataclasses import dataclass +from typing import Literal + + +CallerIDType = Literal['main', 'associated', 'anonymous', 'shared'] class CallerIDAnonymous: type = 'anonymous' +@dataclass() +class CallerID: + type: CallerIDType + number: str + + class UserCallerIDService: - def __init__(self, user_dao, incall_dao): + def __init__(self, user_dao, incall_dao, phone_number_dao): self.user_dao = user_dao self.incall_dao = incall_dao + self.phone_number_dao = phone_number_dao def search(self, user_id, tenant_uuid, parameters): callerids = [] - if main_callerid := self.incall_dao.find_main_callerid(tenant_uuid): - callerids.append(main_callerid) + if main_callerid := self.phone_number_dao.find_by( + main=True, tenant_uuids=[tenant_uuid] + ): + callerids.append(CallerID(type='main', number=main_callerid.number)) + shared_callerids = self.phone_number_dao.find_all_by( + shared=True, main=False, tenant_uuids=[tenant_uuid] + ) + callerids.extend( + CallerID(type='shared', number=callerid.number) + for callerid in shared_callerids + ) callerids.extend(self.user_dao.list_outgoing_callerid_associated(user_id)) callerids.append(CallerIDAnonymous) return len(callerids), callerids def build_service(): - return UserCallerIDService(user_dao, incall_dao) + return UserCallerIDService(user_dao, incall_dao, phone_number_dao) From 4f16f2e31a2f44887a4e6bb92a6d4398ff0ad655 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 12:00:04 -0500 Subject: [PATCH 03/10] added 'phonenumbers' library dependency --- debian/control | 1 + requirements.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/debian/control b/debian/control index 21d742177..55b5ad7a2 100644 --- a/debian/control +++ b/debian/control @@ -24,6 +24,7 @@ Depends: python3-requests, python3-werkzeug, python3-tz, + python3-phonenumbers, wazo-auth-client-python3, wazo-provd-client-python3, wazo-bus-python3, diff --git a/requirements.txt b/requirements.txt index 45e3afb93..39317466b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,3 +25,4 @@ sqlalchemy-utils==0.36.8 # from xivo-dao stevedore==4.0.2 unidecode==1.2.0 # from xivo-dao werkzeug==1.0.1 +phonenumbers==8.12.1 \ No newline at end of file From a77778a451db8c871ed08bb0ee34eb344339a4d6 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 12:01:38 -0500 Subject: [PATCH 04/10] integration_tests: test_user_callerid verify deduplication in case of phone number used as user incall but also a shared callerid --- integration_tests/suite/base/test_user_callerid.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/integration_tests/suite/base/test_user_callerid.py b/integration_tests/suite/base/test_user_callerid.py index 3ef6432be..91857e336 100644 --- a/integration_tests/suite/base/test_user_callerid.py +++ b/integration_tests/suite/base/test_user_callerid.py @@ -76,15 +76,21 @@ def test_list_with_shared(phone_number, user): @fixtures.phone_number(main=True, number='5555551234') @fixtures.phone_number(shared=True, number='5555551235') +@fixtures.extension(exten='5555551235', context=INCALL_CONTEXT) +@fixtures.incall() @fixtures.extension(exten='5555556789', context=INCALL_CONTEXT) @fixtures.incall() @fixtures.user() -def test_list_with_all_type(main_number, shared_number, extension, incall, user): +def test_list_with_all_type( + main_number, shared_number, extension, incall, extension2, incall2, user +): destination = {'type': 'user', 'user_id': user['id']} confd.incalls(incall['id']).put(destination=destination).assert_updated() + confd.incalls(incall2['id']).put(destination=destination).assert_updated() with a.incall_extension(incall, extension): - response = confd.users(user['uuid']).callerids.outgoing.get() + with a.incall_extension(incall2, extension2): + response = confd.users(user['uuid']).callerids.outgoing.get() expected = [ {'type': 'main', 'number': '5555551234'}, From 5909ee6cf2de002332ee6b3a4e7cc5fdc7a55644 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 12:04:48 -0500 Subject: [PATCH 05/10] user_callerid: deduplicate callerids between incalls and phone numbers why: a phone number may be shared as a callerid but also used as a user DID --- wazo_confd/plugins/user_callerid/service.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/wazo_confd/plugins/user_callerid/service.py b/wazo_confd/plugins/user_callerid/service.py index 6a6c90917..d8e7e5570 100644 --- a/wazo_confd/plugins/user_callerid/service.py +++ b/wazo_confd/plugins/user_callerid/service.py @@ -6,7 +6,7 @@ from xivo_dao.resources.phone_number import dao as phone_number_dao from dataclasses import dataclass from typing import Literal - +import phonenumbers CallerIDType = Literal['main', 'associated', 'anonymous', 'shared'] @@ -21,6 +21,17 @@ class CallerID: number: str +def same_phone_number(number1: str, number2: str) -> bool: + ''' + compare two strings semantically as phone numbers + ''' + result = phonenumbers.is_number_match(number1, number2) + if result == phonenumbers.MatchType.NO_MATCH: + return False + else: + return True + + class UserCallerIDService: def __init__(self, user_dao, incall_dao, phone_number_dao): self.user_dao = user_dao @@ -40,7 +51,11 @@ def search(self, user_id, tenant_uuid, parameters): CallerID(type='shared', number=callerid.number) for callerid in shared_callerids ) - callerids.extend(self.user_dao.list_outgoing_callerid_associated(user_id)) + callerids.extend( + callerid + for callerid in self.user_dao.list_outgoing_callerid_associated(user_id) + if not any(same_phone_number(callerid.number, c.number) for c in callerids) + ) callerids.append(CallerIDAnonymous) return len(callerids), callerids From 62c2624895003cd903be5fe26b62ce3172bc52fb Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 15:51:51 -0500 Subject: [PATCH 06/10] user_callerid: test coverage for same_phone_number --- .../plugins/user_callerid/tests/__init__.py | 0 .../user_callerid/tests/test_service.py | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 wazo_confd/plugins/user_callerid/tests/__init__.py create mode 100644 wazo_confd/plugins/user_callerid/tests/test_service.py diff --git a/wazo_confd/plugins/user_callerid/tests/__init__.py b/wazo_confd/plugins/user_callerid/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/wazo_confd/plugins/user_callerid/tests/test_service.py b/wazo_confd/plugins/user_callerid/tests/test_service.py new file mode 100644 index 000000000..b529f55a0 --- /dev/null +++ b/wazo_confd/plugins/user_callerid/tests/test_service.py @@ -0,0 +1,25 @@ +import unittest + +from ..service import same_phone_number + + +class TestSamePhoneNumber(unittest.TestCase): + def test_exact(self): + numbers = ['1234567890', '11234567890', '+11234567890' '4567890', '911'] + for number in numbers: + self.assertTrue(same_phone_number(number, number), number) + + def test_actually_different(self): + number1 = '11234567890' + number2 = '11234567891' + self.assertFalse(same_phone_number(number1, number2), (number1, number2)) + + def test_different_country(self): + number1 = '11234567890' + number2 = '21234567890' + self.assertFalse(same_phone_number(number1, number2), (number1, number2)) + + def test_different_country_one_e164(self): + number1 = '+11234567890' + number2 = '21234567890' + self.assertFalse(same_phone_number(number1, number2), (number1, number2)) From d7249ef7e118c4221bf6884d2f8de1021f2a54d1 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 15:52:33 -0500 Subject: [PATCH 07/10] user_callerid: same_phone_number changed to only accept EXACT_MATCH and NSN_MATCH --- wazo_confd/plugins/user_callerid/service.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wazo_confd/plugins/user_callerid/service.py b/wazo_confd/plugins/user_callerid/service.py index d8e7e5570..8b79759b0 100644 --- a/wazo_confd/plugins/user_callerid/service.py +++ b/wazo_confd/plugins/user_callerid/service.py @@ -26,10 +26,10 @@ def same_phone_number(number1: str, number2: str) -> bool: compare two strings semantically as phone numbers ''' result = phonenumbers.is_number_match(number1, number2) - if result == phonenumbers.MatchType.NO_MATCH: - return False - else: - return True + return result in ( + phonenumbers.MatchType.EXACT_MATCH, + phonenumbers.MatchType.NSN_MATCH, + ) class UserCallerIDService: From f50615c2507d1be1bb9e73ab730207d4c8fdd32f Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 15:55:09 -0500 Subject: [PATCH 08/10] user_callerid: extracted type alias to types module --- wazo_confd/plugins/user_callerid/service.py | 8 ++++---- wazo_confd/plugins/user_callerid/types.py | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 wazo_confd/plugins/user_callerid/types.py diff --git a/wazo_confd/plugins/user_callerid/service.py b/wazo_confd/plugins/user_callerid/service.py index 8b79759b0..987f50122 100644 --- a/wazo_confd/plugins/user_callerid/service.py +++ b/wazo_confd/plugins/user_callerid/service.py @@ -1,14 +1,14 @@ # Copyright 2024 The Wazo Authors (see the AUTHORS file) # SPDX-License-Identifier: GPL-3.0-or-later +import phonenumbers +from dataclasses import dataclass + from xivo_dao.resources.user import dao as user_dao from xivo_dao.resources.incall import dao as incall_dao from xivo_dao.resources.phone_number import dao as phone_number_dao -from dataclasses import dataclass -from typing import Literal -import phonenumbers -CallerIDType = Literal['main', 'associated', 'anonymous', 'shared'] +from .types import CallerIDType class CallerIDAnonymous: diff --git a/wazo_confd/plugins/user_callerid/types.py b/wazo_confd/plugins/user_callerid/types.py new file mode 100644 index 000000000..ea407b33d --- /dev/null +++ b/wazo_confd/plugins/user_callerid/types.py @@ -0,0 +1,3 @@ +from typing import Literal + +CallerIDType = Literal['main', 'associated', 'anonymous', 'shared'] From 1e3e1ab3a66f686171717bcb20e516b35f5b9ce6 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Wed, 13 Nov 2024 16:52:56 -0500 Subject: [PATCH 09/10] user_callerid: changed caller ids precedence order why: makes more sense to ensure 'direct' numbers are displayed as such --- wazo_confd/plugins/user_callerid/service.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/wazo_confd/plugins/user_callerid/service.py b/wazo_confd/plugins/user_callerid/service.py index 987f50122..ad559cc82 100644 --- a/wazo_confd/plugins/user_callerid/service.py +++ b/wazo_confd/plugins/user_callerid/service.py @@ -44,16 +44,20 @@ def search(self, user_id, tenant_uuid, parameters): main=True, tenant_uuids=[tenant_uuid] ): callerids.append(CallerID(type='main', number=main_callerid.number)) + + # consider "associated" caller ids from incalls + # as having precedence over shared phone numbers + callerids.extend( + callerid + for callerid in self.user_dao.list_outgoing_callerid_associated(user_id) + if not any(same_phone_number(callerid.number, c.number) for c in callerids) + ) shared_callerids = self.phone_number_dao.find_all_by( shared=True, main=False, tenant_uuids=[tenant_uuid] ) callerids.extend( CallerID(type='shared', number=callerid.number) for callerid in shared_callerids - ) - callerids.extend( - callerid - for callerid in self.user_dao.list_outgoing_callerid_associated(user_id) if not any(same_phone_number(callerid.number, c.number) for c in callerids) ) callerids.append(CallerIDAnonymous) From ff784acf79943b06bc6db45b653d9fe78ed8a1f5 Mon Sep 17 00:00:00 2001 From: Charles Langlois Date: Thu, 14 Nov 2024 10:00:37 -0500 Subject: [PATCH 10/10] integration_tests: test_user_callerid: associated/shared caller id precedence --- integration_tests/suite/base/test_user_callerid.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/integration_tests/suite/base/test_user_callerid.py b/integration_tests/suite/base/test_user_callerid.py index 91857e336..d56a5b09f 100644 --- a/integration_tests/suite/base/test_user_callerid.py +++ b/integration_tests/suite/base/test_user_callerid.py @@ -76,26 +76,23 @@ def test_list_with_shared(phone_number, user): @fixtures.phone_number(main=True, number='5555551234') @fixtures.phone_number(shared=True, number='5555551235') +@fixtures.phone_number(shared=True, number='5555551236') @fixtures.extension(exten='5555551235', context=INCALL_CONTEXT) @fixtures.incall() -@fixtures.extension(exten='5555556789', context=INCALL_CONTEXT) -@fixtures.incall() @fixtures.user() def test_list_with_all_type( - main_number, shared_number, extension, incall, extension2, incall2, user + main_number, shared_number1, shared_number2, extension, incall, user ): destination = {'type': 'user', 'user_id': user['id']} confd.incalls(incall['id']).put(destination=destination).assert_updated() - confd.incalls(incall2['id']).put(destination=destination).assert_updated() with a.incall_extension(incall, extension): - with a.incall_extension(incall2, extension2): - response = confd.users(user['uuid']).callerids.outgoing.get() + response = confd.users(user['uuid']).callerids.outgoing.get() expected = [ {'type': 'main', 'number': '5555551234'}, - {'type': 'shared', 'number': '5555551235'}, - {'type': 'associated', 'number': '5555556789'}, + {'type': 'associated', 'number': '5555551235'}, + {'type': 'shared', 'number': '5555551236'}, {'type': 'anonymous'}, ] assert_that(response.items, contains_inanyorder(*expected))