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

[12.0][stock_available_mrp] Migration + make compatible with other stock_available_immediately #644

Merged
merged 18 commits into from Aug 31, 2019

Conversation

florian-dacosta
Copy link
Contributor

Hi, This PR has 2 commits.
First one is standard migration, second one try to solve a problem I have detected.

If I understand well, the purpose code in product_template.py that I removed (https://github.com/OCA/stock-logistics-warehouse/blob/11.0/stock_available_mrp/models/product_template.py#L14), was to be sure, in case a template with variants, that the immediately usable qty of our bom is the sum of all real planned qties of the variable (by default, virtual qty) + the potential quantity, which is the highest potential of its variants.

Indeed, we see in method _compute_available_quantities_dict of stock_available module : https://github.com/OCA/stock-logistics-warehouse/blob/12.0/stock_available/models/product_template.py#L29
that the computed immediately_usable_qty of our template is the sum of all quantities of the variants, but, the quantities of the variant, already include the potential quantity !
See : https://github.com/OCA/stock-logistics-warehouse/blob/11.0/stock_available_mrp/models/product_product.py#L122

So, without this code (https://github.com/OCA/stock-logistics-warehouse/blob/11.0/stock_available_mrp/models/product_template.py#L14) we add in template qty_immediately_usable the potential qty of all variants even though we want only to count the highest potential qty of the variant...

Anyway, the problem is that it makes it incompatible with others modules like stock_available_immediately. (and maybe others).
Indeed, stock_available_immediately remove the incoming quantity from the virtual quantity.
But the patch I removed (stock_available_immediately) go from virtual quantity once more...

That's what I have tried to correct in my second commit. For this I need to modify the logic in stock_available.
(+ make impossible to have a negative potential quantity, because I guess it makes no sense...but this have nothing to do with the problem described!)

It seems to work fine and the test passes without any problem.
But any opinion is welcome. @lmignon @Cedric-Pigeon @rafaelbn @tschanzt @simahawk @grindtildeath

@rousseldenis rousseldenis added this to the 12.0 milestone Jul 6, 2019
@rousseldenis
Copy link
Sponsor Contributor

@Cedric-Pigeon

)

res[product.id]['potential_qty'] = potential_qty
res[product.id]['immediately_usable_qty'] += potential_qty
Copy link
Contributor

Choose a reason for hiding this comment

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

I not sure of this...
Selección_302
I have a bom for a glass cookcing table
Selección_301
Also, I create a work order like this
Selección_303
As yo can see...
inmediately_usable_qty is 1 because is virtual_available and you increase all potential_qty so inmediately_usable_qty is 6.. Is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hello @sergio-teruel
Yes it is the behavior of the module, I did not change this.
The potential quantity is added to the immediately_usable_qty and it is intended.

@OCA-git-bot OCA-git-bot mentioned this pull request Jul 24, 2019
54 tasks
@rousseldenis
Copy link
Sponsor Contributor

@rafaelbn
Copy link
Member

Please @florian-dacosta take a quick read also in #611 Thank you for this great contribution! 😄

@florian-dacosta
Copy link
Contributor Author

@rousseldenis
It was not done in version 11? I'll take a look to include it in this PR then.

@rafaelbn Thanks for the pointer.
I won't include this here since it depends on a patch on Odoo...
But when it will be merged, if it is, we could be a improvment PR

@florian-dacosta
Copy link
Contributor Author

@rousseldenis Actually, both PR you pointed concern more the stock_available module, which has already been migrated to 12, and wich seems to contains all modifications.
But it does not concern this module (stock_available_mrp).
Unless I am missing something...
So nothing to do I guess.

bom_id = fields.Many2one(
'mrp.bom',
compute='_compute_bom_id',
string='Bill of Materials'
Copy link
Member

Choose a reason for hiding this comment

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

Change this string due to:

2019-08-16 15:08:49,323 175 WARNING openerp_test odoo.addons.base.models.ir_model: Two fields (bom_ids, bom_id) of product.product() have the same label: Bill of Materials.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@oca-clabot
Copy link

Hey @florian-dacosta, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Rebased to 12.0-ocabot-merge-pr-644-by-pedrobaeza-bump-no, awaiting test results.

@OCA-git-bot
Copy link
Contributor

@pedrobaeza The merge process could not be finalized, because command oca-gen-addon-readme --org-name OCA --repo-name stock-logistics-warehouse --branch 12.0 --addons-dir . --commit failed with output:

./stock_available_mrp/README.rst:81: (SEVERE/4) Title level inconsistent:

Authors
~~~~~~~
Traceback (most recent call last):
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 349, in check_subsection
    level = title_styles.index(style) + 1
ValueError: '~' is not in list

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/oca-gen-addon-readme", line 11, in <module>
    load_entry_point('oca-maintainers-tools', 'console_scripts', 'oca-gen-addon-readme')()
  File "/ocamt/lib/python3.6/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/ocamt/lib/python3.6/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/ocamt/lib/python3.6/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/ocamt/lib/python3.6/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/ocamt/src/oca-maintainers-tools/tools/gen_addon_readme.py", line 310, in gen_addon_readme
    check_rst(readme_filename)
  File "/ocamt/src/oca-maintainers-tools/tools/gen_addon_readme.py", line 241, in check_rst
    settings_overrides=RST2HTML_SETTINGS,
  File "/ocamt/lib/python3.6/site-packages/docutils/core.py", line 380, in publish_file
    enable_exit_status=enable_exit_status)
  File "/ocamt/lib/python3.6/site-packages/docutils/core.py", line 664, in publish_programmatically
    output = pub.publish(enable_exit_status=enable_exit_status)
  File "/ocamt/lib/python3.6/site-packages/docutils/core.py", line 217, in publish
    self.settings)
  File "/ocamt/lib/python3.6/site-packages/docutils/readers/__init__.py", line 71, in read
    self.parse()
  File "/ocamt/lib/python3.6/site-packages/docutils/readers/__init__.py", line 77, in parse
    self.parser.parse(self.input, document)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/__init__.py", line 191, in parse
    self.statemachine.run(inputlines, document, inliner=self.inliner)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 171, in run
    input_source=document['source'])
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 239, in run
    context, state, transitions)
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 460, in check_line
    return method(match, context, next_state)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 3007, in text
    self.section(title.lstrip(), source, style, lineno + 1, messages)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 327, in section
    self.new_subsection(title, lineno, messages)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 395, in new_subsection
    node=section_node, match_titles=True)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 282, in nested_parse
    node=node, match_titles=match_titles)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 196, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 239, in run
    context, state, transitions)
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 460, in check_line
    return method(match, context, next_state)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 2771, in underline
    self.section(title, source, style, lineno - 1, messages)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 327, in section
    self.new_subsection(title, lineno, messages)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 395, in new_subsection
    node=section_node, match_titles=True)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 282, in nested_parse
    node=node, match_titles=match_titles)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 196, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 239, in run
    context, state, transitions)
  File "/ocamt/lib/python3.6/site-packages/docutils/statemachine.py", line 460, in check_line
    return method(match, context, next_state)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 2771, in underline
    self.section(title, source, style, lineno - 1, messages)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 326, in section
    if self.check_subsection(source, style, lineno):
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 355, in check_subsection
    self.parent += self.title_inconsistent(source, lineno)
  File "/ocamt/lib/python3.6/site-packages/docutils/parsers/rst/states.py", line 373, in title_inconsistent
    line=lineno)
  File "/ocamt/lib/python3.6/site-packages/docutils/utils/__init__.py", line 237, in severe
    return self.system_message(self.SEVERE_LEVEL, *args, **kwargs)
  File "/ocamt/lib/python3.6/site-packages/docutils/utils/__init__.py", line 195, in system_message
    raise SystemMessage(msg, level)
docutils.utils.SystemMessage: ./stock_available_mrp/README.rst:81: (SEVERE/4) Title level inconsistent:

Authors
~~~~~~~

sbidoul and others added 14 commits August 31, 2019 09:23
Compute potential quantities for both product templates and variants. To keep the code simple, only the biggest potential of any single variant is accounted for in the template's potential.
Take all levels of phantom BoM into account, respects validity dates etc. thanks to the use of the standard method _bom_explode, as suggested by @gdgellatly in OCA#5 (comment)
Improve tests, rewritten in python.
Adhere to new file/manifest/README conventions.
Simplify copyright headers
sudo is not required since mrp.bom are readable to groups with access to the qty_x fields on a product. Moreover using sudo to retrive the bom will ignore the company_id defined on the bom
Record rules used to not be checked on stock quants, but now they are since Odoo's commit 2fd14db (odoo/odoo@2fd14db).
In our test we changed the company of the products and BoMs but we neglected that the stock was not attached to the right company, and that made the test fail.
To fix that, make the test inventory for the right company.
Since there is a little inconsistency in the demo data with a negative quantity of an unrelated product, use the `partial` filter for the inventories instead of the `none` filter, so that no wrong inventory lines are added automatically.
    * mrp_bom.name has been deleted.
    * mrp_bom_line.type moved to mrp_bom.type.
    * Fix missing group_mrp_user issue.
    * Change versions
@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Rebased to 12.0-ocabot-merge-pr-644-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Aug 31, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit 1312f68 into OCA:12.0 Aug 31, 2019
@OCA-git-bot
Copy link
Contributor

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

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

i-vyshnevska pushed a commit to camptocamp/stock-logistics-warehouse that referenced this pull request Apr 13, 2020
Signed-off-by pedrobaeza
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