[10][MIGR] account_invoice_pricelist #342

rvalyi · 2017-12-15T16:38:12Z

No description provided.

pedrobaeza · 2017-12-18T17:15:31Z

Why haven't you used the migration guide, but changing 9.0 by 8.0? You need to preserve commit history

rvalyi · 2017-12-18T17:20:05Z

@pedrobaeza my bad. Somebody told me version 8.0 was the most advanced and I admitted it was true without checking if it was ported to 9.0 before... I'll review your 9.0 port and start from here again sorry.

rvalyi · 2017-12-18T17:25:48Z

@pedrobaeza I should say I have been induced in error because I checked too quickly on this addons-table https://github.com/OCA/account-invoicing/tree/10.0#unported-addons where it says last version of
account_invoice_pricelist was 8.0.1.0.0 and I assumed the table was correct.

is it a known issue this table can miss that kind of information? and also is there a reason we don't we have such a table for v11?

pedrobaeza · 2017-12-18T18:40:59Z

@pedrobaeza I should say I have been induced in error because I checked too quickly on this addons-table https://github.com/OCA/account-invoicing/tree/10.0#unported-addons where it says last version of
account_invoice_pricelist was 8.0.1.0.0 and I assumed the table was correct.

This is because the 10.0 branch was created before the module was merged in 9.0

is it a known issue this table can miss that kind of information? and also is there a reason we don't we have such a table for v11?

I removed that tables as part of the 11.0 branch creation for avoiding this kind of situations, as now if the module is not there, it means is not migrated. And even more, there's no possibility of having outdated sources hosted in the repo. That was one of the advantages I enumerated when announcing this change of paradigm if you remember.

bealdav · 2017-12-19T07:09:32Z

account_invoice_pricelist/model/account_invoice.py

+# © 2014-2016 GRAP <http://www.grap.coop>.
+# © 2017 Therp BV <http://therp.nl>.
+# License AGPL-3.0 or later <http://www.gnu.org/licenses/agpl.html>.
+from openerp import api, fields, models


s/openerp/odoo

bealdav · 2017-12-19T07:11:38Z

account_invoice_pricelist/model/__init__.py

+# -*- coding: utf-8 -*-
+# © 2015-2016 GRAP <http://www.grap.coop>.
+# © 2017 Therp BV <http://therp.nl>.
+# License AGPL-3.0 or later <http://www.gnu.org/licenses/agpl.html>.


Not blocking for me but here there is no content (just import), then authors and license can be removed (it's just my opinion ;-) )

bealdav · 2017-12-19T07:16:30Z

account_invoice_pricelist/model/account_invoice.py

+                if hasattr(self.partner_id,
+                           'property_product_pricelist_purchase'):
+                    self.pricelist_id =\
+                        self.partner_id.property_product_pricelist_purchase


Partner model has attribute property_product_pricelist_purchase which is defined in product module.
Why a condition here ?

bealdav · 2017-12-19T07:19:08Z

account_invoice_pricelist/view/account_invoice_view.xml

+                    <button
+                        type="object"
+                        name="button_update_prices_from_pricelist"
+                        string="Update prices from product and/or pricelist"


This button string is too long in the interface (mobile render !? etc)
=> 'Update prices' is sufficient I think

Maybe "Apply Pricelist" ?

Or "Update prices"?

bealdav · 2017-12-19T07:19:48Z

account_invoice_pricelist/model/account_invoice_line.py

+# © 2015-2016 GRAP <http://www.grap.coop>.
+# © 2017 Therp BV <http://therp.nl>.
+# License AGPL-3.0 or later <http://www.gnu.org/licenses/agpl.html>.
+from openerp import models, api


s/openerp/odoo

legalsylvain · 2017-12-20T12:12:06Z

Hi @rvalyi, the res_groups.yml demo file is not useless for me. By this way, it is more easy to test the module. (On runbot, mainly) to avoid functional users to have to set this settings each time.

For that reason, I regurlarly add a demo file, adding admin user to the groups, required for the module.
In that module, if users is not part of pricelist group, it will not have the possibility to test fully the module. (but maybe some groups changed since V8)

If you're agree, to add it again, maybe add a xml file, as yml is deprecated.

Thanks for porting this module.

legalsylvain

Hi @rvalyi, more deeply review.

minor remark, on readme file, I think runbot link is usefull.
I think there are a problem in your code. you don't seems to overload the function _compute_price()
This function should be overloaded to allow to compute price, based on the correct pricelist. Don't you think ? otherwise, changing quantity will not recompute the price, based on the correct pricelist.
you can add you in creditor section ! This work is not a /s/openerp/odoo work, lot of changes since V8.

rvalyi · 2017-12-20T13:08:19Z

@legalsylvain I'll redo the migration starting from the 9.0 branch again anyway. Thanks for your comments, I'll take them into account.

angelmoya · 2018-03-16T11:06:10Z

Hi @rvalyi, did you redo this migration? Maybe I could do it if you don't plan to do.

Thanks

rvalyi · 2018-03-26T16:41:39Z

Hello guys, just wanted to say, I'm on it today finally

rvalyi · 2018-03-30T23:56:52Z

Here is the new PR superseding this one
#368

NL66278 · 2018-04-01T09:13:39Z

account_invoice_pricelist/__manifest__.py

+# -*- coding: utf-8 -*-
+# © 2015-2016 GRAP <http://www.grap.coop>.
+# © 2015-2017 Therp BV <http://therp.nl>.
+# License AGPL-3.0 or later <http://www.gnu.org/licenses/agpl.html>.
 {
    'name': 'Account - Pricelist on Invoices',
    'version': '8.0.1.0.0',


Version number should be updated to 10.0 as well

NL66278 · 2018-04-01T09:17:27Z

account_invoice_pricelist/view/account_invoice_view.xml

+    © 2017 Therp BV <http://therp.nl>.
+    License AGPL-3.0 or later <http://www.gnu.org/licenses/agpl.html>.
+-->
+<openerp>


s/openerp/odoo and <data> is deprecated.

NL66278

👍 LGTM. Some suggestions for style changes and small fixes. See comments.

NL66278 and others added 2 commits December 15, 2017 17:15

forward-port from 8.0: [ENH] Make sure pricelist on invoice is used.

dae5628

forward-port from 8.0: newer 8.0 translations

8ed2011

rvalyi requested a review from alexis-via December 15, 2017 16:38

rvalyi changed the title ~~10 migr account invoice pricelist~~ [10][MIGR] account_invoice_pricelist Dec 15, 2017

pedrobaeza mentioned this pull request Dec 15, 2017

Migration to version 10.0 #194

Closed

34 tasks

rvalyi force-pushed the 10-migr-account_invoice_pricelist branch 3 times, most recently from cd53cac to fde4ecc Compare December 15, 2017 17:48

rvalyi added 2 commits December 15, 2017 20:30

migrate account_invoice_pricelist to v10

cce83de

buggy demo groups are useless in this module

054723c

rvalyi force-pushed the 10-migr-account_invoice_pricelist branch from fde4ecc to 054723c Compare December 15, 2017 19:30

rvalyi added the needs review label Dec 15, 2017

rvalyi requested review from NL66278 and legalsylvain December 18, 2017 11:36

rvalyi mentioned this pull request Dec 18, 2017

[10][MIGR] account invoice pricelist sale #343

Merged

1 task

rvalyi added needs fixing and removed needs review labels Dec 18, 2017

bealdav suggested changes Dec 19, 2017

View reviewed changes

legalsylvain suggested changes Dec 20, 2017

View reviewed changes

rvalyi mentioned this pull request Mar 30, 2018

10 account invoice pricelist from 9 #368

Merged

rvalyi closed this Mar 30, 2018

NL66278 reviewed Apr 1, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10][MIGR] account_invoice_pricelist #342

[10][MIGR] account_invoice_pricelist #342

rvalyi commented Dec 15, 2017

pedrobaeza commented Dec 18, 2017

rvalyi commented Dec 18, 2017

rvalyi commented Dec 18, 2017

pedrobaeza commented Dec 18, 2017

bealdav Dec 19, 2017

bealdav Dec 19, 2017

legalsylvain Dec 20, 2017

bealdav Dec 19, 2017

bealdav Dec 19, 2017

legalsylvain Dec 20, 2017

pedrobaeza Dec 20, 2017

bealdav Dec 19, 2017

legalsylvain commented Dec 20, 2017

legalsylvain left a comment

rvalyi commented Dec 20, 2017

angelmoya commented Mar 16, 2018

rvalyi commented Mar 26, 2018

rvalyi commented Mar 30, 2018

NL66278 Apr 1, 2018

NL66278 Apr 1, 2018

NL66278 left a comment

[10][MIGR] account_invoice_pricelist #342

[10][MIGR] account_invoice_pricelist #342

Conversation

rvalyi commented Dec 15, 2017

pedrobaeza commented Dec 18, 2017

rvalyi commented Dec 18, 2017

rvalyi commented Dec 18, 2017

pedrobaeza commented Dec 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legalsylvain commented Dec 20, 2017

legalsylvain left a comment

Choose a reason for hiding this comment

rvalyi commented Dec 20, 2017

angelmoya commented Mar 16, 2018

rvalyi commented Mar 26, 2018

rvalyi commented Mar 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NL66278 left a comment

Choose a reason for hiding this comment