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

[16.0][ADD] shipment_advice_planner #88

Closed
wants to merge 1 commit into from

Conversation

sbejaoui
Copy link
Contributor

@sbejaoui sbejaoui commented Feb 10, 2023

This module is used to plan ready transfers in shipment advices.

As this is the base module, it provides only a simple
planning mode. Transfers are grouped to have a single shipment advice by warehouse.

@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch 5 times, most recently from 84f8ca8 to b1480f2 Compare February 12, 2023 18:08
@sbejaoui
Copy link
Contributor Author

cc/ @lmignon

@rousseldenis rousseldenis added this to the 16.0 milestone Feb 13, 2023
@rousseldenis
Copy link
Sponsor Contributor

@sbejaoui Thanks for this. Don't forget to put your dependencies links in your PR description to help reviewers.

@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch from b1480f2 to 2792a23 Compare February 13, 2023 10:07
Copy link
Sponsor Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

A little comment but LGTM (Code + Functional tests)

@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch 3 times, most recently from 7e1c4fa to 5b11c92 Compare February 13, 2023 17:13
Copy link
Sponsor Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

LGTM (Code review + Functional)

@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch from 5b11c92 to 2970f90 Compare February 15, 2023 09:51
@sbejaoui
Copy link
Contributor Author

@lmignon , please check my last commit. It adds warehouse and dock to the planner with some data consistency checks.

@simahawk simahawk changed the title [16.0][ADD] - add shipment_advice_planner [16.0][ADD] shipment_advice_planner Mar 10, 2023
@lmignon lmignon force-pushed the 16.0-shipment_advice_planner-sbj branch 2 times, most recently from 0ccee46 to 1b5e1e6 Compare March 29, 2023 12:58
@lmignon
Copy link
Sponsor Contributor

lmignon commented Mar 29, 2023

@rousseldenis @jbaudoux This one is ready for review and then merge 👼

@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch 4 times, most recently from ae2bfc0 to 0239264 Compare July 5, 2023 13:49
@rousseldenis
Copy link
Sponsor Contributor

@sbejaoui Could you look at comments ?

@sbejaoui sbejaoui requested a review from jbaudoux August 31, 2023 19:13
@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch 2 times, most recently from 4205437 to 46a04c9 Compare September 1, 2023 10:39
@rafaelbn
Copy link
Member

rafaelbn commented Sep 1, 2023

Great contribution! @moduon testit

Copy link

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes. I will test it again.

Can you include me for the solution design?

shipment_advice_planner/__manifest__.py Outdated Show resolved Hide resolved
Copy link

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

LGTM, not retested yet

This module is used to plan ready transfers in shipment advices.

As this is the base module, it provides only a simple:  Transfers
are grouped to have a single shipment advice by warehouse.

[IMP] shipment_advice_planner: Add init script

[FIX] - shipment_advice_planner: fix typo

[FIX] - group pickings to plan by picking type instead of the warehouse

[IMP] - shipment_advice_planner: add search for can_be_planned_in_shipment_advice field

[IMP] shipment_advice_planner: shipment advice is created confirmed and arrival and departure date

[IMP] - shipment_advice_planner: add unit tests

[IMP] shipment_advice_planner: add author & maintainer

Co-authored-by: Jacques-Etienne Baudoux <je@bcim.be>

[IMP] add depends_context to _compute_picking_to_plan_ids
@sbejaoui sbejaoui force-pushed the 16.0-shipment_advice_planner-sbj branch from 2e237b6 to 7d36e2d Compare September 26, 2023 11:45
@sbejaoui
Copy link
Contributor Author

@rousseldenis ,
can you update your review please, thanks.

Copy link

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG, 1 minor remark and... the commit msg should not contain the odoo version

@@ -0,0 +1,3 @@
from . import common
Copy link

Choose a reason for hiding this comment

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

no need to import common in any test suite

@@ -0,0 +1,2 @@
* Souheil Bejaoui <souheil.bejaoui@acsone.eu>
* Laurent Mignon <laurent.mignon@acsone.eu>
Copy link

Choose a reason for hiding this comment

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

JE to be added here?

@simahawk
Copy link

simahawk commented Oct 6, 2023

@sbejaoui I had some time and needed to move fwd w/ PRs depending on this one.
I took the chance to address the last comments on #106
and fast track merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants