Merge lp:~openerp-dev/openobject-client-web/inline-list-prompt-action into lp:openobject-client-web/trunk

[Vaibhav Darji]

Inline ListView Prompt action same as form view.

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

* The validation message is terrible, what is "the record"? How is anybody going to understand that? It was OK for form, but for list is needs to be much clearer.
* old listgridValidation(name, o2m) have become far too complex and redundant, please extract redundancies into a function
* In validateForm, `_list_view` can not be `0` or an empty string (or id selector will fail anyway), so you don't need `_list_view != null`, `_list_view` should be enough to test that there is a list view id.
* You probably should not use `is_list_changed` (denoting a boolean value) to store the id of the modified list. Either rename key (to e.g. `modified_list`) or use two different data keys (a boolean to know if a list changed, and then a string to know *which* list changed)
-- listgrid.js
* Why async=false in listgrid save?
* Do not use alert() for errors
* If you have an id (as string) and want to turn it into a jquery id selector, there is now an `idSelector` function in openobject.dom.js, this should be used in ListView.save and in listgridValidation
* Also the `return` after $focus_field is redundant
* In listgridValidation, `0` is not a valid second parameter for parseInt, use `10`.
* jQuery.ajax can take a context keyword argument, this means the `var self=this` aliasing is now unnecessary (just pass `context: this` and in the success callback replace `self` by `this`)

Also more general comments:
* create more, smaller and more focused commits. It's easier to review and cherrypick if each commit does a single job instead of having a single big commit doing 3000 things at once. Intent of change also becomes clearer due to commit messages. You're not limited in number of commits and you don't have to pay for them so...
* Please remember to add comment if you commit/push anything new as launchpad doesn't warn reviewers

review: Needs Fixing

[Krisztian Eyssen (eyssen)]

review: Approve

Unmerged revisions

3736. By Vaibhav Darji on 2010-10-27

[IMP] Improved inline listgrid validation same as form.

=== modified file 'addons/openerp/static/javascript/form.js'
--- addons/openerp/static/javascript/form.js	2010-10-27 09:36:11 +0000
+++ addons/openerp/static/javascript/form.js	2010-10-27 11:26:02 +0000
@@ -1186,14 +1186,38 @@
 }
 
 
-function validateForm(){
-    jQuery('#view_form table tr td:first').find('input:not([type=hidden]), select').change(function(){
-        jQuery('#view_form').data('is_form_changed', true);
-    });
+function validateForm(_list_view){
+    var $form = jQuery('#view_form');
+    if (_list_view != null) {
+        var $list = jQuery('[id="' + _list_view + '"]').removeAttr('current_id');
+        
+        if($form.data('is_list_changed')) {
+            $form.removeData('is_list_changed');
+        }
+        
+        jQuery('table.grid[id="'+_list_view+'_grid'+'"] tr.grid-row[record] td.grid-cell:not(.selector)').find('input, select').change(function(){
+            $form.data('is_list_changed', _list_view);
+            $list.attr('current_id',parseInt(jQuery(this).closest('tr.grid-row').attr('record'), 10) || -1);
+        });
+    }
+    else {
+        jQuery('#view_form table tr td:first').find('input:not([type=hidden]), select').change(function(){
+            $form.data('is_form_changed', true);
+        });
+    }
 }
 
 function validate_action() {
     var $form = jQuery('#view_form');
+    if($form.data('is_list_changed') && confirm('The record has been modified \n Do you want to save it ?')){
+        var _list = $form.data('is_list_changed');
+        var current_id = jQuery('[id="' + _list + '"]').attr('current_id');
+        var record_id = jQuery('[id="' + _list + '"]').attr('_record_id');
+        if(current_id != null) {
+            new ListView(_list).save(current_id, record_id);
+        }
+    }
+    
     if ($form.data('is_form_changed') && confirm('The record has been modified \n Do you want to save it ?')) {
         if (!validate_required($form.get(0))) {
             return;
@@ -1210,6 +1234,7 @@
             }
         });
     }
+    
     if (arguments.length) {
         var params = arguments[0];
         var action = arguments[1];

=== modified file 'addons/openerp/static/javascript/listgrid.js'
--- addons/openerp/static/javascript/listgrid.js	2010-10-27 07:13:40 +0000
+++ addons/openerp/static/javascript/listgrid.js	2010-10-27 11:26:02 +0000
@@ -544,44 +544,49 @@
         args['_terp_source'] = this.name;
 
         var self = this;
-        var req = openobject.http.postJSON('/openerp/listgrid/save', args);
-
         var $current_record = jQuery('table[id="'+this.name+'_grid'+'"]').find('tr.grid-row[record="'+id+'"]');
-        req.addCallback(function(obj) {
-            if (obj.error) {
-                error_display(obj.error);
-
-                var $focus_field;
-                for (var k in data) {
-                    var $req_field = $current_record.find('td [id="'+'_terp_listfields/'+k+'"].requiredfield');
-
-                    if(!$req_field.length)
-                        continue;
-                    if($req_field.attr('kind') == 'many2one') {
-                        $req_field = $current_record.find('td [id="'+'_terp_listfields/'+k+'_text'+'"]');
-                    }
-
-                    var req_value = $req_field.val();
-                    if(req_value == '') {
-                        $req_field.addClass('errorfield');
-                        if(!$focus_field) {
-                            $focus_field = $req_field;
-                        }
-                    } else if($req_field.hasClass('errorfield')) {
-                        $req_field.removeClass('errorfield');
-                    }
-                }
-                if($focus_field) {
-                    $focus_field.focus();
-                }
-            } else {
-                openobject.dom.get(prefix + '_terp_id').value = obj.id;
-                openobject.dom.get(prefix + '_terp_ids').value = obj.ids;
-
-                if(prev_id != undefined) {
-                    self.reload(prev_id , prefix ? 1 : 0);
+        jQuery.ajax({
+            url: '/openerp/listgrid/save',
+            data: args,
+            dataType: 'json',
+            type: 'POST',
+            async: false,
+            error: function(obj) {
+                alert('error'+obj)
+            },
+            success: function(obj) {
+                if(obj.error) {
+                    error_display(obj.error);
+                    var $focus_field;
+                    for (var k in data) {
+                        var $req_field = $current_record.find('td [id="'+'_terp_listfields/'+k+'"].requiredfield');
+                        if(!$req_field.length)
+                            continue;
+                        if($req_field.attr('kind') == 'many2one') {
+                            $req_field = $current_record.find('td [id="'+'_terp_listfields/'+k+'_text'+'"]');
+                        }
+                        var req_value = $req_field.val();
+                        if(req_value == '') {
+                            $req_field.addClass('errorfield');
+                            if(!$focus_field) {
+                                $focus_field = $req_field;
+                            }
+                        } else if($req_field.hasClass('errorfield')) {
+                            $req_field.removeClass('errorfield');
+                        }
+                    }
+                    if($focus_field) {
+                        $focus_field.focus();
+                    }
+                    return;
                 } else {
-                    self.reload(id > 0 ? null : -1, prefix ? 1 : 0);
+                    jQuery('[id="'+prefix+'_terp_id'+'"]').val(obj.id);
+                    jQuery('[id="'+prefix+'_terp_ids'+'"]').val(obj.ids);
+                    if(prev_id != undefined) {
+                        self.reload(prev_id , prefix ? 1 : 0);
+                    } else {
+                        self.reload(id > 0 ? null : -1, prefix ? 1 : 0);
+                    }
                 }
             }
         });
@@ -841,36 +846,25 @@
     }
 });
 
-function validateList(_list) {
-    var $list = jQuery('[id="' + _list + '"]').removeAttr('current_id');
-    var $check = jQuery('table.grid[id="'+_list+'_grid'+'"] tr.grid-row[record] td.grid-cell:not(.selector)').find('input, select');
-    $check.change(function() {
-        $list.attr(
-            'current_id',
-            parseInt(jQuery(this).closest('tr.grid-row').attr('record'), 10) || -1);
-    });
-}
-
-function listgridValidation(_list, o2m, record_id) {
-    o2m = parseInt(o2m, 0);
-    var current_id = jQuery('[id="' + _list + '"]').attr('current_id');
-    // not(null | undefined)
-    if(current_id != null) {
-        if(confirm('The record has been modified \n Do you want to save it ?')) {
-            new ListView(_list).save(current_id, record_id);
-        }
-    } else{
-        if(o2m) {
-            var detail = jQuery('table.one2many[id$="' + _list + '"]').attr('detail');
-            if(record_id == undefined || record_id == -1) {
-                new One2Many(_list, detail).create();
-            } else {
-                new ListView(_list).edit(record_id);
-            }
-        } else if(record_id == -1) {
-            new ListView(_list).create();
+function listgridValidation(_list) {
+    var $list = jQuery('[id="' + _list + '"]');
+    var o2m = parseInt($list.attr('_o2m'), 0);
+    var current_id = $list.attr('current_id');
+    var record_id = $list.attr('_record_id');
+    if (current_id != null) {
+        return;
+    }
+    
+    if (o2m) {
+        var detail = jQuery('table.one2many[id$="' + _list + '"]').attr('detail');
+        if (record_id == undefined || record_id == -1) {
+            new One2Many(_list, detail).create();
         } else {
             new ListView(_list).edit(record_id);
         }
+    } else if(record_id == -1) {
+        new ListView(_list).create();
+    } else {
+        new ListView(_list).edit(record_id);
     }
 }

=== modified file 'addons/openerp/widgets/templates/listgrid.mako'
--- addons/openerp/widgets/templates/listgrid.mako	2010-10-25 12:46:53 +0000
+++ addons/openerp/widgets/templates/listgrid.mako	2010-10-27 11:26:02 +0000
@@ -133,17 +133,22 @@
                                                     ${_('Add')}
                                             </button>
                                         % elif o2m:
-                                            <button title="${_('Create new record.')}" id="${name}_btn_"
-                                                onclick="listgridValidation('${name}', '${o2m or 0}', -1); return false;">
-                                                    ${_('New')}
-                                            </button>
+                                            <button title="${_('Create new record.')}" id="${name}_btn_">${_('New')}</button>
+                                            <script type="text/javascript">
+                                                jQuery('[id=${name}_btn_]').click(function(){
+                                                    jQuery('[id=${name}]').attr({'_o2m': '${o2m or 0}', '_record_id': -1});
+                                                    validate_action('${name}', listgridValidation);
+                                                    return false;
+                                                });
+                                            </script>
                                         % else:
                                             % if not dashboard:
                                                 <button id="${name}_new" title="${_('Create new record.')}">${_('New')}</button>
                                                 % if editors:
                                                     <script type="text/javascript">
                                                         jQuery('[id=${name}_new]').click(function() {
-                                                            listgridValidation('${name}', '${o2m or 0}', -1)
+                                                            jQuery('[id=${name}]').attr({'_o2m': '${o2m or 0}', '_record_id': -1});
+                                                            validate_action('${name}', listgridValidation);
                                                             return false;
                                                         });
                                                     </script>
@@ -160,7 +165,7 @@
                                         % if not m2m and not dashboard and editors:
                                             <script type="text/javascript">
                                                 jQuery(document).ready(function() {
-                                                    validateList('${name}')
+                                                    validateForm('${name}', '${o2m or 0}')
                                                 });
                                             </script>
                                         % endif
@@ -328,7 +333,8 @@
                                jQuery(row).click(function(event) {
                                    if (!jQuery(event.target).is(':input, img, option, td.m2o_coplition')) {
                                        var record_id = parseInt(jQuery(row).attr('record'), 10) || -1;
-                                       listgridValidation('${name}','${o2m or 0}', record_id);
+                                       jQuery('[id=${name}]').attr({'_o2m': '${o2m or 0}', '_record_id': record_id});
+                                       validate_action('${name}', listgridValidation);
                                    }
                                });
                            });