diff --git a/PRDs/FEDEX_SHIPMENT_PICKUP_TYPE.md b/PRDs/FEDEX_SHIPMENT_PICKUP_TYPE.md new file mode 100644 index 000000000..3f4b41f8d --- /dev/null +++ b/PRDs/FEDEX_SHIPMENT_PICKUP_TYPE.md @@ -0,0 +1,191 @@ +# Plan: FedEx Configurable `fedex_pickup_type` Option + +FedEx's `pickupType` field (which describes how a shipper tenders a package — drop off, scheduled, or on-call) is hardcoded to `DROPOFF_AT_FEDEX_LOCATION` in both the rate and shipment request builders. This adds it as a standard FedEx `ShippingOption` so API consumers can send any of the three valid Ship API values. Default behaviour is unchanged (backward-compatible). + +## Context + +`pickupType` is hardcoded as `"DROPOFF_AT_FEDEX_LOCATION"` in both: + +- `modules/connectors/fedex/karrio/providers/fedex/shipment/create.py` (line 293) +- `modules/connectors/fedex/karrio/providers/fedex/rate.py` (line 214) + +Valid FedEx Ship API values (from `ship-api.json`, `rate-api.json`, and the API Reference Guide `#pickuptypes` table): + +| Enumeration | Description | +| --------------------------- | ------------------------------------------------------------------ | +| `DROPOFF_AT_FEDEX_LOCATION` | Shipment will be dropped off at a FedEx Location (current default) | +| `CONTACT_FEDEX_TO_SCHEDULE` | FedEx will be contacted to request a pickup | +| `USE_SCHEDULED_PICKUP` | Shipment will be picked up as part of a regular scheduled pickup | + +Note: `ON_CALL`, `PACKAGE_RETURN_PROGRAM`, `REGULAR_STOP` are Pickup API values only — they do not belong in the shipment/rate request and should not be included in this option. + +## Design Decisions + +- **FedEx-only `ShippingOption`** — no SDK `ShipmentRequest` model changes +- Unified `PickupRequest.pickup_type` (`one_time`/`daily`/`recurring`) is a _different_ concept (it schedules a carrier driver pickup event). The field we are adding answers "how does this shipment get to the carrier?" — set at shipment creation time. +- `dpd_meta_dropoff_type` is a false cognate (controls label format at drop-off point, not tendering method) — not a reference pattern for this change +- `help` text exposure in the API endpoint is out of scope — separate PR +- Purolator (`PickupType`: `DropOff`/`PreScheduled`) and DHL Poland (`dropOffType`: `REGULAR_PICKUP`) also hardcode this concept — noted as future work, not in scope here +- Default remains `DROPOFF_AT_FEDEX_LOCATION` — zero behaviour change for existing integrations + +## Implementation Steps + +### Step 1 — `units.py`: add `FedExPickupType` StrEnum + +File: `modules/connectors/fedex/karrio/providers/fedex/units.py` + +Add before the `ConnectionConfig` class: + +```python +class FedExPickupType(lib.StrEnum): + """How the shipper will tender the package to FedEx (Ship API / Rate API).""" + # Shipper brings the package to a FedEx drop-off location + dropoff_at_fedex_location = "DROPOFF_AT_FEDEX_LOCATION" + # FedEx will be contacted to schedule a one-time pickup + contact_fedex_to_schedule = "CONTACT_FEDEX_TO_SCHEDULE" + # Package will be collected as part of a regular standing pickup schedule + use_scheduled_pickup = "USE_SCHEDULED_PICKUP" +``` + +### Step 2 — `units.py`: add `fedex_pickup_type` to `ShippingOption` + +In the same file, add to the `ShippingOption` enum inside the delivery options group (near `fedex_saturday_delivery`): + +```python +fedex_pickup_type = lib.OptionEnum( + "fedex_pickup_type", + str, + help=( + "How the shipper will tender the package to FedEx. " + "Valid values: DROPOFF_AT_FEDEX_LOCATION, CONTACT_FEDEX_TO_SCHEDULE, USE_SCHEDULED_PICKUP. " + "Defaults to DROPOFF_AT_FEDEX_LOCATION." + ), + meta=dict(category="DELIVERY_OPTIONS"), +) +``` + +### Step 3 — `shipment/create.py`: replace hardcoded `pickupType` + +File: `modules/connectors/fedex/karrio/providers/fedex/shipment/create.py`, line 293 + +Replace: + +```python +pickupType="DROPOFF_AT_FEDEX_LOCATION", +``` + +With: + +```python +pickupType=(options.fedex_pickup_type.state or "DROPOFF_AT_FEDEX_LOCATION"), +``` + +### Step 4 — `rate.py`: replace hardcoded `pickupType` + +File: `modules/connectors/fedex/karrio/providers/fedex/rate.py`, line 214 + +Replace: + +```python +pickupType="DROPOFF_AT_FEDEX_LOCATION", +``` + +With: + +```python +pickupType=(options.fedex_pickup_type.state or "DROPOFF_AT_FEDEX_LOCATION"), +``` + +### Step 5 — `i18n.py`: add translation entry + +File: `modules/connectors/fedex/karrio/providers/fedex/i18n.py` + +Add to `OPTION_NAME_TRANSLATIONS`: + +```python +"fedex_pickup_type": _("FedEx Pickup Type"), +``` + +### Step 6 — `test_shipment.py`: add 2 new test methods + fixture constants + +File: `modules/connectors/fedex/tests/fedex/test_shipment.py` + +Add two fixture constants (minimal diffs of the existing `ShipmentRequest` fixture with only `pickupType` changed): + +```python +ShipmentUseScheduledPickupRequest = { + ... # copy of ShipmentRequest with "pickupType": "USE_SCHEDULED_PICKUP" +} + +ShipmentContactFedexPickupRequest = { + ... # copy of ShipmentRequest with "pickupType": "CONTACT_FEDEX_TO_SCHEDULE" +} +``` + +Add two new test methods to `TestFedExShipping`: + +```python +def test_create_shipment_request_with_use_scheduled_pickup(self): + request = gateway.mapper.create_shipment_request( + models.ShipmentRequest(**{**ShipmentPayload, "options": {"fedex_pickup_type": "USE_SCHEDULED_PICKUP"}}) + ) + self.assertEqual(request.serialize(), ShipmentUseScheduledPickupRequest) + +def test_create_shipment_request_with_contact_fedex_pickup(self): + request = gateway.mapper.create_shipment_request( + models.ShipmentRequest(**{**ShipmentPayload, "options": {"fedex_pickup_type": "CONTACT_FEDEX_TO_SCHEDULE"}}) + ) + self.assertEqual(request.serialize(), ShipmentContactFedexPickupRequest) +``` + +### Step 7 — `test_rate.py`: add 1 new test method + fixture constant + +File: `modules/connectors/fedex/tests/fedex/test_rate.py` + +Add one fixture constant (minimal diff of existing `RateRequest` with only `pickupType` changed): + +```python +RateUseScheduledPickupRequest = { + ... # copy of RateRequest with "pickupType": "USE_SCHEDULED_PICKUP" +} +``` + +Add one new test method to `TestFedExRating`: + +```python +def test_create_rate_request_with_use_scheduled_pickup(self): + request = gateway.mapper.create_rate_request( + models.RateRequest(**{**RatePayload, "options": {"fedex_pickup_type": "USE_SCHEDULED_PICKUP"}}) + ) + self.assertEqual(request.serialize(), RateUseScheduledPickupRequest) +``` + +## Files Changed + +| File | Change | +| -------------------------------------------------------------------- | --------------------------------------------------------------- | +| `modules/connectors/fedex/karrio/providers/fedex/units.py` | Add `FedExPickupType` enum + `ShippingOption.fedex_pickup_type` | +| `modules/connectors/fedex/karrio/providers/fedex/shipment/create.py` | Replace hardcoded `pickupType` at line 293 | +| `modules/connectors/fedex/karrio/providers/fedex/rate.py` | Replace hardcoded `pickupType` at line 214 | +| `modules/connectors/fedex/karrio/providers/fedex/i18n.py` | Add `"fedex_pickup_type"` translation | +| `modules/connectors/fedex/tests/fedex/test_shipment.py` | Add 2 tests + 2 fixture constants | +| `modules/connectors/fedex/tests/fedex/test_rate.py` | Add 1 test + 1 fixture constant | + +## Verification + +```bash +source bin/activate-env +python -m unittest discover -v -f modules/connectors/fedex/tests +``` + +All 7 existing tests must pass unchanged — confirming `DROPOFF_AT_FEDEX_LOCATION` remains the default when no option is supplied. + +The 3 new tests each call `request.serialize()` and directly assert `"pickupType"` in the resulting dict equals the option value passed. + +## Future Work (out of scope) + +- Expose `OptionEnum.help` through `GET /v1/carriers/fedex/options` API endpoint +- Add equivalent `purolator_pickup_type` option to Purolator connector (`DropOff`/`PreScheduled`) +- Add equivalent `dhl_poland_dropoff_type` option to DHL Poland connector +- Consider a unified `tendering_type` field on `ShipmentRequest` once all three carriers are done +- The 'create_label` core module for the Dashboard only renders boolean "CheckBoxField" options and skips other options so we can't see this option via the Dashboard atm diff --git a/modules/connectors/fedex/karrio/providers/fedex/i18n.py b/modules/connectors/fedex/karrio/providers/fedex/i18n.py index 3de72dc52..d280fcc2a 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/i18n.py +++ b/modules/connectors/fedex/karrio/providers/fedex/i18n.py @@ -84,6 +84,7 @@ "fedex_one_rate": _("FedEx One Rate"), "fedex_freight_guarantee": _("FedEx Freight Guarantee"), "fedex_saturday_delivery": _("FedEx Saturday Delivery"), + "fedex_pickup_type": _("FedEx Pickup Type"), "fedex_smart_post_hub_id": _("FedEx Smart Post Hub ID"), "fedex_smart_post_allowed_indicia": _("FedEx Smart Post Allowed Indicia"), "fedex_alcohol": _("FedEx Alcohol"), diff --git a/modules/connectors/fedex/karrio/providers/fedex/rate.py b/modules/connectors/fedex/karrio/providers/fedex/rate.py index 0b30760fb..e4048a14e 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/rate.py +++ b/modules/connectors/fedex/karrio/providers/fedex/rate.py @@ -211,7 +211,7 @@ def rate_request( preferredCurrency=default_currency, rateRequestType=request_types, shipDateStamp=lib.fdate(shipment_date, "%Y-%m-%d"), - pickupType="DROPOFF_AT_FEDEX_LOCATION", + pickupType=(options.fedex_pickup_type.state or "DROPOFF_AT_FEDEX_LOCATION"), requestedPackageLineItems=[ fedex.RequestedPackageLineItemType( subPackagingType=lib.identity( diff --git a/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py b/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py index e9cc86f51..0c3575d25 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py +++ b/modules/connectors/fedex/karrio/providers/fedex/shipment/create.py @@ -290,7 +290,7 @@ def shipment_request( ) ], recipientLocationNumber=None, - pickupType="DROPOFF_AT_FEDEX_LOCATION", + pickupType=(options.fedex_pickup_type.state or "DROPOFF_AT_FEDEX_LOCATION"), serviceType=service, packagingType=lib.identity( provider_units.PackagingType.map( diff --git a/modules/connectors/fedex/karrio/providers/fedex/units.py b/modules/connectors/fedex/karrio/providers/fedex/units.py index 8cb1e3afd..de32ebd7e 100644 --- a/modules/connectors/fedex/karrio/providers/fedex/units.py +++ b/modules/connectors/fedex/karrio/providers/fedex/units.py @@ -232,6 +232,14 @@ class PaymentType(lib.Enum): third_party = "THIRD_PARTY" +class FedExPickupType(lib.StrEnum): + """How the shipper will tender the package to FedEx (Ship API / Rate API).""" + + dropoff_at_fedex_location = "DROPOFF_AT_FEDEX_LOCATION" + contact_fedex_to_schedule = "CONTACT_FEDEX_TO_SCHEDULE" + use_scheduled_pickup = "USE_SCHEDULED_PICKUP" + + class ConnectionConfig(lib.Enum): label_type = lib.OptionEnum("label_type", LabelType) smart_post_hub_id = lib.OptionEnum("smart_post_hub_id") @@ -327,6 +335,16 @@ class ShippingOption(lib.Enum): fedex_one_rate = lib.OptionEnum("FEDEX_ONE_RATE", bool) fedex_freight_guarantee = lib.OptionEnum("FREIGHT_GUARANTEE", bool) fedex_saturday_delivery = lib.OptionEnum("SATURDAY_DELIVERY", bool, meta=dict(category="DELIVERY_OPTIONS")) + fedex_pickup_type = lib.OptionEnum( + "fedex_pickup_type", + str, + help=( + "How the shipper will tender the package to FedEx. " + "Valid values: DROPOFF_AT_FEDEX_LOCATION, CONTACT_FEDEX_TO_SCHEDULE, USE_SCHEDULED_PICKUP. " + "Defaults to DROPOFF_AT_FEDEX_LOCATION." + ), + meta=dict(category="DELIVERY_OPTIONS"), + ) fedex_smart_post_hub_id = lib.OptionEnum("SMART_POST_HUB_ID") fedex_smart_post_allowed_indicia = lib.OptionEnum("SMART_POST_ALLOWED_INDICIA") diff --git a/modules/connectors/fedex/tests/fedex/test_rate.py b/modules/connectors/fedex/tests/fedex/test_rate.py index ee7a0e096..b6db41ac7 100644 --- a/modules/connectors/fedex/tests/fedex/test_rate.py +++ b/modules/connectors/fedex/tests/fedex/test_rate.py @@ -17,6 +17,21 @@ def test_create_rate_request(self): self.assertEqual(request.serialize(), RateRequest) + def test_create_rate_request_with_use_scheduled_pickup(self): + request = gateway.mapper.create_rate_request( + models.RateRequest( + **{ + **RatePayload, + "options": { + **RatePayload["options"], + "fedex_pickup_type": "USE_SCHEDULED_PICKUP", + }, + } + ) + ) + + self.assertEqual(request.serialize(), RateUseScheduledPickupRequest) + def test_get_rate(self): with patch("karrio.mappers.fedex.proxy.lib.request") as mock: mock.return_value = "{}" @@ -344,6 +359,14 @@ def test_rate_request_contains_semantic_address_fields(self): }, } +RateUseScheduledPickupRequest = { + **RateRequest, + "requestedShipment": { + **RateRequest["requestedShipment"], + "pickupType": "USE_SCHEDULED_PICKUP", + }, +} + RateResponse = """{ "transactionId": "4cbf8c73-61eb-4538-a65a-4830fef5d265", "output": { diff --git a/modules/connectors/fedex/tests/fedex/test_shipment.py b/modules/connectors/fedex/tests/fedex/test_shipment.py index 9271758be..4b04adc01 100644 --- a/modules/connectors/fedex/tests/fedex/test_shipment.py +++ b/modules/connectors/fedex/tests/fedex/test_shipment.py @@ -29,6 +29,36 @@ def test_create_shipment_request(self): self.assertEqual(request.serialize(), ShipmentRequest) + def test_create_shipment_request_with_use_scheduled_pickup(self): + request = gateway.mapper.create_shipment_request( + models.ShipmentRequest( + **{ + **ShipmentPayload, + "options": { + **ShipmentPayload["options"], + "fedex_pickup_type": "USE_SCHEDULED_PICKUP", + }, + } + ) + ) + + self.assertEqual(request.serialize(), ShipmentUseScheduledPickupRequest) + + def test_create_shipment_request_with_contact_fedex_pickup(self): + request = gateway.mapper.create_shipment_request( + models.ShipmentRequest( + **{ + **ShipmentPayload, + "options": { + **ShipmentPayload["options"], + "fedex_pickup_type": "CONTACT_FEDEX_TO_SCHEDULE", + }, + } + ) + ) + + self.assertEqual(request.serialize(), ShipmentContactFedexPickupRequest) + def test_create_shipment_request_paid_by_recipient(self): request = gateway.mapper.create_shipment_request( self.ShipmentPaidByRecipientRequest @@ -480,6 +510,22 @@ def test_parse_return_shipment_response(self): "shipAction": "CONFIRM", } +ShipmentUseScheduledPickupRequest = { + **ShipmentRequest, + "requestedShipment": { + **ShipmentRequest["requestedShipment"], + "pickupType": "USE_SCHEDULED_PICKUP", + }, +} + +ShipmentContactFedexPickupRequest = { + **ShipmentRequest, + "requestedShipment": { + **ShipmentRequest["requestedShipment"], + "pickupType": "CONTACT_FEDEX_TO_SCHEDULE", + }, +} + ShipmentPaidByRecipientRequest = { "accountNumber": {"value": "2349857"}, "labelResponseOptions": "LABEL",