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

Implemented: common admin permission on ship now button if any shipment package require tracking (#281) #289

Merged
merged 7 commits into from
Oct 30, 2023

Conversation

alsoK2maan
Copy link

Related Issues

Closes #281

Short Description and Why It's Useful

Screenshots of Visual Changes before/after (If There Are Any)

IMPORTANT NOTICE - Remember to add changelog entry

Contribution and Currently Important Rules Acceptance

Copy link
Contributor

@dt2patel dt2patel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested some variable name changes

// if there are orders with tracking required and label image present
const trackingRequiredOrders = orderList.filter((order: any) => this.isTrackingRequiredForAnyShipmentPackage(order))
if (trackingRequiredOrders.length) {
const orderHasMissingLabelImage = orderList.some((order: any) => order.missingLabelImage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the terminology here should be around "trackingCode" so that even if label image is missing but user puts tracking code on shipment the variable still makes semantic sense.

@@ -10,4 +10,5 @@ export default {
"APP_UNPACK_ORDER": "COMMON_ADMIN",
"APP_RECYCLE_ORDER": "COMMON_ADMIN",
"APP_STOREFULFILLMENT_ADMIN": "STOREFULFILLMENT_ADMIN",
"APP_SHIP_ORDER": "COMMON_ADMIN"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permission name should also include that this is "forceful" like "APP_FORCE_SHIP_ORDER" because for normal orders anyone is allowed to ship them

@@ -332,6 +331,15 @@ export default defineComponent({
handler: async () => {
let orderList = JSON.parse(JSON.stringify(this.completedOrders.list))

// if there are orders with tracking required and label image present
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we only need to exclude those orders for which tracking is required but the shipping label is missing. Please correct the logic.

if (resp.status == 200 && !hasError(resp)) {
!trackingRequiredAndMissingLabelOrders.length
? showToast(translate('Orders shipped successfully'))
: showToast(translate('out of cannot be shipped due to missing tracking codes.', { remainingOrders: trackingRequiredAndMissingLabelOrders.length, totalOrders: packedOrdersCount }))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: showToast(translate('out of cannot be shipped due to missing tracking codes.', { remainingOrders: trackingRequiredAndMissingLabelOrders.length, totalOrders: packedOrdersCount }))
: showToast(translate('out of orders cannot be shipped due to missing tracking codes.', { remainingOrders: trackingRequiredAndMissingLabelOrders.length, totalOrders: packedOrdersCount }))

@@ -609,6 +624,13 @@ export default defineComponent({
},
fetchProductStock(productId: string) {
this.store.dispatch('stock/fetchStock', { productId })
},
isTrackingRequiredForAnyShipmentPackage(order: any) {
if (!order.shipmentPackages) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the length of shipment packages.

@ravilodhi ravilodhi merged commit 74ae56b into hotwax:main Oct 30, 2023
1 check passed
R-Sourabh added a commit to R-Sourabh/fulfillment-pwa that referenced this pull request Apr 16, 2024
R-Sourabh added a commit to R-Sourabh/fulfillment-pwa that referenced this pull request Apr 16, 2024
R-Sourabh added a commit to R-Sourabh/fulfillment-pwa that referenced this pull request Apr 18, 2024
R-Sourabh added a commit to R-Sourabh/fulfillment-pwa that referenced this pull request Apr 22, 2024
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 this pull request may close these issues.

Restrict Marking Orders as Shipped for Tracking-Required Orders
5 participants