From 2899d067f302fc1ab0a65146ce890999e6689657 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Sun, 2 Aug 2026 19:16:48 -0400 Subject: [PATCH 1/3] Closes #496: Add optional link title to URL fields A url-type CustomObjectTypeField now expands into two real DB columns: the URL itself, and an optional _title used as the visible link text on an object's detail page instead of the raw URL (falling back to the URL when no title is set). Mirrors CoordinatesFieldType's existing two-column pattern, but unlike coordinates the primary URL column keeps behaving like any other single-value column -- unique, default, and regex validation all still apply to it. This required making three previously coordinates-only DDL/validation code paths in models.py dict-aware (the generic single-column schema helpers, the unique-conversion probe in clean(), and the backing- column-collision guard), since URL is the first multi-column type to also flow through the generic single-column path via its unique/ default support. No new migration or upgrade script is needed for existing installations: the plugin's existing post_migrate schema-heal pass already covers any nullable non-mixin column whose attribute name doesn't match a user field's own name, which the new title column satisfies the same way coordinates' latitude/longitude columns already do. --- docs/field-attributes.md | 23 +++- netbox_custom_objects/api/serializers.py | 5 + netbox_custom_objects/field_types.py | 47 ++++++- netbox_custom_objects/mixin_migration.py | 8 ++ netbox_custom_objects/models.py | 117 +++++++++++++----- .../netbox_custom_objects/customobject.html | 8 ++ .../templatetags/custom_object_utils.py | 25 ++++ netbox_custom_objects/tests/test_api.py | 74 +++++++++++ .../tests/test_field_types.py | 97 ++++++++++++++- .../tests/test_schema_operations.py | 46 +++++++ netbox_custom_objects/tests/test_views.py | 61 +++++++++ netbox_custom_objects/views.py | 25 ++++ 12 files changed, 499 insertions(+), 37 deletions(-) diff --git a/docs/field-attributes.md b/docs/field-attributes.md index 075e0d3f..6e8476d2 100644 --- a/docs/field-attributes.md +++ b/docs/field-attributes.md @@ -13,7 +13,7 @@ The following attributes are available when creating or editing a Custom Object | `boolean` | True/false | | `date` | Date | | `datetime` | Date and time | -| `url` | URL | +| `url` | URL, with an optional link title | | `json` | Arbitrary JSON value | | `select` | Single selection from a choice set | | `multiselect` | Multiple selections from a choice set | @@ -107,3 +107,24 @@ Behaviour: - **Map link.** Detail views render the coordinates with a **Map** button that opens the location using NetBox's `MAPS_URL` configuration parameter (Google Maps by default). - `Must be unique` and `Default` are not supported for `coordinates` fields. + +## URL Fields + +Field type: `url` + +A `url` field stores the URL value plus an optional human-readable **link title**. +Adding one `url` field named `website` creates two backing columns: + +- `website` — the URL itself +- `website_title` — optional display text shown in place of the raw URL on the + object's detail page + +Behaviour: + +- **Detail page only.** The title is used as the visible link text on the object + detail page. List/table views continue to show the raw URL as plain text. +- **Optional independently.** The title has no relationship to the URL value — + setting one without the other is allowed and harmless. +- **REST API.** The pair is exposed as two flat fields, `` and `_title`. +- `Must be unique` and `Default` continue to apply to the URL value itself, exactly + as for any other `url` field. diff --git a/netbox_custom_objects/api/serializers.py b/netbox_custom_objects/api/serializers.py index 3d117af3..627f5c54 100644 --- a/netbox_custom_objects/api/serializers.py +++ b/netbox_custom_objects/api/serializers.py @@ -502,6 +502,11 @@ def get_serializer_class(model, skip_object_fields=False): if field.type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: custom_field_names += [f"{field.name}_latitude", f"{field.name}_longitude"] continue + # URL fields expand into the URL itself plus an optional title column; + # expose both flat, mirroring the coordinates treatment above. + if field.type == CustomObjectFieldTypeChoices.TYPE_URL: + custom_field_names += [field.name, f"{field.name}_title"] + continue if field.name not in model_field_names: continue # excluded during model generation (e.g. broken FK) if skip_object_fields and field.type in [ diff --git a/netbox_custom_objects/field_types.py b/netbox_custom_objects/field_types.py index 23eb13f0..cbfefbbd 100644 --- a/netbox_custom_objects/field_types.py +++ b/netbox_custom_objects/field_types.py @@ -524,18 +524,63 @@ def get_filterform_field(self, field, **kwargs): class URLFieldType(FieldType): + """ + A URL field. Expands into two real DB columns: ````, the URL itself, and + ``_title``, an optional human-readable title shown in place of the raw URL + on an object's detail page. Unlike CoordinatesFieldType, the primary ```` + column keeps behaving like any other single-value column (unique/default/regex + validation all still apply to it) -- only the title is new. + """ + graphql_annotation = str + @staticmethod + def title_field_name(field): + return f"{field.name}_title" + def get_model_field(self, field, **kwargs): field_kwargs = self._safe_kwargs(**kwargs) field_kwargs.update({"default": field.default, "unique": field.unique}) - return models.URLField(null=True, blank=True, **field_kwargs) + return { + field.name: models.URLField(null=True, blank=True, **field_kwargs), + self.title_field_name(field): models.CharField( + max_length=200, + null=True, + blank=True, + help_text=_("Human-readable text shown instead of the raw URL."), + ), + } def get_form_field(self, field, **kwargs): return LaxURLField( assume_scheme="https", required=field.required, initial=field.default ) + def get_form_fields(self, field): + """ + Return the URL and title form fields, keyed by their backing column names + (mirrors CoordinatesFieldType.get_form_fields). + """ + base_label = field.label or field.name.replace("_", " ").title() + url_field = LaxURLField( + label=base_label, + assume_scheme="https", + required=field.required, + initial=field.default, + ) + title_field = forms.CharField( + label=f"{base_label} ({_('link title')})", + required=False, + max_length=200, + ) + if field.ui_editable != CustomFieldUIEditableChoices.YES: + url_field.disabled = True + title_field.disabled = True + return { + field.name: url_field, + self.title_field_name(field): title_field, + } + def get_filterform_field(self, field, **kwargs): return forms.CharField( label=field, diff --git a/netbox_custom_objects/mixin_migration.py b/netbox_custom_objects/mixin_migration.py index fe46d336..8794bab3 100644 --- a/netbox_custom_objects/mixin_migration.py +++ b/netbox_custom_objects/mixin_migration.py @@ -47,6 +47,14 @@ def _expected_base_fields(cot, model=None): (f.name). This is equivalent to matching by f.column for user-defined COT fields because they are never created with db_column overrides. + Multi-column custom field types (e.g. CoordinatesFieldType's "_latitude"/ + "_longitude", URLFieldType's "_title") are NOT excluded here, since their + backing columns' attribute names never equal the user field's own f.name -- + so this heal pass also covers them as a deliberate side effect. This is why + a newly introduced sub-column for one of those types needs no dedicated + migration/upgrade code: as long as it's nullable, the next post_migrate run + (or `upgrade_custom_objects`) auto-adds it to every existing COT table. + Pass *model* to avoid a second get_model() call when the caller already holds the model reference. """ diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 847f14bf..66763d7a 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -285,6 +285,24 @@ def _apply_deferred_co_field(field_instance): _deferred_co_field_data.set(None) +# Field types whose FieldType.get_model_field() returns a dict of multiple backing +# columns rather than a single Field (coordinates: latitude/longitude; url: the URL +# itself plus an optional title). Used to reject type conversion to/from these types +# (see CustomObjectTypeField.clean()) and to widen the backing-column-collision scan. +MULTI_COLUMN_TYPES = { + CustomObjectFieldTypeChoices.TYPE_COORDINATES, + CustomObjectFieldTypeChoices.TYPE_URL, +} + + +def _primary_model_field(fi): + """fi's own -column Field, unwrapping FieldType.get_model_field()'s dict + return for a multi-column type (coordinates, url) so single-column DDL/validation + code can keep treating every field type uniformly.""" + mf = FIELD_TYPE_CLASS[fi.type]().get_model_field(fi) + return mf[fi.name] if isinstance(mf, dict) else mf + + def _schema_add_field(fi, model, schema_editor, schema_conn): """``add_field`` against *schema_conn*; idempotent (skips if column exists). @@ -292,7 +310,7 @@ def _schema_add_field(fi, model, schema_editor, schema_conn): applied here — call ``_apply_deferred_co_field`` separately after. """ ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) + mf = _primary_model_field(fi) mf.contribute_to_class(model, fi.name) with schema_conn.cursor() as cursor: @@ -342,8 +360,7 @@ def _schema_remove_field(fi, model, schema_editor, schema_conn=None, existing_ta PostgreSQL doesn't reject the call with "pending trigger events". *existing_tables* optionally short-circuits the per-call introspection. """ - ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) + mf = _primary_model_field(fi) mf.contribute_to_class(model, fi.name) if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: @@ -397,8 +414,8 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist ) return - old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) - new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) + old_mf = _primary_model_field(old_fi) + new_mf = _primary_model_field(new_fi) old_mf.contribute_to_class(model, old_fi.name) new_mf.contribute_to_class(model, new_fi.name) @@ -437,7 +454,7 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist new_fi.pk, schema_conn.alias, ) return - live_mf = FIELD_TYPE_CLASS[live_fi.type]().get_model_field(live_fi) + live_mf = _primary_model_field(live_fi) live_mf.contribute_to_class(model, live_fi.name) if live_mf.column not in existing_cols: logger.debug( @@ -955,6 +972,8 @@ def clone_fields(self): # Coordinates fields have no single column; clone the two backing columns. if field_type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: names += [f"{name}_latitude", f"{name}_longitude"] + elif field_type == CustomObjectFieldTypeChoices.TYPE_URL: + names += [name, f"{name}_title"] else: names.append(name) @@ -2667,12 +2686,11 @@ def clean(self): and hasattr(self, '_original') and not self.original.unique ): - field_type = FIELD_TYPE_CLASS[self.type]() - model_field = field_type.get_model_field(self) + model_field = _primary_model_field(self) model = self.custom_object_type.get_model() model_field.contribute_to_class(model, self.name) - old_field = field_type.get_model_field(self.original) + old_field = _primary_model_field(self.original) old_field.contribute_to_class(model, self._original_name) # Route the probe through the branch's connection so the ALTER @@ -2811,46 +2829,53 @@ def clean(self): {"name": _("Cannot rename a polymorphic field after creation.")} ) - # Prevent converting an existing field to or from coordinates. + # Prevent converting an existing field to or from a multi-column type + # (coordinates, url). # - # A coordinates field occupies two concrete columns ("{name}_latitude" and - # "{name}_longitude") while every other field type occupies a single column - # named "{name}". The save() path has no logic to migrate between those two - # shapes, so allowing the conversion would either leave the original column - # orphaned (→ coordinates) or attempt to alter a column that doesn't exist - # (coordinates → other), raising an uncaught database error. Reject it here, + # A multi-column field occupies more than one concrete column ("{name}_latitude"/ + # "{name}_longitude" for coordinates, "{name}"/"{name}_title" for url) while every + # other field type (and, prior to this conversion, the field itself) occupies a + # single column. The save() path has no logic to migrate between those shapes, so + # allowing the conversion would either leave columns orphaned or attempt to alter a + # column that doesn't exist, raising an uncaught database error. Reject it here, # mirroring the polymorphic-flag guard above. - is_coordinates = self.type == CustomObjectFieldTypeChoices.TYPE_COORDINATES - was_coordinates = self._original_type == CustomObjectFieldTypeChoices.TYPE_COORDINATES - if self.pk and not self._state.adding and is_coordinates != was_coordinates: + if ( + self.pk + and not self._state.adding + and self.type != self._original_type + and (self.type in MULTI_COLUMN_TYPES or self._original_type in MULTI_COLUMN_TYPES) + ): raise ValidationError( - {"type": _("Cannot change a field's type to or from coordinates after creation.")} + {"type": _( + "Cannot change a field's type to or from a multi-column type " + "(coordinates, URL) after creation." + )} ) # Guard against backing-column name collisions. # - # A coordinates field expands into "{name}_latitude"/"{name}_longitude". If a - # sibling field already occupies one of those column names (or vice versa: a - # plain field named "_latitude"), the schema editor would issue a - # duplicate-column ALTER and PostgreSQL would raise a ProgrammingError instead - # of a clean validation error. Detect the overlap here. + # A multi-column field expands into more than one real column (see above). If a + # sibling field already occupies one of those column names (or vice versa: a plain + # field named e.g. "_latitude" or "_title"), the schema editor would + # issue a duplicate-column ALTER and PostgreSQL would raise a ProgrammingError + # instead of a clean validation error. Detect the overlap here. if self.custom_object_type_id: def _occupied_columns(name, field_type): if field_type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: return {f"{name}_latitude", f"{name}_longitude"} + if field_type == CustomObjectFieldTypeChoices.TYPE_URL: + return {name, f"{name}_title"} return {name} own_columns = _occupied_columns(self.name, self.type) - # A clash can only involve a coordinates field's expanded columns. If - # this field isn't coordinates, only sibling coordinates fields can - # collide with it — so skip the full sibling scan and query just those - # (usually zero) to avoid an extra queryset on every field's save. - if self.type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: + # A clash can only involve a multi-column field's expanded columns. If this + # field isn't multi-column, only sibling multi-column fields can collide with + # it — so skip the full sibling scan and query just those (usually zero) to + # avoid an extra queryset on every field's save. + if self.type in MULTI_COLUMN_TYPES: siblings = self.custom_object_type.fields.all() else: - siblings = self.custom_object_type.fields.filter( - type=CustomObjectFieldTypeChoices.TYPE_COORDINATES - ) + siblings = self.custom_object_type.fields.filter(type__in=MULTI_COLUMN_TYPES) if self.pk: siblings = siblings.exclude(pk=self.pk) for sibling in siblings: @@ -3516,6 +3541,13 @@ def save(self, *args, **kwargs): else: _schema_add_field(self, model, schema_editor, schema_conn) _apply_deferred_co_field(self) + if self.type == CustomObjectFieldTypeChoices.TYPE_URL: + # A url field's title column has no deferred-CO-field + # replay support (matches the existing coordinates gap). + title_name = field_type.title_field_name(self) + title_field = field_type.get_model_field(self)[title_name] + title_field.contribute_to_class(model, title_name) + schema_editor.add_field(model, title_field) else: if self.type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: # Only a rename touches the schema; other attribute changes @@ -3536,6 +3568,19 @@ def save(self, *args, **kwargs): ) else: _schema_alter_field(self.original, self, model, schema_editor, schema_conn) + if ( + self.type == CustomObjectFieldTypeChoices.TYPE_URL + and self.name != self._original_name + ): + # Only a rename touches the title column; other attribute + # changes are persisted by super().save() below. + old_title = field_type.title_field_name(self.original) + new_title = field_type.title_field_name(self) + old_title_field = field_type.get_model_field(self.original)[old_title] + new_title_field = field_type.get_model_field(self)[new_title] + old_title_field.contribute_to_class(model, old_title) + new_title_field.contribute_to_class(model, new_title) + schema_editor.alter_field(model, old_title_field, new_title_field) # Rewrite historical audit-data keys so any future replay can # resolve old or new name to the current field name. @@ -3660,6 +3705,12 @@ def delete(self, *args, **kwargs): except LookupError: pass _schema_remove_field(self, model, schema_editor, schema_conn=schema_conn) + if self.type == CustomObjectFieldTypeChoices.TYPE_URL: + # Drop the title column alongside the primary URL column. + title_name = field_type.title_field_name(self) + title_field = field_type.get_model_field(self)[title_name] + title_field.contribute_to_class(model, title_name) + schema_editor.remove_field(model, title_field) # Deregister the dropped through model (both polymorphic and plain MULTIOBJECT) # so the cascade-delete collector no longer queries the now-missing table. diff --git a/netbox_custom_objects/templates/netbox_custom_objects/customobject.html b/netbox_custom_objects/templates/netbox_custom_objects/customobject.html index a17d1e6f..44a60426 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/customobject.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/customobject.html @@ -153,6 +153,14 @@ {{ ''|placeholder }} {% endif %} {% endwith %} + {% elif field.type == 'url' %} + {% with url_html=object|get_url_field_html:field %} + {% if url_html %} + {{ url_html }} + {% else %} + {{ ''|placeholder }} + {% endif %} + {% endwith %} {% else %} {% customfield_value field object|get_field_value:field %} {% endif %} diff --git a/netbox_custom_objects/templatetags/custom_object_utils.py b/netbox_custom_objects/templatetags/custom_object_utils.py index e0c65911..3fdf04eb 100644 --- a/netbox_custom_objects/templatetags/custom_object_utils.py +++ b/netbox_custom_objects/templatetags/custom_object_utils.py @@ -1,5 +1,8 @@ from django import template +from django.utils.html import format_html +from django.utils.text import Truncator from extras.choices import CustomFieldUIVisibleChoices +from utilities.validators import url_scheme_is_allowed from netbox_custom_objects.choices import CustomObjectFieldTypeChoices from netbox_custom_objects.models import CustomObjectTypeField @@ -12,6 +15,7 @@ "get_field_is_ui_visible", "get_child_relations", "get_coordinate_map_url", + "get_url_field_html", ) register = template.Library() @@ -68,6 +72,27 @@ def get_field_is_ui_visible(obj, field: CustomObjectTypeField) -> bool: return False +@register.filter(name="get_url_field_html") +def get_url_field_html(obj, field: CustomObjectTypeField): + """ + Render a url-type field as a safe link: the title as link text if set, falling + back to the URL itself, truncated to 70 chars -- mirrors NetBox core's + builtins/customfield_value.html convention for 'url' custom fields, including + its url_scheme_is_allowed() guard against unsafe schemes (e.g. javascript:). + Returns '' when the URL itself is unset, regardless of whether a title is set. + """ + if field.type != CustomObjectFieldTypeChoices.TYPE_URL: + return "" + url = getattr(obj, field.name, None) + if not url: + return "" + title = getattr(obj, f"{field.name}_title", None) + display_text = Truncator(title or url).chars(70) + if url_scheme_is_allowed(url): + return format_html('{}', url, display_text) + return display_text + + @register.filter(name="get_child_relations") def get_child_relations(obj, field: CustomObjectTypeField): return getattr(obj, field.name) diff --git a/netbox_custom_objects/tests/test_api.py b/netbox_custom_objects/tests/test_api.py index e6052745..7660cca6 100644 --- a/netbox_custom_objects/tests/test_api.py +++ b/netbox_custom_objects/tests/test_api.py @@ -2067,3 +2067,77 @@ def test_patch_clearing_both_halves_allowed(self): obj.refresh_from_db() self.assertIsNone(obj.location_latitude) self.assertIsNone(obj.location_longitude) + + +class URLFieldLinkTitleAPITest(CustomObjectsTestCase, NetBoxTestCase): + """REST API behaviour for the url field type's link-title support (issue #496).""" + + @classmethod + def setUpTestData(cls): + cls.cot = CustomObjectType.objects.create( + name="LinkObject", + verbose_name_plural="Link Objects", + slug="link-objects", + ) + cls.create_custom_object_type_field( + cls.cot, name="name", type="text", primary=True, required=True + ) + cls.create_custom_object_type_field(cls.cot, name="website", type="url") + cls.model = cls.cot.get_model() + + def setUp(self): + super().setUp() + self.user = create_test_user("linkuser") + self.client = APIClient() + token_key = create_token(self.user) + self.header = {"HTTP_AUTHORIZATION": f"Token {token_key}"} + perm = ObjectPermission( + name="link all", actions=["view", "add", "change", "delete"] + ) + perm.save() + perm.users.add(self.user) + perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + def _list_url(self): + return reverse( + "plugins-api:netbox_custom_objects-api:customobject-list", + kwargs={"custom_object_type": self.cot.slug}, + ) + + def _detail_url(self, instance): + return reverse( + "plugins-api:netbox_custom_objects-api:customobject-detail", + kwargs={"pk": instance.pk, "custom_object_type": self.cot.slug}, + ) + + def test_serializer_exposes_flat_url_title(self): + """The URL and title columns are serialized as two flat fields.""" + obj = self.model.objects.create( + name="Box", website="https://example.com/", website_title="Example Site", + ) + response = self.client.get(self._detail_url(obj), **self.header) + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + self.assertEqual(response.data["website"], "https://example.com/") + self.assertEqual(response.data["website_title"], "Example Site") + + def test_create_with_url_and_title(self): + """Creating an object via the API persists both the URL and title columns.""" + data = { + "name": "Created box", + "website": "https://example.com/", + "website_title": "Example Site", + } + response = self.client.post(self._list_url(), data, format="json", **self.header) + self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.data) + obj = self.model.objects.get(pk=response.data["id"]) + self.assertEqual(obj.website, "https://example.com/") + self.assertEqual(obj.website_title, "Example Site") + + def test_create_with_url_and_no_title_allowed(self): + """Creating with a URL and no title is accepted (no both-required rule).""" + data = {"name": "Created box", "website": "https://example.com/"} + response = self.client.post(self._list_url(), data, format="json", **self.header) + self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.data) + obj = self.model.objects.get(pk=response.data["id"]) + self.assertEqual(obj.website, "https://example.com/") + self.assertIsNone(obj.website_title) diff --git a/netbox_custom_objects/tests/test_field_types.py b/netbox_custom_objects/tests/test_field_types.py index de85d057..21843eff 100644 --- a/netbox_custom_objects/tests/test_field_types.py +++ b/netbox_custom_objects/tests/test_field_types.py @@ -687,7 +687,10 @@ def test_url_field_validation(self): field.validate("http:/example.com") def test_url_field_model_generation(self): - """Test URL field model generation.""" + """ + Test URL field model generation. A url field expands into two backing + columns: the URL itself and an optional link title (issue #496). + """ self.create_custom_object_type_field( self.custom_object_type, name="website", @@ -696,9 +699,99 @@ def test_url_field_model_generation(self): ) model = self.custom_object_type.get_model() - instance = model.objects.create(name="Test", website="https://example.com") + column_names = {f.name for f in model._meta.local_fields} + self.assertIn("website", column_names) + self.assertIn("website_title", column_names) + instance = model.objects.create( + name="Test", website="https://example.com", website_title="Example Site", + ) self.assertEqual(instance.website, "https://example.com") + self.assertEqual(instance.website_title, "Example Site") + + def test_url_field_title_is_optional(self): + """A url value with no title set is valid (no both-required rule).""" + self.create_custom_object_type_field( + self.custom_object_type, + name="website", + label="Website", + type="url", + ) + model = self.custom_object_type.get_model() + instance = model.objects.create(name="Test", website="https://example.com/") + self.assertEqual(instance.website, "https://example.com/") + self.assertIsNone(instance.website_title) + + def test_url_field_unique_still_enforced(self): + """ + Unique still applies to the URL column itself for a url field (unlike + coordinates, which disallows unique entirely) -- guards the fix making the + clean() uniqueness-conversion probe dict-aware. + """ + self.create_custom_object_type_field( + self.custom_object_type, + name="website", + label="Website", + type="url", + unique=True, + ) + model = self.custom_object_type.get_model() + model.objects.create(name="A", website="https://example.com/a") + with self.assertRaises(ValidationError): + duplicate = model(name="B", website="https://example.com/a") + duplicate.full_clean() + + def test_change_existing_field_to_url_rejected(self): + """An existing non-url field cannot be converted to url.""" + field = self.create_custom_object_type_field( + self.custom_object_type, name="website2", label="Website", type="text", + ) + field.type = "url" + with self.assertRaises(ValidationError): + field.full_clean() + + def test_change_url_field_to_other_type_rejected(self): + """An existing url field cannot be converted to another type.""" + field = self.create_custom_object_type_field( + self.custom_object_type, name="website2", label="Website", type="url", + ) + field.type = "text" + with self.assertRaises(ValidationError): + field.full_clean() + + def test_url_backing_column_collision_rejected(self): + """ + Adding a url field whose title column collides with an existing field's + column raises a ValidationError rather than a DB error. + """ + self.create_custom_object_type_field( + self.custom_object_type, name="website2_title", label="Title", type="text", + ) + field = CustomObjectTypeField( + custom_object_type=self.custom_object_type, + name="website2", + label="Website", + type="url", + ) + with self.assertRaises(ValidationError): + field.full_clean() + + def test_field_colliding_with_url_backing_column_rejected(self): + """ + The reverse collision: a plain field named "_title" cannot be added + when a url field "" already exists. + """ + self.create_custom_object_type_field( + self.custom_object_type, name="website2", label="Website", type="url", + ) + field = CustomObjectTypeField( + custom_object_type=self.custom_object_type, + name="website2_title", + label="Title", + type="text", + ) + with self.assertRaises(ValidationError): + field.full_clean() class JSONFieldTypeTestCase(FieldTypeTestCase): diff --git a/netbox_custom_objects/tests/test_schema_operations.py b/netbox_custom_objects/tests/test_schema_operations.py index 582cc9c6..740c63d1 100644 --- a/netbox_custom_objects/tests/test_schema_operations.py +++ b/netbox_custom_objects/tests/test_schema_operations.py @@ -275,3 +275,49 @@ def test_coordinates_field_delete_drops_both_columns(self): columns = self._db_columns(cot.get_model()) self.assertNotIn('location_latitude', columns) self.assertNotIn('location_longitude', columns) + + def test_url_field_rename_renames_both_columns(self): + """Renaming a url field renames both backing DB columns (issue #496).""" + cot = self.create_custom_object_type(name='urlrename', slug='url-rename') + self.create_custom_object_type_field( + cot, name='name', label='Name', type='text', primary=True, + ) + field = self.create_custom_object_type_field( + cot, name='website', label='Website', type='url', + ) + + columns = self._db_columns(cot.get_model()) + self.assertIn('website', columns) + self.assertIn('website_title', columns) + + # Reload from DB so the rename path has the original snapshot (set in + # from_db) — this mirrors how the edit view loads the field before saving. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.name = 'homepage' + field.save() + + columns = self._db_columns(cot.get_model()) + self.assertNotIn('website', columns) + self.assertNotIn('website_title', columns) + self.assertIn('homepage', columns) + self.assertIn('homepage_title', columns) + + def test_url_field_delete_drops_both_columns(self): + """Deleting a url field drops both backing DB columns (issue #496).""" + cot = self.create_custom_object_type(name='urldelete', slug='url-delete') + self.create_custom_object_type_field( + cot, name='name', label='Name', type='text', primary=True, + ) + field = self.create_custom_object_type_field( + cot, name='website', label='Website', type='url', + ) + + columns = self._db_columns(cot.get_model()) + self.assertIn('website', columns) + self.assertIn('website_title', columns) + + field.delete() + + columns = self._db_columns(cot.get_model()) + self.assertNotIn('website', columns) + self.assertNotIn('website_title', columns) diff --git a/netbox_custom_objects/tests/test_views.py b/netbox_custom_objects/tests/test_views.py index a7944dfe..1148d792 100644 --- a/netbox_custom_objects/tests/test_views.py +++ b/netbox_custom_objects/tests/test_views.py @@ -1072,6 +1072,67 @@ def test_bulk_edit_half_populated_pair_rejected(self): self.assertEqual(obj.location_longitude, Decimal("-74.006000")) +class URLFieldLinkTitleViewTest(CustomObjectsTestCase, TestCase): + """UI form behaviour for the url field type's link-title support (issue #496).""" + + @classmethod + def setUpTestData(cls): + cls.cot = CustomObjectType.objects.create( + name="LinkView", + verbose_name_plural="Link Views", + slug="link-views", + ) + CustomObjectTypeField.objects.create( + custom_object_type=cls.cot, name="name", type="text", primary=True, required=True + ) + CustomObjectTypeField.objects.create( + custom_object_type=cls.cot, name="website", type="url" + ) + cls.model = cls.cot.get_model() + + def setUp(self): + super().setUp() + perm = ObjectPermission( + name="link view all", actions=["view", "add", "change", "delete"] + ) + perm.save() + perm.users.add(self.user) + perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + def _add_url(self): + return reverse( + "plugins:netbox_custom_objects:customobject_add", + kwargs={"custom_object_type": self.cot.slug}, + ) + + def test_add_form_renders_url_and_title_inputs(self): + response = self.client.get(self._add_url()) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "website") + self.assertContains(response, "website_title") + + def test_create_valid_url_with_title(self): + data = { + "name": "Box", + "website": "https://example.com/", + "website_title": "Example Site", + } + response = self.client.post(self._add_url(), data) + self.assertEqual(response.status_code, 302, getattr(response, "content", b"")) + obj = self.model.objects.get(name="Box") + self.assertEqual(obj.website, "https://example.com/") + self.assertEqual(obj.website_title, "Example Site") + + def test_create_valid_url_with_no_title(self): + """A URL with no title is accepted (no both-required rule, unlike coordinates).""" + data = {"name": "Box", "website": "https://example.com/"} + response = self.client.post(self._add_url(), data) + self.assertEqual(response.status_code, 302, getattr(response, "content", b"")) + obj = self.model.objects.get(name="Box") + self.assertEqual(obj.website, "https://example.com/") + self.assertFalse(obj.website_title) + + class QuickAddViewTestCase(CustomObjectsTestCase, TestCase): """ Tests for the quick-add flow in CustomObjectEditView. diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index e8ea8773..2c298296 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -787,6 +787,20 @@ def get_form(self, model): attrs["custom_object_type_coordinates_fields"][field.name] = tuple(sub_names) continue + # URL: one logical field rendered as two grouped url/title inputs. Unlike + # coordinates, there's no "both required" pairing rule, so no tracking dict + # is needed here beyond the generic field-group/rendered-names bookkeeping. + if field.type == CustomObjectFieldTypeChoices.TYPE_URL: + sub_fields = field_type.get_form_fields(field) + sub_names = list(sub_fields.keys()) + for sub_name, sub_field in sub_fields.items(): + attrs[sub_name] = sub_field + attrs["custom_object_type_rendered_names"].add(sub_name) + if group_name not in attrs["custom_object_type_field_groups"]: + attrs["custom_object_type_field_groups"][group_name] = [] + attrs["custom_object_type_field_groups"][group_name].extend(sub_names) + continue + # Polymorphic single-object: type-selector + object-picker pair if field.is_polymorphic and field.type == CustomFieldTypeChoices.TYPE_OBJECT: ct_sub = f"{field.name}__ct" @@ -1212,6 +1226,17 @@ def get_form(self, queryset): attrs["custom_object_type_coordinates_fields"][field.name] = tuple(sub_names) continue + # URL: two optional url/title inputs in bulk edit. No cross-field + # validation rule exists for URL (unlike coordinates), so no tracking + # dict is needed beyond adding the sub-fields themselves. + if field.type == CustomObjectFieldTypeChoices.TYPE_URL: + for sub_name, sub_field in field_type.get_form_fields(field).items(): + sub_field.required = False + sub_field.widget.is_required = False + sub_field.initial = None + attrs[sub_name] = sub_field + continue + # Polymorphic single-object: scope-style type-selector + object-picker pair if field.is_polymorphic and field.type == CustomFieldTypeChoices.TYPE_OBJECT: ct_sub = f"{field.name}__ct" From 637fb70cb36aef98c874a243a3b0e3038ddf1166 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Sun, 2 Aug 2026 19:31:54 -0400 Subject: [PATCH 2/3] Fix CI failure: reimplement scheme check instead of importing it url_scheme_is_allowed() only exists on NetBox's feature branch (added 2026-07-23), not in main or any released version, so importing it broke CI's NetBox-main test matrix leg with an ImportError at plugin load time. This plugin supports NetBox 4.5.2+, well before that function existed. Reimplement the same scheme-allowlist check locally in custom_object_utils.py instead. ALLOWED_URL_SCHEMES itself (the config parameter the function reads) has existed since 2020 and is safe to rely on across the whole supported version range. --- .../templatetags/custom_object_utils.py | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/netbox_custom_objects/templatetags/custom_object_utils.py b/netbox_custom_objects/templatetags/custom_object_utils.py index 3fdf04eb..9de80e40 100644 --- a/netbox_custom_objects/templatetags/custom_object_utils.py +++ b/netbox_custom_objects/templatetags/custom_object_utils.py @@ -1,8 +1,10 @@ +from urllib.parse import urlparse + from django import template from django.utils.html import format_html from django.utils.text import Truncator from extras.choices import CustomFieldUIVisibleChoices -from utilities.validators import url_scheme_is_allowed +from netbox.config import get_config from netbox_custom_objects.choices import CustomObjectFieldTypeChoices from netbox_custom_objects.models import CustomObjectTypeField @@ -72,13 +74,29 @@ def get_field_is_ui_visible(obj, field: CustomObjectTypeField) -> bool: return False +def _url_scheme_is_allowed(value): + """ + Return True if value's URL scheme is permitted by ALLOWED_URL_SCHEMES (a + schemeless/unparseable value is treated as relative and allowed). Reimplemented + locally rather than imported from NetBox core's utilities.validators, since + url_scheme_is_allowed() only exists on NetBox's feature branch (added 2026-07-23) + and this plugin supports NetBox versions well before that -- ALLOWED_URL_SCHEMES + itself has existed since 2020 and is safe to rely on across the supported range. + """ + try: + scheme = urlparse(value).scheme.lower() + except ValueError: + scheme = "" + return not scheme or scheme in get_config().ALLOWED_URL_SCHEMES + + @register.filter(name="get_url_field_html") def get_url_field_html(obj, field: CustomObjectTypeField): """ Render a url-type field as a safe link: the title as link text if set, falling back to the URL itself, truncated to 70 chars -- mirrors NetBox core's builtins/customfield_value.html convention for 'url' custom fields, including - its url_scheme_is_allowed() guard against unsafe schemes (e.g. javascript:). + its scheme-allowlist guard against unsafe schemes (e.g. javascript:). Returns '' when the URL itself is unset, regardless of whether a title is set. """ if field.type != CustomObjectFieldTypeChoices.TYPE_URL: @@ -88,7 +106,7 @@ def get_url_field_html(obj, field: CustomObjectTypeField): return "" title = getattr(obj, f"{field.name}_title", None) display_text = Truncator(title or url).chars(70) - if url_scheme_is_allowed(url): + if _url_scheme_is_allowed(url): return format_html('{}', url, display_text) return display_text From 7711318a50279d073b325a8ddb306b1b96cdd536 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Mon, 3 Aug 2026 15:58:47 -0400 Subject: [PATCH 3/3] Address findings from automated review of #641 - Add tests for get_url_field_html and its security-relevant scheme guard (_url_scheme_is_allowed): allowed scheme with title, allowed scheme falling back to the URL text, a disallowed scheme (e.g. javascript:) rendering as plain text instead of a link, and an unset URL returning ''. - Document that CSV bulk import only populates the URL value, not the title (matches the existing, pre-established limitation for coordinates fields' backing columns). - Make the title column's schema_editor.add_field() call idempotent, checking existing_cols first, matching the established pattern in _schema_add_field() (the URL column already had this guard; the title column's separate add_field() call did not). - Change the title column from CharField(null=True, blank=True) to CharField(blank=True, default=""), so "no title" has one canonical representation instead of two (NULL vs ''). default="" keeps the column eligible for mixin_migration.py's auto-heal pass on existing installations, which requires a column to be nullable or have a Django-level default before auto-adding it. --- docs/field-attributes.md | 3 + netbox_custom_objects/field_types.py | 9 ++- netbox_custom_objects/models.py | 14 ++++- netbox_custom_objects/tests/test_api.py | 2 +- .../tests/test_field_types.py | 62 ++++++++++++++++++- netbox_custom_objects/tests/test_views.py | 2 +- 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/field-attributes.md b/docs/field-attributes.md index 6e8476d2..87f465f5 100644 --- a/docs/field-attributes.md +++ b/docs/field-attributes.md @@ -128,3 +128,6 @@ Behaviour: - **REST API.** The pair is exposed as two flat fields, `` and `_title`. - `Must be unique` and `Default` continue to apply to the URL value itself, exactly as for any other `url` field. +- **CSV import.** Bulk CSV import only populates the URL value; the title column is + not importable via CSV (the same limitation applies to `coordinates` fields' backing + columns). diff --git a/netbox_custom_objects/field_types.py b/netbox_custom_objects/field_types.py index cbfefbbd..359a2f1b 100644 --- a/netbox_custom_objects/field_types.py +++ b/netbox_custom_objects/field_types.py @@ -543,10 +543,17 @@ def get_model_field(self, field, **kwargs): field_kwargs.update({"default": field.default, "unique": field.unique}) return { field.name: models.URLField(null=True, blank=True, **field_kwargs), + # blank=True, default='' (not null=True): a CharField with null=True lets + # "no title" be represented as both NULL (ORM/API create omitting the key) + # and '' (a form submission clearing the field), which would make + # isnull-based filtering unreliable. default='' also keeps this column + # eligible for mixin_migration.py's post_migrate auto-heal pass on + # existing installations, which only auto-ADDs a new column when it is + # nullable or has a Django-level default (see _can_auto_add()). self.title_field_name(field): models.CharField( max_length=200, - null=True, blank=True, + default="", help_text=_("Human-readable text shown instead of the raw URL."), ), } diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 66763d7a..a75fb28e 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -3547,7 +3547,19 @@ def save(self, *args, **kwargs): title_name = field_type.title_field_name(self) title_field = field_type.get_model_field(self)[title_name] title_field.contribute_to_class(model, title_name) - schema_editor.add_field(model, title_field) + with schema_conn.cursor() as cursor: + existing_cols = { + col.name for col in schema_conn.introspection.get_table_description( + cursor, model._meta.db_table + ) + } + if title_field.column in existing_cols: + logger.debug( + '_schema_add_field: %r already exists on %s, skipping', + title_field.column, model._meta.db_table, + ) + else: + schema_editor.add_field(model, title_field) else: if self.type == CustomObjectFieldTypeChoices.TYPE_COORDINATES: # Only a rename touches the schema; other attribute changes diff --git a/netbox_custom_objects/tests/test_api.py b/netbox_custom_objects/tests/test_api.py index 7660cca6..ebb7c927 100644 --- a/netbox_custom_objects/tests/test_api.py +++ b/netbox_custom_objects/tests/test_api.py @@ -2140,4 +2140,4 @@ def test_create_with_url_and_no_title_allowed(self): self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.data) obj = self.model.objects.get(pk=response.data["id"]) self.assertEqual(obj.website, "https://example.com/") - self.assertIsNone(obj.website_title) + self.assertEqual(obj.website_title, "") diff --git a/netbox_custom_objects/tests/test_field_types.py b/netbox_custom_objects/tests/test_field_types.py index 21843eff..34944bcb 100644 --- a/netbox_custom_objects/tests/test_field_types.py +++ b/netbox_custom_objects/tests/test_field_types.py @@ -710,7 +710,10 @@ def test_url_field_model_generation(self): self.assertEqual(instance.website_title, "Example Site") def test_url_field_title_is_optional(self): - """A url value with no title set is valid (no both-required rule).""" + """ + A url value with no title set is valid (no both-required rule). The title + column defaults to '' (not None) so "unset" has one canonical representation. + """ self.create_custom_object_type_field( self.custom_object_type, name="website", @@ -720,7 +723,7 @@ def test_url_field_title_is_optional(self): model = self.custom_object_type.get_model() instance = model.objects.create(name="Test", website="https://example.com/") self.assertEqual(instance.website, "https://example.com/") - self.assertIsNone(instance.website_title) + self.assertEqual(instance.website_title, "") def test_url_field_unique_still_enforced(self): """ @@ -793,6 +796,61 @@ def test_field_colliding_with_url_backing_column_rejected(self): with self.assertRaises(ValidationError): field.full_clean() + def test_get_url_field_html_renders_link_with_title(self): + """get_url_field_html renders using the title as link text when set.""" + from netbox_custom_objects.templatetags.custom_object_utils import get_url_field_html + cotf = self.create_custom_object_type_field( + self.custom_object_type, name="website3", label="Website", type="url", + ) + model = self.custom_object_type.get_model(no_cache=True) + instance = model.objects.create( + name="Test", website3="https://example.com/", website3_title="Example Site", + ) + html = get_url_field_html(instance, cotf) + self.assertIn('href="https://example.com/"', html) + self.assertIn("Example Site", html) + + def test_get_url_field_html_falls_back_to_url_text_without_title(self): + """get_url_field_html uses the URL itself as link text when no title is set.""" + from netbox_custom_objects.templatetags.custom_object_utils import get_url_field_html + cotf = self.create_custom_object_type_field( + self.custom_object_type, name="website4", label="Website", type="url", + ) + model = self.custom_object_type.get_model(no_cache=True) + instance = model.objects.create(name="Test", website4="https://example.com/") + html = get_url_field_html(instance, cotf) + self.assertIn('href="https://example.com/"', html) + self.assertIn(">https://example.com/<", html) + + def test_get_url_field_html_rejects_disallowed_scheme(self): + """ + A disallowed URL scheme (e.g. javascript:) renders as plain text, not a link. + Guards the security-relevant _url_scheme_is_allowed() check -- the value is + assigned directly (bypassing form/URLField validation) to simulate data that + reached the column via any path other than the add/edit form. + """ + from netbox_custom_objects.templatetags.custom_object_utils import get_url_field_html + cotf = self.create_custom_object_type_field( + self.custom_object_type, name="website5", label="Website", type="url", + ) + model = self.custom_object_type.get_model(no_cache=True) + instance = model.objects.create( + name="Test", website5="javascript:alert(1)", website5_title="Click me", + ) + html = get_url_field_html(instance, cotf) + self.assertNotIn("