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

feat(stock): Pick List Scan #30832

Merged
merged 30 commits into from
May 11, 2022
Merged

Conversation

dj12djdjs
Copy link
Collaborator

@dj12djdjs dj12djdjs commented Apr 28, 2022

closes: #30821

Proposed Solution

Please note for the examples bellow there are 4 items Item A - Item D with barcode 'a' corresponding to Item A, etc...

  • Restrict BarcodeScanner from creating new rows.
    By default BarcodeScanner will create a row if batch_no or item_code doesn't exist.
    In the case of pick list only the rows generated from sales order or other supported documents
    should be considered when scanning. Ex. We are trying to add Item B through barcode 'b' but it doesn't add row
    because Item B isn't in the list of items to pick.
    feat-pick-list-scan_prevent-new-row

  • Restrict BarcodeScanner from exceeding maximum pickable qty provided by pick list.
    By default BarcodeScanner will continue to increment the quantity of the scanned item.
    In the case of pick list a maximum quantity will be reached. I've modified the BarcodeScanner opts to support this.
    feat-pick-list-scan_max-qty

  • Support multiple rows with the same item code.
    feat-pick-list-scan_multiple-item_codes

  • Prevent Picked Qty from being fulfilled automatically when pick list is submitted. There should be some validation Ex: Picker didn't pick all the items.
    dont allow delivery note with full qty picked

  • Might be good to have an option prompt for qty after a scan. This way if a picking order calls for 50pcs It won't be necessary to scan all 50pcs. Just one, then input qty to increment.
    prompt qty

Please post your suggestions / feedback.

@ankush
Copy link
Member

ankush commented Apr 28, 2022

Restrict BarcodeScanner from creating new rows.

Makes sense for picklist. I was gonna add this too.

Restrict BarcodeScanner from exceeding the maximum pickable qty provided by picklist.

This also makes sense. However, IMO this check should be implemented in Pick List's JS and not in the barcode scanner? Do you know anywhere else where max qty validation will be required with barcode scanning?

Support multiple rows with the same item code.

Behavior shown in GIF looks good 👍

Prevent Picked Qty from being fulfilled automatically when pick list is submitted. There should be some validation Ex: Picker didn't pick all the items.

Yeah, this will be a good addition. We can just check before_submit if all stock_qty == picked_qty and if not we can warn the user.

Support Simultaneous pickers scanning items for pick list

Not sure if I understand what this means 😅

Two pickers working on same doc on different computers? I highly doubt that's doable because of the way the framework works right now.

@ankush ankush self-assigned this Apr 28, 2022
@dj12djdjs
Copy link
Collaborator Author

Support Simultaneous pickers scanning items for pick list

Not sure if I understand what this means 😅

Two pickers working on same doc on different computers? I highly doubt that's doable because of the way the framework works right now.

I kind of figured this. It is one of the requirements I have for my client. I will have to develop some middle man to handle this.

@szufisher
Copy link
Contributor

please also consider catch and handle scan same serial number multi times, currently duplicate serial number will be added to serial no field and quantity increase by 1. program need to prevent this and feedback as warning.

@ankush
Copy link
Member

ankush commented Apr 29, 2022

I kind of figured this. It is one of the requirements I have for my client. I will have to develop some middle man to handle this.

You can use real-time updates using socket.io for this. Real-time synchronization of normal data structures like JSON (document object) would quite likely lead to conflicts and unexpected behavior 😄 You'll have to implement some kind of locking/synchronization primitives on top of the document object to prevent races.

please also consider catch and handle scan same serial number multi times, currently duplicate serial number will be added to serial no field and quantity increase by 1. program need to prevent this and feedback as warning.

This is fixed in develop version. Since it was part of bigger changes it didn't get backported, maybe I should port fixes backto v13 😅

@dj12djdjs
Copy link
Collaborator Author

This also makes sense. However, IMO this check should be implemented in Pick List's JS and not in the barcode scanner? Do you know anywhere else where max qty validation will be required with barcode scanning?

@ankush I agree with what you're saying. I was originally implementing with that concept in mind, until I came to the realization I needed to be able to scan multiple rows of the same item code. The barcode scan API now relies specifying a maximum quantity so it knows to move to the next item.
https://github.com/dj12djdjs/erpnext/blob/8053f2dbcd835602d86dca520b8629b0ac866656/erpnext/public/js/utils/barcode_scanner.js#L199-L202

@dj12djdjs
Copy link
Collaborator Author

please also consider catch and handle scan same serial number multi times, currently duplicate serial number will be added to serial no field and quantity increase by 1. program need to prevent this and feedback as warning.

This is fixed in develop version. Since it was part of bigger changes it didn't get backported, maybe I should port fixes backto v13

@szufisher It seems this might be fixed in develop branch. I'm assuming serial_no has something to do with batch qty. I have no experience with this workflow.

I've setup test site, could you check if the behavior is working as expected for serial no.?
https://feat-picklist-scan.frappe.cloud/
demo@example.com
password

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #30832 (e9cf5cb) into develop (60eede0) will decrease coverage by 0.16%.
The diff coverage is 66.66%.

❗ Current head e9cf5cb differs from pull request most recent head 861d4b8. Consider uploading reports for the commit 861d4b8 to get more accurate results

@@             Coverage Diff             @@
##           develop   #30832      +/-   ##
===========================================
- Coverage    63.06%   62.90%   -0.17%     
===========================================
  Files          984      983       -1     
  Lines        67166    67064     -102     
===========================================
- Hits         42360    42185     -175     
- Misses       24806    24879      +73     
Impacted Files Coverage Δ
erpnext/stock/doctype/pick_list/pick_list.py 76.63% <66.66%> (-0.13%) ⬇️
...eport/item_variant_details/item_variant_details.py 31.32% <0.00%> (-53.02%) ⬇️
...tch_item_expiry_status/batch_item_expiry_status.py 67.92% <0.00%> (-24.53%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 67.79% <0.00%> (-22.04%) ⬇️
...em_wise_sales_register/item_wise_sales_register.py 51.58% <0.00%> (-9.51%) ⬇️
...e_sales_analytics/supplier_wise_sales_analytics.py 81.96% <0.00%> (-8.20%) ⬇️
erpnext/utilities/transaction_base.py 75.40% <0.00%> (-7.38%) ⬇️
...unts/report/purchase_register/purchase_register.py 71.71% <0.00%> (-6.58%) ⬇️
erpnext/hr/doctype/job_offer/job_offer.py 82.92% <0.00%> (-4.88%) ⬇️
...pnext/setup/doctype/sales_partner/sales_partner.py 65.21% <0.00%> (-4.35%) ⬇️
... and 35 more

@ankush
Copy link
Member

ankush commented May 3, 2022

@ankush I agree with what you're saying. I was originally implementing with that concept in mind, until I came to the realization I needed to be able to scan multiple rows of the same item code. The barcode scan API now relies specifying a maximum quantity so it knows to move to the next item.

makes sense. Let's go with max_qty for now, if I can think of a better solution I'll comment / implement later.

@dj12djdjs dj12djdjs marked this pull request as ready for review May 3, 2022 15:07
@dj12djdjs dj12djdjs requested a review from ankush May 3, 2022 15:07
@dj12djdjs
Copy link
Collaborator Author

@ankush I agree with what you're saying. I was originally implementing with that concept in mind, until I came to the realization I needed to be able to scan multiple rows of the same item code. The barcode scan API now relies specifying a maximum quantity so it knows to move to the next item.

makes sense. Let's go with max_qty for now, if I can think of a better solution I'll comment / implement later.

@ankush I'm going to be implementing barcode scan for Packing Slip as well. Maybe max_qty will be useful over there.

@dj12djdjs dj12djdjs requested a review from ankush May 5, 2022 15:02
@dj12djdjs
Copy link
Collaborator Author

@ankush I've added another feature for prompting qty. It will be useful so user doesn't have to scan an item so many times for high qty. Let me know your thoughts.

@ankush
Copy link
Member

ankush commented May 6, 2022

@ankush I've added another feature for prompting qty. It will be useful so user doesn't have to scan an item so many times for high qty. Let me know your thoughts.

We have also had a similar request, though UX for this needs to be reviewed. Some users would want to scan everything, and some would want a prompt.

There are even cases where users want prompt for buying cycle (where large qty are bought) but individual scans for the selling cycle where qty is usually small. 😬

cc: @marination

@dj12djdjs
Copy link
Collaborator Author

@ankush I've added another feature for prompting qty. It will be useful so user doesn't have to scan an item so many times for high qty. Let me know your thoughts.

We have also had a similar request, though UX for this needs to be reviewed. Some users would want to scan everything, and some would want a prompt.

There are even cases where users want prompt for buying cycle (where large qty are bought) but individual scans for the selling cycle where qty is usually small. 😬

cc: @marination

These are also very useful, without these features barcode scan feature is weak. Although, I think I should publish these in a separate PR as they are more for the whole of the barcode scan API.

Copy link
Member

@ankush ankush left a comment

Choose a reason for hiding this comment

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

last round of minor changes. Tested locally, mostly LGTM.

Parking batch/serial UX for later, it's bit awkward in picklist right now.

ankush added 7 commits May 11, 2022 18:38
Current design of picklist doc doesn't have "picked serial no" so it
doesn't make much sense to scan it. Instead it should increment qty
only. Might add this in future after fixing UX problems
There was separate function for batch row which frequently didn't
receive all the love main function received like:
1. empty row reuse
2. max qty validation

Hence it makes sense to combine these in one fat function
ankush
ankush previously approved these changes May 11, 2022
@ankush
Copy link
Member

ankush commented May 11, 2022

@dj12djdjs please update the docs page. There are some differences here for "prompt" and "scan mode"

we transpile to es2017, so allow use of es2020
@ankush ankush merged commit 2d9b7a4 into frappe:develop May 11, 2022
@ankush ankush changed the title feat(stock): Proposed Pick List Scan feat(stock): Pick List Scan May 11, 2022
@dj12djdjs
Copy link
Collaborator Author

@ankush Will do, apologies for the delay

@dj12djdjs
Copy link
Collaborator Author

@ankush Should I target the doc for V14 only?

@ankush
Copy link
Member

ankush commented May 12, 2022

@dj12djdjs yes. This depends on changes to barcode scanner that aren't on v13.

@dj12djdjs
Copy link
Collaborator Author

docs update https://docs.erpnext.com/compare?wiki_page=8c53d48a87&&compare=0fc7ad55c8

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scanning in picklist
3 participants