Issue 4258

Title
Trying to remove values from on_change One2Many field causes error when field has no value
Priority
bug
Status
resolved
Nosy list
ced, pokoli, roundup-bot
Assigned to
pokoli
Keywords
patch, review

Created on 2014-10-16.18:36:10 by pokoli, last changed 72 months ago by roundup-bot.

Files

File name Uploaded Type Details
issue4258.patch pokoli, 2014-10-20.15:50:45 text/plain view

Messages

New changeset 5945aa825a51 by Sergi Almacellas Abellana in branch '3.4':
Fix on One2Many on_change when field value is not defined for current record
http://hg.tryton.org/tryton/rev/5945aa825a51
New changeset 0e4297b7a388 by Sergi Almacellas Abellana in branch '3.2':
Fix on One2Many on_change when field value is not defined for current record
http://hg.tryton.org/tryton/rev/0e4297b7a388
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-20.15:50:45
Attached the patch
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-17.17:30:23
I updated the review to the original fix. 
Note: The patch is based on the 3.2 branch

I imagine i'm not allowed to push to 3.2, so do you want me to export the patch and upload?
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2014-10-17.11:35:40
OK, let's go for your original fix in <=3.2
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-17.10:34:11
I stop here. Do whatever you want.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2014-10-17.10:32:34
On 17 Oct 10:01, Sergi Almacellas Abellana wrote:
> So why it's fixed in trunk?

Because trunk is improved.
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-17.10:01:17
So why it's fixed in trunk?
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2014-10-17.09:54:47
For me, it doesn't deserve to be fixed.
Fix the on_change values.
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-17.09:32:14
> The explication of the problem seems wrong.
> Record.set_on_change clearly skip unknown fields.
> And the patch seems wrong because at the begining of set_on_change, > _set_default_value is called which set a value.

This is right on trunk but not on 3.2 (where we found the error) and this fixes the error. So i updated the patch.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2014-10-16.20:02:17
The explication of the problem seems wrong.
Record.set_on_change clearly skip unknown fields.
And the patch seems wrong because at the begining of set_on_change, _set_default_value is called which set a value.
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2014-10-16.18:36:08
I found this issue while using the sale_cost_plan [1] module.

When trying to create a cost_plan from the sale_line form i get the following traceback: 

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/view/form_gtk/many2one.py", line 216, in sig_new
    WinForm(screen, callback, new=True, save_current=True)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/win_form.py", line 35, in __init__
    self.screen.new()

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/screen/screen.py", line 448, in new
    record = group.new(default)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/group.py", line 313, in new
    record.default_get()

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/record.py", line 376, in default_get
    self.set_default(vals)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/record.py", line 434, in set_default
    self.validate(softvalidation=True)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/record.py", line 403, in validate
    if not field.validate(self, softvalidation):

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/field.py", line 116, in validate
    self.set_client(record, value)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/field.py", line 404, in set_client
    force_change=force_change)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/field.py", line 141, in set_client
    self.sig_changed(record)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/field.py", line 41, in sig_changed
    record.on_change(self.name, self.attrs['on_change'])

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/record.py", line 540, in on_change
    self.set_on_change(res)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/record.py", line 487, in set_on_change
    field_x2many.set_on_change(self, value)

  File "/home/sergi/nan/projectes/virtualenv/teb/tryton/tryton/gui/window/view_form/model/field.py", line 697, in set_on_change
    for record2 in record.value[self.name]:

This is because the domain inversion sets the product value, and so calls the on_change_product [2] of cost_plan, which returns the following values:

{
'bom': None,
'uom': 2, 
'boms': {'remove': [], 'add': []}
}

Then it tries to set the values for the fields, which breaks on boms as it's not defined for the current record.

I'm was not able to reproduce it on sao (as I can not create sale_lines), but reading the code it seems that the bug is also there. 

 


This also affects 3.2 (so it will be great if it can be backported) and maybe too to older versions. 

[1] https://bitbucket.org/nantic/trytond-sale_cost_plan
[2] https://bitbucket.org/nantic/trytond-product_cost_plan/src/a7ef5b1c6bb447ada36b45d48703047602656dfe/plan.py?at=default#cl-109
History
Date User Action Args
2014-11-10 23:08:25roundup-botsetmessages: + msg18829
2014-10-21 00:11:32roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg18604
2014-10-20 15:50:46pokolisetfiles: + issue4258.patch
messages: + msg18593
keyword: + patch
2014-10-17 17:30:24pokolisetmessages: + msg18567
2014-10-17 11:35:40cedsetassignedto: ced -> pokoli
messages: + msg18562
2014-10-17 10:34:11pokolisetassignedto: pokoli -> ced
messages: + msg18561
2014-10-17 10:32:35cedsetmessages: + msg18560
2014-10-17 10:01:18pokolisetmessages: + msg18559
2014-10-17 09:54:48cedsetmessages: + msg18558
2014-10-17 09:32:15pokolisetreviews: 14581002,12641002 -> 12641002
messages: + msg18557
component: - sao

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