Tryton - Issues

 

Issue6634

Title Unable to purchase asset mixed with services
Priority bug Status testing
Superseder Nosy List ced, pokoli, reviewbot
Type behavior Components account_asset
Assigned To pokoli Keywords review
Reviews 43281002, 43291002, 35131002, 41441002, 43301002
View: 43281002, 43291002, 35131002, 41441002, 43301002

Created on 2017-07-11.13:45:13 by pokoli, last changed by reviewbot.

Messages
review43301002 updated at https://codereview.tryton.org/43301002/#ps20001
review41441002 updated at https://codereview.tryton.org/41441002/#ps20001
review43281002 updated at https://codereview.tryton.org/43281002/#ps80001
review35131002 updated at https://codereview.tryton.org/35131002/#ps20001
New review43301002 at https://codereview.tryton.org/43301002/#ps1
New review41441002 at https://codereview.tryton.org/41441002/#ps1
review43281002 updated at https://codereview.tryton.org/43281002/#ps60001
New review35131002 at https://codereview.tryton.org/35131002/#ps1
review43291002 updated at https://codereview.tryton.org/43291002/#ps1
msg34556 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-12.16:02:44
> For me, all MissingFunction must be replaced by properties.
Done
> Also we will need your initial patch for 4.4.

Added review43291002 without test scenario.
review43281002 updated at https://codereview.tryton.org/43281002/#ps40001
msg34554 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-12.15:20:16
For me, all MissingFunction must be replaced by properties.
Also we will need your initial patch for 4.4.
msg34553 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-12.15:04:19
My fault, I  was implementing the property on template, but adding it to product solves the problem. 

I updated the review
review43281002 updated at https://codereview.tryton.org/43281002/#ps20001
msg34549 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-12.10:10:06
On 2017-07-12 09:58, Sergi Almacellas Abellana wrote:
> El 11/07/17 a les 18:15, Cédric Krier ha escrit:
> > Cédric Krier<cedric.krier@b2ck.com>  added the comment:
> > 
> > On 2017-07-11 18:00, Sergi Almacellas Abellana wrote:
> >> Sergi Almacellas Abellana<sergi@koolpi.com>  added the comment:
> >>
> >> El 11/07/17 a les 16:05, Cédric Krier ha escrit:
> >>>>> For me, the MissingFunction should be changed for a property.
> >>>> And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.
> >>> Like any property.
> >> But which will be the property name and which will be the code?
> > The same.
> > 
> >> Sorry but I don't see how it will fix the problem....
> > There will be no bunch computation.
> 
> I tryied to implement with a property and the current code (raising the error if no value is found). The problem is exactly the same as we are reading the property of all the lines....

But do not read the property of all the lines.
msg34548 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-12.09:58:14
El 11/07/17 a les 18:15, Cédric Krier ha escrit:
> Cédric Krier<cedric.krier@b2ck.com>  added the comment:
> 
> On 2017-07-11 18:00, Sergi Almacellas Abellana wrote:
>> Sergi Almacellas Abellana<sergi@koolpi.com>  added the comment:
>>
>> El 11/07/17 a les 16:05, Cédric Krier ha escrit:
>>>>> For me, the MissingFunction should be changed for a property.
>>>> And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.
>>> Like any property.
>> But which will be the property name and which will be the code?
> The same.
> 
>> Sorry but I don't see how it will fix the problem....
> There will be no bunch computation.

I tryied to implement with a property and the current code (raising the error if no value is found). The problem is exactly the same as we are reading the property of all the lines....
msg34543 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-11.18:15:05
On 2017-07-11 18:00, Sergi Almacellas Abellana wrote:
> 
> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
> 
> El 11/07/17 a les 16:05, Cédric Krier ha escrit:
> >>> For me, the MissingFunction should be changed for a property.
> >> And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.
> > Like any property.
> 
> But which will be the property name and which will be the code?

The same.

> Sorry but I don't see how it will fix the problem....

There will be no bunch computation.
msg34542 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-11.18:00:12
El 11/07/17 a les 16:05, Cédric Krier ha escrit:
>>> For me, the MissingFunction should be changed for a property.
>> And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.
> Like any property.

But which will be the property name and which will be the code?

Sorry but I don't see how it will fix the problem....
msg34541 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-11.16:05:07
On 2017-07-11 15:28, Sergi Almacellas Abellana wrote:
> El 11/07/17 a les 15:16, Cédric Krier ha escrit:
> > I do not agree we need the clear error message of MissingFunction.
> 
> The message is clear:
> 
> http://hg.tryton.org/modules/account_asset/file/6ea71e496bc5/purchase.py#l15
> 
> Indeed, it's clearer than the current one, which shows expense/revenue account when reading the asset account

I do not agree because it does not take into account of the category
etc.

> > For me, the MissingFunction should be changed for a property.
> 
> And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.

Like any property.
msg34540 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-11.15:28:03
El 11/07/17 a les 15:16, Cédric Krier ha escrit:
> I do not agree we need the clear error message of MissingFunction.

The message is clear:

http://hg.tryton.org/modules/account_asset/file/6ea71e496bc5/purchase.py#l15

Indeed, it's clearer than the current one, which shows expense/revenue account when reading the asset account


Other option I though about was disabling the error message via some context switch. But I don't like it so much too. 

> For me, the MissingFunction should be changed for a property.

And how will be the property implemented? I don't see how it can prevent the error without duplicating the current code.
msg34539 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-11.15:16:10
I do not agree we need the clear error message of MissingFunction.
msg34538 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-11.14:49:27
> For me, the MissingFunction should be changed for a property.

Why not making account_assed_used a normal function field? There is already an error message when creating the invoice line from purchase. Currently, this message is not raised due to the MissingFunction error which also caused some confusion when investigating this issue. 

We can assume an empty value on invoice line, which should be probably fixed by entering manualy and account.
msg34537 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-11.14:43:50
For me, the MissingFunction should be changed for a property.
review43281002 updated at https://codereview.tryton.org/43281002/#ps1
msg34535 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-11.14:37:01
A better solution must be found.
msg34532 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-07-11.13:45:13
When processing a purchase with a line of asset product and assets, it complains about that there is no asset_account defined for service product, but this account is not required. 

The problem is that when reading the account_asset_used from the asset product, it tries to read all the values for all the purchase lines, which is not correct as there may be lines that are not of asset type.
History
Date User Action Args
2017-07-14 12:35:26reviewbotsetmessages: + msg34608
2017-07-14 12:35:24reviewbotsetmessages: + msg34607
2017-07-14 12:35:23reviewbotsetmessages: + msg34606
2017-07-14 12:35:19reviewbotsetmessages: + msg34605
2017-07-13 14:10:49reviewbotsetmessages: + msg34590
2017-07-13 14:10:48reviewbotsetreviews: 43281002, 43291002, 35131002, 41441002 -> 43281002, 43291002, 35131002, 41441002, 43301002
2017-07-13 14:10:45reviewbotsetmessages: + msg34589
2017-07-13 14:10:44reviewbotsetreviews: 43281002, 43291002, 35131002 -> 43281002, 43291002, 35131002, 41441002
2017-07-13 14:10:43reviewbotsetmessages: + msg34588
2017-07-13 14:10:41reviewbotsetmessages: + msg34587

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