-
Notifications
You must be signed in to change notification settings - Fork 136
EDU-15445-feat/-update-pick-and-pack-api #1482
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
base: master
Are you sure you want to change the base?
Conversation
c821b28
to
a9839d6
Compare
Thanks for your contribution. The .json file will be checked now with Spectral. |
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.
Request changes
626b575
to
6813f21
Compare
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.
Links are currently unavailable but will soon be fixed.
a35e128
to
028c145
Compare
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.
Please make the different changes reported in this PR.
I finished reviewing about 70% of the changes. I will create another review process at the same time so we can start working on the suggested changes in this check.
"items": { | ||
"type": "array", | ||
"items": { | ||
"$ref": "#/components/schemas/Item" | ||
"$ref": "#/components/schemas/Item", | ||
"description": "Item details." | ||
}, | ||
"description": "List of items." | ||
}, |
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.
This field should be a array of strings.
"items": {
"type": "array",
"items": { "type": "string", "description": "Item detail." }
},
"orderSource": { | ||
"type": "string", | ||
"description": "Order source." | ||
}, |
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.
This field should be a array but is a string.
Suggestion
"orderSource": {
"type": "array",
"items": {
"type": "object",
"properties": {
"rejectedQuantity": { "type": "integer" },
"sellingPrice": { "type": "number" },
"quantity": { "type": "integer" },
"notes": {
"type": "array",
"items": {
"type": "object",
"properties": {
"id": { "type": "string" },
"user": { "type": "string" },
"note": { "type": "string" }
},
"required": ["id", "user", "note"]
}
},
"orderId": { "type": "string", "minLength": 1 },
"replacedQuantity": { "type": "integer" },
"replacedBy": {
"type": "array",
"items": { "type": "string" }
},
"pickedQuantity": { "type": "integer" },
"currencyCode": {
"type": "string",
"description": "ISO 4217 currency code (COP, USD).",
"minLength": 3
},
"listPrice": { "type": "number" },
"status": {
"type": "string",
"enum": [
"PENDING",
"ASSIGNED",
"COLLECTED",
"REPLACED",
"REJECTED",
"REFUNDED",
"RETURNED",
"CANCELLED",
"PENDING_TRANSFERRED",
"TRANSFERRED",
"COMPLETED",
"READY_FOR_PACKING",
"READY_FOR_SHIPPING",
"SHIPPED",
"DELIVERED",
"PACKED",
"PAUSED"
],
"minLength": 1
}
},
"required": [
"rejectedQuantity",
"sellingPrice",
"quantity",
"notes",
"orderId",
"replacedQuantity",
"replacedBy",
"pickedQuantity",
"currencyCode",
"listPrice",
"status"
]
}
}
028c145
to
dd63405
Compare
…thub.com/vtex/openapi-schemas into EDU-15445-feat/-update-pick-and-pack-api
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.
Great work team on the changes you made 🎉! I found several changes that need to be implemented. I left many suggestions and comments in the PR, please take a look at them. Tks!
I finished reviewing 100% of the changes ✔️ .
"referenceCode": { | ||
"type": "string", | ||
"description": "Picked item reference code." | ||
}, |
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.
This field is new in Vtex-OpenAPI and was not present in the original schema, this should be removed.
"referenceCode": { | |
"type": "string", | |
"description": "Picked item reference code." | |
}, |
"orderSource" | ||
], | ||
"type": "object", | ||
"properties": { |
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.
items[].rejectedQuantity
: This field is not defined in entityitems
and this field should be required.items[].replacedQuantity
: This field is not defined in entityitems
and this field should be required.items[].temperature
: . This field is not defined in entityitems
and this field should be required .
"notes": { | ||
"type": "array", | ||
"description": "List of notes.", | ||
"items": { | ||
"type": "string", | ||
"description": "Note." | ||
}, | ||
"description": "List of notes." | ||
"type": "object", | ||
"description": "Note information.", | ||
"properties": { | ||
"date": { | ||
"type": "string", | ||
"description": "Date and time when the note was created." | ||
}, | ||
"type": { | ||
"type": "string", | ||
"description": "Type of the note.", | ||
"enum": ["COMMENT", "ERROR"] | ||
}, | ||
"content": { | ||
"type": "string", | ||
"description": "Content of the note." | ||
}, | ||
"author": { | ||
"type": "string", | ||
"description": "Author of the note." | ||
} | ||
} | ||
} | ||
}, |
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.
pickerAssignment[].notes
: This field should be a array
of strings
but is a array of objects.
"notes": { | |
"type": "array", | |
"description": "List of notes.", | |
"items": { | |
"type": "string", | |
"description": "Note." | |
}, | |
"description": "List of notes." | |
"type": "object", | |
"description": "Note information.", | |
"properties": { | |
"date": { | |
"type": "string", | |
"description": "Date and time when the note was created." | |
}, | |
"type": { | |
"type": "string", | |
"description": "Type of the note.", | |
"enum": ["COMMENT", "ERROR"] | |
}, | |
"content": { | |
"type": "string", | |
"description": "Content of the note." | |
}, | |
"author": { | |
"type": "string", | |
"description": "Author of the note." | |
} | |
} | |
} | |
}, | |
"notes": { | |
"type": "array", | |
"description": "List of notes.", | |
"items": { | |
"type": "string", | |
"description": "Note information." | |
} | |
}, |
"type": "array", | ||
"items": { | ||
"$ref": "#/components/schemas/GetWorksheetResponse", | ||
"description": "Historical worksheet." | ||
"$ref": "#/components/schemas/ItemRejected", |
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.
Reusing this schema conflicts with in components.schemas.GetOrderResponse.properties.rejectedItems.items.$ref
. For this property, I recommend defining a new schema because they handle different fields.
"replacedItems": { | ||
"type": "array", | ||
"items": { | ||
"$ref": "#/components/schemas/Item", | ||
"$ref": "#/components/schemas/ItemReplaced", |
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.
Reusing this schema conflicts with components.schemas.GetOrderResponse.properties.replacedItems.items.$ref
. For this property, I recommend defining a new schema because they handle different fields.
Warning
@@ -3044,21 +3705,13 @@ | |||
"seller", | |||
"status", |
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.
status
This field should be a enum but is not defined as such.
suggestion
"status": {
"type": "string",
"description": "Shipment status.",
"enum": [
"CREATED",
"PENDING",
"ASSIGNED",
"PICKED",
"ON_ROUTE",
"INCIDENT",
"RETURNED",
"DELIVERED",
"CANCELED",
"ON_HOLD",
"TRANSFER"
]
}
"status", | ||
"carrierName", | ||
"paymentMethod", | ||
"tags", | ||
"deliveredDate", | ||
"customerId", | ||
"customerAddressId", | ||
"source", | ||
"destination", | ||
"courierId", | ||
"courierName", | ||
"seller", | ||
"packagesQuantity", | ||
"itemsQuantity", |
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.
Good work on these changes, but we need to make the following changes to these properties in components.schemas.ListShipmentsResponse
:
courierId
: This field should be nullable but it is not nullable.courierName
: This field should be nullable but it is not nullable.paymentMethod
: This field should be an enum but it is not defined as one."paymentMethod": { "type": "string", "description": "Payment method.", "enum": ["ONLINE", "CASH", "CREDIT_CARD", "DEBIT_CARD", "OTHER"] }
serviceType
: This field should be an enum but it is not defined as one."serviceType": { "type": "string", "description": "Service type.", "enum": ["PICKUP_DELIVERY", "DELIVERY", "PICKUP"] }
status
: This field should be an enum but it is not defined as one."status": { "type": "string", "description": "Shipment status.", "enum": [ "CREATED", "PENDING", "ASSIGNED", "PICKED", "ON_ROUTE", "INCIDENT", "RETURNED", "DELIVERED", "CANCELED", "ON_HOLD", "TRANSFER" ] }
"servers": [ | ||
{ | ||
"url": "https://api.pick-and-pack.com/prod", | ||
"url": "https://api.pick-and-pack.com/prod/v1", | ||
"description": "Server to access the Pick and Pack API." | ||
} | ||
] |
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.
Perfect 🎉!
@@ -141,7 +335,7 @@ | |||
], | |||
"servers": [ | |||
{ | |||
"url": "https://api.pick-and-pack.com/prod", | |||
"url": "https://api.pick-and-pack.com/prod/v1", |
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.
In the VTEX documentation, it's using this endpoint by default for all resources: "https://auth.pickingnpacking.com/prod", even though each resource has its own "servers" set up explicitly.
This happens because it always takes the first position from the globally(root) defined server array.
I suggest changing the position of the globally defined servers from this:
"servers": [
{
"url": "https://auth.pickingnpacking.com/prod",
"description": "Server to access the authentication necessary to make a request in the Pick and Pack API."
},
{
"url": "https://api.pick-and-pack.com/prod/v1",
"description": "Server to access the Pick and Pack API."
}
],
to this:
"servers": [
{
"url": "https://api.pick-and-pack.com/prod/v1",
"description": "Server to access the Pick and Pack API."
},
{
"url": "https://auth.pickingnpacking.com/prod",
"description": "Server to access the authentication necessary to make a request in the Pick and Pack API."
}
],
This is done so the default endpoint becomes: "https://api.pick-and-pack.com/prod/v1"
"description": "Server to access the Pick and Pack API." | ||
} | ||
] | ||
} | ||
}, | ||
"/shipments/{orderId}": { | ||
"/shipments/": { |
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.
"/shipments/": { | |
"/shipments": { |
Types of changes
Changelog
Do not forget to update your changes to our Developer Portal's changelog. Did you create a release note?