[FIX] website: correct slug parsing

addons/website/models/ir_http.py

@@ -10,7 +10,7 @@ import werkzeug.routing
 import openerp
 from openerp.addons.base import ir
 from openerp.addons.base.ir import ir_qweb
-from openerp.addons.website.models.website import slug, url_for
+from openerp.addons.website.models.website import slug, url_for, _UNSLUG_RE
 from openerp.http import request
 from openerp.osv import orm
 
@@ -202,7 +202,7 @@ class ModelConverter(ir.ir_http.ModelConverter):
     def __init__(self, url_map, model=False, domain='[]'):
         super(ModelConverter, self).__init__(url_map, model)
         self.domain = domain
-        self.regex = r'(?:[A-Za-z0-9-_]+?-)?(\d+)(?=$|/)'
+        self.regex = _UNSLUG_RE.pattern
 
     def to_url(self, value):
         return slug(value)
@@ -211,7 +211,7 @@ class ModelConverter(ir.ir_http.ModelConverter):
         m = re.match(self.regex, value)
         _uid = RequestUID(value=value, match=m, converter=self)
         return request.registry[self.model].browse(
-            request.cr, _uid, int(m.group(1)), context=request.context)
+            request.cr, _uid, int(m.group(2)), context=request.context)
 
     def generate(self, cr, uid, query=None, args=None, context=None):
         obj = request.registry[self.model]

addons/website/models/website.py

@@ -118,11 +118,23 @@ def slug(value):
     else:
         # assume name_search result tuple
         id, name = value
-    slugname = slugify(name or '')
+    slugname = slugify(name or '').strip().strip('-')
     if not slugname:
         return str(id)
     return "%s-%d" % (slugname, id)
 
+
+_UNSLUG_RE = re.compile(r'(?:(\w{1,2}|\w[a-z0-9-_]+?\w)-)?(-?\d+)(?=$|/)', re.I)
+
+def unslug(s):
+    """Extract slug and id from a string.
+        Always return un 2-tuple (str|None, int|None)
+    """
+    m = _UNSLUG_RE.match(s)
+    if not m:
+        return None, None
+    return m.group(1), int(m.group(2))
+
 def urlplus(url, params):
     return werkzeug.Href(url)(params or None)
 

addons/website/tests/test_converter.py

@@ -9,11 +9,32 @@ from lxml.builder import E
 from openerp.tests import common
 from openerp.addons.base.ir import ir_qweb
 from openerp.addons.website.models.ir_qweb import html_to_text
-from openerp.addons.website.models.website import slugify
+from openerp.addons.website.models.website import slugify, unslug
 
 impl = getDOMImplementation()
 document = impl.createDocument(None, None, None)
 
+class TestUnslug(unittest2.TestCase):
+    def test_unslug(self):
+        tests = {
+            '': (None, None),
+            'foo': (None, None),
+            'foo-': (None, None),
+            '-': (None, None),
+            'foo-1': ('foo', 1),
+            'foo-bar-1': ('foo-bar', 1),
+            'foo--1': ('foo', -1),
+            '1': (None, 1),
+            '1-1': ('1', 1),
+            '--1': (None, None),
+            'foo---1': (None, None),
+            'foo1': (None, None),
+        }
+
+        for slug, expected in tests.iteritems():
+            self.assertEqual(unslug(slug), expected)
+
+
 class TestHTMLToText(unittest2.TestCase):
     def test_rawstring(self):
         self.assertEqual(

addons/website_crm_partner_assign/controllers/main.py

@@ -1,10 +1,9 @@
 # -*- coding: utf-8 -*-
-import re
 import werkzeug
 from openerp import SUPERUSER_ID
 from openerp.addons.web import http
 from openerp.addons.web.http import request
-from openerp.addons.website.models.website import slug
+from openerp.addons.website.models.website import slug, unslug
 from openerp.tools.translate import _
 
 
@@ -131,7 +130,7 @@ class WebsiteCrmPartnerAssign(http.Controller):
     # Do not use semantic controller due to SUPERUSER_ID
     @http.route(['/partners/<partner_id>'], type='http', auth="public", website=True)
     def partners_detail(self, partner_id, partner_name='', **post):
-        mo = re.search('([-0-9]+)$', str(partner_id))
+        _, partner_id = unslug(partner_id)
         current_grade, current_country = None, None
         grade_id = post.get('grade_id')
         country_id = post.get('country_id')
@@ -143,8 +142,7 @@ class WebsiteCrmPartnerAssign(http.Controller):
             country_ids = request.registry['res.country'].exists(request.cr, request.uid, int(country_id), context=request.context)
             if country_ids:
                 current_country = request.registry['res.country'].browse(request.cr, request.uid, country_ids[0], context=request.context)
-        if mo:
-            partner_id = int(mo.group(1))
+        if partner_id:
             partner = request.registry['res.partner'].browse(request.cr, SUPERUSER_ID, partner_id, context=request.context)
             if partner.exists() and partner.website_published:
                 values = {

addons/website_customer/controllers/main.py

@@ -1,9 +1,8 @@
 # -*- coding: utf-8 -*-
-import re
-
 import openerp
 from openerp import SUPERUSER_ID
 from openerp.addons.web import http
+from openerp.addons.website.models.website import unslug
 from openerp.tools.translate import _
 from openerp.addons.web.http import request
 import werkzeug.urls
@@ -84,9 +83,8 @@ class WebsiteCustomer(http.Controller):
     # Do not use semantic controller due to SUPERUSER_ID
     @http.route(['/customers/<partner_id>'], type='http', auth="public", website=True)
     def partners_detail(self, partner_id, **post):
-        mo = re.search('([-0-9]+)$', str(partner_id))
-        if mo:
-            partner_id = int(mo.group(1))
+        _, partner_id = unslug(partner_id)
+        if partner_id:
             partner = request.registry['res.partner'].browse(request.cr, SUPERUSER_ID, partner_id, context=request.context)
             if partner.exists() and partner.website_published:
                 values = {}

addons/website_membership/controllers/main.py

@@ -1,9 +1,8 @@
 # -*- coding: utf-8 -*-
-import re
-
 from openerp import SUPERUSER_ID
 from openerp.addons.web import http
 from openerp.addons.web.http import request
+from openerp.addons.website.models.website import unslug
 from openerp.tools.translate import _
 
 import werkzeug.urls
@@ -105,9 +104,8 @@ class WebsiteMembership(http.Controller):
     # Do not use semantic controller due to SUPERUSER_ID
     @http.route(['/members/<partner_id>'], type='http', auth="public", website=True)
     def partners_detail(self, partner_id, **post):
-        mo = re.search('([-0-9]+)$', str(partner_id))
-        if mo:
-            partner_id = int(mo.group(1))
+        _, partner_id = unslug(partner_id)
+        if partner_id:
             partner = request.registry['res.partner'].browse(request.cr, SUPERUSER_ID, partner_id, context=request.context)
             if partner.exists() and partner.website_published:
                 values = {}