Skip to content

Commit b712157

Browse files
committed
fix(portaswitch): paginate contacts over filtered account set (WT-1774)
In ACCOUNTS mode the single-customer non-search branch computed the page offset in post-_passes_filter contacts but passed it to get_account_list, whose offset counts raw PortaBilling rows. Every page consumed more raw rows than the next page's offset advanced by, so consecutive pages overlapped by a margin that grew with the page number. items_total was PortaBilling's unfiltered total, so the client built pages that did not exist and the last one came back empty. - Fetch every account of every office in parallel 1000-row chunks and paginate in memory; for a customer outside an office hierarchy all_i_customers == [main_i_customer], so this collapses the former hierarchy, large-page and incremental branches into one. - Order the accounts by i_account before slicing: get_account_list is never sent an ORDER BY, so row order across separate calls is unspecified. Search results keep their relevance order. - One pagination path for all sub-modes, so the slice offset and items_total always describe the same row set. - De-duplicate by i_account in _get_all_accounts_by_customer. Also fixes items_per_page == 1000 returning the first page for every page (>= when fetching vs > when paginating, unreachable given the le=1000 cap in main.py), and the fetch loop being skipped entirely when CONTACTS_CUSTOM held at least items_per_page entries.
1 parent 7202392 commit b712157

2 files changed

Lines changed: 444 additions & 72 deletions

File tree

app/bss/adapters/portaswitch/adapter.py

Lines changed: 45 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,6 @@ async def _get_aliases_for_ext(ext):
895895
# Get custom contacts (needed for both search and non-search modes)
896896
custom_contacts = [Serializer.get_contact_info_by_custom_entry(entry) for entry in
897897
self._portaswitch_settings.CONTACTS_CUSTOM]
898-
custom_contacts_count = len(custom_contacts)
899898

900899
# When filtering by extension and in a hierarchy, use the unified extensions list
901900
# (get_main_office_extensions returns extensions across all offices) as the filter
@@ -1015,57 +1014,29 @@ async def _fetch_ext_account(i_acc):
10151014
logging.debug(f"Failed to search by alias DID {search_stripped}: {e}")
10161015

10171016
accounts = list(accounts_dict.values())
1018-
elif is_hierarchy:
1019-
# Multi-customer: fetch all offices in parallel and paginate in-memory
1017+
else:
1018+
# Non-search: fetch every account of every office in parallel chunks of
1019+
# 1000 and paginate in-memory. API-level LIMIT/OFFSET cannot be used here
1020+
# (WT-1774): the offset would have to be expressed in raw PortaBilling
1021+
# rows, while pages are counted in post-`_passes_filter` contacts. Mixing
1022+
# the two made consecutive pages overlap by a margin that grew with the
1023+
# page number. For a customer outside an office hierarchy
1024+
# all_i_customers == [main_i_customer], so this is a single fan-out.
10201025
accounts_per_customer = await _gather_limited(
10211026
[self._get_all_accounts_by_customer(c) for c in all_i_customers]
10221027
)
1023-
accounts = [acc for accs in accounts_per_customer for acc in accs]
1024-
else:
1025-
# Single customer: use API-level pagination for efficiency.
1026-
# PortaBilling's documented per-call maximum is 1000; requesting more silently
1027-
# returns at most 1000 records. When items_per_page >= 1000, use chunked
1028-
# fetching to guarantee correct in-memory pagination across the full dataset.
1029-
if items_per_page >= MAX_API_LIMIT:
1030-
# Page size at or above PortaBilling's per-request maximum: fetch all accounts
1031-
# in chunks of 1000 and paginate in-memory.
1032-
accounts = await self._get_all_accounts_by_customer(main_i_customer)
1033-
total_count_from_api = 0
1034-
else:
1035-
# Normal page: loop until we have exactly `target` filtered accounts.
1036-
# A single fetch with a fixed buffer is insufficient when many accounts
1037-
# are filtered (blocked, current user, no extension): the buffer may be
1038-
# exhausted before we reach `target` filtered results.
1039-
if page == 1 and custom_contacts_count > 0:
1040-
target = items_per_page - custom_contacts_count
1041-
fetch_offset = 0
1042-
else:
1043-
target = items_per_page
1044-
fetch_offset = max(0, (page - 1) * items_per_page - custom_contacts_count)
1045-
1046-
accounts = []
1047-
total_count_from_api = 0
1048-
while len(accounts) < target:
1049-
needed = target - len(accounts)
1050-
result = await self._admin_api.get_account_list(
1051-
main_i_customer,
1052-
limit=min(needed + FILTER_BUFFER, MAX_API_LIMIT),
1053-
offset=fetch_offset,
1054-
)
1055-
total_count_from_api = result.get("total", 0)
1056-
batch = result.get("account_list") or []
1057-
if not batch:
1058-
break
1059-
for account in batch:
1060-
if _passes_filter(account):
1061-
accounts.append(account)
1062-
if len(accounts) >= target:
1063-
break
1064-
if len(batch) < needed + FILTER_BUFFER:
1065-
break # PortaBilling has no more records
1066-
fetch_offset += len(batch)
1067-
1068-
# Filter accounts (for search/hierarchy paths; single-customer loop pre-filters)
1028+
# Order deterministically: get_account_list is never sent an ORDER BY,
1029+
# so the row order of separate calls — the parallel chunks here, and the
1030+
# next page's own request — is unspecified. Sorting on i_account makes
1031+
# page boundaries reproducible across requests. Search results are left
1032+
# alone: their order is relevance, the fields being probed in priority
1033+
# order (id, firstname, lastname, extension_name, email).
1034+
accounts = sorted(
1035+
(acc for accs in accounts_per_customer for acc in accs),
1036+
key=lambda a: int(a["i_account"]),
1037+
)
1038+
1039+
# Filter accounts
10691040
filtered_accounts = [a for a in accounts if _passes_filter(a)]
10701041

10711042
# Build contacts from accounts
@@ -1085,30 +1056,23 @@ async def _fetch_ext_account(i_acc):
10851056
search_lower in (contact.numbers.main or "").lower())
10861057
]
10871058

1088-
# Add custom contacts (only on first page for non-search / hierarchy modes)
1089-
if search or is_hierarchy or page == 1:
1090-
account_contacts.extend(custom_contacts)
1059+
# Add custom contacts; they tail the directory on the last page
1060+
account_contacts.extend(custom_contacts)
10911061

1092-
# Apply pagination
1093-
if search or is_hierarchy or items_per_page > MAX_API_LIMIT:
1094-
# In-memory pagination for search, multi-customer hierarchy, and large pages.
1095-
# In search mode: use the API-reported total (max across fields) as total_count
1096-
# rather than len(account_contacts), because the bounded fetch only retrieves
1097-
# enough records for the current page — len() would severely undercount.
1098-
if search:
1099-
total_count = max(search_total_from_api, len(account_contacts))
1100-
else:
1101-
total_count = len(account_contacts)
1102-
start_idx = (page - 1) * items_per_page
1103-
end_idx = start_idx + items_per_page
1104-
contacts = account_contacts[start_idx:end_idx]
1062+
# Apply pagination. One code path for every sub-mode so that the slice offset
1063+
# and items_total always describe the same row set (WT-1774): reporting
1064+
# PortaBilling's unfiltered total made the client build pages that do not
1065+
# exist, hence the empty last page.
1066+
# In search mode: use the API-reported total (max across fields) as total_count
1067+
# rather than len(account_contacts), because the bounded fetch only retrieves
1068+
# enough records for the current page — len() would severely undercount.
1069+
if search:
1070+
total_count = max(search_total_from_api, len(account_contacts))
11051071
else:
1106-
# For single-customer non-search mode, pagination is applied via API.
1107-
# items_total comes from PortaBilling and may be slightly higher than the
1108-
# actual retrievable count (e.g. current user and blocked accounts are
1109-
# filtered out adapter-side but counted by PortaBilling).
1110-
contacts = account_contacts[:items_per_page]
1111-
total_count = total_count_from_api + (len(custom_contacts) if page == 1 else 0)
1072+
total_count = len(account_contacts)
1073+
start_idx = (page - 1) * items_per_page
1074+
end_idx = start_idx + items_per_page
1075+
contacts = account_contacts[start_idx:end_idx]
11121076

11131077
case PortaSwitchContactsSelectingMode.PHONEBOOK:
11141078
custom_contacts = [Serializer.get_contact_info_by_custom_entry(entry) for entry in
@@ -1867,7 +1831,16 @@ async def _fetch_page(offset):
18671831
# itself invoked concurrently across customers — can't dead-lock.
18681832
remaining_pages = await _gather_limited([_fetch_page(o) for o in remaining_offsets])
18691833

1870-
return first_page + [acc for page in remaining_pages for acc in page]
1834+
# De-duplicate by i_account: the chunks are separate LIMIT/OFFSET queries and
1835+
# get_account_list is never sent an ORDER BY, so an unstable row order between
1836+
# them could otherwise repeat a record in the assembled list (WT-1774). This
1837+
# bounds the damage but is not a cure — a row that lands in no chunk at all
1838+
# would still be missed; that needs a server-side ORDER BY.
1839+
by_i_account: dict[int, dict] = {}
1840+
for account in first_page + [acc for page in remaining_pages for acc in page]:
1841+
by_i_account[int(account["i_account"])] = account
1842+
1843+
return list(by_i_account.values())
18711844

18721845
async def _get_or_create_api_token(self, account_info: dict) -> Optional[str]:
18731846
"""Return the account's api_token, creating and persisting one if absent."""

0 commit comments

Comments
 (0)