Merge lp:~openerp-dev/openobject-client-web/one2many-onchange into lp:openobject-client-web/trunk

[Sananaz (Open ERP)]

Implement onchange for one2many widget.
For testing refer Bug #671608.

[Navrang Oza (noz-tiny)]

@XMO,

I have it, and its working fine. But still want you to check once.
Thanks.

[Xavier (Open ERP) (xmo-deactivatedaccount)]

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, getFirstParentByTagAndClassName, INPUT)
* 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('_terp_default_o2m/'+k) && !value) {
        if (getElement('_terp_default_o2m/'+k).value=='' && !value)
            continue;
        else if(getElement('_terp_default_o2m/'+k).value && !value) {
            getElement('_terp_default_o2m/'+k).value = '';
            new ListView(k).reload();
        }
    }

  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'].get(name).get('views').get(o2m_view_type)

  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.

review: Needs Fixing

[lp:~openerp-dev/openobject-client-web/one2many-onchange]

3924. By Sananaz (Open ERP) on 2010-11-24

[FIX] Fixed json code.

3925. By Sananaz (Open ERP) on 2010-11-24

[FIX] Removed ajaxStop to prevent recursive onchange (Call onchange for default values).

3926. By Sananaz (Open ERP) on 2010-11-24

[FIX] Change mochikit call to jQuery for default onchange on calendar date.

3927. By Sananaz (Open ERP) on 2010-11-24

[FIX] Fixed reviewed Code for o2m defaults.

[Sananaz (Open ERP) (sma-tiny)]

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.

[lp:~openerp-dev/openobject-client-web/one2many-onchange]

3928. By Xavier (Open ERP) on 2010-11-24

[FIX] id selector for default_o2m, condition not checking for length on jQuery object, condition on val() content

3929. By Xavier (Open ERP) on 2010-11-24

[IMP] use jQuery.post instead of jQuery.ajax as we're not setting the error handlers

3930. By Xavier (Open ERP) on 2010-11-24

[IMP] id selectors (use escaping function instead of arbitrary attribute selector and string munging)

3931. By Xavier (Open ERP) on 2010-11-24

[IMP] use .val() instead of .attr('value')

3932. By Xavier (Open ERP) on 2010-11-24

[IMP] cleanup some jquery code

[Xavier (Open ERP) (xmo-deactivatedaccount)]

Works for me, sma could you check the changes I added, just to be sure i didn't break anything?

review: Approve

=== modified file 'addons/openerp/controllers/listgrid.py'
--- addons/openerp/controllers/listgrid.py	2010-11-17 12:21:33 +0000
+++ addons/openerp/controllers/listgrid.py	2010-11-24 11:46:04 +0000
@@ -26,6 +26,9 @@
 # You can see the MPL licence at: http://www.mozilla.org/MPL/MPL-1.1.html
 #
 ###############################################################################
+import copy
+import simplejson
+
 import cherrypy
 from openerp.controllers import SecuredController
 from openerp.utils import rpc, TinyDict, TinyForm, TinyFormError, context_with_concurrency_info, cache
@@ -141,6 +144,34 @@
         o2m_value.pop(int(index))
         return dict(o2m_value=ustr(o2m_value))
 
+    @expose('json')
+    def get_o2m_defaults(self, o2m_values, model, o2m_model, name, view_type, view_id,
+                         o2m_view_type, o2m_view_id, editable, limit, offset, o2m_context, o2m_domain):
+
+        view_id = view_id or False
+        o2m_view_id = ast.literal_eval(o2m_view_id) or False
+        o2m_view_type = o2m_view_type or 'form'
+        context = dict(ast.literal_eval(o2m_context), **rpc.session.context)
+        o2m_values = simplejson.loads(o2m_values)
+        
+        for o2m in o2m_values:
+            o2m['id'] = 0
+
+        if o2m_view_id:
+            view = cache.fields_view_get(o2m_model, o2m_view_id, o2m_view_type, context)
+        else:
+            view = cache.fields_view_get(model, view_id, view_type, rpc.session.context)
+            view = view['fields'][name]['views'][o2m_view_type]
+
+        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)
+        view=ustr(list_view.render())
+        formated_o2m_values = []
+        for o2m in o2m_values:
+            o2m.pop('id', None)
+            formated_o2m_values.append((0, 0, o2m))
+
+        return dict(view=view, formated_o2m_values=ustr(formated_o2m_values))
+
     @expose()
     def multiple_groupby(self, model, name, grp_domain, group_by, view_id, view_type, parent_group, group_level, groups, no_leaf, **kw):
         grp_domain = ast.literal_eval(grp_domain)

=== modified file 'addons/openerp/static/javascript/form.js'
--- addons/openerp/static/javascript/form.js	2010-11-24 09:27:06 +0000
+++ addons/openerp/static/javascript/form.js	2010-11-24 11:46:04 +0000
@@ -522,8 +522,57 @@
 
             if ((fld.value !== value) || flag) {
                 fld.value = value;
+                var $current_field = jQuery(fld);
+                var kind = $current_field.attr('kind');
 
-                var kind = jQuery(fld).attr('kind');
+                if (!kind) {
+                    var $default_o2m = jQuery(idSelector('_terp_default_o2m/'+k));
+                    if ($current_field.hasClass('gridview')){
+                        if ($default_o2m.length && !value) {
+                            if($default_o2m.val()) {
+                                $default_o2m.val('');
+                                new ListView(k).reload();
+                            } else {
+                                continue;
+                            }
+                        }
+                        else {
+                            jQuery.post(
+                                '/openerp/listgrid/get_o2m_defaults', {
+                                    o2m_values: serializeJSON(value),
+                                    model: jQuery('#_terp_model').val(),
+                                    o2m_model: jQuery(idSelector(prefix+k+'/_terp_model')).val(),
+                                    name: k,
+                                    view_type: jQuery('#_terp_view_type').val(),
+                                    view_id: jQuery('#_terp_view_id').val(),
+                                    o2m_view_type: jQuery(idSelector(prefix+k+'/_terp_view_type')).val(),
+                                    o2m_view_id: jQuery(idSelector(prefix+k+'/_terp_view_id')).val(),
+                                    editable: jQuery(idSelector(prefix+k+'/_terp_editable')).val(),
+                                    limit: jQuery(idSelector(prefix+k+'/_terp_limit')).val(),
+                                    offset: jQuery(idSelector(prefix+k+'/_terp_offset')).val(),
+                                    o2m_context: jQuery(idSelector(prefix+k+'/_terp_context')).val(),
+                                    o2m_domain: jQuery(idSelector(prefix+k+'/_terp_domain')).val()
+                                }, function(obj) {
+                                    $current_field.closest('.list-a').replaceWith(obj.view);
+                                    if ($default_o2m.length) {
+                                        $default_o2m.val(obj.formated_o2m_values);
+                                    }
+                                    else {
+                                        jQuery(idSelector(k)).parents('td.o2m_cell').append(
+                                            jQuery('<input>', {
+                                                id: '_terp_default_o2m/'+k,
+                                                type: 'hidden',
+                                                name:'_terp_default_o2m/'+k,
+                                                value: obj.formated_o2m_values
+                                            })
+                                        );
+                                    }
+                                }, 'json');
+                        }
+                    }
+                    fld.__lock_onchange = true;
+                    return;
+                }
 
                 switch (kind) {
                     case 'picture':

=== modified file 'addons/openerp/static/javascript/openerp/openerp.base.js'
--- addons/openerp/static/javascript/openerp/openerp.base.js	2010-11-23 14:01:24 +0000
+++ addons/openerp/static/javascript/openerp/openerp.base.js	2010-11-24 11:46:04 +0000
@@ -216,7 +216,7 @@
     }
 });
 
-jQuery(document).bind('ready ajaxStop', function (){
+jQuery(document).bind('ready', function (){
     var $caller = jQuery('[callback]:not([type="hidden"]):not([value=""]):not([disabled]):not([readonly]))');
     $caller.each(function(){
         if (jQuery(this).attr('kind') == 'boolean') {

=== modified file 'addons/openerp/widgets/form/_form.py'
--- addons/openerp/widgets/form/_form.py	2010-11-22 10:09:57 +0000
+++ addons/openerp/widgets/form/_form.py	2010-11-24 11:46:04 +0000
@@ -438,7 +438,7 @@
 #            self.default = 0.0
 
     def set_value(self, value):
-        self.default = value
+        self.default = value or 0.0
 
 register_widget(Float, ["float"])
 

=== modified file 'addons/openerp/widgets/listgrid.py'
--- addons/openerp/widgets/listgrid.py	2010-11-22 13:31:06 +0000
+++ addons/openerp/widgets/listgrid.py	2010-11-24 11:46:04 +0000
@@ -153,7 +153,7 @@
 
         proxy = rpc.RPCProxy(model)
 
-        if not self.o2m and not self.m2m and not terp_params.get('_terp_search_text'):
+        if not kw.get('default_data') and not self.o2m and not self.m2m and not terp_params.get('_terp_search_text'):
             if self.limit > 0:
                 if self.sort_key:
                     ids = proxy.search(search_param, self.offset, self.limit, self.sort_key + ' ' +self.sort_order, context)

=== modified file 'addons/view_calendar/controllers/templates/calpopup.mako'
--- addons/view_calendar/controllers/templates/calpopup.mako	2010-10-20 12:41:41 +0000
+++ addons/view_calendar/controllers/templates/calpopup.mako	2010-11-24 11:46:04 +0000
@@ -23,16 +23,20 @@
                 '_terp_context': openobject.dom.get('_terp_context').value
             };
 
-            var req = openobject.http.postJSON('/view_calendar/calpopup/get_defaults', params);
-            req.addCallback(function(obj){
-                forEach(items(obj), function(item){
-                    var k = item[0];
-                    var v = item[1];
-
-                    var e = openobject.dom.get(k);
-
-                    if (e) e.value = v;
-                });
+            jQuery.ajax({
+                url:'/view_calendar/calpopup/get_defaults',
+                dataType: 'json',
+                type: 'POST',
+                data: params,
+                success: function(obj) {
+                    forEach(items(obj), function(item){
+                        var k = item[0];
+                        var v = item[1];
+                        var e = openobject.dom.get(k);
+
+                        if (e) e.value = v;
+                    });
+                }
             });
         }