Skip to content

[BE] Implement the external_id not nullable constraint as described in openedx_tagging ADR 12 #758

Description

@thelmick-unicon

User Story

As a Platform Administrator, I want every tag in every taxonomy to always carry an external identifier, whether I assigned it or the system generated it, in order to reference and act on any competency by its Competency ID without hand-assigning one to each competency first.

Acceptance Criteria

Note that until #680 is complete, the only way to see a tag’s external identifier is to view it in the export of the taxonomy.

# Upgrading an instance that already holds tags

Scenario: Existing tags without an external identifier receive one
  Given an existing taxonomy containing tags that have no external identifier
  When an operator upgrades the instance
  Then every one of those tags reports an external identifier derived from the tag's own name
  And each tag keeps its name, its position in the hierarchy, and its existing content associations

Scenario: An external identifier the institution already assigned is never overwritten
  Given a tag whose external identifier was assigned by the institution
  When an operator upgrades the instance
  Then that tag reports exactly the external identifier it had before the upgrade

Scenario: A generated external identifier would duplicate one already in use in the same taxonomy
  Given a taxonomy containing a tag with no external identifier
  And another tag in that same taxonomy whose institution-assigned external identifier is the same text as the first tag's name
  When an operator upgrades the instance
  Then the upgrade completes
  And the two tags report external identifiers that differ from each other
  And the institution-assigned identifier is unchanged

Scenario: A tag name is too long to be used as an external identifier
  Given a tag with no external identifier whose name is longer than an external identifier is allowed to be (> 255 characters)
  When an operator upgrades the instance
  Then the upgrade completes
  And that tag reports a non-empty external identifier derived from the start of its name
  And that external identifier differs from every other external identifier in its taxonomy

Scenario: A taxonomy at the scale of a large third-party framework
  Given an instance containing a taxonomy at the scale of a large third-party skills framework (e.g. Lightcast Taxonomy), whose tags have no external identifier
  When an operator upgrades the instance
  Then the upgrade completes without manual intervention
  And every tag in that taxonomy reports an external identifier

Scenario: Rolling the upgrade back
  Given an operator has upgraded an instance and external identifiers were generated for tags that had none
  When the operator rolls that upgrade back
  Then the rollback completes
  And every tag still reports the external identifier it reported after the upgrade

# Creating and changing tags afterwards

Scenario: Creating a tag through the tagging API without supplying an external identifier
  Given a taxonomy that permits changes
  When a caller creates a tag in it through the tagging API without supplying an external identifier
  Then the tag is created rather than rejected
  And it reports an external identifier derived from the name the caller supplied

Scenario: Creating a tag without going through the tagging API
  Given a taxonomy that permits changes
  When a plugin or another Open edX app stores a new tag in it directly, without going through the tagging API and without supplying an external identifier
  Then the tag is stored rather than rejected
  And it reports an external identifier derived from the name that was stored

Scenario: Creating a tag and supplying an external identifier
  Given a taxonomy that permits changes
  When a caller creates a tag in it and supplies an external identifier
  Then the tag reports exactly the external identifier the caller supplied

Scenario: Creating a tag whose generated external identifier would duplicate an existing one
  Given a taxonomy containing a tag whose external identifier was generated by the system
  When a caller creates another tag whose name would generate that same external identifier
  Then the tag is created
  And the two tags report external identifiers that differ from each other

Scenario: Creating a tag with an external identifier already in use in the same taxonomy
  Given a taxonomy containing a tag with a given external identifier
  When a caller creates another tag and supplies that same external identifier
  Then the request is rejected with an error reporting the conflict
  And no new tag is created

Scenario: Creating a tag with an external identifier that differs from an existing one only in letter case
  Given a taxonomy containing a tag with a given external identifier
  When a caller creates another tag and supplies an external identifier that differs from it only in letter case
  Then the request is rejected with an error reporting the conflict
  And no new tag is created

Scenario: Renaming a tag leaves its external identifier alone
  Given a tag whose external identifier was generated from its name
  When an author renames that tag
  Then the tag reports the same external identifier as before the rename

# Behavior that must not change

Scenario: A free-text taxonomy is unaffected
  Given a free-text taxonomy, where authors type arbitrary values instead of choosing from a list
  When an operator upgrades the instance and an author then tags content with a new free-text value
  Then tagging succeeds exactly as before the upgrade
  And requests that list that taxonomy's values continue to report no external identifier for them

Scenario: A read-only taxonomy receives external identifiers and stays read-only
  Given a read-only taxonomy whose tags have no external identifier
  When an operator upgrades the instance
  Then its tags report external identifiers
  And attempts to add, rename, or delete tags in that taxonomy are still rejected
  And attempts to import into that taxonomy are still rejected

Scenario: Exporting and re-importing a taxonomy that previously had no external identifiers
   Given a taxonomy whose container was created some way other than the tag-import wizard (for example, directly through the taxonomy-creation API, Django admin, or a system-defined taxonomy created by a migration), and whose tags were then added one at a time through the ordinary taxonomy editor's "Add tag" action, so that before the upgrade none of them carry an external identifier
  When an operator upgrades the instance, exports that taxonomy, and re-imports the exported file unchanged
  Then the import reports that there is nothing to change
  And no tag is created, renamed, re-parented, or deleted

Scenario: Replacing a taxonomy's tags by import still removes the tags left out of the file
  Given a taxonomy whose tags received generated external identifiers during the upgrade
  And a file exported from that taxonomy after the upgrade, with one of those tags removed from it
  When an operator imports that file to replace the taxonomy's tags
  Then the removed tag is deleted
  And any children it had are preserved and re-parented, the same as before the upgrade

Scenario: An export file taken before the upgrade fails safely when added to a taxonomy
 Given an export file produced before the upgrade, from a taxonomy whose container was created some way other than the tag-import wizard (for example, directly through the taxonomy-creation API, Django admin, or a system-defined taxonomy created by a migration) and whose tags were then added one at a time through the ordinary taxonomy editor's "Add tag" action, so the file's id column holds each tag's pre-upgrade database id instead of a real external identifier
  When an operator re-imports that file after upgrading, adding to the taxonomy's existing tags rather than replacing them
  Then the import reports validation errors identifying the rows it cannot match
  And no tag is created, renamed, re-parented, or deleted

Description

Today a tag carries an external identifier only when one arrives through file import in the id field, which is required. A tag created through the taxonomy editing interface or the tagging API has none at all.

The external identifier in this ticket is the one administrators see as the Competency ID on the Competency Management page. It applies to tags in every taxonomy, not only competency taxonomies, because this library has no concept of a competency taxonomy and is not to gain one.

Nothing marks or records an external identifier as generated. Administrators cannot tell a generated external identifier from one their institution assigned, and this ticket adds nothing to make that possible.

Technical Details

This section is background and a suggested approach, not the source of truth. The User Story and Acceptance Criteria define what must be true when the work is done; this exists to save the implementer some thinking.

In short

Identifier format. There's one constant for the maximum length (255 characters), and one small function that turns a tag's name into a candidate identifier. First try: the name itself, trimmed and cut to fit. If that's taken, later tries shorten the name further and append "-2", "-3", etc.

New tags get an identifier automatically. When a tag is saved with no identifier, the code generates one from its name. It checks the database once to see if that name is already used in the same taxonomy. If it's free, done. If not, it pulls the list of similar identifiers already in use (one more query) and picks the next free "-2", "-3", etc. from memory, no more database trips. Comparisons ignore letter case, matching how the database itself decides two identifiers are "the same." Renaming a tag never regenerates its identifier, only a genuinely new, empty one gets generated.

Backfilling existing tags during the upgrade. The database change happens in two steps: first fill in every empty identifier, then add the "can't be empty" rule to the column. The fill-in step works one taxonomy at a time, first noting which identifiers already exist there so it never overwrites one an institution assigned by hand, then assigning free identifiers to the rest in a stable order. It's written so that if it gets interrupted partway through, re-running it picks up where it left off instead of redoing work.

Rolling back. Rolling this migration back only loosens the "can't be empty" rule on the column; it deliberately leaves the data alone, none of the generated identifiers are removed.

Cleanup. A few places in the code have special-case logic for "this tag has no identifier." Since that state can no longer exist, those special cases get deleted. One side effect: a taxonomy export file taken before the upgrade won't match anything if re-imported afterward, because the identifiers it references are gone. That's expected, not a bug the ticket needs to prevent, operators are expected to re-export after upgrading.

Tests. The test data needs every tag to already have an identifier, since loading test fixtures skips the normal "generate on save" logic entirely. A new test library lets a test simulate "the database as it looked right before this migration," which is necessary because a freshly-built test database has no old data to actually backfill. Tests also need to run against MySQL, not just the faster SQLite most tests use, because MySQL is where the case-insensitive uniqueness rule and the column-rebuild step actually get exercised.

Implementation specifics an implementer will need

Exact file and function names: the constant and derivation function live in src/openedx_tagging/models/utils.py, as TAG_EXTERNAL_ID_MAX_LENGTH and tag_external_id_candidate().

Exact truncation formula: first attempt is value.strip()[:255].strip(); later attempts reserve 8 characters for a hyphen and suffix, value.strip()[:255 - 8].strip() + f"-{attempt}".

Which methods must not regenerate an identifier: Taxonomy.update_tag() and the import pathway's RenameTag.execute() both save an existing tag and must leave its identifier untouched.

The specific database operations to use: .exists() for the single-candidate check, .values_list(..., flat=True).distinct() for grouping by taxonomy, .iterator() for reading, and bulk_update(..., batch_size=...) for writing, so this stays a bounded number of queries even on a large taxonomy.

The migration's exact shape: new file src/openedx_tagging/migrations/0022_tag_external_id_not_null.py, atomic = False, depending on 0021_remove_system_defined_add_read_only, running RunPython(backfill, reverse_backfill) before AlterField.

The duplicate-identifier error path: Taxonomy.add_tag() must call the existing Taxonomy.validate_external_id() and raise ValueError, which the REST layer already turns into a 400; without it, a duplicate reaches the database as an IntegrityError and surfaces as a generic 500.

Signatures and docstrings stay put: Taxonomy.add_tag() and api.add_tag_to_taxonomy() keep external_id: str | None = None; only the docstring narrows, so this isn't a DEPR case.

Case-insensitive comparison must match the database, not just Django: use str.casefold(), not __iexact, since casefold() is at least as aggressive as the field's own collations (utf8mb4_unicode_ci on MySQL, NOCASE on SQLite); this keeps the comparison from admitting a match the database would still reject as a duplicate.

Four specific places assume a tag can have no identifier, and all four get removed: TagImportPlan._get_tag_id()'s str(tag.id) fallback branch, ImportAction._get_tag()'s tag_set.get(value=..., external_id=None) fallback, Tag.display_str()'s identifier-less branch, and the inline comment on the external_id field explaining the old NULL rationale.

TagData.external_id and TagDataSerializer.external_id stay nullable: free-text taxonomies have no Tag rows, so this response contract doesn't narrow along with the model field.

New test dependency needs a requirements regeneration, not a hand-edit: add django-test-migrations to requirements/test.in, then regenerate test.txt, quality.txt, and dev.txt with make compile-requirements.

The fixture must carry real identifiers: tests/openedx_tagging/fixtures/tagging.yaml needs every tag row given an external_id equal to its own value, since loaddata calls Model.save_base(raw=True) and never runs Tag.save()'s generation logic.

That fixture change touches specific modules: tests/openedx_tagging/test_models.py, tests/openedx_tagging/test_views.py, and the import/export tests reached through tests/openedx_tagging/import_export/mixins.py; free-text expectations keep a null identifier.

Three tests get deleted, not updated: test_import_removing_no_external_id, test_import_removing_with_childs_no_external_id, and test_import_same_value_without_external_id in tests/openedx_tagging/import_export/test_api.py, each builds a now-impossible no-identifier tag and duplicates an adjacent test that already supplies one.

Specific edge cases to test on the derivation rule directly: a bare value, a value needing truncation, a collision against an institution-assigned identifier, a collision chain, a case-insensitive collision, whitespace at the truncation boundary, and distinctness of consecutive candidates.

Specific generation paths to test: Tag.save(), Taxonomy.add_tag(), and a bare Tag.objects.create(), plus duplicate-identifier rejection and confirming a rename leaves the identifier alone.

Testing the backfill needs the migrator fixture from django-test-migrations, inserting null-identifier rows through the historical pre-migration model; running the ordinary suite proves nothing here, since a freshly built test database starts with zero rows for the backfill to act on.

Import/export round trip needs explicit coverage: a post-upgrade export re-imports as no changes, and a pre-upgrade file is rejected.

The MySQL test run specifically must pass, not just SQLite: it's the only backend exercising the case-insensitive unique index and the MODIFY COLUMN ... NOT NULL rebuild; tox already points Django targets at mysql_test_settings.

No PII annotation work needed: oel_tagging.Tag is already recorded as no_pii in .annotation_safe_list.yml.

Out of scope

Files to create and modify

New files

File Purpose
src/openedx_tagging/migrations/0022_tag_external_id_not_null.py backfill every null identifier, then make the column NOT NULL
tests/openedx_tagging/test_migrations.py exercise the backfill against pre-migration data

Modified files

File Nature of modification
src/openedx_tagging/models/utils.py add the length constant and the derivation helper
src/openedx_tagging/models/base.py generate the identifier in Tag.save(); drop null=True on external_id; reject a duplicate supplied identifier in Taxonomy.add_tag(); remove the identifier-less branches and the stale field comment
src/openedx_tagging/api.py docstring for add_tag_to_taxonomy()'s narrowed outcome
src/openedx_tagging/import_export/import_plan.py _get_tag_id() and its docstring
src/openedx_tagging/import_export/actions.py ImportAction._get_tag()
requirements/test.in add the new test-only dependency
tests/openedx_tagging/fixtures/tagging.yaml give every tag row an identifier
tests/openedx_tagging/test_models.py tests for the derivation rule, generation, duplicate rejection, and rename; update fixture-tag expectations
tests/openedx_tagging/test_views.py update tag-listing and import-plan expectations for fixture tags
tests/openedx_tagging/import_export/test_api.py add round-trip coverage; remove the three tests whose premise is no longer reachable
tests/openedx_tagging/import_export/test_import_plan.py update plan expectations that assume fixture tags have no identifier
tests/openedx_tagging/import_export/test_parsers.py update export expectations now that the id column carries an identifier, not a database id
Context
  • docs/openedx_tagging/decisions/0012-non-nullable-tag-external-id.rst is the design this ticket implements, currently in proposed status. 0010-mutable-tag-external-id.rst establishes the identifier as institution-editable, 0011-cross-instance-taxonomy-identity.rst matches tags across instances by identifier, and 0013-competency-taxonomy-detection.rst is why the rule is generic across all taxonomies.
  • src/openedx_django_lib/collations.py: case_insensitive_char_field() documents which collations make the identifier's uniqueness constraint case-insensitive, and on which backends.
  • src/openedx_content/backcompat/publishing/migrations/0010_backfill_dependencies.py: how this repo writes a data migration that must keep working against historical models.
  • [BE] Support editing an existing tag's external_id (Competency ID) on taxonomy re-import #673, which owns changing an identifier after creation, is a dependency of this work. [EPIC-CC] Initial Competency Management Page #720 is the parent use case.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Ready for Community Review

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions