-
-
Notifications
You must be signed in to change notification settings - Fork 618
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
[ADD] new module pos_picking_load #183
Conversation
}, | ||
|
||
on_click_picking: function(event){ | ||
this.load_picking(parseInt(event.target.parentNode.dataset.pickingId)); |
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.
add a radix:
parseInt(event.target..., 10)
pickingline.innerHTML = pickingline_html; | ||
pickingline = pickingline.childNodes[1]; | ||
pickingline.addEventListener('click', this.on_click_picking); | ||
contents.appendChild(pickingline); |
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.
it's better (for the perfs) if you add all nodes in one DOM call.
you may also add the event listener on the collection instead of each row
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.
Thanks for your review, I'll do the changes. About that point, could you point me some samples ?
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.
var self = this; | ||
var contents = this.$el[0].querySelector('.picking-list-contents'); | ||
contents.innerHTML = ""; | ||
for (var i = 0, len = pickings.length; i < len; i++){ |
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.
(just for the sytle: you can use .forEach()
or .map()
here instead of for loop.
And in javascript the scope delimited by functions and not {})
[IMP] improve JS display for picking list
Dear Sylvain, you are the POS maestro ;-) This module is a great example of how to extend the POS functionality. I have no code remarks, and I tested the described functionality succesfully. My only gripe is that there is no connection between the original sale order and the new picking anymore. Did you look into linking the new picking to the sale order through the procurement group, or even reuse the existing stock moves if possible, to improve tracking of products? I can propose a change into your branch if you like. Or in case you think this change can be considered after the merge of this PR, I will give my approval right away. |
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.
Code review and test
👍 |
Hi @StefanRijnhart. You're fully right about the lake of connection with the original sale order (forced to 'done') / stock picking (forced to 'canceled') / and stock moves (forced to 'canceled'). The main reasons are :
But yes, your idea could be a great improvment. We could imagine to overload create_picking (https://github.com/odoo/odoo/blob/8.0/addons/point_of_sale/point_of_sale.py#L830) function if the pos.order come from an existing picking, and altering the original picking (and moves), instead of creating a new one, and cancelling the previous one, and then setting the altered picking to the picking_id field of the pos.order. As this PR has two 👍, I propose to merge it, and to improve it in another PR. Please ping me when it's done, so that I'll review it. |
@legalsylvain perfect! I will work in it in the next two weeks or so. |
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
[ADD] new module pos_picking_load
Hi all.
This PR add a new module pos_picking_load that allow you to load pickings created by sale module, in the point of sale, to finish the workflow via the point of sale.
You can see the whole description in the readme file with screenshots.
https://github.com/grap/pos/blob/8.0_ADD_pos_picking_load/pos_picking_load/README.rst
In a few words:
new button available
![load_picking_01](https://user-images.githubusercontent.com/3407482/27388379-9ca8cd4e-569b-11e7-8eec-26af30419fe8.png)
List of pickings:
![load_picking_02](https://user-images.githubusercontent.com/3407482/27388400-ad5c4a9e-569b-11e7-87db-bdfd869761f4.png)
Once the picking is loaded:
![load_picking_03](https://user-images.githubusercontent.com/3407482/27388502-f7305b10-569b-11e7-9b0f-fe7c05391239.png)
Thanks for your review.