-
-
Notifications
You must be signed in to change notification settings - Fork 240
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
Added update_shipment_tracking_details and list_shipment_boxes #1615
Conversation
Reviewer's Guide by SourceryThis PR adds two new endpoints to the FulfillmentInbound API class: list_shipment_boxes for retrieving box package information and update_shipment_tracking_details for updating shipment tracking information. Both endpoints are implemented as decorated methods using the sp_endpoint decorator with appropriate HTTP methods and URL patterns. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @abrihter - I've reviewed your changes - here's some feedback:
Overall Comments:
- The docstring for list_shipment_boxes has incorrect parameter descriptions - pageSize description is currently copying the shipment description. Please update with correct parameter descriptions.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sp_endpoint('/inbound/fba/<version>/inboundPlans/{}/shipments/{}/trackingDetails', method='PUT') | ||
def update_shipment_tracking_details(self, inboundPlanId, shipmentId, **kwargs) -> ApiResponse: | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider adding validation for the required 'body' parameter
The method should validate that the body parameter is present and contains the required structure before making the request to prevent silent failures.
@sp_endpoint('/inbound/fba/<version>/inboundPlans/{}/shipments/{}/trackingDetails', method='PUT') | |
def update_shipment_tracking_details(self, inboundPlanId, shipmentId, **kwargs) -> ApiResponse: | |
""" | |
@sp_endpoint('/inbound/fba/<version>/inboundPlans/{}/shipments/{}/trackingDetails', method='PUT') | |
def update_shipment_tracking_details(self, inboundPlanId, shipmentId, **kwargs) -> ApiResponse: | |
"""Updates the tracking details for a shipment. | |
Args: | |
inboundPlanId (str): The identifier for the inbound plan. | |
shipmentId (str): The identifier for the shipment. | |
**kwargs: Additional arguments | |
body (dict): Required. The request body containing tracking details. | |
Must contain: | |
- trackingDetails (list): List of tracking information | |
Returns: | |
ApiResponse | |
Raises: | |
ValueError: If the required body parameter or its required structure is missing | |
""" | |
if 'body' not in kwargs: | |
raise ValueError("The 'body' parameter is required for updating tracking details") | |
if not isinstance(kwargs['body'].get('trackingDetails'), list): | |
raise ValueError("The 'body' must contain 'trackingDetails' as a list") | |
Added two new endpoints (FulfillmentInbound)
updateShipmentTrackingDetails
listShipmentBoxes
Summary by Sourcery
Add two new endpoints to the FulfillmentInbound API: 'list_shipment_boxes' for retrieving shipment box details and 'update_shipment_tracking_details' for updating shipment tracking information.
New Features: