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 split beesdoo product product main supplier #411

Merged

Conversation

victor-champonnois
Copy link
Member

@victor-champonnois victor-champonnois commented Jun 9, 2022

fix #392
fix #409
fix #391

Split part of beesdoo_product into:

  • product_main_supplier
  • sale_suggested_price
  • sale_edit_price_wizard (renamed to sale_adapt_price_wizard)

The dependency graph is the following :
sale_edit_price_wizard -> sale_suggested_price -> product_main_supplier

Todo : merge #320

Migration is done in modules with this method

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch 3 times, most recently from b9eb927 to 402fc16 Compare June 9, 2022 09:00
@victor-champonnois victor-champonnois marked this pull request as draft June 9, 2022 09:01
@victor-champonnois
Copy link
Member Author

victor-champonnois commented Jun 9, 2022

@carmenbianca Could you review this commit c43d439? It merges the changes from your PR #320
Since it is a bit complex I would like a check on it.

@victor-champonnois victor-champonnois marked this pull request as ready for review June 9, 2022 12:31
@victor-champonnois
Copy link
Member Author

victor-champonnois commented Jul 11, 2022

status: waiting for #320 to be merged and then backport it

@victor-champonnois
Copy link
Member Author

victor-champonnois commented Jul 29, 2022

todo:

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from ea8948c to 44812ed Compare September 16, 2022 13:48
@victor-champonnois
Copy link
Member Author

todo:

* rebase and modify the PR to account for [[IMP] beesdoo_product: On changing purchase price, set sales price to suggested price #320](https://github.com/beescoop/Obeesdoo/pull/320)

Done

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2022

Codecov Report

Merging #411 (4709d64) into 12.0 (fa7679b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             12.0     #411   +/-   ##
=======================================
  Coverage   39.26%   39.26%           
=======================================
  Files          44       44           
  Lines        1541     1541           
  Branches      293      293           
=======================================
  Hits          605      605           
  Misses        933      933           
  Partials        3        3           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@victor-champonnois
Copy link
Member Author

Todo : wait for #261 to be merged and rebase

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from c2d2205 to a2ba475 Compare September 20, 2022 12:26
@victor-champonnois victor-champonnois changed the base branch from 12.0 to 12.0-add-search_main_seller_id_product_code September 20, 2022 12:27
@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from a2ba475 to b97190b Compare September 20, 2022 12:49
@victor-champonnois
Copy link
Member Author

@robinkeunen @carmenbianca @huguesdk @remytms ready for review

@victor-champonnois
Copy link
Member Author

@polchampion deployed in test, ready for testing
in particular, need to test that all the functionalities related to the price list, the suggested price, and the main supplier are OK.

@victor-champonnois victor-champonnois force-pushed the 12.0-add-search_main_seller_id_product_code branch from b3de0ed to bfe27f2 Compare September 22, 2022 14:22
Base automatically changed from 12.0-add-search_main_seller_id_product_code to 12.0 September 22, 2022 14:44
@victor-champonnois
Copy link
Member Author

@polchampion Voici les fonctionnalités affectées par le split:

Module product_main_supplier

  • Add a main supplier
  • Allow searching a product given the product reference of the main
    supplier (also known as the product_code of the product.supplierinfo
    object).

Module suggested_price

  • Add a 'Suggested Price' field on products, and a 'Product Margin' field on Partners (Vendors) and Product Categories.
    The first margin is used if set, otherwise the second margin (which has a default value) is used.
  • The reference price on which this margin is applied (supplier price or sale price)
    can be selected in the general settings.
  • Also, sale and supplier taxes that are of type 'percentage' and that are marked as 'included in price'
    are taken into account when computing the suggested price.
  • Round suggested price to 5 cents

Module sale_adapt_price_wizard

  • Adds "Edit Price" submenu on Point Of Sale, Purchase and Sale modules.
    The user lands on an editable List View with the following columns :

    • Name (name)
    • Main Seller (main_seller_id)
    • Purchase Price (purchase_price)
    • Purchase Unit of Measure (uom_po_id)
    • Suggested Price (suggested_price)
    • Sales Price (list_price)
    • Unit of Measure (uom_id)

    The only editable field is Purchase Price.
    Through "Action > Adapt Sales Price", the user can, on the selected products,
    adapt the Sales Price according to the Suggested Price.
    Alternatively, 'Automatically write suggested price' can be enabled to make
    this last step automatic.

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from 07f9fec to ed9bed1 Compare September 27, 2022 08:50
Copy link
Collaborator

@polchampion polchampion left a comment

Choose a reason for hiding this comment

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

@victor-champonnois Thanks a lot for the detailed functionalities, this makes it a lot easier to test.

Module product_main_supplier

  • Adds a main supplier
  • Allows searching a product given the product reference of the main
    supplier (also known as the product_code of the product.supplierinfo
    object).

Module suggested_price

  • Adds a 'Suggested Price' field on products templates
  • Adds a 'Product Margin' field on Partners (Vendors) and Product Categories.
  • The first margin is used if set, otherwise the second margin (which has a default value) is used.
  • The reference price on which this margin is applied (supplier price or sale price) can be selected in the general settings. --> Somebody please explain to me the sense of the "applied on sale price" computation, cause I never understood that piece of maths.
  • (Not tested) Also, sale and supplier taxes that are of type 'percentage' and that are marked as 'included in price' are taken into account when computing the suggested price.
  • Round suggested price to 5 cents --> NOK - What should I test?

Module sale_adapt_price_wizard

  • Adds "Edit Price" submenu on Point Of Sale, Purchase and Sale modules --> NOK on POS app - ALSO : please move the "Edit Price" menu item before the "Configuration" menu item.
  • The user lands on an editable List View with the following columns :
    Name (name)
    Main Seller (main_seller_id)
    Purchase Price (purchase_price)
    Purchase Unit of Measure (uom_po_id)
    Suggested Price (suggested_price)
    Sales Price (list_price)
    Unit of Measure (uom_id)
    --> AND Sales price last updated
  • The only editable field is Purchase Price.
  • Through "Action > Adapt Sales Price", the user can, on the selected products, adapt the Sales Price according to the Suggested Price. --> NOK see error copied below
  • Alternatively, 'Automatically write suggested price' can be enabled to make
    this last step automatic. --> NOK doesn't work
Odoo Server Error
Traceback (most recent call last):
  File "/home/odoo12/src/odoo/odoo/http.py", line 656, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/home/odoo12/src/odoo/odoo/http.py", line 314, in _handle_exception
    raise pycompat.reraise(type(exception), exception, sys.exc_info()[2])
  File "/home/odoo12/src/odoo/odoo/tools/pycompat.py", line 87, in reraise
    raise value
  File "/home/odoo12/src/odoo/odoo/http.py", line 698, in dispatch
    result = self._call_function(**self.params)
  File "/home/odoo12/src/odoo/odoo/http.py", line 346, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/home/odoo12/src/odoo/odoo/service/model.py", line 98, in wrapper
    return f(dbname, *args, **kwargs)
  File "/home/odoo12/src/odoo/odoo/http.py", line 339, in checked_call
    result = self.endpoint(*a, **kw)
  File "/home/odoo12/src/odoo/odoo/http.py", line 941, in __call__
    return self.method(*args, **kw)
  File "/home/odoo12/src/odoo/odoo/http.py", line 519, in response_wrap
    response = f(*args, **kw)
  File "/home/odoo12/src/odoo/addons/web/controllers/main.py", line 971, in call_button
    action = self._call_kw(model, method, args, {})
  File "/home/odoo12/src/odoo/addons/web/controllers/main.py", line 959, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/home/odoo12/src/odoo/odoo/api.py", line 759, in call_kw
    return _call_kw_multi(method, model, args, kwargs)
  File "/home/odoo12/src/odoo/odoo/api.py", line 746, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/home/odoo12/src/coopiteasy/obeesdoo/sale_adapt_price_wizard/wizard/adapt_sales_price_wizard.py", line 25, in adapt_sales_price
    product.adapt_list_price(vals)
AttributeError: 'product.template' object has no attribute 'adapt_list_price'```

@victor-champonnois
Copy link
Member Author

@polchampion on which DB did you test ?

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch 2 times, most recently from 5611068 to 542d7be Compare October 21, 2022 12:11
Copy link
Collaborator

@remytms remytms left a comment

Choose a reason for hiding this comment

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

Some remarks. But very good work. :) Thanks

product_main_supplier/models/product_supplierinfo.py Outdated Show resolved Hide resolved
product_main_supplier/models/product_template.py Outdated Show resolved Hide resolved
product_main_supplier/models/product_template.py Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
Use either "seller" or "supplier", not both.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this comment. Can you point me where supplier is used instead of seller ? Except for the object supplierinfo but that's an Odoo choice. And changing the name of this object is not into our hands.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a real difference in english about supplier and seller ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

How I see now that the name of the module is product_main_supplier, is that what you point here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No I though supplier and seller could be used interchangeably, and that we should harmonize, but from what you said above, they are not the same. So the comment can indeed be removed. Is that right ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I found that the code is well named like this. So for me that's ok. But someone else may have other point of view. I don't think it really important.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK removing it then, let's discuss about this in the issue and then decide what to do

sale_adapt_price_wizard/readme/CONTRIBUTORS.rst Outdated Show resolved Hide resolved
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

{
"name": "sale_suggested_price",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use Sale Suggested Price as name. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@victor-champonnois Did you forget this point or is there a reason for the name of this module with _ ?

sale_suggested_price/readme/CONTRIBUTORS.rst Outdated Show resolved Hide resolved
@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from 542d7be to 35b20e6 Compare October 24, 2022 12:42
@victor-champonnois
Copy link
Member Author

@remytms thx for the review, ready for re-review !

Copy link
Collaborator

@remytms remytms left a comment

Choose a reason for hiding this comment

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

It seams there is a small change needed. :)

# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

{
"name": "sale_suggested_price",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@victor-champonnois Did you forget this point or is there a reason for the name of this module with _ ?

@remytms
Copy link
Collaborator

remytms commented Oct 24, 2022

@victor-champonnois The change of Beescoop to BEES coop in README.rst breaks it. Maybe, try to regenerate it with oac-gen-addon-readme or simply remove modification in the README.rst. I will be updated later if needed (also module will move, so it's not important if README are not with the right name).

@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch 2 times, most recently from 96f47bc to f305451 Compare October 24, 2022 13:37
@victor-champonnois
Copy link
Member Author

@remytms all fixed !

Copy link
Collaborator

@remytms remytms left a comment

Choose a reason for hiding this comment

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

3 readme to be regenerated than it will be totally good. :)

product_main_supplier/README.rst Outdated Show resolved Hide resolved
sale_adapt_price_wizard/README.rst Outdated Show resolved Hide resolved
sale_suggested_price/README.rst Outdated Show resolved Hide resolved
@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from f305451 to 30fd945 Compare October 24, 2022 14:06
Copy link
Collaborator

@remytms remytms left a comment

Choose a reason for hiding this comment

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

Yeah ! :)

Please note that this model makes assumptions when computing the suggested price:

- It supposes that each product has only one supplier and that products coming from multiple suppliers
occur as duplicated products with one supplier each.
Copy link
Member

Choose a reason for hiding this comment

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

Can you add to this file the new modules you created ?

@github-grap-bot
Copy link
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). 🤖

@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from 30fd945 to 16068d1 Compare October 28, 2022 08:50
@polchampion
Copy link
Collaborator

Functionally validated on bees-test and spp-test:

Module product_main_supplier

  • Adds a main supplier
  • Allows searching a product given the product reference of the main
    supplier (also known as the product_code of the product.supplierinfo
    object).

Module suggested_price

  • Adds a 'Suggested Price' field on products templates
  • Adds a 'Product Margin' field on Partners (Vendors) and Product Categories.
  • The seller’s margin is used if set, otherwise the category margin is used if set, otherwise the margin set on the parent category is used etc.
  • The reference price on which this margin is applied (supplier price or sale price) can be selected in the Purchase general settings.
  • Sale and supplier taxes that are of type 'percentage' and that are marked as 'included in price' are taken into account when computing the suggested price.
  • Adds a config to round suggested price to 5 cents in the product category.

Module sale_adapt_price_wizard

  • Adds "Edit Price" submenu on Purchase and Sale modules -

  • The user lands on an editable List View with the following columns :
    Name (name)
    Main Seller (main_seller_id)
    Purchase Price (purchase_price)
    Purchase Price Last Updated (purchase_price_write _date)
    Purchase Unit of Measure (uom_po_id)
    Suggested Price (suggested_price)
    Sales Price (list_price)
    Sales Price Last Updated (list_price_write_date)
    Unit of Measure (uom_id)

  • The only editable field is Purchase Price.

  • Through "Action > Adapt Sales Price", the user can, on the selected products, adapt the Sales Price according to the Suggested Price.

  • Alternatively, the option 'Automatically write suggested price' in the Purchase General Settings can be enabled to make this last step automatic.

into:
- product_main_supplier
- sale_suggested_price
- sale_adapt_price_wizard

[REF] split beesdoo_product -> product_main_supplier

[FIX] sale_suggested_price dep on product_main_supplier

[ADD] module sale_edit_price_wizard

[REF] rename sale_edit_price_wizard ->  sale_adapt_price_wizard

[REF] move rounding_method to sale_suggested_price

[REF] refactor following PR 320

[REF] search product_code -> product_main_seller

[FIX] bug adapt_sales_price

[IMP] use float_compare to compare prices

[REF] move list_price_write_date to sale_adapt_price_wizard

[FIX] beesdoo_product: main_seller_id search

[FIX] run precommit

fix
@victor-champonnois victor-champonnois force-pushed the 12.0-split-beesdoo_product-product_main_supplier branch from 16068d1 to fa8695a Compare October 28, 2022 15:38
@remytms
Copy link
Collaborator

remytms commented Oct 30, 2022

/ocabot merge nobump

@github-grap-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-411-by-remytms-bump-nobump, awaiting test results.

@github-grap-bot github-grap-bot merged commit 0516ca7 into 12.0 Oct 30, 2022
@github-grap-bot github-grap-bot deleted the 12.0-split-beesdoo_product-product_main_supplier branch October 30, 2022 19:31
@github-grap-bot
Copy link
Contributor

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

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