Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions djangocms_link/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@
except (ModuleNotFoundError, ImportError): # pragma: no cover
File = None

from djangocms_link.validators import AnchorValidator, ExtendedURLValidator
from djangocms_link.validators import (
AnchorValidator,
ExtendedURLValidator,
RelativeURLValidator,
)


MINIMUM_INPUT_LENGTH = getattr(settings, "DJANGOCMS_LINK_MINIMUM_INPUT_LENGTH", 0)
Expand Down Expand Up @@ -146,6 +150,7 @@ def optgroups(self, name: str, value: str, attr: dict | None = None):
# Configure the LinkWidget
link_types = {
"internal_link": _("Internal link"),
"relative_link": _("Relative link"),
"external_link": _("External link/anchor"),
}
if File:
Expand All @@ -155,7 +160,15 @@ def optgroups(self, name: str, value: str, attr: dict | None = None):
allowed_link_types = getattr(
settings,
"DJANGOCMS_LINK_ALLOWED_LINK_TYPES",
("internal_link", "external_link", "file_link", "anchor", "mailto", "tel"),
(
"internal_link",
"relative_link",
"external_link",
"file_link",
"anchor",
"mailto",
"tel",
),
)

# Adjust example uri schemes to allowed link types
Expand Down Expand Up @@ -195,6 +208,16 @@ def optgroups(self, name: str, value: str, attr: dict | None = None):
).format(example_uri_scheme),
},
), # External link input
"relative_link": TextInput(
attrs={
"widget": "relative_link",
"placeholder": _("/some/path - optionally append #anchor"),
"data-help": _(
"Provide a relative link. Optionally, add an #anchor "
"(including the #) to scroll to."
).format(example_uri_scheme),
},
), # Relative link input
"internal_link": LinkAutoCompleteWidget(
attrs={
"widget": "internal_link",
Expand Down Expand Up @@ -291,6 +314,9 @@ class LinkFormField(Field):
external_link_validators = [
ExtendedURLValidator(allowed_link_types=allowed_link_types)
]
relative_link_validators = [
RelativeURLValidator(allowed_link_types=allowed_link_types)
]
internal_link_validators = []
file_link_validators = []
anchor_validators = [AnchorValidator()]
Expand All @@ -314,6 +340,10 @@ def prepare_value(self, value: dict) -> list[str | None]:
pos = self._get_pos("external_link")
multi_value[0] = "external_link"
multi_value[pos] = value["external_link"]
elif "relative_link" in value:
pos = self._get_pos("relative_link")
multi_value[0] = "relative_link"
multi_value[pos] = value["relative_link"]
elif "internal_link" in value:
pos = self._get_pos("internal_link")
anchor_pos = self._get_pos("anchor")
Expand Down
9 changes: 7 additions & 2 deletions djangocms_link/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ def get_link(link_field_value: dict, site_id: int | None = None) -> str | None:
if link_field_value["external_link"].startswith("tel:"):
return link_field_value["external_link"].replace(" ", "")
return link_field_value["external_link"] or None
elif "relative_link" in link_field_value:
return link_field_value["relative_link"] or None
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Relative links are now returned directly; consider normalization for leading/trailing whitespace.

Strip whitespace from relative links before returning to prevent subtle bugs.

Suggested change
elif "relative_link" in link_field_value:
return link_field_value["relative_link"] or None
elif "relative_link" in link_field_value:
relative_link = link_field_value["relative_link"]
if relative_link is not None:
relative_link = relative_link.strip()
return relative_link or None


if "__cache__" in link_field_value:
return link_field_value["__cache__"] or None
Expand Down Expand Up @@ -82,7 +84,10 @@ def __init__(self, initial=None, **kwargs):
if isinstance(initial, dict):
self.update(initial)
elif isinstance(initial, str):
self["external_link"] = initial
if initial.startswith("/"):
self["relative_link"] = initial
else:
self["external_link"] = initial
elif isinstance(initial, File):
self["file_link"] = initial.pk
elif isinstance(initial, models.Model):
Expand All @@ -100,7 +105,7 @@ def url(self) -> str:

@property
def type(self) -> str:
for key in ("internal_link", "file_link"):
for key in ("relative_link", "internal_link", "file_link"):
if key in self:
return key
if "external_link" in self:
Expand Down
6 changes: 4 additions & 2 deletions djangocms_link/static/djangocms_link/link-widget.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
min-width: unset;
}
}
.external_link, .internal_link, .file_link, .anchor, .site {
.external_link, .relative_link, .internal_link, .file_link, .anchor, .site {
display: none;
padding: 0;
select, input {
Expand All @@ -23,7 +23,8 @@
width: 100% !important;
}
}
.external_link {
.external_link,
.relative_link {
width: 75%;
}
.internal_link {
Expand All @@ -37,6 +38,7 @@
margin-top: 0.5em;
}
&[data-type="external_link"] .external_link,
&[data-type="relative_link"] .relative_link,
&[data-type="internal_link"] .internal_link,
&[data-type="internal_link"] .site,
&[data-type="internal_link"] .anchor
Expand Down
29 changes: 29 additions & 0 deletions djangocms_link/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,32 @@ def __call__(self, value: str):
):
return EmailValidator()(value[7:])
return super().__call__(value)


@deconstructible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider refactoring the validation logic to use early returns and a helper function for error handling to improve readability.

Here’s one way to collapse the nested logic into a straight‐line of early checks plus a single helper for raising errors. This keeps every check, but it’s easier to scan and maintains the same behavior:

@deconstructible
class RelativeURLValidator:
    message = _("Enter a valid relative link")
    code = "invalid"

    def __init__(self, allowed_link_types: list = None, **kwargs):
        self.allowed_link_types = allowed_link_types
        super().__init__(**kwargs)

    def __call__(self, value: str):
        # simple helper to reduce repetition
        def _fail():
            raise ValidationError(self.message, code=self.code, params={"value": value})

        if not isinstance(value, str):
            _fail()
        if len(value) > URLValidator.max_length:
            _fail()
        if URLValidator.unsafe_chars.intersection(value):
            _fail()

        # must start with “/”
        if not value.startswith("/"):
            _fail()
        # if caller has restricted link types, enforce “relative_link”
        if self.allowed_link_types and "relative_link" not in self.allowed_link_types:
            _fail()

        return value

Steps to apply:

  1. Extract a small _fail() inner function so you don’t repeat the ValidationError call.
  2. Turn each condition into an early return (_fail()), so you avoid nested and/or logic.
  3. Split the “must start with /” check from the “allowed_link_types” check to keep each test atomic.

class RelativeURLValidator:
message = _("Enter a valid relative link")
code = "invalid"

def __init__(self, allowed_link_types: list = None, **kwargs):
self.allowed_link_types = allowed_link_types
super().__init__(**kwargs)

def __call__(self, value: str):
if not isinstance(value, str) or len(value) > URLValidator.max_length:
raise ValidationError(self.message, code=self.code, params={"value": value})
if URLValidator.unsafe_chars.intersection(value):
raise ValidationError(self.message, code=self.code, params={"value": value})
if (
value.startswith("/") and (
self.allowed_link_types is not None
and "relative_link" not in self.allowed_link_types
)
or not value.startswith("/")
):
raise ValidationError(
self.message,
code=self.code,
params={"value": value},
)
return value
33 changes: 26 additions & 7 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,31 @@ class LinkForm(forms.Form):
'<select name="link_field_0" class="js-link-widget-selector" data-help="No destination selected. '
'Use the dropdown to select a destination." required id="id_link_field_0">'
'<option value="internal_link">Internal link</option>'
'<option value="relative_link">Relative link</option>'
'<option value="external_link">External link/anchor</option>'
'<option value="file_link">File link</option></select>',
form_html,
)
# Render internal URL field
self.assertIn(
'<select name="link_field_2" widget="internal_link" '
'<select name="link_field_3" widget="internal_link" '
'data-help="Select from available internal destinations. Optionally, add an anchor to scroll to." '
'data-placeholder="" required id="id_link_field_2" class="admin-autocomplete" data-ajax--cache="true" '
'data-placeholder="" required id="id_link_field_3" class="admin-autocomplete" data-ajax--cache="true" '
'data-ajax--delay="250" data-ajax--type="GET" data-ajax--url="/en/admin/djangocms_link/link/urls" '
'data-theme="admin-autocomplete" data-allow-clear="true" '
'data-minimum-input-length="0" lang="en">'
'<option value=""></option><option value="" selected>None</option>'
"</select>",
form_html,
)
# Render relative URL field
self.assertIn(
'<input type="text" name="link_field_2" widget="relative_link" '
'placeholder="/some/path - optionally append #anchor" '
'data-help="Provide a relative link. Optionally, add an #anchor (including the #) to scroll to." '
'required id="id_link_field_2">',
form_html,
)
# Render external URL field
self.assertIn(
'<input type="url" name="link_field_1" widget="external_link" '
Expand All @@ -68,6 +77,7 @@ class LinkNotRequiredForm(forms.Form):
'Use the dropdown to select a destination." id="id_link_field_0">'
'<option value="empty">---------</option>'
'<option value="internal_link">Internal link</option>'
'<option value="relative_link">Relative link</option>'
'<option value="external_link">External link/anchor</option>'
'<option value="file_link">File link</option></select>',
str(LinkNotRequiredForm()),
Expand All @@ -91,6 +101,7 @@ def check_value(value):
self.assertEqual(form.cleaned_data["link_field"], value)

check_value({"internal_link": f"cms.page:{self.page.id}", "anchor": "#anchor"})
check_value({"relative_link": "/some/path"})
check_value({"external_link": "https://example.com"})
check_value({"external_link": "#anchor"})
check_value({"file_link": str(self.file.id)})
Expand All @@ -101,21 +112,29 @@ class LinkForm(forms.Form):
link_field = LinkFormField(required=False)

form = LinkForm(initial={"link_field": {"internal_link": f"cms.page:{self.page.id}"}})
self.assertEqual(form["link_field"].value(), ["internal_link", None, f"cms.page:{self.page.id}", "", None])
self.assertEqual(form["link_field"].value(), ["internal_link", None, None, f"cms.page:{self.page.id}", "", None])

def test_form_field_initial_works_relative(self):
class LinkForm(forms.Form):
link_field = LinkFormField(required=False)

some_string = "/some/path"
form = LinkForm(initial={"link_field": {"relative_link": some_string}})
self.assertEqual(form["link_field"].value(), ["relative_link", None, some_string, None, None, None])

def test_form_field_initial_works_external(self):
class LinkForm(forms.Form):
link_field = LinkFormField(required=False)

some_string = "https://example.com"
form = LinkForm(initial={"link_field": {"external_link": some_string}})
self.assertEqual(form["link_field"].value(), ["external_link", some_string, None, None, None])
self.assertEqual(form["link_field"].value(), ["external_link", some_string, None, None, None, None])

def test_widget_renders_selection(self):
widget = LinkWidget()
pre_select_page = len(widget.widgets) * [None]
pre_select_page[0] = "internal_link"
pre_select_page[2] = f"cms.page:{self.page.id}"
pre_select_page[3] = f"cms.page:{self.page.id}"
rendered_widget = widget.render(
"link_field", pre_select_page, attrs={"id": "id_link_field"}
)
Expand All @@ -129,14 +148,14 @@ def test_widget_renders_site_selector(self):
widget = LinkWidget(site_selector=True)
pre_select_page = len(widget.widgets) * [None]
pre_select_page[0] = "internal_link"
pre_select_page[2] = f"cms.page:{self.page.id}"
pre_select_page[3] = f"cms.page:{self.page.id}"
rendered_widget = widget.render(
"link_field", pre_select_page, attrs={"id": "id_link_field"}
)

# Subwidget is present
self.assertIn(
'<select name="link_field_2" class="js-link-site-widget admin-autocomplete" widget="site"',
'<select name="link_field_3" class="js-link-site-widget admin-autocomplete" widget="site"',
rendered_widget,
)
# Current site is pre-selected
Expand Down
11 changes: 11 additions & 0 deletions tests/test_link_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ def test_external_link(self):
self.assertEqual(link1.type, "external_link")
self.assertEqual(link2.type, "external_link")

def test_relative_link(self):
link1 = LinkDict({"relative_link": "/some/path"})
link2 = LinkDict("/other/path")

self.assertEqual(link1.url, "/some/path")
self.assertEqual(link2.url, "/other/path")
self.assertEqual(str(link1), "/some/path")
self.assertEqual(str(link2), "/other/path")
self.assertEqual(link1.type, "relative_link")
self.assertEqual(link2.type, "relative_link")

def test_file_link(self):
file = File.objects.create(file=get_random_string(5))
link1 = LinkDict({"file_link": file.pk})
Expand Down
19 changes: 18 additions & 1 deletion tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ def setUp(self):
target=TARGET_CHOICES[0][0],
attributes="{'data-type', 'link'}",
)
self.relative_link = Link.objects.create(
template="default",
name="My Link",
link={"relative_link": "/some/path#some_id"},
target=TARGET_CHOICES[0][0],
attributes="{'data-type', 'link'}",
)
self.external_link = Link.objects.create(
template="default",
name="My Link",
Expand Down Expand Up @@ -65,7 +72,7 @@ def tearDown(self):

def test_link_instance(self):
instances = Link.objects.all()
self.assertEqual(instances.count(), 6) # 4 instances created in setUp
self.assertEqual(instances.count(), 7) # 7 instances created in setUp

instance = Link.objects.first()
self.assertEqual(instance.template, "default")
Expand Down Expand Up @@ -97,6 +104,9 @@ def test_get_link(self):
self.internal_link.get_link(), self.page.get_absolute_url() + "#some_id"
)
self.assertEqual(self.file_link.get_link(), self.file.url)
self.assertEqual(
self.relative_link.get_link(), "/some/path#some_id"
)
self.assertEqual(
self.external_link.get_link(), "https://www.django-cms.org/#some_id"
)
Expand All @@ -111,6 +121,9 @@ def test_get_url_template_tag(self):
to_url(self.internal_link.link), self.page.get_absolute_url() + "#some_id"
)
self.assertEqual(to_url(self.file_link.link), self.file.url)
self.assertEqual(
to_url(self.relative_link.link), "/some/path#some_id"
)
self.assertEqual(
to_url(self.external_link.link), "https://www.django-cms.org/#some_id"
)
Expand All @@ -130,6 +143,10 @@ def test_to_link_template_tag(self):
"__cache__": self.page.get_absolute_url(),
}
)
self.assertEqual(
to_link("/some/path#some_id"),
{"relative_link": "/some/path#some_id"},
)
self.assertEqual(
to_link("https://www.django-cms.org/#some_id"),
{"external_link": "https://www.django-cms.org/#some_id"},
Expand Down