NEW : allow stock management by product #30996
Conversation
| /** | ||
| * @var int | ||
| */ | ||
| public $no_button_copy; |
There was a problem hiding this comment.
This property is no more used in develop. Can you remove it?
| /** | ||
| * @var int | ||
| */ | ||
| public $no_button_copy; |
There was a problem hiding this comment.
This property is no more used in develop. Can you remove it?
There was a problem hiding this comment.
@eldy I just did it.
thomas-Ngr
force-pushed
the
develop_new-stockableproduct_3
branch
from
aeda220 to
39d5603
Compare
thomas-Ngr
force-pushed
the
develop_new-stockableproduct_3
branch
from
39d5603 to
151692f
Compare
eldy
added
the
PR to fix - See feedback in comments
thomas-Ngr
force-pushed
the
develop_new-stockableproduct_3
branch
from
bc8840d to
151692f
Compare
htdocs/product/card.php
Outdated
| @@ -813,6 +816,9 @@ | |||
| $object->fk_default_bom = 0; | |||
| } | |||
|
|
|||
| // managed_in_stock | |||
| $object->stockable_product = GETPOSTISSET('stockable_product'); | |||
There was a problem hiding this comment.
This line looks strange.
if stockable_product = 0,
you wil get
$object->stockable_product = true.
i think what you want to do is
$object->stockable_product = GETPOSTINT('stockable_product');
There was a problem hiding this comment.
No, since stockable_product is a checkbox.
- When not checked, $_POST['stockable_product'] is not set
- When checked, $_POST['stockable_product'] is equal to string 'on'. As a consequence,
GETPOSTINT('stockable_product') == 0.
The exact way to deal with it is $object->stockable_product = (int) GETPOST('stockable_product') == 'on'. But it makes useless operations and it does not harm security to $object->stockable_product = GETPOSTISSET('stockable_product'); .
I will cast it to (int).
| * | ||
| * @var boolean | ||
| */ | ||
| public $stockable_product = true; |
There was a problem hiding this comment.
Using boolean must be avoid as it can't be used to evolve, fr example when we need a third state.
It is better to always use int (even for boolean) so
/* @var integer */
public $stockable_product = 1;
htdocs/expedition/card.php
Outdated
| $qty .= '_'.$j; | ||
| } | ||
|
|
||
| if ($p->stockable_product == Product::DISABLED_STOCK) { |
There was a problem hiding this comment.
This part of code from line 366 to 389 looks strange:
Currently, default is to have all product managed as stockable.
So we should have code added to avoid to do something when product is not stock managed. But here yo add code to do more things when stock is not managed.
I think we should not have this part of code from line 366 to 389 and we should have the code to do or not action depending of stockable status somewhere later.
If we remove this part of code, where does the program hangs ?
There was a problem hiding this comment.
You are right. I changed the test in Exepedition::addline() to not test stock remaining if product is not managed in stock. see 2407fe0.
For products with a batch/serial, it makes sense to force stock management. I made some changes in product/card to avoid having batch/lot products not managed in stock.
eldy
added
github_note
…t with disabled stock management
thomas-Ngr
force-pushed
the
develop_new-stockableproduct_3
branch
8 times, most recently
from
5e90c18 to
a7e42cc
Compare
thomas-Ngr
force-pushed
the
develop_new-stockableproduct_3
branch
from
a7e42cc to
8834529
Compare
| $line = new ExpeditionLigne($this->db); // always set $line for PHAN analyser. @phan-var-force muse be used after an assignation, and there is no assignation for $line. | ||
| while ($i < $num) { | ||
| $obj = $this->db->fetch_object($resql); | ||
|
|
||
|
|
||
| if ($originline > 0 && $originline == $obj->fk_elementdet) { | ||
| '@phan-var-force ExpeditionLigne $line'; // $line from previous loop | ||
| // '@phan-var-force ExpeditionLigne $line'; // $line from previous loop |
There was a problem hiding this comment.
@phan-var-force (l.1732) must be used after an assignation, and we don't have any assignation for $line at the beginning of the loop.
Declaring $line before starting the loop is the solution I found to pass Phan tests.
NEW : allow stock management by product
When using product stocks, users may want to have some products for which they don't want to manage stocks, for example in case of subcontracting.
This PR:
Discussion started on #26184 and #26200
SQL Part : #26190
follow-up on #30230 (closed) because file expedition.class has been split into two files.