[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 #713

RodrigoBM · 2023-10-03T11:47:49Z

issue #534

Migration from 14.0

...very_carrier_package_measure_required/tests/test_delivery_carrier_package_measure_require.py

xAdrianCif

Tech review, thanks for the job!

RodrigoBM · 2023-11-22T11:23:35Z

is it possible to merge this, @OCA/logistics-maintainers, thank you

RodrigoBM · 2023-11-22T11:25:48Z

ping @TDu @rousseldenis @simahawk @jbaudoux @lmignon

jorgemartinez-factorlibre

Tech review, good job @RodrigoBM

github-actions · 2024-05-12T12:26:31Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

hugosantosred · 2024-05-21T11:13:01Z

@OCA/logistics-maintainers Could you review for merge or apply the no stale label to this PR? Thanks!

simahawk · 2024-05-21T11:19:37Z

@RodrigoBM can you rebase pls?

simahawk

@TDu ping for review ;)

This module adds the possibility to flag some measurements on package to be required for delivery.

The required dimensions on a package need not to be enforce before the picking is being done (package with content). Because that blocks any preparation on the package before it is delivered. But we still want to be able to check if the package is valid at will. This can be done with the `delivery_pkg_measure__ignore_package_content` context key.

RodrigoBM · 2024-05-21T11:59:41Z

@RodrigoBM can you rebase pls?

Done @simahawk

delivery_carrier_package_measure_required/models/stock_picking.py

jbaudoux · 2024-05-21T13:07:31Z

delivery_carrier_package_measure_required/models/stock_quant_package.py

-            "delivery_pkg_measure__ignore_package_content"
+            "delivery_pkg_measure__ignore_package_content", False
+        )
+        force_validation = self.env.context.get(
+            "delivery_pkg_measure__force_validation_package", False
        )
        for package in self:
-            if not ignore_pack_content and not package.quant_ids:
+            if ignore_pack_content or (not force_validation and not package.quant_ids):


I think we better stick to previously existing context variable (and not invert behavior). I don't find it clearer with the new name.

@jbaudoux , I think this is better now.

Before the changes we don't check the dimensions in the case if ignore_pack_content is False and the package has no quants.
We also have a variable that is passed by context which is delivery_pkg_measure__ignore_package_content that if you pass it with the value set to True you would expect it not to check the dimensions but this is not the case.

Option1:
ignore_pack_content = True, package.quant_ids = True
(not ignore_pack_content and not package.quant_ids) if == False --> Check dimensions
Opción 2:
ignore_pack_content = True, package.quant_ids = False
(not ignore_pack_content and not package.quant_ids) if == False --> Check dimensions
Opción 3:
ignore_pack_content = False, package.quant_ids = True
(not ignore_pack_content and not package.quant_ids) if == False --> Check dimensions
Opción 4:
ignore_pack_content = False, package.quant_ids = False
(not ignore_pack_content and not package.quant_ids) if == True --> No Check dimensions

But what if we want to be able to skip the dimension check in other cases?

Option1:
ignore_pack_content = False, force_validation = True, package.quant_ids = True
ignore_pack_content or (not force_validation and not package.quant_ids) if = False --> Check dimensions
Opción 2:
ignore_pack_content = False, force_validation = True, package.quant_ids = False
ignore_pack_content or (not force_validation and not package.quant_ids) if = False --> Check dimensions
Opción 3:
ignore_pack_content = False, force_validation = False, package.quant_ids = True
ignore_pack_content or (not force_validation and not package.quant_ids) if = False --> Check dimensions
Opción 4:
ignore_pack_content = False, force_validation = False, package.quant_ids = False
ignore_pack_content or (not force_validation and not package.quant_ids) if = True --> No Check dimensions
Opción 5:
ignore_pack_content = True
ignore_pack_content or (not force_validation and not package.quant_ids) if = True --> No Check dimensions

What do you think about this? this can go in another commit if you think it is more correct.

however, the delivery_pkg_measure__ignore_package_content context I have not seen used in other OCA modules.

I can't find any usage of this context variable. @TDu What was the purpose of it?
Do we need any of those context variables? If this is useless, I'm more in favor of simplifying. We can already configure which dimension is required.

I understand @jbaudoux , but now I have client developments that need these contexts. We can change this "if ignore_pack_content or (not force_validation and not package.quant_ids):" to "if self.ignore_check_dimensions():" and have everyone inherit the method to whatever we need.

As commented in the code that content key delivery_pkg_measure__ignore_package_content was meant to be used to force asking for dimension even for empty packages.

I think the idea behind it was, you receive a new product in your warehouse, all packaging (A, B) for that product are not configured properly. The product arrives in packaging A. But you want the user to configure all packaging (A and B) for that product on first reception.

Now indeed it does not look like it is used 🤔

Is it a problem to accept this new proposal? Do you need it to be in a separate commit?

Thanks for the clarification @TDu
@RodrigoBM Your current proposal is fine for me. It is clear and covers all the cases.
Just put that in a separate commit :)

Done @jbaudoux, sorry for the delay

OCA-git-bot · 2024-05-21T13:07:50Z

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). 🤖

OCA-git-bot · 2024-05-21T13:07:50Z

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). 🤖

jbaudoux · 2024-05-21T13:12:15Z

/ocabot migration delivery_carrier_package_measure_required

TDu · 2024-05-23T08:03:47Z

delivery_carrier_package_measure_required/models/stock_picking.py

+        If package is None all result packages on the stock pickings are checked
+        otherwise only the one


Suggested change

If package is None all result packages on the stock pickings are checked

otherwise only the one

docstring out of sync with the code ?

Now that I think about it, I think so

this is now resolved, @TDu?

RodrigoBM · 2024-05-27T07:15:51Z

ping @TDu @jbaudoux

jbaudoux · 2024-05-30T07:58:56Z

@RodrigoBM As a general rule, can you keep commit subject (first line) short and put description in the body (separated by a blank line from the subject).

Like:

delivery_carrier_package_measure_required: put in pack

Add measures to package when you add product to pack

add measures to package when you add producto to pack.

…ceptions greater granularity is implemented to control possible dimension control exceptions.

RodrigoBM · 2024-05-30T08:09:28Z

@RodrigoBM As a general rule, can you keep commit subject (first line) short and put description in the body (separated by a blank line from the subject).

Like:
delivery_carrier_package_measure_required: put in pack

Add measures to package when you add product to pack

Done. I will take this tip in consideration in the next contributions.

jbaudoux

Thanks

jbaudoux · 2024-05-30T08:20:43Z

/ocabot merge nobump

OCA-git-bot · 2024-05-30T08:21:03Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-713-by-jbaudoux-bump-nobump, awaiting test results.

OCA-git-bot · 2024-05-30T08:28:12Z

Congratulations, your PR was merged at fb001cb. Thanks a lot for contributing to OCA. ❤️

RodrigoBM changed the title ~~[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0~~ [WIP][16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 Oct 3, 2023

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from 2094b7e to c2d36c1 Compare October 3, 2023 12:00

RodrigoBM mentioned this pull request Oct 3, 2023

Migration to version 16.0 #534

Open

32 tasks

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from c2d36c1 to f839883 Compare October 3, 2023 13:32

xAdrianCif reviewed Oct 4, 2023

View reviewed changes

...very_carrier_package_measure_required/tests/test_delivery_carrier_package_measure_require.py Outdated Show resolved Hide resolved

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch 2 times, most recently from 0623c66 to 8379158 Compare October 4, 2023 11:32

xAdrianCif approved these changes Oct 4, 2023

View reviewed changes

RodrigoBM changed the title ~~[WIP][16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0~~ [16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 Oct 19, 2023

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from 8379158 to 1452fbe Compare October 23, 2023 09:53

jorgemartinez-factorlibre approved these changes Jan 8, 2024

View reviewed changes

OCA-git-bot added the approved label Jan 8, 2024

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch 3 times, most recently from d8a54b9 to 6b0ef2a Compare January 10, 2024 17:05

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 12, 2024

simahawk added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels May 21, 2024

simahawk reviewed May 21, 2024

View reviewed changes

TDu and others added 6 commits May 21, 2024 13:58

Add delivery_carrier_package_measure_required

45f2d11

This module adds the possibility to flag some measurements on package to be required for delivery.

delivery_carrier_package_measure_required 14.0.1.0.1

781528f

dc_package_measure_required: fix warning constrains

c36b170

delivery_carrier_package_measure_required 14.0.1.0.2

4202b4e

Added translation using Weblate (Spanish)

299caf6

[IMP] delivery_carrier_package_measure_required: pre-commit stuff

5f13364

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from 6b0ef2a to f05c941 Compare May 21, 2024 11:58

jbaudoux reviewed May 21, 2024

View reviewed changes

OCA-git-bot added ready to merge labels May 21, 2024

OCA-git-bot added this to the 16.0 milestone May 21, 2024

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from f05c941 to a285edf Compare May 22, 2024 13:54

TDu reviewed May 23, 2024

View reviewed changes

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from a285edf to 62f0a35 Compare May 23, 2024 09:05

[MIG] delivery_carrier_package_measure_required: Migration to 16.0"

ba92579

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from 62f0a35 to ca06d2b Compare May 30, 2024 07:35

RodrigoBM added 2 commits May 30, 2024 10:05

[IMP] delivery_carrier_package_measure_required: put in pack

9d544d6

add measures to package when you add producto to pack.

[IMP] delivery_carrier_package_measure_required: dimension control ex…

dbe2017

…ceptions greater granularity is implemented to control possible dimension control exceptions.

RodrigoBM force-pushed the 16.0-mig-delivery_carrier_package_measure_required branch from ca06d2b to dbe2017 Compare May 30, 2024 08:07

jbaudoux approved these changes May 30, 2024

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label May 30, 2024

OCA-git-bot merged commit b422a9c into OCA:16.0 May 30, 2024
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels May 30, 2024

RodrigoBM deleted the 16.0-mig-delivery_carrier_package_measure_required branch May 30, 2024 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 #713

[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 #713

RodrigoBM commented Oct 3, 2023

xAdrianCif left a comment

RodrigoBM commented Nov 22, 2023

RodrigoBM commented Nov 22, 2023

jorgemartinez-factorlibre left a comment

github-actions bot commented May 12, 2024

hugosantosred commented May 21, 2024 •

edited

Loading

simahawk commented May 21, 2024

simahawk left a comment

RodrigoBM commented May 21, 2024

jbaudoux May 21, 2024

RodrigoBM May 22, 2024

RodrigoBM May 22, 2024

jbaudoux May 22, 2024

RodrigoBM May 23, 2024

RodrigoBM May 23, 2024

TDu May 23, 2024 •

edited

Loading

RodrigoBM May 23, 2024

jbaudoux May 27, 2024

RodrigoBM May 30, 2024

OCA-git-bot commented May 21, 2024

OCA-git-bot commented May 21, 2024

jbaudoux commented May 21, 2024

TDu May 23, 2024

RodrigoBM May 23, 2024

RodrigoBM May 23, 2024

RodrigoBM May 24, 2024 •

edited

Loading

RodrigoBM commented May 27, 2024

jbaudoux commented May 30, 2024 •

edited

Loading

RodrigoBM commented May 30, 2024

jbaudoux left a comment

jbaudoux commented May 30, 2024

OCA-git-bot commented May 30, 2024

OCA-git-bot commented May 30, 2024

		If package is None all result packages on the stock pickings are checked
		otherwise only the one

[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 #713

[16.0][MIG] delivery_carrier_package_measure_required: Migration to 16.0 #713

Conversation

RodrigoBM commented Oct 3, 2023

xAdrianCif left a comment

Choose a reason for hiding this comment

RodrigoBM commented Nov 22, 2023

RodrigoBM commented Nov 22, 2023

jorgemartinez-factorlibre left a comment

Choose a reason for hiding this comment

github-actions bot commented May 12, 2024

hugosantosred commented May 21, 2024 • edited Loading

simahawk commented May 21, 2024

simahawk left a comment

Choose a reason for hiding this comment

RodrigoBM commented May 21, 2024

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

TDu May 23, 2024 • edited Loading

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

OCA-git-bot commented May 21, 2024

OCA-git-bot commented May 21, 2024

jbaudoux commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RodrigoBM May 24, 2024 • edited Loading

Choose a reason for hiding this comment

RodrigoBM commented May 27, 2024

jbaudoux commented May 30, 2024 • edited Loading

RodrigoBM commented May 30, 2024

jbaudoux left a comment

Choose a reason for hiding this comment

jbaudoux commented May 30, 2024

OCA-git-bot commented May 30, 2024

OCA-git-bot commented May 30, 2024

hugosantosred commented May 21, 2024 •

edited

Loading

TDu May 23, 2024 •

edited

Loading

RodrigoBM May 24, 2024 •

edited

Loading

jbaudoux commented May 30, 2024 •

edited

Loading