Merge lp:~openerp-dev/openobject-client-web/one2many-onchange into lp:openobject-client-web/trunk
- one2many-onchange
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 3947 | ||||
Proposed branch: | lp:~openerp-dev/openobject-client-web/one2many-onchange | ||||
Merge into: | lp:openobject-client-web/trunk | ||||
Diff against target: |
184 lines (+98/-14) 6 files modified
addons/openerp/controllers/listgrid.py (+31/-0) addons/openerp/static/javascript/form.js (+50/-1) addons/openerp/static/javascript/openerp/openerp.base.js (+1/-1) addons/openerp/widgets/form/_form.py (+1/-1) addons/openerp/widgets/listgrid.py (+1/-1) addons/view_calendar/controllers/templates/calpopup.mako (+14/-10) |
||||
To merge this branch: | bzr merge lp:~openerp-dev/openobject-client-web/one2many-onchange | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Xavier (Open ERP) (community) | Approve | ||
Sananaz (Open ERP) | Pending | ||
Review via email:
|
Commit message
Description of the change
Implement onchange for one2many widget.
For testing refer Bug #671608.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Navrang Oza (noz-tiny) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
Seems to fix the bug, so good.
=On the other hand, bugs:=
* Variable `class` generates a syntax error in Safari, and it's a reserved word in javascript anyway so it should not be used.
=Style/coding in JS code:=
* Is not needed as only used once, and `.attr('class') == 'classname'` is bad code, should use jQuery's `.hasClass` method in case e.g. there is more than one class on the element
* Please do not introduce new mochikit code, we're trying to get rid of it, use jQuery instead (e.g. getElement, getFirstParentB
* Also why all those temporary variables at line 540? They're only used once, their name is not much clearer than they would be within map itself.
* Same with `request`, it's only used once in `addCallback` call, why not just chain `addCallback` with `postJSON`?
* And in any case, `addCallback` is a mochikit API (and so is `postJSON`, transitively), please use jQuery facilities instead in new code.
* Remember to terminate statements with `;` always (line 571), check IDE to see if can not display warning, should be able to.
* jQuery(fld) is called a lot, should probably be memoized to e.g. `$current_field`
* There is code which looks redundant:
if (getElement(
if (getElement(
else if(getElement(
new ListView(
}
}
First line already mandates `!value` test, why re-test it at both other cases? And if you remove it, tests at line 2 and line 4 are pretty much opposite of each other so should be able to do with only one test (and an `else` clause rather than `else if`) no?
= In listgrid.py =
* Why is simplejson imported within method (and therefore re-imported at each execution) rather than just at module toplevel?
* Why do you need copy.deepcopy?
* Please avoid using eval unless you have to. What is content of o2m_view_id? Why can't you use `literal_eval` or an explicit call constructor call?
* Same question for o2m_context and o2m_values (which is first loaded via simplejson and then eval'ed?)
* line 39, doesn't make sense to use `.get`:
view = view['fields'
if `name` is not in `view['fields']`. `.get(name)` is going to return `None` which is going to raise an error when trying to call `.get('views')` on it. So why use `.get`, why not just `[name]`? Same question for the calls after that.
- 3924. By Sananaz (Open ERP)
-
[FIX] Fixed json code.
- 3925. By Sananaz (Open ERP)
-
[FIX] Removed ajaxStop to prevent recursive onchange (Call onchange for default values).
- 3926. By Sananaz (Open ERP)
-
[FIX] Change mochikit call to jQuery for default onchange on calendar date.
- 3927. By Sananaz (Open ERP)
-
[FIX] Fixed reviewed Code for o2m defaults.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Sananaz (Open ERP) (sma-tiny) wrote : | # |
Hello XMO,
I have improved all changes suggested by you and related to other issues also.
I request you to please check it and merge with trunk branch.
Thank you.
- 3928. By Xavier (Open ERP)
-
[FIX] id selector for default_o2m, condition not checking for length on jQuery object, condition on val() content
- 3929. By Xavier (Open ERP)
-
[IMP] use jQuery.post instead of jQuery.ajax as we're not setting the error handlers
- 3930. By Xavier (Open ERP)
-
[IMP] id selectors (use escaping function instead of arbitrary attribute selector and string munging)
- 3931. By Xavier (Open ERP)
-
[IMP] use .val() instead of .attr('value')
- 3932. By Xavier (Open ERP)
-
[IMP] cleanup some jquery code
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Xavier (Open ERP) (xmo-deactivatedaccount) wrote : | # |
Works for me, sma could you check the changes I added, just to be sure i didn't break anything?
Preview Diff
1 | === modified file 'addons/openerp/controllers/listgrid.py' | |||
2 | --- addons/openerp/controllers/listgrid.py 2010-11-17 12:21:33 +0000 | |||
3 | +++ addons/openerp/controllers/listgrid.py 2010-11-24 11:46:04 +0000 | |||
4 | @@ -26,6 +26,9 @@ | |||
5 | 26 | # You can see the MPL licence at: http://www.mozilla.org/MPL/MPL-1.1.html | 26 | # You can see the MPL licence at: http://www.mozilla.org/MPL/MPL-1.1.html |
6 | 27 | # | 27 | # |
7 | 28 | ############################################################################### | 28 | ############################################################################### |
8 | 29 | import copy | ||
9 | 30 | import simplejson | ||
10 | 31 | |||
11 | 29 | import cherrypy | 32 | import cherrypy |
12 | 30 | from openerp.controllers import SecuredController | 33 | from openerp.controllers import SecuredController |
13 | 31 | from openerp.utils import rpc, TinyDict, TinyForm, TinyFormError, context_with_concurrency_info, cache | 34 | from openerp.utils import rpc, TinyDict, TinyForm, TinyFormError, context_with_concurrency_info, cache |
14 | @@ -141,6 +144,34 @@ | |||
15 | 141 | o2m_value.pop(int(index)) | 144 | o2m_value.pop(int(index)) |
16 | 142 | return dict(o2m_value=ustr(o2m_value)) | 145 | return dict(o2m_value=ustr(o2m_value)) |
17 | 143 | 146 | ||
18 | 147 | @expose('json') | ||
19 | 148 | def get_o2m_defaults(self, o2m_values, model, o2m_model, name, view_type, view_id, | ||
20 | 149 | o2m_view_type, o2m_view_id, editable, limit, offset, o2m_context, o2m_domain): | ||
21 | 150 | |||
22 | 151 | view_id = view_id or False | ||
23 | 152 | o2m_view_id = ast.literal_eval(o2m_view_id) or False | ||
24 | 153 | o2m_view_type = o2m_view_type or 'form' | ||
25 | 154 | context = dict(ast.literal_eval(o2m_context), **rpc.session.context) | ||
26 | 155 | o2m_values = simplejson.loads(o2m_values) | ||
27 | 156 | |||
28 | 157 | for o2m in o2m_values: | ||
29 | 158 | o2m['id'] = 0 | ||
30 | 159 | |||
31 | 160 | if o2m_view_id: | ||
32 | 161 | view = cache.fields_view_get(o2m_model, o2m_view_id, o2m_view_type, context) | ||
33 | 162 | else: | ||
34 | 163 | view = cache.fields_view_get(model, view_id, view_type, rpc.session.context) | ||
35 | 164 | view = view['fields'][name]['views'][o2m_view_type] | ||
36 | 165 | |||
37 | 166 | list_view = listgrid.List(name, model, view, ids=None, domain=o2m_domain, context=context, default_data=copy.deepcopy(o2m_values), limit=20, editable= editable,o2m=1) | ||
38 | 167 | view=ustr(list_view.render()) | ||
39 | 168 | formated_o2m_values = [] | ||
40 | 169 | for o2m in o2m_values: | ||
41 | 170 | o2m.pop('id', None) | ||
42 | 171 | formated_o2m_values.append((0, 0, o2m)) | ||
43 | 172 | |||
44 | 173 | return dict(view=view, formated_o2m_values=ustr(formated_o2m_values)) | ||
45 | 174 | |||
46 | 144 | @expose() | 175 | @expose() |
47 | 145 | def multiple_groupby(self, model, name, grp_domain, group_by, view_id, view_type, parent_group, group_level, groups, no_leaf, **kw): | 176 | def multiple_groupby(self, model, name, grp_domain, group_by, view_id, view_type, parent_group, group_level, groups, no_leaf, **kw): |
48 | 146 | grp_domain = ast.literal_eval(grp_domain) | 177 | grp_domain = ast.literal_eval(grp_domain) |
49 | 147 | 178 | ||
50 | === modified file 'addons/openerp/static/javascript/form.js' | |||
51 | --- addons/openerp/static/javascript/form.js 2010-11-24 09:27:06 +0000 | |||
52 | +++ addons/openerp/static/javascript/form.js 2010-11-24 11:46:04 +0000 | |||
53 | @@ -522,8 +522,57 @@ | |||
54 | 522 | 522 | ||
55 | 523 | if ((fld.value !== value) || flag) { | 523 | if ((fld.value !== value) || flag) { |
56 | 524 | fld.value = value; | 524 | fld.value = value; |
57 | 525 | var $current_field = jQuery(fld); | ||
58 | 526 | var kind = $current_field.attr('kind'); | ||
59 | 525 | 527 | ||
61 | 526 | var kind = jQuery(fld).attr('kind'); | 528 | if (!kind) { |
62 | 529 | var $default_o2m = jQuery(idSelector('_terp_default_o2m/'+k)); | ||
63 | 530 | if ($current_field.hasClass('gridview')){ | ||
64 | 531 | if ($default_o2m.length && !value) { | ||
65 | 532 | if($default_o2m.val()) { | ||
66 | 533 | $default_o2m.val(''); | ||
67 | 534 | new ListView(k).reload(); | ||
68 | 535 | } else { | ||
69 | 536 | continue; | ||
70 | 537 | } | ||
71 | 538 | } | ||
72 | 539 | else { | ||
73 | 540 | jQuery.post( | ||
74 | 541 | '/openerp/listgrid/get_o2m_defaults', { | ||
75 | 542 | o2m_values: serializeJSON(value), | ||
76 | 543 | model: jQuery('#_terp_model').val(), | ||
77 | 544 | o2m_model: jQuery(idSelector(prefix+k+'/_terp_model')).val(), | ||
78 | 545 | name: k, | ||
79 | 546 | view_type: jQuery('#_terp_view_type').val(), | ||
80 | 547 | view_id: jQuery('#_terp_view_id').val(), | ||
81 | 548 | o2m_view_type: jQuery(idSelector(prefix+k+'/_terp_view_type')).val(), | ||
82 | 549 | o2m_view_id: jQuery(idSelector(prefix+k+'/_terp_view_id')).val(), | ||
83 | 550 | editable: jQuery(idSelector(prefix+k+'/_terp_editable')).val(), | ||
84 | 551 | limit: jQuery(idSelector(prefix+k+'/_terp_limit')).val(), | ||
85 | 552 | offset: jQuery(idSelector(prefix+k+'/_terp_offset')).val(), | ||
86 | 553 | o2m_context: jQuery(idSelector(prefix+k+'/_terp_context')).val(), | ||
87 | 554 | o2m_domain: jQuery(idSelector(prefix+k+'/_terp_domain')).val() | ||
88 | 555 | }, function(obj) { | ||
89 | 556 | $current_field.closest('.list-a').replaceWith(obj.view); | ||
90 | 557 | if ($default_o2m.length) { | ||
91 | 558 | $default_o2m.val(obj.formated_o2m_values); | ||
92 | 559 | } | ||
93 | 560 | else { | ||
94 | 561 | jQuery(idSelector(k)).parents('td.o2m_cell').append( | ||
95 | 562 | jQuery('<input>', { | ||
96 | 563 | id: '_terp_default_o2m/'+k, | ||
97 | 564 | type: 'hidden', | ||
98 | 565 | name:'_terp_default_o2m/'+k, | ||
99 | 566 | value: obj.formated_o2m_values | ||
100 | 567 | }) | ||
101 | 568 | ); | ||
102 | 569 | } | ||
103 | 570 | }, 'json'); | ||
104 | 571 | } | ||
105 | 572 | } | ||
106 | 573 | fld.__lock_onchange = true; | ||
107 | 574 | return; | ||
108 | 575 | } | ||
109 | 527 | 576 | ||
110 | 528 | switch (kind) { | 577 | switch (kind) { |
111 | 529 | case 'picture': | 578 | case 'picture': |
112 | 530 | 579 | ||
113 | === modified file 'addons/openerp/static/javascript/openerp/openerp.base.js' | |||
114 | --- addons/openerp/static/javascript/openerp/openerp.base.js 2010-11-23 14:01:24 +0000 | |||
115 | +++ addons/openerp/static/javascript/openerp/openerp.base.js 2010-11-24 11:46:04 +0000 | |||
116 | @@ -216,7 +216,7 @@ | |||
117 | 216 | } | 216 | } |
118 | 217 | }); | 217 | }); |
119 | 218 | 218 | ||
121 | 219 | jQuery(document).bind('ready ajaxStop', function (){ | 219 | jQuery(document).bind('ready', function (){ |
122 | 220 | var $caller = jQuery('[callback]:not([type="hidden"]):not([value=""]):not([disabled]):not([readonly]))'); | 220 | var $caller = jQuery('[callback]:not([type="hidden"]):not([value=""]):not([disabled]):not([readonly]))'); |
123 | 221 | $caller.each(function(){ | 221 | $caller.each(function(){ |
124 | 222 | if (jQuery(this).attr('kind') == 'boolean') { | 222 | if (jQuery(this).attr('kind') == 'boolean') { |
125 | 223 | 223 | ||
126 | === modified file 'addons/openerp/widgets/form/_form.py' | |||
127 | --- addons/openerp/widgets/form/_form.py 2010-11-22 10:09:57 +0000 | |||
128 | +++ addons/openerp/widgets/form/_form.py 2010-11-24 11:46:04 +0000 | |||
129 | @@ -438,7 +438,7 @@ | |||
130 | 438 | # self.default = 0.0 | 438 | # self.default = 0.0 |
131 | 439 | 439 | ||
132 | 440 | def set_value(self, value): | 440 | def set_value(self, value): |
134 | 441 | self.default = value | 441 | self.default = value or 0.0 |
135 | 442 | 442 | ||
136 | 443 | register_widget(Float, ["float"]) | 443 | register_widget(Float, ["float"]) |
137 | 444 | 444 | ||
138 | 445 | 445 | ||
139 | === modified file 'addons/openerp/widgets/listgrid.py' | |||
140 | --- addons/openerp/widgets/listgrid.py 2010-11-22 13:31:06 +0000 | |||
141 | +++ addons/openerp/widgets/listgrid.py 2010-11-24 11:46:04 +0000 | |||
142 | @@ -153,7 +153,7 @@ | |||
143 | 153 | 153 | ||
144 | 154 | proxy = rpc.RPCProxy(model) | 154 | proxy = rpc.RPCProxy(model) |
145 | 155 | 155 | ||
147 | 156 | if not self.o2m and not self.m2m and not terp_params.get('_terp_search_text'): | 156 | if not kw.get('default_data') and not self.o2m and not self.m2m and not terp_params.get('_terp_search_text'): |
148 | 157 | if self.limit > 0: | 157 | if self.limit > 0: |
149 | 158 | if self.sort_key: | 158 | if self.sort_key: |
150 | 159 | ids = proxy.search(search_param, self.offset, self.limit, self.sort_key + ' ' +self.sort_order, context) | 159 | ids = proxy.search(search_param, self.offset, self.limit, self.sort_key + ' ' +self.sort_order, context) |
151 | 160 | 160 | ||
152 | === modified file 'addons/view_calendar/controllers/templates/calpopup.mako' | |||
153 | --- addons/view_calendar/controllers/templates/calpopup.mako 2010-10-20 12:41:41 +0000 | |||
154 | +++ addons/view_calendar/controllers/templates/calpopup.mako 2010-11-24 11:46:04 +0000 | |||
155 | @@ -23,16 +23,20 @@ | |||
156 | 23 | '_terp_context': openobject.dom.get('_terp_context').value | 23 | '_terp_context': openobject.dom.get('_terp_context').value |
157 | 24 | }; | 24 | }; |
158 | 25 | 25 | ||
169 | 26 | var req = openobject.http.postJSON('/view_calendar/calpopup/get_defaults', params); | 26 | jQuery.ajax({ |
170 | 27 | req.addCallback(function(obj){ | 27 | url:'/view_calendar/calpopup/get_defaults', |
171 | 28 | forEach(items(obj), function(item){ | 28 | dataType: 'json', |
172 | 29 | var k = item[0]; | 29 | type: 'POST', |
173 | 30 | var v = item[1]; | 30 | data: params, |
174 | 31 | 31 | success: function(obj) { | |
175 | 32 | var e = openobject.dom.get(k); | 32 | forEach(items(obj), function(item){ |
176 | 33 | 33 | var k = item[0]; | |
177 | 34 | if (e) e.value = v; | 34 | var v = item[1]; |
178 | 35 | }); | 35 | var e = openobject.dom.get(k); |
179 | 36 | |||
180 | 37 | if (e) e.value = v; | ||
181 | 38 | }); | ||
182 | 39 | } | ||
183 | 36 | }); | 40 | }); |
184 | 37 | } | 41 | } |
185 | 38 | 42 |
@XMO,
I have it, and its working fine. But still want you to check once.
Thanks.