Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canada Post - Individual Group IDs Created for Each Shipment, Causing Inefficient Use of End-of-Day Manifests #679

Closed
minchaminder opened this issue Sep 10, 2024 · 10 comments · Fixed by #698

Comments

@minchaminder
Copy link

minchaminder commented Sep 10, 2024

Description:
A representative from Canada Post has reported an issue with how Karrio handles Group IDs for Canada Post shipments. The concern is that each shipment is being assigned its own Group ID, which quickly leads to hitting the limits for End-of-Day (EOD) manifests. This is inefficient compared to the expected behavior of grouping multiple shipments under a single Group ID.

Expected Behavior:

  • Karrio should group multiple shipments under a single Group ID, allowing users to process all shipments in one End-of-Day (EOD) manifest.
  • This would prevent the rapid exhaustion of the maximum allowed manifests and streamline the process for users.
  • Additionally, the manifest GUI should be updated to operate by Group IDs rather than individual shipments, so users can easily manage and transmit shipments in bulk, reducing the number of manifests needed.

Current Behavior:

  • Each individual shipment is assigned its own unique Group ID, even if multiple shipments are created around the same time.
  • This results in unnecessary consumption of Group IDs and increases the likelihood of reaching the manifest limit more quickly.
  • The process becomes inefficient, requiring users to transmit each shipment separately rather than in bulk.

Steps to Reproduce:

  1. Create multiple Canada Post shipments via Karrio.
  2. Observe that each shipment is assigned a unique Group ID.
  3. This leads to inefficient use of Group IDs, making it more likely to hit the EOD manifest limit sooner than expected.

Impact:
This behavior can lead to faster consumption of the EOD manifest limits and makes shipment management less efficient, forcing users to manage multiple manifests instead of a single grouped one. Updating the manifest GUI to work by groups rather than individual shipments would further improve efficiency.

For reference from Canada Post:

Performance limitations
To avoid a timeout of our servers, please follow these recommendations:
· Do not include more than 30 groups per manifest (i.e., maximum of 30 group-ids in one Transmit Shipments request.)
· Do not put more than 5,000 shipments in one group.

System limitations
To avoid an error, please do not exceed the following limits before performing a Transmit Shipments call:
· Maximum of 50 groups per manifest (error 9109 if exceeded).
· Maximum of 10,000 shipments in one group (error 9110 if exceeded).
· Maximum of 10,000 shipments across multiple groups (error 9108 if exceeded).

Body (REST)

<transmit-set xmlns=http://www.canadapost.ca/ws/manifest-v8>
<group-ids>
<group-id>00f31fb4954641a8e50b260d287238b0</group-id>
<group-id>67fcc2c694854eaeb78031ca272b1a0c</group-id>
<group-id>6b7f2091144642bfb7032a4060dd4e9a</group-id>
</group-ids>
<requested-shipping-point>X0X0X0</requested-shipping-point>
<cpc-pickup-indicator>true</cpc-pickup-indicator>
<detailed-manifests>true</detailed-manifests>
<method-of-payment>Account</method-of-payment>
<manifest-address>
<manifest-company>COMPANY INC</manifest-company>
<manifest-name>COMPANY INC</manifest-name>
<phone-number>888-888-8888</phone-number>
<address-details>
<address-line-1>123 MAIN STREET</address-line-1>
<city>YOUR CITY</city>
<prov-state>QC</prov-state>
<postal-zip-code>X0X0X0</postal-zip-code>
</address-details>
</manifest-address>
</transmit-set>
@jacobshilitz
Copy link
Collaborator

@danh91 what do you think about using todays date or the let the api pass the group_id ?

also I think we need to remove customer_request_ids becouse its now the same as the group_id and it's wrong

customer_request_ids = [f"{group_id}" for _ in range(len(packages))]

canada post docs:

(Character String - up to 35 characters)
A unique ID that you can define for this shipment. It must be unique or the request will be rejected.

Supplier Account vendors must use this field to provide a unique identifier for any shipment that is a pre-authorized payment request.

You can also use this ID recover shipment details, such as the label, without the need for the information provided in the Create Shipment response. Instead, just use this ID in a request to Get Shipments.

@danh91
Copy link
Member

danh91 commented Sep 11, 2024

That's one way to appro

@danh91 what do you think about using todays date or the let the api pass the group_id ?

Yes using the date can be a way to address it.

also I think we need to remove customer_request_ids becouse its now the same as the group_id and it's wrong

customer_request_ids = [f"{group_id}" for _ in range(len(packages))]

canada post docs:

(Character String - up to 35 characters)
A unique ID that you can define for this shipment. It must be unique or the request will be rejected.

Supplier Account vendors must use this field to provide a unique identifier for any shipment that is a pre-authorized payment request.

You can also use this ID recover shipment details, such as the label, without the need for the information provided in the Create Shipment response. Instead, just use this ID in a request to Get Shipments.

The customer_request_ids was supposed to be appended with the index to make uniq. I must have missed it during testing. It is used later as a way to retrieve shipments and process manifests.

Once the uniq issue is solved, Is there a reason to completely remove it?

@jacobshilitz
Copy link
Collaborator

Once the uniq issue is solved, Is there a reason to completely remove it?

  1. I couldn't see its being used later, I only see shipment_identifiers being used

  2. Once group_id is not unique, we need to either use uuid just for that, and make unique per parcel,

p.s. I release now that you use group_id for mulit-parcel shipment 🤔

@jacobshilitz
Copy link
Collaborator

also we need to change the cancel method that instead of void it request shipment refund.

@danh91
Copy link
Member

danh91 commented Sep 11, 2024

also we need to change the cancel method that instead of void it request shipment refund.

😅 It is way more complex than that. It is supposed to:

  • request refunds then void when the package is submitted
  • void if the package hasn't been submitted

If it is not behaving like this then we have a bug. In fact if you call cancel for something that has been submitted it will fail because it needs a refund call prior

@jacobshilitz
Copy link
Collaborator

jacobshilitz commented Sep 11, 2024

😅 It is way more complex than that. It is supposed to:

  • request refunds then void when the package is submitted
  • void if the package hasn't been submitted

If it is not behaving like this then we have a bug. In fact if you call cancel for something that has been submitted it will fail because it needs a refund call prior

exactly! and even worse it show in dashboard that it's canceled, as stated in #682

Shipment is marked as purchased I don't see a type submitted

also noticed another bug where the manifest dashboard take the first shipments reference and set it as the reference for the manifest

@danh91
Copy link
Member

danh91 commented Sep 11, 2024

submitted is a way of saying you are not going to create a manifest for it later so they mark it as purchased.
submitted == purchased

when you don't submit and add a group_id instead, it means you plan on manifesting at the end of the day.
So if you cancel before manifesting, it will just void without requesting refunds. because it wasn't submitted.
When you manifest then it gets marked as purchased.

At least that's how I understood it.

@jacobshilitz
Copy link
Collaborator

submitted is a way of saying you are not going to create a manifest for it later so they mark it as purchased. submitted == purchased

when you don't submit and add a group_id instead, it means you plan on manifesting at the end of the day. So if you cancel before manifesting, it will just void without requesting refunds. because it wasn't submitted. When you manifest then it gets marked as purchased.

At least that's how I understood it.

oh my bad, I thought you have submitted status in karrio.

that's correct, but I can't find the logic of canceling in the codebase, but this is how it supposed to be.

@danh91
Copy link
Member

danh91 commented Sep 11, 2024

Here is the current implementation. it is done in the proxy.py file

def cancel_shipment(self, requests: lib.Serializable) -> lib.Deserializable:
# retrieve shipment infos to check if refund is necessary
infos: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
lambda shipment_id: (
shipment_id,
lib.request(
url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{shipment_id}",
trace=self.trace_as("xml"),
method="GET",
headers={
"Content-Type": "application/vnd.cpc.shipment-v8+xml",
"Accept": "application/vnd.cpc.shipment-v8+xml",
"Authorization": f"Basic {self.settings.authorization}",
"Accept-language": f"{self.settings.language}-CA",
},
),
),
requests.serialize(),
)
# make refund requests for submitted shipments
refunds: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
lambda payload: (
payload["id"],
(
lib.request(
url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{payload['id']}/refund",
trace=self.trace_as("xml"),
data=payload["data"],
method="POST",
headers={
"Content-Type": "application/vnd.cpc.shipment-v8+xml",
"Accept": "application/vnd.cpc.shipment-v8+xml",
"Authorization": f"Basic {self.settings.authorization}",
"Accept-language": f"{self.settings.language}-CA",
},
)
if payload["data"] is not None
else payload["info"]
),
),
[
dict(
id=_,
info=info,
data=provider_utils.parse_submitted_shipment(info, requests.ctx),
)
for _, info in infos
],
)
# make cancel requests for non-submitted shipments
responses: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
lambda payload: (
payload["id"],
(
lib.request(
url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{payload['id']}",
trace=self.trace_as("xml"),
method="DELETE",
headers={
"Content-Type": "application/vnd.cpc.shipment-v8+xml",
"Accept": "application/vnd.cpc.shipment-v8+xml",
"Authorization": f"Basic {self.settings.authorization}",
"Accept-language": f"{self.settings.language}-CA",
},
)
if payload["refunded"]
else payload["response"]
),
),
[
dict(
id=_,
response=response,
refunded=(
getattr(lib.to_element(response), "tag", None)
!= "shipment-refund-request-info"
),
)
for _, response in refunds
],
)
return lib.Deserializable(
responses, lambda ___: [(_, lib.to_element(__)) for _, __ in ___]
)

@minchaminder
Copy link
Author

We also need to account for the possibility of multiple accounts for the same carrier, such as Canada Post. Therefore, the manifesting process must be exclusive per account to avoid conflicts or errors when generating manifests.

@danh91 danh91 linked a pull request Sep 28, 2024 that will close this issue
9 tasks
@danh91 danh91 mentioned this issue Sep 28, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants