From 8d4e99c6a6e2281b00ec25e870065d0728e27606 Mon Sep 17 00:00:00 2001
From: Romain Derie <rde@odoo.com>
Date: Mon, 12 Dec 2022 13:42:04 +0000
Subject: [PATCH] [FIX] website: fix multiple error in submenu template

- `clean_url` method was wrongly flag as `api.model`
- the `unslug_url` was not called for the URL comparison in the dropdown
  case, meaning that `/shop/prod-1` would not match `/shop/product-1` as
  it should (and as it does for regular non dropdown menu)
- the `active` class was actually never working for the dropdown case,
  as the class was added on the wrong element (`li` instead of `a`)

The code was hard to read (mainly because huge python conditions in XML)
and kinda redundant, going through an util method should be clearer and
help reading the template XML.

task-3096367
opw-3091427

Part-of: odoo/odoo#107782
---
 addons/website/models/website_menu.py      | 27 ++++++++++++++++++++--
 addons/website/views/website_templates.xml | 12 ++++------
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/addons/website/models/website_menu.py b/addons/website/models/website_menu.py
index 736393f7cd8..af54eea1430 100644
--- a/addons/website/models/website_menu.py
+++ b/addons/website/models/website_menu.py
@@ -5,6 +5,7 @@ import werkzeug.exceptions
 import werkzeug.urls
 
 from odoo import api, fields, models
+from odoo.addons.http_routing.models.ir_http import unslug_url
 from odoo.http import request
 from odoo.tools.translate import html_translate
 
@@ -123,8 +124,7 @@ class Menu(models.Model):
                 visible = False
             menu.is_visible = visible
 
-    @api.model
-    def clean_url(self):
+    def _clean_url(self):
         # clean the url with heuristic
         if self.page_id:
             url = self.page_id.sudo().url
@@ -138,6 +138,29 @@ class Menu(models.Model):
                     url = '/%s' % self.url
         return url
 
+    def _is_active(self):
+        if not request:
+            return False
+
+        request_url = unslug_url(request.httprequest.path)
+
+        if not self.child_id:
+            # Don't compare to `url` as it could be shadowed by the linked
+            # website page's URL
+            menu_url = self._clean_url()
+            if not menu_url:
+                return False
+            menu_url = unslug_url(menu_url)
+            if request_url == menu_url:
+                return True
+        else:
+            # Child match (dropdown menu), `self` is just a parent/container,
+            # don't check its URL, consider only its children
+            if any(request_url == unslug_url(child.url) for child in self.child_id if child.url):
+                return True
+
+        return False
+
     # would be better to take a menu_id as argument
     @api.model
     def get_tree(self, website_id, menu_id=None):
diff --git a/addons/website/views/website_templates.xml b/addons/website/views/website_templates.xml
index 6f4708115fe..8889c16fb64 100644
--- a/addons/website/views/website_templates.xml
+++ b/addons/website/views/website_templates.xml
@@ -4,19 +4,17 @@
 <template id="submenu" name="Submenu">
     <t t-set="show_dropdown" t-value="(submenu.is_mega_menu and submenu.is_visible) or submenu.child_id.filtered(lambda menu: menu.is_visible)"/>
     <li t-if="submenu.is_visible and not (submenu.child_id or submenu.is_mega_menu)" t-attf-class="#{item_class or ''}">
-        <a t-att-href="submenu.clean_url()"
-            t-attf-class="#{link_class or ''} #{'active' if submenu.clean_url() and unslug_url(request.httprequest.path) == unslug_url(submenu.clean_url()) else ''}"
+        <a t-att-href="submenu._clean_url()"
+            t-attf-class="#{link_class or ''} #{submenu._is_active() and 'active'}"
             role="menuitem"
             t-ignore="true"
             t-att-target="'_blank' if submenu.new_window else None">
             <span t-field="submenu.name"/>
         </a>
     </li>
-    <li t-elif="show_dropdown" t-attf-class="#{item_class or ''} dropdown #{
-        (submenu.clean_url() and submenu.clean_url() != '/' and any(request.httprequest.path == child.url for child in submenu.child_id if child.url) or
-         (submenu.clean_url() and request.httprequest.path == submenu.clean_url())) and 'active'
-        } #{submenu.is_mega_menu and 'position-static'}">
-        <a t-attf-class="#{link_class or ''} dropdown-toggle #{submenu.is_mega_menu and 'o_mega_menu_toggle'}" data-bs-toggle="dropdown" href="#">
+    <li t-elif="show_dropdown" t-attf-class="#{item_class or ''} dropdown #{submenu.is_mega_menu and 'position-static'}">
+        <a t-attf-class="#{link_class or ''} dropdown-toggle #{submenu.is_mega_menu and 'o_mega_menu_toggle'} #{submenu._is_active() and 'active'}"
+           data-bs-toggle="dropdown" href="#">
             <span t-field="submenu.name"/>
         </a>
         <div t-if="submenu.is_mega_menu"