Closes #496: Add optional link title to URL fields

[bctiemann]

Closes: #496

Summary

• 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 itself when no title is set). List/table views are unaffected -- still plain-text URL.
• 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 exactly as before.
• This required making three previously coordinates-only DDL/validation code paths in models.py dict-aware (the generic single-column schema helpers used by every field type, the unique-conversion probe in clean(), and the backing-column-collision guard), since URL is the first multi-column type that also needs to 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. Documented this explicitly in mixin_migration.py.

Upgrading

Existing url fields gain the new <name>_title column automatically — no manual migration is needed. On the main schema this happens the next time manage.py migrate runs (or immediately via manage.py upgrade_custom_objects, which also supports --dry-run). On a NetBox Branching branch it happens the next time that branch itself is migrated (its "Migrate branch" action, available whenever the branch's migration state lags behind main).

This will need to be captured in a release note for the next minor release in which this feature ships.

Test plan

• New/extended tests across test_field_types.py (URLFieldTypeTestCase: model generation, title-optional, unique still enforced, type-conversion rejection both directions, backing-column collision both directions), test_schema_operations.py (rename and delete both drop/rename the title column), test_api.py (serializer exposes both columns flat, create round-trip with and without a title), test_views.py (add form renders both inputs, create with and without a title).
• ruff check clean across the whole package.
• Ran the full plugin test suite (1109 tests) against a NetBox 4.6.6 checkout. All new/changed tests pass. The only pre-existing failures (20, unrelated to this change) trace to an environment gap in that test setup -- netbox_branching is importable but not enabled in PLUGINS, which breaks any test touching CustomObjectTypeField/CustomObjectType rename or delete, confirmed by the identical failure occurring on the unmodified, pre-existing coordinates rename test -- plus one unrelated csv_update_data scaffolding gap in generic view tests.

[bctiemann]

@claude /review and limit response only to areas of potential concern.

[github-actions]

Claude finished @bctiemann's task in 6m 45s —— View job

---

Review: Areas of Potential Concern

Limiting to concerns only, as requested.

---

1. No tests for get_url_field_html / _url_scheme_is_allowed — including the security-relevant scheme guard

_url_scheme_is_allowed is the only line of defense preventing a javascript: URL from rendering as a clickable link. There are no tests for this function or the get_url_field_html filter directly. The existing view tests only check HTTP 302 redirects after form submission; they don't render the detail page and verify the output HTML.

In particular, the following cases are untested:

• A URL with a disallowed scheme (e.g., javascript:alert(1)) renders as plain text, not a link
• A URL with a title renders <a href="...">Title</a>
• A URL with no title falls back to the URL text
• An object with no URL at all renders '' (the placeholder path)

This is a gap worth filling given that _url_scheme_is_allowed is a reimplemented local copy of a NetBox core function, and any divergence from NetBox's version (e.g., new scheme normalisation logic upstream) would be silent.

Fix this →%20URL%20with%20allowed%20scheme%20renders%20as%20%3Ca%20href%3D%22...%22%3E%3C%2Fa%3E%3B%202)%20javascript%3A%20URL%20renders%20as%20plain%20text%2C%20no%20link%3B%203)%20title%20is%20used%20as%20link%20text%20when%20set%3B%204)%20URL%20text%20used%20as%20fallback%20when%20title%20is%20absent.%20Also%20add%20unit%20tests%20for%20_url_scheme_is_allowed%20in%20test_field_types.py.&repo=netboxlabs/netbox-custom-objects)

---

2. CSV bulk import silently drops the _title column

CustomObjectBulkImportView (views.py:1452) builds its form by iterating fields and calling get_annotated_form_field(field, for_csv_import=True). URLFieldType doesn't override get_annotated_form_field, so it calls the single-field get_form_field, which returns only a LaxURLField for the URL column. The _title column is never added to the import form.

A user exporting a COT with titled URLs and re-importing the CSV will silently lose all link titles. There's no error and no documentation warning. This could be acceptable as an MVP limitation, but it should at minimum be documented in docs/field-attributes.md.

---

3. Title column add_field call is not idempotent (unlike the URL column)

In CustomObjectTypeField.save() (models.py:3544), when adding a new URL field:

_schema_add_field(self, model, schema_editor, schema_conn)   # idempotent — checks column exists first
_apply_deferred_co_field(self)
if self.type == CustomObjectFieldTypeChoices.TYPE_URL:
    ...
    schema_editor.add_field(model, title_field)               # NOT idempotent

_schema_add_field explicitly checks whether the column already exists before issuing ALTER TABLE. The bare schema_editor.add_field(model, title_field) call does not — it would raise DuplicateColumn if retried in the rare case where the URL column was added but the transaction for the title column was not yet committed when the process was interrupted. In practice both operations commit together, so the risk is low. However, it's worth applying the same idempotency guard (checking existing_cols before calling add_field) to match the established pattern in _schema_add_field.

---

4. CharField(null=True, blank=True) creates dual empty states

The title model field (field_types.py):

models.CharField(max_length=200, null=True, blank=True, ...)

Django's CharField with null=True means "no title" can be represented as either NULL (ORM create without passing title) or "" (form submission with empty field). The display logic handles both via title or url, but comparisons like website_title="" vs website_title__isnull=True will behave differently, and assertIsNone(obj.website_title) in the API test would silently pass for NULL but fail for "".

The conventional Django solution is to store NULL consistently for "not set" by overriding the model's clean() to coerce "" → None. Or drop null=True and treat empty string as "no title" consistently. Neither is urgent, but the dual-state can cause subtle filtering bugs.

---

5. Minor: MULTI_COLUMN_TYPES extends type-conversion guard but URL → URL rename is not re-tested

The new guard in clean():

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)
):

The condition requires self.type != self._original_type, so a URL field being renamed (same type, different name) correctly skips this branch. But the test test_change_existing_field_to_url_rejected starts from a text field, not from another multi-column type. A conversion between two different multi-column types (e.g., hypothetically coordinates → url) would also hit this guard, but that path isn't tested. Not a current bug (only two multi-column types exist and they share the same guard), but worth a comment noting the intent for future field types.

---

[pheus]

Thanks for working on this. The normal URL and title flow looks good.

I found three cases that still need another pass: upgrading existing branches, deferred replay during squash operations, and rename/history handling for the second backing column. I also left two smaller comments about form help text and the field-deletion warning.

I’m requesting changes for now.

[bctiemann]

Thanks for the thorough review, @pheus! Pushed 14d6bbb addressing all five points:

1. Branch-upgrade heal gap — added heal_branch() in mixin_migration.py, wired to netbox-branching's own post_migrate signal (the one Branch.migrate() actually fires, since it never triggers Django's core post_migrate). This runs the existing heal_all_cots() pass against the branch's own connection inside activate_branch(), so a branch provisioned before this release now gets the _title column added to its own schema. Added a regression test (BranchUpgradeHealTestCase) that simulates a pre-upgrade branch by dropping the column from only the branch's schema and asserts heal_branch() restores it there without touching main's.

2. Deferred replay dropping the title value — _apply_deferred_co_field() now matches and applies the <name>_title key independently of the base URL key, since the two can be set independently. Added URL fields (with both URL and title values) to the shared test_comprehensive_merge_and_revert test, which runs under both the iterative and squash strategies, plus to MissingFieldTypesTestCase.

3. Naive title-column rename + missing audit-key rewrite — extracted the conflict-aware rename logic already used by _schema_alter_field() into a shared _alter_column_with_rename_conflict_resolution() helper, and reused it for the title column, so an independent-rename conflict (branch renames A→B, main independently renames A→C) resolves the title column the same way it already resolves the primary column. Also added a second _rename_objectchange_field_key() call for the title key. Added two regression tests: a straightforward rename that replays/reverts both values, and a rename-conflict test mirroring the existing test_sequential_renames_both_sides_merge pattern but asserting the title column converges correctly too.

4. Missing help_text — URLFieldType.get_form_fields() now renders the field's configured description as help_text on the URL input, and the title input has its own help_text.

5. Deletion-impact preview omitting title-only objects — both the count and the dependent-objects list in CustomObjectTypeFieldDeleteView now match on URL-set OR title-set, since either can hold a value independently.

[pheus]

Thanks for the follow-up.

I found three remaining upgrade and replay issues: existing branches still need a reliable trigger for the schema heal, deferred title values are applied before the backing column exists, and pre-existing <url>_title field collisions need to be handled safely.

I’ve left the details inline. I’m requesting changes for now, but this looks close.

[pheus]

Thanks for the follow-up. The automatic branch trigger and deferred replay ordering look good now, and surfacing existing column collisions is a useful improvement.

I found two remaining upgrade-safety issues: the branch sweep can operate against a missing or outdated schema, and the collision warning currently suggests a recovery path that can move another field's data. I also left one small documentation correction.

I’m requesting changes for those upgrade cases, but the main URL/title implementation looks close.

[pheus]

Could we validate and isolate each branch before healing it?

FAILED can mean provisioning failed, in which case the schema has already been dropped. Since the branch connection uses <branch>,<main> as its search path, those queries can then fall through to main. This also includes branches with pending migrations, where the current ORM may not match the branch schema yet.

Please verify the schema exists, skip branches with a pending migration plan, and handle failures per branch. The branch post_migrate hook can heal skipped branches after migration.

[pheus]

Could we make the recovery guidance more specific? Renaming either field is not safe.

For an existing website / website_title pair, renaming website also renames the shared website_title column. That moves the sibling field's data into the URL title and leaves the sibling without its own column.

The safe path is to rename the field whose own name matches the colliding column, then rerun the heal. A data-preservation test for that recovery flow would be helpful.

[pheus]

Could we update this sentence to match the new upgrade path? Existing branch schemas are now healed by the main migrate or upgrade_custom_objects run; the branch migration hook is only the secondary path.

[pheus]

Could we keep only the optional netbox_branching imports local?

I don't see an import-cycle or app-loading reason for delaying detect_backing_column_collisions, and django.apps.apps is also safe to import at module scope. Moving those to the regular imports would make it clearer that the remaining lazy imports are specifically required for the optional Branching integration.