Skip to content

Commit 5e6ab29

Browse files
authored
MPT-16614 [Monitoring] KeyError - Error synchronizing agreement AGR-2… (#706)
…076-2603-9245 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> Closes [MPT-16614](https://softwareone.atlassian.net/browse/MPT-16614) - Add PriceManager to centralize price retrieval and missing-price notification logic for agreement and subscription sync flows. - Refactor agreement and subscription sync to use PriceManager: build processable lines, fetch prices via PriceManager, skip lines without prices, and only update unit price when a price is available. - Add fallback lookup against MPT pricelist for SKUs missing Airtable prices and trigger centralized notify_missing_prices for unresolved SKUs. - Add unit tests for PriceManager and new missing-price scenarios; update existing sync tests and fixtures to support the new price retrieval path. - Migrate Airtable model queries to structured Field-based formulas (EQ/Field/BLANK/NE/GT/LTE) and switch model access to meta-based API/base/table/view patterns; update tests accordingly. - Short-circuit global-customer deployment flow by returning early when GC agreement deployment creation fails. - Bump pyairtable dependency to 3.3.* and add a per-file-ignore for a lint rule in pyproject.toml. <!-- end of auto-generated comment: release notes by coderabbit.ai --> [MPT-16614]: https://softwareone.atlassian.net/browse/MPT-16614?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
2 parents 8ba5af7 + fa986c8 commit 5e6ab29

File tree

11 files changed

+550
-132
lines changed

11 files changed

+550
-132
lines changed

adobe_vipm/airtable/models.py

Lines changed: 43 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
from mpt_extension_sdk.runtime.djapp.conf import get_for_product
99
from pyairtable.formulas import (
1010
AND,
11-
EQUAL,
12-
FIELD,
13-
GREATER,
14-
LESS_EQUAL,
11+
BLANK,
12+
EQ,
13+
GT,
1514
LOWER,
16-
NOT_EQUAL,
15+
LTE,
16+
NE,
1717
OR,
18-
STR_VALUE,
19-
to_airtable_value,
18+
Field,
2019
)
2120
from pyairtable.orm import Model, fields
2221
from requests import HTTPError
@@ -290,7 +289,7 @@ def get_offer_ids_by_membership_id(product_id: str, membership_id: str) -> list[
290289
list: List of SKUs that belong to the given membership.
291290
"""
292291
offer_model = get_offer_model(AirTableBaseInfo.for_migrations(product_id))
293-
formula = EQUAL(FIELD("membership_id"), STR_VALUE(membership_id))
292+
formula = EQ(Field("membership_id"), membership_id)
294293

295294
return [offer.offer_id for offer in offer_model.all(formula=formula)]
296295

@@ -320,8 +319,8 @@ def get_transfers_to_process(product_id: str):
320319
transfer_model = get_transfer_model(AirTableBaseInfo.for_migrations(product_id))
321320
return transfer_model.all(
322321
formula=OR(
323-
EQUAL(FIELD("status"), STR_VALUE(STATUS_INIT)),
324-
EQUAL(FIELD("status"), STR_VALUE(STATUS_RESCHEDULED)),
322+
EQ(Field("status"), STATUS_INIT),
323+
EQ(Field("status"), STATUS_RESCHEDULED),
325324
),
326325
)
327326

@@ -338,7 +337,7 @@ def get_transfers_to_check(product_id: str):
338337
"""
339338
transfer_model = get_transfer_model(AirTableBaseInfo.for_migrations(product_id))
340339
return transfer_model.all(
341-
formula=EQUAL(FIELD("status"), STR_VALUE(STATUS_RUNNING)),
340+
formula=EQ(Field("status"), STATUS_RUNNING),
342341
)
343342

344343

@@ -361,15 +360,15 @@ def get_transfer_by_authorization_membership_or_customer(
361360
transfer_model = get_transfer_model(AirTableBaseInfo.for_migrations(product_id))
362361
transfers = transfer_model.all(
363362
formula=AND(
364-
EQUAL(FIELD("authorization_uk"), STR_VALUE(authorization_uk)),
363+
EQ(Field("authorization_uk"), authorization_uk),
365364
OR(
366-
EQUAL(
367-
LOWER(FIELD("membership_id")),
368-
LOWER(STR_VALUE(membership_or_customer_id)),
365+
EQ(
366+
LOWER(Field("membership_id")),
367+
LOWER(membership_or_customer_id),
369368
),
370-
EQUAL(FIELD("customer_id"), STR_VALUE(membership_or_customer_id)),
369+
EQ(Field("customer_id"), membership_or_customer_id),
371370
),
372-
NOT_EQUAL(FIELD("status"), STR_VALUE(STATUS_DUPLICATED)),
371+
NE(Field("status"), STATUS_DUPLICATED),
373372
),
374373
)
375374

@@ -387,9 +386,9 @@ def get_transfer_link(transfer) -> str:
387386
str: The link to the transfer record or None in case of an error.
388387
"""
389388
try:
390-
base_id = transfer.Meta.base_id
391-
table_id = transfer.get_table().id
392-
view_id = transfer.get_table().schema().view("Transfer View").id
389+
base_id = transfer.meta.base.id
390+
table_id = transfer.meta.table.id
391+
view_id = transfer.meta.table.schema().view("Transfer View").id
393392
record_id = transfer.id
394393
except HTTPError:
395394
return None
@@ -441,10 +440,10 @@ def _get_prices_for_skus_from_airtable(
441440
pricelist_model = get_pricelist_model(AirTableBaseInfo.for_pricing(product_id))
442441
return pricelist_model.all(
443442
formula=AND(
444-
EQUAL(FIELD("currency"), to_airtable_value(currency)),
445-
EQUAL(FIELD("valid_until"), "BLANK()"),
443+
EQ(Field("currency"), currency),
444+
EQ(Field("valid_until"), BLANK()),
446445
OR(
447-
*[EQUAL(FIELD(column_name), to_airtable_value(sku)) for sku in skus],
446+
*[EQ(Field(column_name), sku) for sku in skus],
448447
),
449448
),
450449
)
@@ -464,6 +463,8 @@ def get_prices_for_skus(product_id: str, currency: str, skus: list[str]) -> dict
464463
Returns:
465464
dict: A dictionary with SKU, purchase price items.
466465
"""
466+
if not skus:
467+
return {}
467468
items = _get_prices_for_skus_from_airtable(product_id, currency, skus, "sku")
468469
return {item.sku: item.unit_pp for item in items}
469470

@@ -480,6 +481,8 @@ def get_skus_with_available_prices(product_id: str, currency: str, skus: list[st
480481
Returns:
481482
set: A set of SKUs if the price is available.
482483
"""
484+
if not skus:
485+
return set()
483486
items = _get_prices_for_skus_from_airtable(product_id, currency, skus, "partial_sku")
484487
return {item.partial_sku for item in items}
485488

@@ -496,6 +499,8 @@ def get_skus_with_available_prices_3yc(
496499
start_date: The date in which the 3YC started.
497500
skus: List of SKUs which purchase prices must be retrieved.
498501
"""
502+
if not skus:
503+
return set()
499504
items = _get_prices_3yc_for_skus_from_airtable(
500505
product_id, currency, start_date, skus, "partial_sku"
501506
)
@@ -508,16 +513,16 @@ def _get_prices_3yc_for_skus_from_airtable(
508513
pricelist_model = get_pricelist_model(AirTableBaseInfo.for_pricing(product_id))
509514
return pricelist_model.all(
510515
formula=AND(
511-
EQUAL(FIELD("currency"), to_airtable_value(currency)),
516+
EQ(Field("currency"), currency),
512517
OR(
513-
EQUAL(FIELD("valid_until"), "BLANK()"),
518+
EQ(Field("valid_until"), BLANK()),
514519
AND(
515-
LESS_EQUAL(FIELD("valid_from"), STR_VALUE(to_airtable_value(start_date))),
516-
GREATER(FIELD("valid_until"), STR_VALUE(to_airtable_value(start_date))),
520+
LTE(Field("valid_from"), start_date),
521+
GT(Field("valid_until"), start_date),
517522
),
518523
),
519524
OR(
520-
*[EQUAL(FIELD(column_name), to_airtable_value(sku)) for sku in skus],
525+
*[EQ(Field(column_name), sku) for sku in skus],
521526
),
522527
),
523528
sort=["-valid_until"],
@@ -662,10 +667,10 @@ def get_gc_main_agreement(product_id: str, authorization_uk: str, membership_or_
662667
)
663668
gc_main_agreements = gc_main_agreement_model.all(
664669
formula=AND(
665-
EQUAL(FIELD("authorization_uk"), STR_VALUE(authorization_uk)),
670+
EQ(Field("authorization_uk"), authorization_uk),
666671
OR(
667-
EQUAL(FIELD("membership_id"), STR_VALUE(membership_or_customer_id)),
668-
EQUAL(FIELD("customer_id"), STR_VALUE(membership_or_customer_id)),
672+
EQ(Field("membership_id"), membership_or_customer_id),
673+
EQ(Field("customer_id"), membership_or_customer_id),
669674
),
670675
),
671676
)
@@ -691,7 +696,7 @@ def get_gc_agreement_deployments_by_main_agreement(product_id: str, main_agreeme
691696
)
692697
return gc_agreement_deployment_model.all(
693698
formula=AND(
694-
EQUAL(FIELD("main_agreement_id"), STR_VALUE(main_agreement_id)),
699+
EQ(Field("main_agreement_id"), main_agreement_id),
695700
),
696701
)
697702

@@ -714,8 +719,8 @@ def get_gc_agreement_deployments_to_check(product_id: str):
714719
)
715720
return gc_agreement_deployment_model.all(
716721
formula=OR(
717-
EQUAL(FIELD("status"), STR_VALUE(STATUS_GC_PENDING)),
718-
EQUAL(FIELD("status"), STR_VALUE(STATUS_GC_ERROR)),
722+
EQ(Field("status"), STATUS_GC_PENDING),
723+
EQ(Field("status"), STATUS_GC_ERROR),
719724
),
720725
)
721726

@@ -734,11 +739,10 @@ def get_agreement_deployment_view_link(product_id: str) -> str | None:
734739
gc_agreement_deployment_model = get_gc_agreement_deployment_model(
735740
AirTableBaseInfo.for_migrations(product_id)
736741
)
737-
base_id = gc_agreement_deployment_model.Meta.base_id
738-
table_id = gc_agreement_deployment_model.get_table().id
742+
base_id = gc_agreement_deployment_model.meta.base.id
743+
table_id = gc_agreement_deployment_model.meta.table.id
739744
view_id = (
740-
gc_agreement_deployment_model
741-
.get_table()
745+
gc_agreement_deployment_model.meta.table
742746
.schema()
743747
.view(
744748
"Agreement Deployments View",
@@ -787,14 +791,11 @@ def from_short_id(cls, vendor_external_id: str):
787791
Returns:
788792
AdobeProductMapping: entity of the AdobeProductMapping.
789793
"""
790-
entity = cls.first(
791-
formula=EQUAL(FIELD("vendor_external_id"), STR_VALUE(vendor_external_id))
792-
)
794+
entity = cls.first(formula=EQ(Field("vendor_external_id"), vendor_external_id))
793795
if entity is None:
794796
raise AdobeProductNotFoundError(
795797
f"AdobeProduct with vendor_external_id `{vendor_external_id}` not found."
796798
)
797-
798799
return entity
799800

800801
def is_consumable(self) -> bool:

adobe_vipm/flows/global_customer.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,7 @@ def process_agreement_deployment( # noqa: C901
642642
listing,
643643
licensee,
644644
)
645+
645646
if not gc_agreement_id:
646647
return
647648

adobe_vipm/flows/sync/agreement.py

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from adobe_vipm.flows.mpt import get_agreements_by_3yc_commitment_request_invitation
3131
from adobe_vipm.flows.sync.asset import AssetSyncer
3232
from adobe_vipm.flows.sync.helper import check_adobe_subscription_id
33+
from adobe_vipm.flows.sync.price_manager import PriceManager
3334
from adobe_vipm.flows.sync.subscription import SubscriptionSyncer
3435
from adobe_vipm.flows.utils import notify_agreement_unhandled_exception_in_teams
3536
from adobe_vipm.flows.utils.market_segment import get_market_segment
@@ -520,18 +521,21 @@ def _update_3yc_ordering_params(self, commitment_info: dict, parameters: dict):
520521
def _update_agreement_line_prices(
521522
self, agreement: dict, currency: str, product_id: str
522523
) -> None:
523-
agreement_lines = []
524-
for line in agreement["lines"]:
525-
if line["item"]["externalIds"]["vendor"] != "adobe-reseller-transfer":
526-
actual_sku = models.get_adobe_sku(line["item"]["externalIds"]["vendor"])
527-
agreement_lines.append((
528-
line,
529-
flows_utils.get_sku_with_discount_level(actual_sku, self._adobe_customer),
530-
))
524+
agreement_lines = self._get_processable_agreement_lines(agreement)
525+
price_manager = PriceManager(
526+
self._mpt_client,
527+
self._adobe_customer,
528+
agreement_lines,
529+
self.agreement_id,
530+
self._agreement["listing"]["priceList"]["id"],
531+
)
532+
skus = [sku for _, sku in agreement_lines]
533+
prices = price_manager.get_sku_prices_for_agreement_lines(skus, product_id, currency)
531534

532-
skus = [item[1] for item in agreement_lines]
533-
prices = models.get_sku_price(self._adobe_customer, skus, product_id, currency)
534535
for line, actual_sku in agreement_lines:
536+
if actual_sku not in prices:
537+
continue
538+
535539
current_price = line["price"]["unitPP"]
536540
line["price"]["unitPP"] = prices[actual_sku]
537541
logger.info(
@@ -542,6 +546,14 @@ def _update_agreement_line_prices(
542546
prices[actual_sku],
543547
)
544548

549+
def _get_processable_agreement_lines(self, agreement: dict) -> list[tuple[dict, str]]:
550+
agreement_lines = []
551+
for line in agreement["lines"]:
552+
if line["item"]["externalIds"]["vendor"] != "adobe-reseller-transfer":
553+
actual_sku = models.get_adobe_sku(line["item"]["externalIds"]["vendor"])
554+
agreement_lines.append((line, actual_sku))
555+
return agreement_lines
556+
545557
# REFACTOR: get method must not update subscriptions in mpt or terminate a subscription
546558
def _get_subscriptions_for_update(self, agreement: dict) -> list[tuple[dict, dict, str]]: # noqa: C901
547559
logger.info("Getting subscriptions for update for agreement %s", agreement["id"])
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
from mpt_extension_sdk.mpt_http import mpt
2+
3+
from adobe_vipm.airtable import models
4+
from adobe_vipm.flows import utils as flows_utils
5+
from adobe_vipm.utils import get_commitment_start_date
6+
7+
8+
class PriceManager:
9+
"""
10+
PriceManager class to manage the prices to update the agreement or subscription.
11+
12+
Attributes:
13+
mpt_client: The MPT client.
14+
adobe_customer: The Adobe customer.
15+
agreement: The agreement data.
16+
agreement_lines: The agreement lines.
17+
"""
18+
19+
def __init__(self, mpt_client, adobe_customer, lines, agreement_id, pricelist_id):
20+
self._mpt_client = mpt_client
21+
self._adobe_customer = adobe_customer
22+
self._lines = lines
23+
self._agreement_id = agreement_id
24+
self._pricelist_id = pricelist_id
25+
26+
def get_sku_prices_for_agreement_lines(self, skus, product_id, currency):
27+
"""Get the prices for the given SKUs.
28+
29+
Args:
30+
skus: The SKUs to get the prices for.
31+
product_id: The product ID.
32+
currency: The currency.
33+
34+
Returns:
35+
A dictionary with the list of SKUs and the prices.
36+
"""
37+
prices = models.get_sku_price(self._adobe_customer, skus, product_id, currency)
38+
missing_prices_skus = []
39+
for line, actual_sku in self._lines:
40+
if actual_sku not in prices:
41+
missing_prices_skus.append(actual_sku)
42+
mpt_price = mpt.get_item_prices_by_pricelist_id(
43+
self._mpt_client,
44+
self._pricelist_id,
45+
line["item"]["id"],
46+
)
47+
if mpt_price:
48+
prices[actual_sku] = mpt_price[0]["unitPP"]
49+
50+
self._notify_missing_prices(missing_prices_skus, product_id, currency)
51+
return prices
52+
53+
def _notify_missing_prices(self, missing_prices_skus, product_id, currency):
54+
if missing_prices_skus:
55+
flows_utils.notify_missing_prices(
56+
self._agreement_id,
57+
missing_prices_skus,
58+
product_id,
59+
currency,
60+
get_commitment_start_date(self._adobe_customer),
61+
)

0 commit comments

Comments
 (0)