Merge lp:~openerp-dev/openobject-client-web/trunk-improved-searchview-cpa into lp:openobject-client-web/trunk

[Chirag Patel (OpenERP)]

Hello,

Fixed search view issues, Check with the following steps.

steps:
Go to list view of partner(same - any object), add your custom filter and go to form view,switch back to list view and try to apply filter - group by or any thing.It doesn't work.I have to clear the filter first and then need to restart again so for example :
    i) first I have filtered my data with country and opened form view for particular partner but then I feel I need some more filtration so will go back to list view
    ii) and will try to add another filter,let say language - but at this time filters are not working ,neither custom nor default.Even group by is not working either.

Thanks.

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

* Why do you replace a JSON load by an eval (unrestricted too, not just a literal eval)?

* Why do you munge around with the filter value (removing '%') when the problem here is that this value should *not* have %'s in it because the server takes care of adding the '%' markers to ilike values?

* Could you explain what the problem was and why adding a conditional fixes it?

* from an efficiency standpoint, you added a conditional test within a loop. The loop is performed on the parent's children, and you test on the children's parent, so you could just perform the test on jQuery(this) outside the loop, which would avoid traversing the children and going up to the parent again. Either filter as now (via an if) or use .filter

* do not test visibility by checking for a CSS property, there is more to visibility than that and jQuery takes care of it via the `:visible` and `:hidden` pseudo-classes. They're also much clearer in terms of intent.

review: Needs Fixing

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

Also, please correctly indent the code you edit, as you can see in the diff view below the indentation is all broken up.

[lp:~openerp-dev/openobject-client-web/trunk-improved-searchview-cpa]

4585. By Chirag Patel (OpenERP) on 2011-02-18

[Fix] Fixed reviewed code.

[Chirag Patel (OpenERP) (cpa-openerp)]

review: Needs Resubmitting

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

Could you reply to questions 1 and 3? (I'm not saying there's a problem there, I just want to understand)

review: Needs Information

[Chirag Patel (OpenERP) (cpa-openerp)]

> * Why do you replace a JSON load by an eval (unrestricted too, not just a
> literal eval)?

In commit 4584, I removed json loads because when passing the search value in search.js function "switch_searchView", javascript stops execution because we eval the value like "[[u'country', u'ilike', u'%fr%']]". So eval value from py and passing it to script.

>
> * Why do you munge around with the filter value (removing '%') when the
> problem here is that this value should *not* have %'s in it because the server
> takes care of adding the '%' markers to ilike values?
>
> * Could you explain what the problem was and why adding a conditional fixes
> it?

And I put condition in loop to get object. But it was mistake so I changed accordingly as you said.

>
> * from an efficiency standpoint, you added a conditional test within a loop.
> The loop is performed on the parent's children, and you test on the children's
> parent, so you could just perform the test on jQuery(this) outside the loop,
> which would avoid traversing the children and going up to the parent again.
> Either filter as now (via an if) or use .filter
>
> * do not test visibility by checking for a CSS property, there is more to
> visibility than that and jQuery takes care of it via the `:visible` and
> `:hidden` pseudo-classes. They're also much clearer in terms of intent.

=== modified file 'addons/openerp/controllers/search.py'
--- addons/openerp/controllers/search.py	2011-02-17 17:46:51 +0000
+++ addons/openerp/controllers/search.py	2011-02-18 08:55:33 +0000
@@ -25,8 +25,6 @@
 from error_page import _ep
 from openobject import rpc
 from openobject.tools import expose, ast
-import simplejson
-
 
 class Search(Form):
 
@@ -337,7 +335,7 @@
         if not custom_domains:
             custom_domains = []
         else:
-            custom_domains = simplejson.loads(custom_domains)
+            custom_domains = ast.literal_eval(custom_domains)
         
         # conversion of the pseudo domain from the javascript to a valid domain
         ncustom_domain = []

=== modified file 'addons/openerp/static/javascript/search.js'
--- addons/openerp/static/javascript/search.js	2011-02-17 16:05:42 +0000
+++ addons/openerp/static/javascript/search.js	2011-02-18 08:55:33 +0000
@@ -271,7 +271,7 @@
 function display_Customfilters(all_domains, group_by_ctx) {
     var Allrecords = {};
 	var error = false;
-    jQuery('tbody',idSelector('filter_option_table')).each(function () {
+    jQuery('tbody:visible',idSelector('filter_option_table')).each(function () {
         var record = {};
         var pid = jQuery(this).index();
 
@@ -285,11 +285,11 @@
                 rec[$fieldname.attr('value')] = $constraint_value.val();
                 record[id] = rec;
             } else {
-				$constraint_value.addClass('errorfield').val(_('Invalid Value')).click(function() {
-					jQuery(this).val('').removeClass('errorfield')
-				})
-				error = true;
-			}
+            	$constraint_value.addClass('errorfield').val(_('Invalid Value')).click(function() {
+            		jQuery(this).val('').removeClass('errorfield')
+            	})
+            	error = true;
+            }
         });
 
         if (jQuery.keys(record).length != 0){
@@ -360,9 +360,6 @@
 								if (isOrderable(type)) {
 									comparison = (comparison == 'ilike' ? '=' : '!=');
 								}
-								else {
-									value = '%' + value + '%';
-								}
 								break;
 							case '<':
 							case '>':