[MIG][10.0]product_supplier_info_for_customer by AaronHForgeFlow · Pull Request #292 · OCA/product-attribute

AaronHForgeFlow · 2017-09-29T14:16:04Z

Use product supplier info also for customers

This modules allows to use supplier info structure, available in Inventory tab of the product form, also for defining customer information, allowing to define prices per customer and product.

Update: Added back the option to import supplierinfo prices to PL items

Known issues

As discussed here: [MGR] Migrated product_customer_code to version 10 #236 The module product_customer_code is deprecated in favor of this one.
The product.supplierinfo form view is overwritten as the absence of name attributes make it impossible to inherit.
This module only import prices from supplierinfo for customers not suppliers.

elicoidal

some details but overall LGTM
Functional tests are OK
Good job!

elicoidal · 2017-10-23T00:39:55Z

product_supplierinfo_for_customer/README.rst

+
+* Product prices through this method are only guaranteed on the standard sale
+  order workflow. Other custom flows maybe don't reflect the price.
+* The minimum quantity will not also be applied on sale orders.


"neither will apply "

elicoidal · 2017-10-23T00:40:03Z

product_supplierinfo_for_customer/__init__.py

@@ -0,0 +1,5 @@
+# -*- coding: utf-8 -*-
+##############################################################################


No need for copyright notice in __init__.py

elicoidal · 2017-10-23T00:40:21Z

product_supplierinfo_for_customer/__manifest__.py

@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).


Please use standard copyright headers

Please add the copyright holder

elicoidal · 2017-10-23T03:57:39Z

product_supplierinfo_for_customer/README.rst

+
+For these prices to be used in sale prices calculations, you will have
+to create a pricelist with a rule with option "Based on" with the value
+"Supplier prices on the product form" (although the text is not clear enough).


Why not change the name to "Partner prices on the product form"?

Sorry for the mistake. This text is obsolete as that options is not available in v10. Currently the prices in the pricelist are not passed to the SO. I'm wondering if adding this feature back is really needed or that can be implemented in other way 😕

Personally I feel it is redundant with the PL management system so I would not fight for it

elicoidal · 2017-10-23T03:58:13Z

product_supplierinfo_for_customer/README.rst

+
+The product code / product name specified for the customer can be reflected
+on the sale orders using module `product_supplierinfo_for_customer_sale
+<https://github.com/OCA/product-attribute/tree/8.0/product_supplierinfo_for_customer_sale>`_


👍 The module is not migrated yet, so I'll just remove this section. When migrating product_supplierinfo_for_customer_sale this readme will be updated

elicoidal · 2017-10-23T03:59:35Z

product_supplierinfo_for_customer/README.rst

+* Product prices through this method are only guaranteed on the standard sale
+  order workflow. Other custom flows maybe don't reflect the price.
+* The minimum quantity will not also be applied on sale orders.
+* Computed fields in product.supplierinfo object won't properly work for


won't work properly
(although I do not really understand what the sentence means)

This is not valid in v10, so removed.

elicoidal · 2017-10-24T00:18:50Z

product_supplierinfo_for_customer/README.rst

+
+For these prices to be used in sale prices calculations, you will have
+to create a pricelist with a rule with option "Based on" with the value
+"Supplier prices on the product form" (although the text is not clear enough).


Personally I feel it is redundant with the PL management system so I would not fight for it

elicoidal · 2017-10-24T00:19:55Z

product_supplierinfo_for_customer/__manifest__.py

@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).


Please add the copyright holder

AaronHForgeFlow · 2017-10-24T07:56:35Z

@elicoidal Now there's no option in the pricelist to choose the price from the supplierinfo. I opened an issue here: #297 If it's ok to all I'll put that feature back

elicoidal · 2017-10-24T09:18:57Z

I mean for customer function that links to supplierinfo table is redundant with standard pricelist (set up price x product).
@aheficent

AaronHForgeFlow · 2017-10-24T09:41:53Z

@elicoidal But it's not possible to associate a pricelist to a specific customer, so if I need to specify a fix price for a customer it's not possible.

elicoidal · 2017-10-24T09:49:59Z

@aheficent I am not quite sure what you mean by " it's not possible to associate a pricelist to a specific customer".
IMO you can:

Create a Pricelist called "Pricelist Agrolait"
add rules one by one per product / price or any other filter
Assign the PL to Agrolait (and not assign it to another customer for sure)

Maybe I did not understand your meaning

AaronHForgeFlow · 2017-10-24T10:03:31Z

@elicoidal In this case step3 is totally manual, isn't it? Users have to search and select the proper PL in SO among other PL for other customers.

pedrobaeza · 2017-10-24T10:18:48Z

@elicoidal it's not direct to have a fixed price per customer (although I don't see this as a viable way of doing business, but you know, customers demand...), so this will fill the gap.

AaronHForgeFlow · 2017-10-24T11:28:27Z

Is ok then to add a partner_id field to product.pricelist.item model?

elicoidal · 2017-10-24T12:15:15Z

@pedrobaeza @aheficent

In this case step3 is totally manual, isn't it? Users have to search and select the proper PL in SO among other PL for other customers.

As manual but far less than selecting n times the customer in supplierinfo table (in my case you have only one selection of the price list in the customer)
NB: you could add a partner_id in the PL and on_change to automatically assign the PL in the customer

it's not direct to have a fixed price per customer (although I don't see this as a viable way of doing business, but you know, customers demand...), so this will fill the gap.

Not sure about your meaning here.

As said, I am not against the feature (and I am not blocking it) but I think it is redundant and I do not see really the advantage of your solution compared with the standard pricelist (or features that could not be done via pricelist).

AaronHForgeFlow · 2017-10-24T13:42:56Z

As you can specify the PL in the customer I don't like the idea of putting the partner in the PL. I think that a wizard that allows to manually import the prices from the supplier.info to the PL it's better.

PS: From my side it's ok to leave the module as it is, however, the module will be losing some of the features it had in v8.

elicoidal · 2017-10-25T01:43:30Z

@aheficent Anyway your added feature would apply for both suppliers and customer so actually I think adding it does add something that current version does not provide (PL function of product partner price).
So probably good idea to add it (up to you 👅 )

AaronHForgeFlow · 2017-10-26T10:13:08Z

@elicoidal Added the features but in button. Some issues found:

It's not possible to create pricelist items in an onchange method. If this is ok I'll move the button to a wizard.
I restricted the importing to customers because that was the purpose of this module, get the specific prices from customers.

elicoidal · 2017-10-27T00:33:42Z

@aheficent Sorry but I am not following you. Which button and/or wizard are you talking about?

AaronHForgeFlow · 2017-10-27T07:15:14Z

@elicoidal Button in the PL to import supplier.info into PL items

elicoidal · 2017-10-27T09:51:28Z

@aheficent Here I am not sure to follow: this would duplicate the data (with possible sync erreur/mistake).
IMO this is not desirable: why not forward port the feature of "Supplier price in Product form" in the PL? It is not possible?

Otherwise I would suggest not to use prices in product for customers and hide them in the form.

All in all I have the feeling we are making a complex feature out of simple problem.

AaronHForgeFlow · 2017-10-27T10:06:00Z

@elicoidal That's what I suggested first. To forward port the feature of "Supplier price in Product form" in the PL. That option was on PL items not in the PL itself because the product is selected in the PL item. I prefer this also.

elicoidal · 2017-10-27T10:18:58Z

@aheficent In summary. options:

forward port supplier in product: redundant (but acceptable IMO)
No use the price in product (but then needs to be comfortable to use pricelist)

I dont like the button option (for duplicated data)

…option

…elds

…lder.

…tomer: Complete README.

pedrobaeza · 2018-06-28T19:57:49Z

See comments from v9.

MiquelRForgeFlow · 2018-06-29T11:04:36Z

updated

AaronHForgeFlow · 2018-06-29T13:34:29Z

@elicoidal IMO your comments were attended, is it possible to update your review? Thanks

elicoidal

LGTM. Merging given the feedback and time under review

pedrobaeza · 2018-06-30T00:14:38Z

@elicoidal you have missed my comment #292 (comment). @aheficent please make another PR with the changes that you have already made on v9

elicoidal · 2018-06-30T09:37:38Z

@pedrobaeza sorry about that: I had the feeling it was covered.

MiquelRForgeFlow · 2018-07-02T09:50:05Z

@pedrobaeza your comments were covered.

AaronHForgeFlow changed the title ~~[MIG]product_supplier_info_for_customer~~ [MIG][10.0]product_supplier_info_for_customer Sep 29, 2017

AaronHForgeFlow mentioned this pull request Sep 29, 2017

Migration to version 10.0 #184

Closed

35 tasks

AaronHForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch from 6dc8992 to c40373e Compare September 29, 2017 14:33

AaronHForgeFlow changed the title ~~[MIG][10.0]product_supplier_info_for_customer~~ [MIG][10.0]product_supplier_info_for_customer (WIP) Sep 29, 2017

AaronHForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch from c40373e to 912369c Compare September 29, 2017 16:10

AaronHForgeFlow changed the title ~~[MIG][10.0]product_supplier_info_for_customer (WIP)~~ [MIG][10.0]product_supplier_info_for_customer Oct 10, 2017

AaronHForgeFlow changed the title ~~[MIG][10.0]product_supplier_info_for_customer~~ [MIG][10.0]product_supplier_info_for_customer (WIP) Oct 10, 2017

AaronHForgeFlow changed the title ~~[MIG][10.0]product_supplier_info_for_customer (WIP)~~ [MIG][10.0]product_supplier_info_for_customer Oct 17, 2017

AaronHForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch 2 times, most recently from bd04317 to 8ce61b0 Compare October 17, 2017 08:33

eLBati mentioned this pull request Oct 20, 2017

[RFC] Product Customer Code #296

Closed

elicoidal suggested changes Oct 23, 2017

View reviewed changes

AaronHForgeFlow mentioned this pull request Oct 23, 2017

[RFC][10.0]pricelists based on vendor/customer price #297

Closed

elicoidal suggested changes Oct 24, 2017

View reviewed changes

oihane and others added 11 commits June 28, 2018 17:48

Related to OCA#638

bb4cacb

Slovene translations of odoomrp modules

5063d21

product_supplierinfo_for_customer: Avoid side effects + improve README

b30eaf0

<product_supplierinfo_for_customer> Create 'customer' type from menu …

34099b8

…option

<product_supplierinfo_for_customer> Added know issue with computed fi…

9193ae8

…elds

product_supplierinfo_for_customer: Create "demo" data, and "tests" fo…

4c607f4

…lder.

license target link

5aab198

OCA as author added

9030f72

OCA Transbot updated translations from Transifex

0bf236e

product_customer_code: New README. [REF] product_supplierinfo_for_cus…

b7bbd52

…tomer: Complete README.

product_supplierinfo_for_customer to v9

2c82565

MiquelRForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch from a0818cf to 2c2f3ec Compare June 28, 2018 15:49

MiquelRForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch from 0eb6f56 to dd31c75 Compare June 29, 2018 10:55

AaronHForgeFlow and others added 6 commits June 29, 2018 12:58

[MIG] product_supplierinfo_for_customer to v10

a275bb9

[IMP] Add back the option to import the prices from supplierinfo to PL

0dc95a3

[FIX] product_multi_category: E7901 error pylint in readme

50d1172

[FIX] product_sequence: E8103 sql-injection pylint error

092bffe

[FIX] customer_pricing_in_product_view: E7901 pylint error

e6d6619

[FIX] product_variant_inactive: E7901 pylint error

3868474

MiquelRForgeFlow force-pushed the 10.0-mig-product_supplierinfo_for_customer branch from dd31c75 to 3868474 Compare June 29, 2018 10:58

MiquelRForgeFlow approved these changes Jun 29, 2018

View reviewed changes

elicoidal approved these changes Jun 30, 2018

View reviewed changes

elicoidal merged commit 4f7cb0f into OCA:10.0 Jun 30, 2018

MiquelRForgeFlow deleted the 10.0-mig-product_supplierinfo_for_customer branch July 2, 2018 09:50

		@@ -0,0 +1,5 @@
		# -- coding: utf-8 --
		##############################################################################

		@@ -0,0 +1,26 @@
		# -- coding: utf-8 --
		# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).

Uh oh!

Conversation

AaronHForgeFlow commented Sep 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Use product supplier info also for customers

Known issues

Uh oh!

elicoidal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronHForgeFlow commented Oct 24, 2017

Uh oh!

elicoidal commented Oct 24, 2017

Uh oh!

AaronHForgeFlow commented Oct 24, 2017

Uh oh!

elicoidal commented Oct 24, 2017

Uh oh!

AaronHForgeFlow commented Oct 24, 2017

Uh oh!

pedrobaeza commented Oct 24, 2017

Uh oh!

AaronHForgeFlow commented Oct 24, 2017

Uh oh!

elicoidal commented Oct 24, 2017

Uh oh!

AaronHForgeFlow commented Oct 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elicoidal commented Oct 25, 2017

Uh oh!

AaronHForgeFlow commented Oct 26, 2017

Uh oh!

elicoidal commented Oct 27, 2017

Uh oh!

AaronHForgeFlow commented Oct 27, 2017

Uh oh!

elicoidal commented Oct 27, 2017

Uh oh!

AaronHForgeFlow commented Oct 27, 2017

Uh oh!

elicoidal commented Oct 27, 2017

Uh oh!

pedrobaeza commented Jun 28, 2018

Uh oh!

MiquelRForgeFlow commented Jun 29, 2018

Uh oh!

AaronHForgeFlow commented Jun 29, 2018

Uh oh!

elicoidal left a comment

Choose a reason for hiding this comment

Uh oh!

pedrobaeza commented Jun 30, 2018

Uh oh!

AaronHForgeFlow commented Sep 29, 2017 •

edited

Loading

AaronHForgeFlow commented Oct 24, 2017 •

edited

Loading