diff --git a/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py b/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py index e9cc86f51..5b613a399 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py +++ b/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py @@ -490,16 +490,11 @@ def shipment_request( shipper.company_name or shipper.contact, max=35 ), comments=None, - customerReferences=( - [ - fedex.CustomerReferenceType( - customerReferenceType="INVOICE_NUMBER", - value=customs.invoice, - ) - ] - if customs.invoice - else [] - ), + customerReferences=provider_utils.collect_customer_references( + payload, + customs, + options, + )["commercial_invoice"], taxesOrMiscellaneousCharge=None, taxesOrMiscellaneousChargeType=None, freightCharge=None, @@ -714,7 +709,11 @@ def shipment_request( fedex.RequestedPackageLineItemType( sequenceNumber=None, subPackagingType="OTHER", - customerReferences=[], + customerReferences=provider_utils.collect_customer_references( + payload, + customs, + options, + )["package"], declaredValue=fedex.TotalDeclaredValueType( amount=lib.identity( lib.to_money(package.total_value) diff --git a/modules/connectors/fedex/karrio/providers/fedex/utils.py b/modules/connectors/fedex/karrio/providers/fedex/utils.py index ae6c75a80..7c85dde31 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/utils.py +++ b/modules/connectors/fedex/karrio/providers/fedex/utils.py @@ -1,8 +1,11 @@ +import karrio.schemas.fedex.shipping_request as shipping_request import karrio.schemas.fedex.tracking_document_request as fedex import gzip import typing import karrio.lib as lib import karrio.core as core +import karrio.core.models as models +import karrio.core.units as units class Settings(core.Settings): @@ -100,3 +103,90 @@ def state_code(address: lib.units.ComputedAddress) -> str: if address.state_code.lower() == "qc" and address.country_code == "CA" else address.state_code ) + + +# Max lengths sourced from FedEx API spec (vendor/ship-api.json, CustomerReference schema). +# Note: CUSTOMER_REFERENCE allows 40 chars for Express but only 30 for Ground. +# We use the conservative Ground limit (30) to ensure all service types work correctly. +CUSTOMER_REFERENCE_MAX_LENGTH = { + "CUSTOMER_REFERENCE": 30, + "DEPARTMENT_NUMBER": 30, + "INVOICE_NUMBER": 30, + "P_O_NUMBER": 30, + "RMA_ASSOCIATION": 20, +} + + +def build_customer_reference( + reference_type: str, + value: typing.Optional[str], +) -> typing.Optional[shipping_request.CustomerReferenceType]: + parsed_value = lib.text( + value, + max=CUSTOMER_REFERENCE_MAX_LENGTH.get(reference_type, 30), + ) + + if not any(parsed_value or ""): + return None + + return shipping_request.CustomerReferenceType( + customerReferenceType=reference_type, + value=parsed_value, + ) + + +def collect_customer_references( + payload: models.ShipmentRequest, + customs: models.Customs, + options: units.ShippingOptions, +) -> typing.Dict[str, typing.List[shipping_request.CustomerReferenceType]]: + raw_options = payload.options or {} + invoice_number = lib.text(customs.invoice) or lib.text(options.invoice_number.state) + + # NOTE: These references are sent via the option, but many don't current have a UI exposure. + # would the label 'metadata' be a better fit for these values? + references = { + "CUSTOMER_REFERENCE": build_customer_reference( + "CUSTOMER_REFERENCE", + payload.reference, + ), + "DEPARTMENT_NUMBER": build_customer_reference( + "DEPARTMENT_NUMBER", + lib.text( + raw_options.get("fedex_department_number") + or raw_options.get("department_number") + ), + ), + "INVOICE_NUMBER": build_customer_reference("INVOICE_NUMBER", invoice_number), + "P_O_NUMBER": build_customer_reference( + "P_O_NUMBER", + lib.text(raw_options.get("fedex_po_number") or raw_options.get("po_number")), + ), + "RMA_ASSOCIATION": build_customer_reference( + "RMA_ASSOCIATION", + lib.text( + raw_options.get("fedex_rma_association") + or raw_options.get("rma_association") + ), + ), + } + + return { + "commercial_invoice": [ + reference + for key in ["INVOICE_NUMBER", "CUSTOMER_REFERENCE", "DEPARTMENT_NUMBER"] + for reference in [references[key]] + if reference is not None + ], + "package": [ + reference + for key in [ + "CUSTOMER_REFERENCE", + "DEPARTMENT_NUMBER", + "P_O_NUMBER", + "RMA_ASSOCIATION", + ] + for reference in [references[key]] + if reference is not None + ], + } diff --git a/modules/connectors/fedex/tests/fedex/test_shipment.py b/modules/connectors/fedex/tests/fedex/test_shipment.py index 9271758be..9eb4b9152 100644 --- a/modules/connectors/fedex/tests/fedex/test_shipment.py +++ b/modules/connectors/fedex/tests/fedex/test_shipment.py @@ -335,7 +335,8 @@ def test_parse_return_shipment_response(self): "customsClearanceDetail": { "commercialInvoice": { "customerReferences": [ - {"customerReferenceType": "INVOICE_NUMBER", "value": "123456789"} + {"customerReferenceType": "INVOICE_NUMBER", "value": "123456789"}, + {"customerReferenceType": "CUSTOMER_REFERENCE", "value": "#Order 11111"}, ], "originatorName": "Input Your Information", "termsOfSale": "DDU", @@ -418,6 +419,12 @@ def test_parse_return_shipment_response(self): "units": "IN", "width": 12.0, }, + "customerReferences": [ + { + "customerReferenceType": "CUSTOMER_REFERENCE", + "value": "#Order 11111", + } + ], "groupPackageCount": 1, "packageSpecialServices": {}, "subPackagingType": "OTHER", @@ -523,6 +530,12 @@ def test_parse_return_shipment_response(self): "units": "IN", "width": 12.0, }, + "customerReferences": [ + { + "customerReferenceType": "CUSTOMER_REFERENCE", + "value": "#Order 11111", + } + ], "groupPackageCount": 1, "packageSpecialServices": {}, "subPackagingType": "OTHER", @@ -594,7 +607,8 @@ def test_parse_return_shipment_response(self): "customsClearanceDetail": { "commercialInvoice": { "customerReferences": [ - {"customerReferenceType": "INVOICE_NUMBER", "value": "123456789"} + {"customerReferenceType": "INVOICE_NUMBER", "value": "123456789"}, + {"customerReferenceType": "CUSTOMER_REFERENCE", "value": "#Order 11111"}, ], "originatorName": "Input Your Information", "termsOfSale": "DDU", @@ -677,6 +691,12 @@ def test_parse_return_shipment_response(self): "units": "IN", "width": 12, }, + "customerReferences": [ + { + "customerReferenceType": "CUSTOMER_REFERENCE", + "value": "#Order 11111", + } + ], "groupPackageCount": 1, "packageSpecialServices": {}, "subPackagingType": "OTHER", @@ -690,6 +710,12 @@ def test_parse_return_shipment_response(self): "units": "IN", "width": 11, }, + "customerReferences": [ + { + "customerReferenceType": "CUSTOMER_REFERENCE", + "value": "#Order 11111", + } + ], "groupPackageCount": 1, "packageSpecialServices": {}, "subPackagingType": "OTHER", diff --git a/modules/connectors/fedex/tests/fedex/test_utils.py b/modules/connectors/fedex/tests/fedex/test_utils.py new file mode 100644 index 000000000..9386c59d3 --- /dev/null +++ b/modules/connectors/fedex/tests/fedex/test_utils.py @@ -0,0 +1,168 @@ +import unittest +from unittest import mock + +import karrio.providers.fedex.utils as provider_utils + + +class TestFedExBuildCustomerReference(unittest.TestCase): + def test_returns_none_for_none_value(self): + result = provider_utils.build_customer_reference("CUSTOMER_REFERENCE", None) + self.assertIsNone(result) + + def test_returns_none_for_empty_string(self): + result = provider_utils.build_customer_reference("CUSTOMER_REFERENCE", "") + self.assertIsNone(result) + + def test_returns_reference_for_valid_value(self): + result = provider_utils.build_customer_reference("CUSTOMER_REFERENCE", "Ref-123") + self.assertIsNotNone(result) + self.assertEqual(result.customerReferenceType, "CUSTOMER_REFERENCE") + self.assertEqual(result.value, "Ref-123") + + def test_truncates_customer_reference_to_30_chars(self): + result = provider_utils.build_customer_reference("CUSTOMER_REFERENCE", "X" * 35) + self.assertIsNotNone(result) + self.assertEqual(len(result.value), 30) + + def test_truncates_rma_association_to_20_chars(self): + result = provider_utils.build_customer_reference("RMA_ASSOCIATION", "R" * 25) + self.assertIsNotNone(result) + self.assertEqual(len(result.value), 20) + + def test_preserves_value_at_exact_max_length(self): + exact_value = "X" * 30 + result = provider_utils.build_customer_reference("CUSTOMER_REFERENCE", exact_value) + self.assertIsNotNone(result) + self.assertEqual(result.value, exact_value) + + def test_preserves_value_under_max_length(self): + result = provider_utils.build_customer_reference("DEPARTMENT_NUMBER", "DEPT-001") + self.assertIsNotNone(result) + self.assertEqual(result.value, "DEPT-001") + + +class TestFedExCollectCustomerReferences(unittest.TestCase): + def _make_payload(self, reference=None, options=None): + payload = mock.MagicMock() + payload.reference = reference + payload.options = options or {} + return payload + + def _make_customs(self, invoice=None): + customs = mock.MagicMock() + customs.invoice = invoice + return customs + + def _make_options(self, invoice_number=None): + options = mock.MagicMock() + options.invoice_number.state = invoice_number + return options + + def test_returns_commercial_invoice_and_package_keys(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(), + self._make_options(), + ) + self.assertIn("commercial_invoice", result) + self.assertIn("package", result) + + def test_empty_references_excluded_when_no_data(self): + result = provider_utils.collect_customer_references( + self._make_payload(reference=None), + self._make_customs(invoice=None), + self._make_options(invoice_number=None), + ) + self.assertEqual(result["commercial_invoice"], []) + self.assertEqual(result["package"], []) + + def test_invoice_number_in_commercial_invoice_from_customs(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(invoice="INV-123"), + self._make_options(), + ) + values = {r.customerReferenceType: r.value for r in result["commercial_invoice"]} + self.assertIn("INVOICE_NUMBER", values) + self.assertEqual(values["INVOICE_NUMBER"], "INV-123") + + def test_invoice_number_falls_back_to_options(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(invoice=None), + self._make_options(invoice_number="OPT-INV-789"), + ) + values = {r.customerReferenceType: r.value for r in result["commercial_invoice"]} + self.assertIn("INVOICE_NUMBER", values) + self.assertEqual(values["INVOICE_NUMBER"], "OPT-INV-789") + + def test_customs_invoice_takes_priority_over_options(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(invoice="CUSTOMS-INV"), + self._make_options(invoice_number="OPT-INV"), + ) + values = {r.customerReferenceType: r.value for r in result["commercial_invoice"]} + self.assertEqual(values["INVOICE_NUMBER"], "CUSTOMS-INV") + + def test_customer_reference_in_package_from_payload(self): + result = provider_utils.collect_customer_references( + self._make_payload(reference="#MyOrder"), + self._make_customs(), + self._make_options(), + ) + types = [r.customerReferenceType for r in result["package"]] + self.assertIn("CUSTOMER_REFERENCE", types) + + def test_package_reference_number_is_ignored_for_po_number(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(), + self._make_options(), + ) + types = [r.customerReferenceType for r in result["package"]] + self.assertNotIn("P_O_NUMBER", types) + + def test_po_number_falls_back_to_options(self): + result = provider_utils.collect_customer_references( + self._make_payload(options={"fedex_po_number": "OPT-PO-999"}), + self._make_customs(), + self._make_options(), + ) + values = {r.customerReferenceType: r.value for r in result["package"]} + self.assertIn("P_O_NUMBER", values) + self.assertEqual(values["P_O_NUMBER"], "OPT-PO-999") + + def test_invoice_number_excluded_from_package_references(self): + result = provider_utils.collect_customer_references( + self._make_payload(), + self._make_customs(invoice="INV-123"), + self._make_options(), + ) + package_types = [r.customerReferenceType for r in result["package"]] + self.assertNotIn("INVOICE_NUMBER", package_types) + + def test_customer_reference_appears_in_both_commercial_and_package(self): + result = provider_utils.collect_customer_references( + self._make_payload(reference="#SharedRef"), + self._make_customs(), + self._make_options(), + ) + commercial_types = [r.customerReferenceType for r in result["commercial_invoice"]] + package_types = [r.customerReferenceType for r in result["package"]] + self.assertIn("CUSTOMER_REFERENCE", commercial_types) + self.assertIn("CUSTOMER_REFERENCE", package_types) + + def test_rma_association_in_package_from_options(self): + result = provider_utils.collect_customer_references( + self._make_payload(options={"fedex_rma_association": "RMA-001"}), + self._make_customs(), + self._make_options(), + ) + values = {r.customerReferenceType: r.value for r in result["package"]} + self.assertIn("RMA_ASSOCIATION", values) + self.assertEqual(values["RMA_ASSOCIATION"], "RMA-001") + + +if __name__ == "__main__": + unittest.main()