Skip to content

[16.0][IMP] sell_only_by_packaging: support negative rounding when converting packaging qty#3248

Closed
QuocDuong1306 wants to merge 2 commits intoOCA:16.0from
QuocDuong1306:16.0-imp-sell_only_by_packaging
Closed

[16.0][IMP] sell_only_by_packaging: support negative rounding when converting packaging qty#3248
QuocDuong1306 wants to merge 2 commits intoOCA:16.0from
QuocDuong1306:16.0-imp-sell_only_by_packaging

Conversation

@QuocDuong1306
Copy link
Copy Markdown

Currently, on upstream, It's possible to support creating return orders when filling negative quantities in product_uom_qty on Sale Order lines.

This PR will help to round down the product quantities to the nearest multiple of the packaging quantity in that case.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

float_compare(
qty,
0.0,
precision_rounding=0.001,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why this value and not another ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @rousseldenis , I just based on the original one (positive rounding, here). Do you have any suggestion for this rounding?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, of course, using the product rounding.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank @rousseldenis for your suggestions, a new commit has been added

Copy link
Copy Markdown
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

LGTM

@rousseldenis rousseldenis added this to the 16.0 milestone Sep 17, 2024
@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-3248-by-rousseldenis-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 17, 2024
Signed-off-by rousseldenis
@OCA-git-bot
Copy link
Copy Markdown
Contributor

@rousseldenis The merge process could not be finalized, because command oca-gen-addon-readme --if-source-changed --org-name OCA --repo-name sale-workflow --branch 16.0 --addons-dir /tmp/tmp7uf7_sw0 --commit failed with output:

/tmp/tmp7uf7_sw0/sell_only_by_packaging/README.rst:45: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.
Traceback (most recent call last):
  File "/usr/local/bin/oca-gen-addon-readme", line 8, in <module>
    sys.exit(gen_addon_readme())
             ^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/src/oca-maintainers-tools/tools/gen_addon_readme.py", line 575, in gen_addon_readme
    check_rst(readme_filename)
  File "/ocamt/src/oca-maintainers-tools/tools/gen_addon_readme.py", line 398, in check_rst
    publish_file(
  File "/ocamt/lib/python3.12/site-packages/docutils/core.py", line 422, in publish_file
    output, publisher = publish_programmatically(
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/core.py", line 722, in publish_programmatically
    output = publisher.publish(enable_exit_status=enable_exit_status)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/core.py", line 234, in publish
    self.document = self.reader.read(self.source, self.parser,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/readers/__init__.py", line 70, in read
    self.parse()
  File "/ocamt/lib/python3.12/site-packages/docutils/readers/__init__.py", line 76, in parse
    self.parser.parse(self.input, document)
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/__init__.py", line 184, in parse
    self.statemachine.run(inputlines, document, inliner=self.inliner)
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 169, in run
    results = StateMachineWS.run(self, input_lines, input_offset,
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/statemachine.py", line 233, in run
    context, next_state, result = self.check_line(
                                  ^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/statemachine.py", line 445, in check_line
    return method(match, context, next_state)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 3034, in text
    self.section(title.lstrip(), source, style, lineno + 1, messages)
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 325, in section
    self.new_subsection(title, lineno, messages)
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 391, in new_subsection
    newabsoffset = self.nested_parse(
                   ^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 279, in nested_parse
    state_machine.run(block, input_offset, memo=self.memo,
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 195, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/statemachine.py", line 233, in run
    context, next_state, result = self.check_line(
                                  ^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/statemachine.py", line 445, in check_line
    return method(match, context, next_state)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 1282, in bullet
    self.parent += self.unindent_warning('Bullet list')
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/parsers/rst/states.py", line 432, in unindent_warning
    return self.reporter.warning('%s ends without a blank line; '
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/utils/__init__.py", line 224, in warning
    return self.system_message(self.WARNING_LEVEL, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ocamt/lib/python3.12/site-packages/docutils/utils/__init__.py", line 197, in system_message
    raise SystemMessage(msg, level)
docutils.utils.SystemMessage: /tmp/tmp7uf7_sw0/sell_only_by_packaging/README.rst:45: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-imp-sell_only_by_packaging branch from 93569c4 to f8637f8 Compare September 19, 2024 15:19
@ivs-cetmix
Copy link
Copy Markdown
Member

/ocabot rebase

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-imp-sell_only_by_packaging branch from f8637f8 to 444f5da Compare May 11, 2025 19:42
@ivs-cetmix
Copy link
Copy Markdown
Member

Hi @QuocDuong1306 , thank you for your contribution! Could you please squash the commits?

For example, if your packaging is set to sell by 5 units and the employee fill
the quantity with 3, the quantity will be automatically replaced by 5 (it always rounds up).
For example,
- To sell packaging (fill positive product quantities), if your packaging is set to sell by 5 units and the employee fill
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a blank line before the list, like this:

For example:

- To sell....

@cyrilmanuel
Copy link
Copy Markdown

@QuocDuong1306 hi, could you make the modification to let this pr be merge please :) thanks a lot for your effort

Copy link
Copy Markdown

@alexey-pelykh alexey-pelykh 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 working on this, nice addition for return order support.

The rounding logic in _convert_packaging_qty looks correct to me -- the forced_qty -= q adjustment for negative quantities properly rounds away from zero (toward more negative), which is the expected mirror of the existing round-up behavior for positive quantities.

Good catch also on switching precision_rounding=0.001 to precision_rounding=uom.rounding -- using the UoM's actual precision is more correct than the hardcoded value.

Tests cover the key negative-qty scenarios well.

Two things still needed before this can merge:

  1. The RST formatting issue that @ivs-cetmix already flagged in README.rst also exists in readme/DESCRIPTION.rst (same source). The bullet list needs a blank line before it and the continuation lines should be indented to align with the list item text. This is what caused oca-gen-addon-readme to fail on the previous merge attempt.

  2. Commits need squashing as requested.

@cyrilmanuel
Copy link
Copy Markdown

hi guys, i've superseed this pr here --> #4196

you can close this one :)

@rousseldenis
Copy link
Copy Markdown
Contributor

Closing as solved by: #4196

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants