From bacce76767f6e2346fab8cf68ab13d0c6c42e1f6 Mon Sep 17 00:00:00 2001
From: Romain Courteaud <romain@nexedi.com>
Date: Thu, 30 Apr 2009 13:06:30 +0000
Subject: [PATCH] Do not use WebSection_zGetDocumentValueList when
 portal_catalog is enough.

Simplify WebSection_zGetDocumentValueList to the case it as to handle in order to improve maintainance.

WebSection_zGetDocumentValueList heavily depends on MySQL implementation, as it seems that the "GROUP BY reference" returns the first line of the subselect (which is the expected one due to "ORDER BY priority DESC"). But, without any explicit reference of this behaviour in MySQL documentation, wrong results may be returned with another MySQL version.

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@26745 20353a03-c40f-0410-a6d1-a30d3c3de9de
---
 .../WebSection_getDocumentValueListBase.xml   | 21 +++-
 .../WebSection_zGetDocumentValueList.xml      | 98 +++++--------------
 bt5/erp5_web/bt/revision                      |  2 +-
 3 files changed, 39 insertions(+), 82 deletions(-)

diff --git a/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_getDocumentValueListBase.xml b/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_getDocumentValueListBase.xml
index e1ebf173da..5766bf62f2 100644
--- a/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_getDocumentValueListBase.xml
+++ b/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_getDocumentValueListBase.xml
@@ -113,16 +113,26 @@ try:\n
     language = portal.Localizer.get_selected_language()\n
 \n
   if validation_state is None:\n
+    # XXX hardcoded validation state list.\n
+    # Use predicate or layout property instead\n
     validation_state = (\'released\', \'released_alive\', \'published\', \n
                         \'published_alive\', \'shared\', \'shared_alive\', \n
                         \'public\', \'validated\')\n
   kw[\'validation_state\'] = validation_state\n
 \n
-  return current_section.WebSection_zGetDocumentValueList(language=language,\n
-                                                          all_languages=all_languages,\n
-                                                          all_versions=all_versions,\n
-                                                          src__=src__,\n
-                                                          kw=kw)\n
+  if \'sort_on\' not in kw:\n
+    # XXX Do not sort by default, as it increases query time\n
+    kw[\'sort_on\'] = [(\'int_index\', \'DESC\'), (\'reference\', \'DESC\')]\n
+\n
+  if not all_versions:\n
+    return current_section.WebSection_zGetDocumentValueList(language=language,\n
+                                                            all_languages=all_languages,\n
+                                                            src__=src__,\n
+                                                            kw=kw)\n
+  else:\n
+    if not all_languages:\n
+      kw[\'language\'] = language\n
+    return current_section.portal_catalog(src__=src__, **kw)\n
 \n
 except Unauthorized:\n
   return []\n
@@ -177,6 +187,7 @@ except Unauthorized:\n
                             <string>current_section</string>
                             <string>None</string>
                             <string>_write_</string>
+                            <string>_apply_</string>
                           </tuple>
                         </value>
                     </item>
diff --git a/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_zGetDocumentValueList.xml b/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_zGetDocumentValueList.xml
index 4e6268d47a..0efd7f81f2 100644
--- a/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_zGetDocumentValueList.xml
+++ b/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_zGetDocumentValueList.xml
@@ -37,12 +37,6 @@
                                   <dictionary/>
                                 </value>
                             </item>
-                            <item>
-                                <key> <string>all_versions</string> </key>
-                                <value>
-                                  <dictionary/>
-                                </value>
-                            </item>
                             <item>
                                 <key> <string>kw</string> </key>
                                 <value>
@@ -64,7 +58,6 @@
                           <list>
                             <string>language</string>
                             <string>all_languages</string>
-                            <string>all_versions</string>
                             <string>kw</string>
                           </list>
                         </value>
@@ -84,7 +77,6 @@
             <key> <string>arguments_src</string> </key>
             <value> <string>language\r\n
 all_languages\r\n
-all_versions\r\n
 kw</string> </value>
         </item>
         <item>
@@ -135,20 +127,16 @@ kw</string> </value>
     this method should be refactored to use catalog only.\n
 \n
      The subquery is named catalog to prevent use another LEFT JOIN.\n
+\n
+     XXX This query heavily depends on MySQL implementation, \n
+     as it seems that the "GROUP BY reference" returns the first line of the subselect \n
+     (which is the expected one due to "ORDER BY priority DESC").\n
+     But, without any explicit reference of this behaviour in MySQL\n
+     documentation, wrong results may be returned with another MySQL version.\n
   </dtml-comment>\n
 \n
   SELECT\n
-    catalog.path AS path,\n
-    catalog.uid AS uid,\n
-    catalog.modification_date,\n
-    catalog.reference,\n
-    catalog.int_index,\n
-    catalog.creation_date,\n
-    catalog.title,\n
-    catalog.priority\n
-    <dtml-if is_translation_order_by>\n
-      , catalog.translated_message\n
-    </dtml-if>\n
+    *\n
   FROM\n
     (\n
       SELECT\n
@@ -157,23 +145,13 @@ kw</string> </value>
         catalog.int_index,\n
         catalog.modification_date,\n
         catalog.reference,\n
-        my_versioning.language,\n
-        my_versioning.version,\n
         catalog.creation_date,\n
         catalog.title,\n
-        <dtml-if all_versions>\n
-        <dtml-else>\n
         CONCAT(CASE my_versioning.language\n
                    WHEN <dtml-sqlvar language type="string"> THEN \'3\'\n
                    WHEN \'en\' THEN \'2\'\n
                    ELSE \'1\' END,\n
-            my_versioning.version) AS current_priority,\n
-        </dtml-if>\n
-        MAX(CONCAT(CASE my_versioning.language\n
-                   WHEN <dtml-sqlvar language type="string"> THEN \'3\'\n
-                   WHEN \'en\' THEN \'2\'\n
-                   ELSE \'1\' END,\n
-            my_versioning.version)) AS priority\n
+               my_versioning.version) AS priority\n
         <dtml-if is_translation_order_by>\n
           , related_translation_intermediate.translated_message\n
         </dtml-if>\n
@@ -196,29 +174,20 @@ kw</string> </value>
           AND my_versioning.language = <dtml-sqlvar language type="string">\n
         </dtml-if>\n
 \n
-      GROUP BY\n
-        catalog.uid\n
       ORDER BY\n
         priority DESC\n
+\n
     ) AS catalog\n
 \n
-  <dtml-if all_versions>\n
-  <dtml-else>\n
-    WHERE\n
-      catalog.priority = catalog.current_priority\n
-    GROUP BY reference\n
-  </dtml-if>\n
+  GROUP BY reference\n
 \n
-  ORDER BY\n
   <dtml-if "query[\'order_by_expression\']">\n
+    ORDER BY\n
     <dtml-if is_translation_order_by>\n
       translated_message\n
     <dtml-else>\n
       <dtml-var "query[\'order_by_expression\']">\n
     </dtml-if>\n
-  <dtml-else> \t \t \n
-     catalog.int_index,\n
-     catalog.reference\n
   </dtml-if>\n
 \n
   <dtml-if "query[\'limit_expression\']">\n
@@ -278,20 +247,16 @@ kw</string> </value>
     this method should be refactored to use catalog only.\n
 \n
      The subquery is named catalog to prevent use another LEFT JOIN.\n
+\n
+     XXX This query heavily depends on MySQL implementation, \n
+     as it seems that the "GROUP BY reference" returns the first line of the subselect \n
+     (which is the expected one due to "ORDER BY priority DESC").\n
+     But, without any explicit reference of this behaviour in MySQL\n
+     documentation, wrong results may be returned with another MySQL version.\n
   </dtml-comment>\n
 \n
   SELECT\n
-    catalog.path AS path,\n
-    catalog.uid AS uid,\n
-    catalog.modification_date,\n
-    catalog.reference,\n
-    catalog.int_index,\n
-    catalog.creation_date,\n
-    catalog.title,\n
-    catalog.priority\n
-    <dtml-if is_translation_order_by>\n
-      , catalog.translated_message\n
-    </dtml-if>\n
+    *\n
   FROM\n
     (\n
       SELECT\n
@@ -300,23 +265,13 @@ kw</string> </value>
         catalog.int_index,\n
         catalog.modification_date,\n
         catalog.reference,\n
-        my_versioning.language,\n
-        my_versioning.version,\n
         catalog.creation_date,\n
         catalog.title,\n
-        <dtml-if all_versions>\n
-        <dtml-else>\n
         CONCAT(CASE my_versioning.language\n
                    WHEN <dtml-sqlvar language type="string"> THEN \'3\'\n
                    WHEN \'en\' THEN \'2\'\n
                    ELSE \'1\' END,\n
-            my_versioning.version) AS current_priority,\n
-        </dtml-if>\n
-        MAX(CONCAT(CASE my_versioning.language\n
-                   WHEN <dtml-sqlvar language type="string"> THEN \'3\'\n
-                   WHEN \'en\' THEN \'2\'\n
-                   ELSE \'1\' END,\n
-            my_versioning.version)) AS priority\n
+               my_versioning.version) AS priority\n
         <dtml-if is_translation_order_by>\n
           , related_translation_intermediate.translated_message\n
         </dtml-if>\n
@@ -339,29 +294,20 @@ kw</string> </value>
           AND my_versioning.language = <dtml-sqlvar language type="string">\n
         </dtml-if>\n
 \n
-      GROUP BY\n
-        catalog.uid\n
       ORDER BY\n
         priority DESC\n
+\n
     ) AS catalog\n
 \n
-  <dtml-if all_versions>\n
-  <dtml-else>\n
-    WHERE\n
-      catalog.priority = catalog.current_priority\n
-    GROUP BY reference\n
-  </dtml-if>\n
+  GROUP BY reference\n
 \n
-  ORDER BY\n
   <dtml-if "query[\'order_by_expression\']">\n
+    ORDER BY\n
     <dtml-if is_translation_order_by>\n
       translated_message\n
     <dtml-else>\n
       <dtml-var "query[\'order_by_expression\']">\n
     </dtml-if>\n
-  <dtml-else> \t \t \n
-     catalog.int_index,\n
-     catalog.reference\n
   </dtml-if>\n
 \n
   <dtml-if "query[\'limit_expression\']">\n
diff --git a/bt5/erp5_web/bt/revision b/bt5/erp5_web/bt/revision
index a58cdef3e9..f0b2b124d1 100644
--- a/bt5/erp5_web/bt/revision
+++ b/bt5/erp5_web/bt/revision
@@ -1 +1 @@
-802
\ No newline at end of file
+803
\ No newline at end of file
-- 
2.30.9