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

[10.0] MIG sale documents comments, renaming in sale_comment_propagation #509

Closed
wants to merge 17 commits into from
Closed

[10.0] MIG sale documents comments, renaming in sale_comment_propagation #509

wants to merge 17 commits into from

Conversation

hurrinico
Copy link

@hurrinico hurrinico commented Jul 18, 2017

Porting of @eLBati version
#468

@@ -7,7 +7,7 @@

Copy link
Member

Choose a reason for hiding this comment

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

@hurrinico __manifest.py -> __manifest__.py

@hurrinico
Copy link
Author

@eLBati fixed error manifest and flake8

@eLBati
Copy link
Member

eLBati commented Jul 18, 2017

@hurrinico

sale_documents_comments/models/account.py:19:9: F841 local variable 'val' is assigned to but never used
sale_documents_comments/models/sale.py:32:1: W293 blank line contains whitespace
sale_documents_comments/models/sale.py:36:9: F841 local variable 'val' is assigned to but never used

@hurrinico
Copy link
Author

@eLBati travis is green

@pedrobaeza
Copy link
Member

Check my comment about module name in #468 (comment)

@hurrinico
Copy link
Author

@pedrobaeza done

@hurrinico hurrinico changed the title [10.0] MIG sale documents comments [10.0] MIG sale documents comments, renaming in sale_comment_propagation Jul 19, 2017

In sale order

.. image:: /sale_documents_comments/images/sale_comments.png
Copy link
Member

Choose a reason for hiding this comment

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

fix path after renaming

@pedrobaeza
Copy link
Member

@chienandalu please review thie PR and propose the module renaming in OpenUpgrade v9 to assure path from odoomrp-wip v8 module to OCA one on v9/v10

Copy link
Member

@chienandalu chienandalu 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 your work, @hurrinico! Some comments below:

  • Maybe you could try to squash some commits together (mainly the translation ones)
  • Tests would be very welcome.

# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html

from openerp import models, fields, api
from odoo import models, fields, api
Copy link
Member

Choose a reason for hiding this comment

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

Sort alphabetically

@@ -49,6 +49,8 @@ Contributors
* Pedro M. Baeza <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Please, change it to [email protected]

# © 2015 Esther Martín <[email protected]> - Avanzosc S.L.
# Copyright 2015 Ainara Galdona - AvanzOSC
# Copyright 2015 Oihane Crucelaegui - AvanzOSC
# Copyright 2015 Esther Martín <[email protected]> - Avanzosc S.L.
Copy link
Member

Choose a reason for hiding this comment

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

There's no real need of authoring in __init__.py files. Better to keep them as minimalistic as possible.

@@ -18,7 +18,8 @@
"author": "OdooMRP team, "
"AvanzOSC, "
"Serv. Tecnol. Avanzados - Pedro M. Baeza, "
Copy link
Member

Choose a reason for hiding this comment

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

Please, change it to Pedro's new company Tecnativa

@@ -18,7 +18,8 @@
"author": "OdooMRP team, "
"AvanzOSC, "
"Serv. Tecnol. Avanzados - Pedro M. Baeza, "
"Odoo Community Association (OCA)",
"Odoo Community Association (OCA), "
"Agile Businnes Group",
"category": "Sales",
"website": "http://www.odoomrp.com",
Copy link
Member

Choose a reason for hiding this comment

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

# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html

from openerp import models, fields
from odoo import models, fields
Copy link
Member

Choose a reason for hiding this comment

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

Sort alphabetically

# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html

from openerp import models, fields, api
from odoo import models, fields, api
Copy link
Member

Choose a reason for hiding this comment

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

Sort alphabetically

# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html

from openerp import api, models, fields
from odoo import api, models, fields
Copy link
Member

Choose a reason for hiding this comment

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

Sort alphabetically


{
"name": "Comments for sale documents (order, picking and invoice)",
"version": "10.0.0.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Follow guidelines for version numbering: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#version-numbers. This migration should start on 10.0.1.0.0 and then bump according to said guidelines.

string='Propagated internal comments')

@api.one
@api.onchange('partner_id')
Copy link
Member

Choose a reason for hiding this comment

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

Don't use @api.one with @api.onchange or it will crash.
https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-10.0

(besides @api.one is deprecated)

Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

Tested in runbot this module has an incompatibility with sale_order_margin_percent

  File "/home/odoo/build/OCA/sale-workflow/sale_order_margin_percent/models/sale_order_margin.py", line 15, in _compute_percent
    if self.margin and self.amount_untaxed:
  File "/.repo_requirements/odoo/odoo/fields.py", line 864, in __get__
    record.ensure_one()
  File "/.repo_requirements/odoo/odoo/models.py", line 4826, in ensure_one
    raise ValueError("Expected singleton: %s" % self)
ValueError: Expected singleton: sale.order(18, 19)

With this module you can add specific comments to a customer, for sale order,
delivery order and invoices. Part of the info will be passed from one to other.
Those data will be automatically added to each item.

Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment that this module don't add this comments to any report. A user could expect that.

@rafaelbn
Copy link
Member

Hi @hurrinico could you please to attend comments ? Let us know, thanks!

@hurrinico
Copy link
Author

Hi @pedrobaeza i'll work on it as soon as possible.

oihane and others added 12 commits October 26, 2017 09:36
[MOD] Vistos cambios necesarios despues del PR

[MOD]

[MOD]

Update stock.py

Según código corregido issue 80, pero hay que probar
[IMP] PEP8
[FIX] Pylint fixing <sale_documents_comments>

[FIX] Pylint fixing <warning_log>

[MOD] Translated module <sale_documents_comments>
[IMP] sale_documents_comments: añadir comentario del parent cuando exista + pasar codigo a la nueva api

[FIX] sale_documents_comments: visual indentation corregido

[IMP] sale_documents_comments : inicializar las variables en caso de no tener comentario
[FIX] <sale_documents_comments> space adding at start avoided

[IMP] <sale_documents_comments> required changes in string replacing

[FIX] <sale_documents_comments> Comments not taken from partner fixed
[IMP] <sale_documents_comments> Just in case, take order partner_invoice_id
if partner_id is not present
pedrobaeza and others added 4 commits October 26, 2017 09:36
…nt 'type'

IMP using OCA templates

IMP description and images

IMP copyright headers following OCA template
and renaming in sale_documents_comments sale_comment_propagation
@hurrinico
Copy link
Author

Fixed commit list and set runbot and travis green

@pedrobaeza
Copy link
Member

@tafaRU can you update your review?

Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

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

Code review, and check ❌ Travis

res['sale_comment'] = '\n'.join(comment_list)
return res

@api.multi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@api.multi

if partner._get_invoice_comments():
pcomment_list.append(partner._get_invoice_comments())
values['sale_comment'] = '\n'.join(pcomment_list)
return super(StockPicking, self)._create_invoice_from_picking(
Copy link
Member

Choose a reason for hiding this comment

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

I can't find this method in v10 (in v8 was stock_account.stock.stock_picking._create_invoice_from_picking)

@rousseldenis
Copy link
Contributor

@hurrinico @SimoRubi Is this still applicable ?

@rousseldenis
Copy link
Contributor

Closing as very old. Feel free to reopen it if needed.

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.