[16.0][ADD] stock_quant_package_fast_move #1428

angelinaanaki · 2023-11-15T07:54:10Z

This PR is dependent on changes introduced in PR #1422, specifically on the module stock_picking_move_package_to_package.

rousseldenis · 2023-11-15T08:13:37Z

@angelinaanaki Thanks for this. Couldn't you use a batch picking flow instead (several picking at once) - and maybe extend that process ?

ivs-cetmix · 2023-11-15T09:50:24Z

@angelinaanaki Thanks for this. Couldn't you use a batch picking flow instead (several picking at once) - and maybe extend that process ?

Hi @rousseldenis just wondering why should we use the batch picking flow if we can transfer multiple package in a singe picking using package level?

rousseldenis · 2023-11-15T09:52:07Z

@angelinaanaki Thanks for this. Couldn't you use a batch picking flow instead (several picking at once) - and maybe extend that process ?

Hi @rousseldenis just wondering why should we use the batch picking flow if we can transfer multiple package in a singe picking using package level?

Ok, need testing the flow

ivs-cetmix

Code review LGTM. Please fix comment regarding link format in the /readme files and squash commits afterwards

stock_quant_package_fast_move/readme/CONTRIBUTORS.md

Bearnard21

All features work as expected

Aldeigja

Functional LGTM

geomer198

LGTM!

DemchukM

In some files, you have an incorrect license reference.
The link is http://www.gnu.org/licenses/lgpl and you need http://www.gnu.org/licenses/agpl

Please review the files and correct them.

Everything else is fine

DemchukM

LGTM

DemchukM · 2024-01-07T15:59:58Z

stock_quant_package_fast_move/models/stock_quant_package.py

+        or if the destination package does not belong to the specified location.
+        """
+        # Check if the location is different from the current location
+        if location == self[0].location_id and not destination_package:


Why do you only check the location_id in the first package?

What will happen if packages from different location_ids are selected?

Before this, validation ensures the source location is the same for all packages. - https://github.com/OCA/stock-logistics-workflow/pull/1428/files#diff-8d3622c031c6c7da6edf3c7e1d6e83021cacb13df6f5cc2f25b518262fae59deR47

I can't spot where is that check? And why is this necessary that they are all in the same source location?

Suggested change

if location == self[0].location_id and not destination_package:

if location in self.location_id and not destination_package:

stock_quant_package_fast_move/wizard/stock_quant_package_fast_move_wizard.py

stock_quant_package_fast_move/models/res_company.py

stock_quant_package_fast_move/wizard/stock_quant_package_fast_move_wizard.py

yelizariev · 2024-01-08T14:14:27Z

stock_quant_package_fast_move/models/stock_quant_package.py

+    def create_new_package(self):
+        """
+        Create a new stock package.
+
+        Returns:
+        - package (recordset of stock.quant.package): The newly created package object.
+        """
+        package = self.env["stock.quant.package"].create({})
+        return package


Maybe just create a package inline?

Maybe just create a package inline?

This is in case someone would need to implement some custom flow here

DemchukM

LGTM

yelizariev

LGTM

ivs-cetmix · 2024-02-15T13:16:14Z

stock_quant_package_fast_move/models/stock_quant_package.py

        picking.action_confirm()
-        picking.button_validate()
+        if no_validate:


Should't it be if not no_validate ?

If you check 'Don't Validate', the created internal picking won't be validated. Otherwise, it will be validated and the package will directly moved to the specified destination location

ivs-cetmix · 2024-02-15T14:06:38Z

stock_quant_package_fast_move/models/stock_quant_package.py

        """
        Move packages to a specified location.

        Parameters:
        - location (recordset of stock.location): The destination location.
        - destination_package (recordset of stock.quant.package, optional):
        Optional destination package. If provided, it must belong to the specified location.
-        - no_validate (boolean, optional):
+        - validate (boolean, optional):


If set to True, the created picking will be validated. Otherwise it will remain in the "Ready" state.

Thank you for your review. Could you please check it again?

ivs-cetmix

LGTM

ivs-cetmix · 2024-03-06T20:34:09Z

stock_quant_package_fast_move/models/stock_quant_package.py

+        }
+
+    def check_source_location(self):
+        if any(package.location_id != self[0].location_id for package in self):


Please remove this restriction because this scenario is possible.

ivs-cetmix

LGTM

GabbasovDinar

LGTM

hoangtrann · 2024-08-01T17:20:54Z

stock_quant_package_fast_move/models/stock_quant_package.py

+        - validate (boolean, optional):
+        If set to True, the created picking will be validated.
+        Otherwise it will remain in the "Ready" state. Defaults to False.


validate is required in args, is this docstring accurate?

hoangtrann · 2024-08-09T11:21:21Z

Tested and it's working as expected.

However, I noticed that when I choose not to Validate in the pop-up wizard, regardless of choosing Destination Package or Put in new Pack, the Package Content is blank.

Validating the picking and go back to Package Content, it's now showing.

angelinaanaki · 2024-08-09T14:56:57Z

Tested and it's working as expected.

However, I noticed that when I choose not to Validate in the pop-up wizard, regardless of choosing Destination Package or Put in new Pack, the Package Content is blank.

Validating the picking and go back to Package Content, it's now showing.

@hoangtrann Thank you very much for your review! The issue was due to the moves being created only at the package level while in the 'draft' state, as seen here. I resolved this by adding the 'done' state after the picking confirmation. Could you please review it again and let me know your thoughts?

hoangtrann

Looking good, package is now showing content if not validate immediately

ivs-cetmix · 2024-08-20T11:16:21Z

test-requirements.txt

stock_picking_move_package_to_package was merged, please remove it from thetest-requirements

Done. I’ve removed stock_picking_move_package_to_package from the test-requirements.

OCA-git-bot · 2024-08-24T14:00:01Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

ivs-cetmix · 2024-11-13T21:45:51Z

Hey @OCA/logistics-maintainers would be great to have this PR merged. Not only it's already "ready for merge" but also has "fast" in the module name! 😄

jbaudoux

Just minor remarks. I wonder if all the options are necessary (validate & batch)?

What happens if the moved package was reserved by another operation? Do you want to allow the move?

jbaudoux · 2024-11-13T22:24:03Z

stock_quant_package_fast_move/models/res_company.py

+        string="Package Move Operation",
+        domain="[('show_entire_packs', '=', True)]",
+    )
+    use_batch_transfers = fields.Boolean()


Is there any use for having this option?

jbaudoux · 2024-11-13T22:25:25Z

stock_quant_package_fast_move/models/stock_quant_package.py

+            "context": self.env.context,
+        }
+
+    def create_batch_transfer(


Suggested change

def create_batch_transfer(

def _move_with_batch_transfer(

jbaudoux · 2024-11-13T22:26:49Z

stock_quant_package_fast_move/models/stock_quant_package.py

+        use_batch_transfers = active_company.use_batch_transfers
+
+        if use_batch_transfers:
+            self.create_batch_transfer(


Suggested change

self.create_batch_transfer(

self._move_with_batch_transfer(

jbaudoux · 2024-11-13T22:28:42Z

stock_quant_package_fast_move/models/stock_quant_package.py

+                location, destination_package, validate, picking_type_id
+            )
+        else:
+            # Create a picking


If you create a method for above option, create also a method for this option like
_move_with_picking(location, destination_package, validate, picking_type_id)
to be consistent

jbaudoux · 2024-11-13T22:30:27Z

stock_quant_package_fast_move/models/stock_quant_package.py

+
+        return True
+
+    def create_new_package(self):


If you need this hook, then move it inside the wizard as this serves the wizard only

jbaudoux · 2024-11-13T22:32:40Z

stock_quant_package_fast_move/wizard/stock_quant_package_fast_move_wizard.py

+        "stock.location", "Destination Location", required=True
+    )
+    put_in_new_package = fields.Boolean("Put in New Package")
+    validate = fields.Boolean()


Is there any use for not validating the movement?

@jbaudoux yes, sometimes we create a picking and then validate it later when goods a physically moved.

jbaudoux · 2024-11-13T22:41:22Z

stock_quant_package_fast_move/readme/USAGE.md

@@ -0,0 +1,15 @@
+- Go to **Inventory -> Products -> Packages**.
+
+- Open a package or select several packages in the list view. **Important**: All selected packages must be located in the same location.


The important restriction was dropped. This need to be removed then. And the location check here to be fixed https://github.com/OCA/stock-logistics-workflow/pull/1428/files#r1444030377

ivs-cetmix · 2024-11-15T10:22:03Z

Just minor remarks. I wonder if all the options are necessary (validate & batch)?

What happens if the moved package was reserved by another operation? Do you want to allow the move?

Hi @jbaudoux , first of all - thank you for your review! Regarding your question - sounds reasonable. Let us check this.

Return created picking after move package to new location

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 05b91f5 to 8b5b8ab Compare November 15, 2023 08:54

angelinaanaki marked this pull request as ready for review November 15, 2023 09:00

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from f2fa39a to e48963d Compare November 15, 2023 13:43

ivs-cetmix approved these changes Nov 21, 2023

View reviewed changes

stock_quant_package_fast_move/readme/CONTRIBUTORS.md Outdated Show resolved Hide resolved

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 58899f6 to 372a0b2 Compare November 21, 2023 12:53

Bearnard21 approved these changes Nov 26, 2023

View reviewed changes

OCA-git-bot added the approved label Nov 26, 2023

ivs-cetmix force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 372a0b2 to f2d8214 Compare November 26, 2023 17:26

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 2669c53 to 4ae70da Compare December 10, 2023 13:43

Aldeigja approved these changes Dec 12, 2023

View reviewed changes

geomer198 approved these changes Dec 28, 2023

View reviewed changes

DemchukM reviewed Dec 28, 2023

View reviewed changes

DemchukM approved these changes Dec 31, 2023

View reviewed changes

DemchukM reviewed Jan 7, 2024

View reviewed changes

stock_quant_package_fast_move/wizard/stock_quant_package_fast_move_wizard.py Outdated Show resolved Hide resolved

yelizariev reviewed Jan 8, 2024

View reviewed changes

angelinaanaki requested review from DemchukM and yelizariev January 10, 2024 15:11

DemchukM approved these changes Jan 10, 2024

View reviewed changes

yelizariev approved these changes Jan 11, 2024

View reviewed changes

ivs-cetmix suggested changes Feb 15, 2024

View reviewed changes

OCA-git-bot removed the approved label Feb 15, 2024

ivs-cetmix reviewed Feb 15, 2024

View reviewed changes

ivs-cetmix approved these changes Feb 15, 2024

View reviewed changes

OCA-git-bot added the approved label Feb 15, 2024

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch 2 times, most recently from 9cdb07a to 8f56ef0 Compare February 15, 2024 16:04

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 8f56ef0 to 888da7c Compare February 26, 2024 21:08

ivs-cetmix suggested changes Mar 6, 2024

View reviewed changes

OCA-git-bot removed the approved label Mar 6, 2024

ivs-cetmix approved these changes Mar 6, 2024

View reviewed changes

OCA-git-bot added the approved label Mar 6, 2024

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch 2 times, most recently from 1bbcb4b to d0a585a Compare March 6, 2024 22:04

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 1c7779d to cbf149d Compare July 3, 2024 07:13

GabbasovDinar approved these changes Aug 1, 2024

View reviewed changes

hoangtrann reviewed Aug 1, 2024

View reviewed changes

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 99bff74 to b70bc8b Compare August 3, 2024 11:26

hoangtrann approved these changes Aug 4, 2024

View reviewed changes

hoangtrann approved these changes Aug 9, 2024

View reviewed changes

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch 2 times, most recently from eb21513 to 874ec61 Compare August 15, 2024 06:25

ivs-cetmix reviewed Aug 20, 2024

View reviewed changes

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch 2 times, most recently from a1c45ce to 729119c Compare August 20, 2024 12:26

OCA-git-bot added the ready to merge label Aug 24, 2024

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from 729119c to d43ab73 Compare August 27, 2024 20:46

[16.0][ADD] stock_quant_package_fast_move

dafbc70

angelinaanaki force-pushed the 16.0-t3043-stock_quant_package_fast_move-new_module branch from ee64a74 to dafbc70 Compare August 27, 2024 21:36

jbaudoux reviewed Nov 13, 2024

View reviewed changes

[IMP] stock_quant_package_fast_move: Return picking

c032910

Return created picking after move package to new location

	if location == self[0].location_id and not destination_package:
	if location in self.location_id and not destination_package:

		@@ -0,0 +1,15 @@
		- Go to Inventory -> Products -> Packages.

		- Open a package or select several packages in the list view. Important: All selected packages must be located in the same location.

[16.0][ADD] stock_quant_package_fast_move #1428

Are you sure you want to change the base?

[16.0][ADD] stock_quant_package_fast_move #1428

Conversation

angelinaanaki commented Nov 15, 2023

rousseldenis commented Nov 15, 2023 • edited Loading

ivs-cetmix commented Nov 15, 2023

rousseldenis commented Nov 15, 2023

ivs-cetmix left a comment

Choose a reason for hiding this comment

Bearnard21 left a comment

Choose a reason for hiding this comment

Aldeigja left a comment

Choose a reason for hiding this comment

geomer198 left a comment

Choose a reason for hiding this comment

DemchukM left a comment

Choose a reason for hiding this comment

DemchukM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DemchukM left a comment

Choose a reason for hiding this comment

yelizariev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivs-cetmix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivs-cetmix left a comment

Choose a reason for hiding this comment

GabbasovDinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoangtrann commented Aug 9, 2024

angelinaanaki commented Aug 9, 2024

hoangtrann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OCA-git-bot commented Aug 24, 2024

ivs-cetmix commented Nov 13, 2024

jbaudoux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivs-cetmix commented Nov 15, 2024

rousseldenis commented Nov 15, 2023 •

edited

Loading