[Fixes #13997] Fixes Custom license saved without identifier by sijandh35 · Pull Request #14094 · GeoNode/geonode

sijandh35 · 2026-04-03T10:24:42Z

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

For all pull requests:

Confirm you have read the contribution guidelines
You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
The issue connected to the PR must have Labels and Milestone assigned
PR for bug fixes and small new features are presented as a single commit
PR title must be in the form "[Fixes #<issue_number>] Title of the PR"
New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented

Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.

gemini-code-assist

Code Review

This pull request introduces an automated identifier generation for the License model by overriding the save method to assign a UUID if one is missing. The review feedback suggests using the string representation of the UUID instead of the hex format to ensure consistency with other models in the project.

…stead of the identifier for license

etj

Since we're setting the identifier as unique, in the migration we need to make sure there are no dups (mostly there can be duplicated empty strings, but let's also check and fix for not-empty dups)

etj · 2026-04-21T09:47:39Z

+    for lic in License.objects.order_by("id"):
+        identifier = lic.identifier
+        if not identifier or identifier in seen:
+            lic.identifier = str(uuid.uuid4())


we expect this identifier to be a mnemonic token.
I would try and copy the abbreviation field, maybe slugifing it, and if it's there's a duplicate adding a short random suffix to make it unique.

Copilot

Pull request overview

Updates GeoNode’s metadata license handling to avoid relying on License.identifier (which may be empty/duplicated for custom licenses) by switching the metadata editor/API to use the License database PK and enforcing identifier uniqueness at the model layer.

Changes:

Switch metadata license serialization/autocomplete to use License.id (PK) instead of License.identifier.
Update metadata JSON schema and tests to treat license id as an integer.
Enforce License.identifier uniqueness and add a data migration to populate missing/duplicate identifiers.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
geonode/metadata/tests/tests.py	Updates license autocomplete tests to assert PK-based ids.
geonode/metadata/tests/test_handlers.py	Updates handler tests to expect PK-based license serialization/deserialization.
geonode/metadata/schemas/base.json	Changes license `id` type from string to integer in the metadata schema.
geonode/metadata/handlers/base.py	Serializes/deserializes license using `License.id` rather than `identifier`.
geonode/metadata/api/views.py	Returns PK-based ids from the licenses autocomplete endpoint.
geonode/base/models.py	Makes `License.identifier` unique (and implicitly editable again).
geonode/base/migrations/0098_alter_license_identifier.py	Populates/fixes identifiers and adds a DB-level unique constraint.
geonode/api/tests.py	Updates metadata payload in API tests to send PK-based license ids.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T15:41:52Z

+        return License.objects.get(id=field_value["id"]) if field_value else None




LicenseSubHandler.deserialize now assumes field_value["id"] is a DB PK. Existing callers (including older metadata payloads/tests) previously sent License.identifier strings; with this change, non-numeric identifiers will raise (e.g., invalid int) and metadata saves will fail. Consider supporting both formats during a transition (e.g., if id is an int / numeric string, lookup by pk; otherwise fall back to identifier), and optionally accept either via the JSON schema.

Suggested change

return License.objects.get(id=field_value["id"]) if field_value else None

if not field_value:

return None

license_id = field_value["id"]

if isinstance(license_id, int) or (isinstance(license_id, str) and license_id.isdigit()):

return License.objects.get(id=license_id)

return License.objects.get(identifier=license_id)

Copilot · 2026-04-23T15:41:52Z

        ret.append(
            {
-                "id": record.identifier,
+                "id": record.id,


The license autocomplete response changed id from record.identifier to record.id (int). If any clients still rely on identifier here, they'll break. Consider including both fields (e.g., keep id as PK for the editor, but also return identifier) or providing a backward-compatible fallback until consumers are updated.

Suggested change

"id": record.id,

"id": record.id,

"identifier": record.identifier,

Copilot · 2026-04-23T15:41:53Z

     "properties": {
       "id": {
-         "type": "string"
+         "type": "integer"


Changing license.properties.id from string to integer is a breaking schema change for /api/v2/metadata/instance/* clients that still send License.identifier. If you intend to keep backward compatibility, consider using oneOf (integer pk OR string identifier) during a deprecation window to avoid hard failures on older payloads.

Suggested change

"type": "integer"

"oneOf": [

{

"type": "integer"

},

{

"type": "string"

}

]

Copilot · 2026-04-23T15:41:53Z

+logger = logging.getLogger(__name__)
+
+MAX_LEN = 255
+SUFFIX_LEN = 7  


SUFFIX_LEN = 7 has trailing whitespace, which will fail common linting checks (and adds noise in diffs). Remove the trailing spaces.

Suggested change

SUFFIX_LEN = 7

SUFFIX_LEN = 7

Copilot · 2026-04-23T15:41:53Z

+    base = base[: MAX_LEN - SUFFIX_LEN]
+    candidate = base
+    while candidate in seen:
+        candidate = f"{base}-{uuid.uuid4().hex[:6]}"
+    return candidate[:MAX_LEN]


_unique_identifier truncates base to MAX_LEN - SUFFIX_LEN unconditionally, which means all generated identifiers are capped at 248 chars even when no suffix is needed (potentially unnecessary data loss). Consider only reserving SUFFIX_LEN when a suffix is actually appended (i.e., keep up to 255 when candidate is unique, otherwise truncate to leave room for -xxxxxx).

Suggested change

base = base[: MAX_LEN - SUFFIX_LEN]

candidate = base

while candidate in seen:

candidate = f"{base}-{uuid.uuid4().hex[:6]}"

return candidate[:MAX_LEN]

candidate = base[:MAX_LEN]

if candidate not in seen:

return candidate

suffixed_base = base[: MAX_LEN - SUFFIX_LEN]

candidate = f"{suffixed_base}-{uuid.uuid4().hex[:6]}"

while candidate in seen:

candidate = f"{suffixed_base}-{uuid.uuid4().hex[:6]}"

return candidate

etj · 2026-04-23T16:19:59Z

@copilot review

Copilot · 2026-04-23T16:20:10Z

@etj Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 5d44da47-0b07-41b9-b2e9-cd08a738e4e5

Sorry for the inconvenience!

…_unique_identifier Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/027e8dd4-c8f9-43f2-bde1-6ec4f305bc2f Co-authored-by: etj <717359+etj@users.noreply.github.com>

cla-bot · 2026-04-23T16:22:40Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

Copilot · 2026-04-23T16:22:49Z

Since we're setting the identifier as unique, in the migration we need to make sure there are no dups (mostly there can be duplicated empty strings, but let's also check and fix for not-empty dups)

The migration populate_license_identifiers already handles both cases:

Duplicated empty strings: empty/blank identifiers fall through and get a unique value generated from the abbreviation field (slugified), falling back to "license".
Non-empty duplicates: if a non-empty identifier already appeared in seen, it gets replaced with a unique variant and a warning is logged.

I also cleaned up the migration in commit b29bff9:

Removed trailing whitespace on SUFFIX_LEN = 7
Fixed _unique_identifier to only reserve space for the suffix when one is actually needed (so identifiers that are already unique can be up to 255 chars instead of being capped at 248)
Added the missing blank line before class Migration

[Fixes #13997] Fixes Custom license saved without identifier

c042304

cla-bot Bot added the cla-signed CLA Bot: community license agreement signed label Apr 3, 2026

gemini-code-assist Bot reviewed Apr 3, 2026

View reviewed changes

Comment thread geonode/base/models.py Outdated

sijandh35 assigned sijandh35 and unassigned sijandh35 Apr 3, 2026

[Fixes #13997] correct review suggestion

d47de00

sijandh35 self-assigned this Apr 3, 2026

sijandh35 requested a review from mattiagiupponi April 3, 2026 11:17

sijandh35 assigned mattiagiupponi and sijandh35 and unassigned sijandh35 and mattiagiupponi Apr 3, 2026

sijandh35 marked this pull request as draft April 3, 2026 15:17

[Fixes #13997] make identifier editable and populate existing

9892943

sijandh35 requested a review from etj April 8, 2026 07:30

sijandh35 marked this pull request as ready for review April 8, 2026 07:32

[Fixes #13997] change the metadata editor procedures to use the id in…

c2515f8

…stead of the identifier for license

sijandh35 assigned etj and unassigned sijandh35 Apr 8, 2026

[Fixes #13997] fix test case failure due to change

b4df12f

etj requested changes Apr 8, 2026

View reviewed changes

[Fixes #13997] fixes for duplicates identifier also

f3c253e

sijandh35 requested a review from etj April 9, 2026 11:01

This was linked to issues Apr 17, 2026

Issue in License model #14093

Open

Custom license saved without identifier -> Not usable in metadata form #13997

Open

mattiagiupponi approved these changes Apr 21, 2026

View reviewed changes

etj reviewed Apr 21, 2026

View reviewed changes

etj requested changes Apr 21, 2026

View reviewed changes

[Fixes #13997] make identifier mnemonic token

63fe57f

sijandh35 requested a review from etj April 23, 2026 10:28

[Fixes #13997] make duplicate unique and limit size

fe86f89

etj approved these changes Apr 23, 2026

View reviewed changes

etj requested a review from Copilot April 23, 2026 15:36

Copilot started reviewing on behalf of etj April 23, 2026 15:36 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Copilot started work on behalf of etj April 23, 2026 16:22 View session

[Fixes #13997] fix migration: remove trailing whitespace and improve …

b29bff9

…_unique_identifier Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/027e8dd4-c8f9-43f2-bde1-6ec4f305bc2f Co-authored-by: etj <717359+etj@users.noreply.github.com>

cla-bot Bot removed the cla-signed CLA Bot: community license agreement signed label Apr 23, 2026

Copilot AI requested a review from etj April 23, 2026 16:23

Copilot finished work on behalf of etj April 23, 2026 16:23

		return License.objects.get(id=field_value["id"]) if field_value else None

-        return License.objects.get(id=field_value["id"]) if field_value else None
+        if not field_value:
+            return None
+        license_id = field_value["id"]
+        if isinstance(license_id, int) or (isinstance(license_id, str) and license_id.isdigit()):
+            return License.objects.get(id=license_id)
+        return License.objects.get(identifier=license_id)

	"id": record.id,
	"id": record.id,
	"identifier": record.identifier,

Conversation

sijandh35 commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

etj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etj Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

sijandh35 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

etj commented Apr 23, 2026

Uh oh!

Copilot AI commented Apr 23, 2026

Uh oh!

cla-bot Bot commented Apr 23, 2026

Uh oh!

Copilot AI commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sijandh35 commented Apr 3, 2026 •

edited

Loading

etj left a comment •

edited

Loading