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

[REF] full refactoring: #3

Conversation

legalsylvain
Copy link
Collaborator

@legalsylvain legalsylvain commented Nov 26, 2016

Refactoring

  • OCA convention (folder name, file name, field name)
  • change module name from account_invoice_product_supplier_price_update to account_invoice_supplierinfo_update
  • remove product_variant_supplierinfo dependency. variant management will be done in an extra glue module
  • remove useless product_supplierinfo_tree_price_info dependency.
  • Split big functions in some many to allow overloading, and add a some prepare functions

Fixes

  • Add an extra button 'update_supplierinfo' on supplier invoice form to avoid to change button type and allow user to update supplierinfo, even if user is not member of 'group_account_invoice'.

Improvement

  • Set colors to mention if partnerinfo is created, or updated, or if supplierinfo is fully created
  • display price variation (in %)
  • account_invoice_supplierinfo_update_discount. Glue module with OCA purchase-workflow / product_supplierinfo_discount, to manage extra discount field

Extra work to do

After merging, extra module to do :

  • account_invoice_supplierinfo_update_variant. Glue module with OCA product-variant / product_variant_supplierinfo, to manage correctly supplierinfo in variants implementation. (with to_variant field)

- OCA convention
- module renamed
- modularity in function. (prepare function added)
@legalsylvain
Copy link
Collaborator Author

Hi @chafique-delli, @sebastienbeau,

I just refactored all the module you proposed. Please take a time to make a technical and functional review.

About your use in production, module rename will not be a problem, as your module do not introduce persistent field. (only transient model). So a simple uninstallation / installation during the migration will work.

If you have plugged your production buildout on your branch, please freeze it until account_invoice_supplierinfo_update_variant is done, to avoid regression.

kind regards.

Note Travis is red because OCA / 8.0 is red.
ref : OCA#185

Copy link
Member

@chafique-delli chafique-delli left a comment

Choose a reason for hiding this comment

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

@legalsylvain , The user will not always know when to use the 'Update Supplier Information' button on the invoice, he will no longer have a tendency to directly validate the invoice.
I think it would be more sensible to have an option in the configuration of the billing which will allow the updating of the supplier prices.


.. image:: ./account_invoice_supplierinfo_update/static/description/supplier_invoice_form.png


Copy link
Member

Choose a reason for hiding this comment

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

Hi @legalsylvain , fix the README.rst file because we have 3 times the same image that appears 'supplier_invoice_form.png' in view form of module.

It creates a new supplier information line if there is not or it updates the
first.

This module add an extra button 'Check Supplier Informations' on supplier
Copy link
Member

Choose a reason for hiding this comment

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

replace 'Check Supplier Informations' by 'Update Supplier Informations'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed the name of the button.

lines = []

for line in self.invoice_line:
propose_update = True
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO if there is no product, set propose_update to False.

@chafique-delli
Copy link
Member

chafique-delli commented Dec 13, 2016

@legalsylvain , When you have verified the supplier information on a draft invoice without updating the prices and then want to delete it, this is not possible because we have a record created in the table 'wizard.update .invoice.supplierinfo 'with the id of the invoice.
Add ondelete='cascade' attribute on invoice_id field in wizard.update .invoice.supplierinfo.

[FIX] various improvments
@legalsylvain
Copy link
Collaborator Author

Hi @chafique-delli. I did commit according to your remarks.

regards.

@chafique-delli
Copy link
Member

Hi @legalsylvain , thank you for your work.
It's good for me. 👍

@legalsylvain legalsylvain merged commit 7d2a190 into akretion:8.0-product-cost-price-update Dec 13, 2016
@legalsylvain legalsylvain deleted the 8.0_refactor_invoice_supplier branch December 13, 2016 17:00
mbcosta pushed a commit that referenced this pull request Oct 4, 2021
…d_option-dro

[13.0][FIX] stock_picking_return_refund_option: Fix dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants