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

11.0 mig product variant configurator #100

Merged
merged 28 commits into from
Oct 24, 2018

Conversation

angelmoya
Copy link
Member

No description provided.

@pedrobaeza
Copy link
Member

Travis is red. Can you check?

@pedrobaeza pedrobaeza added this to the 11.0 milestone Aug 8, 2018
@angelmoya angelmoya force-pushed the 11.0-mig-product_variant_configurator branch 2 times, most recently from 6fafa9a to c387963 Compare August 15, 2018 10:49
Copy link

@zaoral zaoral left a comment

Choose a reason for hiding this comment

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

Hi @angelmoya

I have made a quick review and I have suggested some changes, aslo asked some questions.
Will be great if you check it out.

Best Regards

product_variant_configurator/README.rst Show resolved Hide resolved
product_variant_configurator/README.rst Show resolved Hide resolved
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->

<odoo>
<data>
Copy link

Choose a reason for hiding this comment

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

Why use an xml file and not a "ir.model.access.csv" file as oca guideline suggest? Also, data tag is deprecated, you can removed if you like from here and all it occurrences

Copy link
Member Author

Choose a reason for hiding this comment

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

t's inherited from previous version, I will migrate

@pedrobaeza
Copy link
Member

@angelmoya I need this module soon. What is your ETA? What is left for working on it if not possible to finish it soon?

@angelmoya
Copy link
Member Author

Hi @pedrobaeza I'm testing now, I don't needed soon, but I'm working now, maybe we can do it this days

@pedrobaeza
Copy link
Member

OK, thanks. I will review it and tell you if everything seems correct.

@angelmoya angelmoya force-pushed the 11.0-mig-product_variant_configurator branch from 9e7e05f to 0dfdffd Compare September 17, 2018 13:55
@angelmoya
Copy link
Member Author

Hi @pedrobaeza travis is green

@@ -0,0 +1,12 @@
# © 2015 Oihane Crucelaegui - AvanzOSC
# © 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
Copy link
Member

Choose a reason for hiding this comment

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

Change © by Copyright

product_variant_configurator/__manifest__.py Show resolved Hide resolved
'Tecnativa,'
'ACSONE SA/NV,'
'Odoo Community Association (OCA)',
'website': 'https://odoo-community.org/',
Copy link
Member

Choose a reason for hiding this comment

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

@angelmoya
Copy link
Member Author

@ernestotejeda changes done

zakiuu and others added 11 commits October 24, 2018 16:28
This module has been ported from the odoomrp repo.
the module name in the odoo mrp repo was product_variants_no_automatic_creation
the following improvement where done while porting this module:

* Improve the readme file with a technical description on how the module should
  be used with other modules.
* Add attribute col=4 to group parent for a better desplay of product category
  for view.
* Remove unnecessary @api.multi before compute or onchange methods.
* Move product.configurator.attribute model to the new
  product_configurator_attribute.py file.
* Add unit test for product.configurator.attribute model that includes test for
  _compute_possible_value_ids and _compute_price_extra methods.
* Rename XML files in views and security folder as per OCA guidelines.
* Improve test coverage for product_configurator.
* Add unit test for purchase.order and purchase.order.line models.
* Separate the purchase.order and purchase.order.line models code into two files.
* Rename XML files as per OCA guidelines.
* product_variant_configurator: set product_tmpl_id in
  onchange_product_id_product_configurator_old_api and
  onchange_product_id_product_configurator methods for product.configurator model.
* Add unit test for onchange_product_tmpl_id and onchange_product_attribute_ids.
…doomrp repo. the following improvements were done while porting this module:

* Adapt files as per OCA guidelines.
* Add unit test
What helps is setting the many2many_tags which seems to have an
influence even though the field is invisible. Without this you sometimes
get a warning asking if you want to discard the last change.
Setting the computed field readonly is also good practice.

[IMP] new api style in onchange for updating attributes list
[IMP] better to avoid overrides of onchange method (OCA#2)
[REF] refactoring of the onchanges

This should be easier to understand and cover all cases of
changing product template and product in any order, so you
always get a consistent list of attributes.

[FIX] fix tests in purchase_product_variant_configurator

[FIX] avoid systematic warning when creating new product templates
because I don't know how to emulate a true onchange that
populates _origin
… are for sale only

plus, fix the visibility rule of the button,
to show the button if there are attributes on the product
(otherwise it would stay hidden until the first 2 variants
have been created)

[FIX] more robust configurator onchanges

[FIX] return values for onchange_product_attribute_ids
…vepoint to discard created product variant in case of exception
sbidoul and others added 16 commits October 24, 2018 16:28
* Issue with cache invalidation on create
* remove unused _create_variant_from_vals
* hide price_extra not applicable to purchase for now
* remove import on removed procurement_order.py file
* Fix tests in purchase_product_variant_configurator
* Fix tests in product_variant_configurator
…elete' attribute on the field owner_id declared as Integer is required since this field is declared as inverse of the field product_attribute_ids of the abstract model product.configurator
…f.attribute_line_ids/not tmpl.attribute_line_ids
With this, you can decide at product level which attributes are required
to be filled. Default is true for keeping the same logic with previous
version.
@pedrobaeza pedrobaeza force-pushed the 11.0-mig-product_variant_configurator branch from 79daf88 to 9a8e3fb Compare October 24, 2018 15:06
@pedrobaeza
Copy link
Member

@angelmoya you didn't respect commit history (it's already added), and I have found some unwanted changes in the code that I have reverted:

  • required for product.attribute.line default changed
  • with_context in tests with a dictionary instead of keyword argument that removes previous context.
  • An unlink instead of [(5, )] operation that cuts the link.
  • Some stylistic only changes

I have also added README by fragments.

Merging if CIs goes well.

@pedrobaeza pedrobaeza force-pushed the 11.0-mig-product_variant_configurator branch from 9a8e3fb to 6e4acbf Compare October 24, 2018 16:23
@pedrobaeza
Copy link
Member

OK, I have seen that the required=False by default in product.attribute.line is a sane default for avoiding problems with other modules, so I restored it. Merging this one. Thanks to all.

@pedrobaeza pedrobaeza merged commit 1b030a5 into OCA:11.0 Oct 24, 2018
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.

10 participants