Fix incorrect URL generation for missing optional groups

Exclude None values from kwargs in RegexPattern to ensure
translate_url() handles optional named groups correctly.
Adds tests to verify correct URL translation when optional
parameters are absent or present.

Addresses issues with URL translation when optional arguments are missing.
This commit is contained in:
utkarsh.arya@zomato.com 2025-11-15 22:59:59 +00:00
parent e286711879
commit 86454bb0cb
5 changed files with 32 additions and 1 deletions

View file

@ -153,7 +153,7 @@ class RegexPattern(CheckURLMixin):
# If there are any named groups, use those as kwargs, ignoring # If there are any named groups, use those as kwargs, ignoring
# non-named groups. Otherwise, pass all non-named arguments as # non-named groups. Otherwise, pass all non-named arguments as
# positional arguments. # positional arguments.
kwargs = match.groupdict() kwargs = {k: v for k, v in match.groupdict().items() if v is not None}
args = () if kwargs else match.groups() args = () if kwargs else match.groups()
return path[match.end():], args, kwargs return path[match.end():], args, kwargs
return None return None

View file

@ -157,6 +157,16 @@ class URLTranslationTests(URLTestCaseBase):
self.assertEqual(translate_url('/en/account/register/', 'nl'), '/nl/profiel/registreren/') self.assertEqual(translate_url('/en/account/register/', 'nl'), '/nl/profiel/registreren/')
# path() URL pattern # path() URL pattern
self.assertEqual(translate_url('/en/account/register-as-path/', 'nl'), '/nl/profiel/registreren-als-pad/') self.assertEqual(translate_url('/en/account/register-as-path/', 'nl'), '/nl/profiel/registreren-als-pad/')
# URL with optional parameter missing
self.assertEqual(
translate_url('/en/with-arguments/regular-argument/', 'nl'),
'/nl/with-arguments/regular-argument/',
)
# URL with optional parameter present
self.assertEqual(
translate_url('/en/with-arguments/regular-argument/optional.html', 'nl'),
'/nl/with-arguments/regular-argument/optional.html',
)
self.assertEqual(translation.get_language(), 'en') self.assertEqual(translation.get_language(), 'en')
with translation.override('nl'): with translation.override('nl'):

View file

@ -15,6 +15,11 @@ urlpatterns = [
urlpatterns += i18n_patterns( urlpatterns += i18n_patterns(
path('prefixed/', view, name='prefixed'), path('prefixed/', view, name='prefixed'),
path('prefixed.xml', view, name='prefixed_xml'), path('prefixed.xml', view, name='prefixed_xml'),
re_path(
_(r'^with-arguments/(?P<argument>[\w-]+)/(?:(?P<optional>[\w-]+)\.html)?$'),
view,
name='with-arguments',
),
re_path(_(r'^users/$'), view, name='users'), re_path(_(r'^users/$'), view, name='users'),
re_path(_(r'^account/'), include('i18n.patterns.urls.namespace', namespace='account')), re_path(_(r'^account/'), include('i18n.patterns.urls.namespace', namespace='account')),
) )

View file

@ -11,6 +11,7 @@ urlpatterns = [
path('users/<id>/', views.empty_view, name='user-with-id'), path('users/<id>/', views.empty_view, name='user-with-id'),
path('included_urls/', include('urlpatterns.included_urls')), path('included_urls/', include('urlpatterns.included_urls')),
re_path(r'^regex/(?P<pk>[0-9]+)/$', views.empty_view, name='regex'), re_path(r'^regex/(?P<pk>[0-9]+)/$', views.empty_view, name='regex'),
re_path(r'^regex_optional/(?P<arg1>\d+)/(?:(?P<arg2>\d+)/)?$', views.empty_view, name='regex_optional'),
path('', include('urlpatterns.more_urls')), path('', include('urlpatterns.more_urls')),
path('<lang>/<path:url>/', views.empty_view, name='lang-and-path'), path('<lang>/<path:url>/', views.empty_view, name='lang-and-path'),
] ]

View file

@ -54,6 +54,21 @@ class SimplifiedURLTests(SimpleTestCase):
self.assertEqual(match.kwargs, {'pk': '1'}) self.assertEqual(match.kwargs, {'pk': '1'})
self.assertEqual(match.route, '^regex/(?P<pk>[0-9]+)/$') self.assertEqual(match.route, '^regex/(?P<pk>[0-9]+)/$')
def test_re_path_with_optional_parameter(self):
# Test with both optional parameter present and missing
for url, expected_kwargs in (
('/regex_optional/1/2/', {'arg1': '1', 'arg2': '2'}),
('/regex_optional/1/', {'arg1': '1'}),
):
with self.subTest(url=url):
match = resolve(url)
self.assertEqual(match.url_name, 'regex_optional')
self.assertEqual(match.kwargs, expected_kwargs)
self.assertEqual(
match.route,
r'^regex_optional/(?P<arg1>\d+)/(?:(?P<arg2>\d+)/)?$',
)
def test_path_lookup_with_inclusion(self): def test_path_lookup_with_inclusion(self):
match = resolve('/included_urls/extra/something/') match = resolve('/included_urls/extra/something/')
self.assertEqual(match.url_name, 'inner-extra') self.assertEqual(match.url_name, 'inner-extra')