Tryton - Issues

 

Issue7474

Title Use multivalue api to access product multivalue fields
Priority feature Status testing
Superseder Nosy List ced, pokoli, reviewbot
Type feature request Components account_dunning_fee, account_stock_anglo_saxon, account_stock_continental, carrier, product, product_cost_fifo, production, purchase, sale, stock, stock_forecast
Assigned To pokoli Keywords review
Reviews 52311002, 48401002, 45221002, 45231002, 52321002, 52331002, 51331002, 45221003, 42211002, 51351002, 50471002, 50481002, 49421002, 52341002, 52341003, 49421003
View: 52311002, 48401002, 45221002, 45231002, 52321002, 52331002, 51331002, 45221003, 42211002, 51351002, 50471002, 50481002, 49421002, 52341002, 52341003, 49421003

Created on 2018-05-28.17:55:10 by pokoli, last changed by reviewbot.

Messages
review42211002 updated at https://codereview.tryton.org/42211002/#ps60001
review42211002 updated at https://codereview.tryton.org/42211002/#ps40001
review52311002 updated at https://codereview.tryton.org/52311002/#ps40001
review48401002 updated at https://codereview.tryton.org/48401002/#ps40001
review51331002 updated at https://codereview.tryton.org/51331002/#ps20001
review52331002 updated at https://codereview.tryton.org/52331002/#ps20001
review52321002 updated at https://codereview.tryton.org/52321002/#ps20001
review45231002 updated at https://codereview.tryton.org/45231002/#ps40001
review42211002 updated at https://codereview.tryton.org/42211002/#ps20001
review45221003 updated at https://codereview.tryton.org/45221003/#ps20001
review52311002 updated at https://codereview.tryton.org/52311002/#ps20001
msg41262 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-06.09:09:03
I think there is also a second cases which is quite similar. It is when an operational record get MultiValue from its Configuration model. I think we could also have a method: get_config(self, name, **pattern) and callers does not need to have a pattern keyword argument. This method may be decorated with the field.depends to be callable from on_change's.
msg41260 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-06.08:56:10
I made some reviews and for me the proposed solution is not satisfactory.
I think there is common pattern which is to retrieve MultiValue property from a referential record (like a product) linked to an operational record (like a stock move). In this case almost every time, the pattern comes from the operational record (like the company of the stock move). So for me, we should have a method to retrieve those properties on the operation record (like Move.get_product_multivalue(self, name, **pattern)). The caller of such method does not need to have a pattern keyword argument by default because I think it will be very rare (and it will still be hackable with context).
review49421003 updated at https://codereview.tryton.org/49421003/#ps20001
review52341002 updated at https://codereview.tryton.org/52341002/#ps20001
review49421002 updated at https://codereview.tryton.org/49421002/#ps20001
review52341003 updated at https://codereview.tryton.org/52341003/#ps20001
New review49421003 at https://codereview.tryton.org/49421003/#ps1
New review52341003 at https://codereview.tryton.org/52341003/#ps1
New review52341002 at https://codereview.tryton.org/52341002/#ps1
New review49421002 at https://codereview.tryton.org/49421002/#ps1
New review50481002 at https://codereview.tryton.org/50481002/#ps1
msg41114 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-05-30.17:20:55
I think we are now ready for testing as all the modules have been reviewed.
New review50471002 at https://codereview.tryton.org/50471002/#ps1
New review51351002 at https://codereview.tryton.org/51351002/#ps1
review45231002 updated at https://codereview.tryton.org/45231002/#ps20001
New review42211002 at https://codereview.tryton.org/42211002/#ps1
New review45221003 at https://codereview.tryton.org/45221003/#ps1
New review51331002 at https://codereview.tryton.org/51331002/#ps1
New review52331002 at https://codereview.tryton.org/52331002/#ps1
New review52321002 at https://codereview.tryton.org/52321002/#ps1
New review45231002 at https://codereview.tryton.org/45231002/#ps1
New review45221002 at https://codereview.tryton.org/45221002/#ps1
New review48401002 at https://codereview.tryton.org/48401002/#ps1
New review52311002 at https://codereview.tryton.org/52311002/#ps1
msg41075 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-05-28.18:07:41
I do not think it is needed for method to get a pattern argument. Normally this can be managed with context.
msg41074 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-05-28.18:05:36
There is still carrier, purchase and sale reviews missing which define an API for accessing the product attributes (get_sale_price, get_purchase_pricet)

I think this API should accept a pattern argument which will be passed to the product instance.
msg41073 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-05-28.17:55:10
This is required to ensure that all processes work as expected when issue4080 is resolved. 

There are other fields that should be also reviewed but I will create smaller issues to go step by step and ease the review process
History
Date User Action Args
2018-07-06 17:51:00reviewbotsetmessages: + msg42093
2018-06-22 14:04:45reviewbotsetmessages: + msg41592
2018-06-22 14:04:43reviewbotsetmessages: + msg41591
2018-06-11 15:04:00reviewbotsetmessages: + msg41338
2018-06-11 15:03:57reviewbotsetmessages: + msg41337
2018-06-11 15:03:54reviewbotsetmessages: + msg41336
2018-06-11 14:28:36reviewbotsetmessages: + msg41334
2018-06-11 14:28:34reviewbotsetmessages: + msg41333
2018-06-11 14:28:32reviewbotsetmessages: + msg41332
2018-06-11 14:28:26reviewbotsetmessages: + msg41331

Showing 10 items. Show all history (warning: this could be VERY long)