|
| 1 | +.. _openedx-tagging-adr-0013: |
| 2 | + |
| 3 | +13. Competency taxonomy detection in openedx-platform |
| 4 | +======================================================= |
| 5 | + |
| 6 | +Status |
| 7 | +------ |
| 8 | + |
| 9 | +Accepted |
| 10 | + |
| 11 | +Context |
| 12 | +------- |
| 13 | + |
| 14 | +The taxonomy Get endpoints need to report whether a taxonomy is a Competency Taxonomy, so |
| 15 | +that Studio can badge Competency Taxonomies and gate access to the Competency Management |
| 16 | +page, symmetrically with the Create/Import endpoint's ``taxonomy_type`` field. |
| 17 | +``CompetencyTaxonomy`` is a Django multi-table-inheritance subclass of |
| 18 | +``Taxonomy``, owned by the CBE applet and defined in |
| 19 | +`ADR 0002 <../../openedx_learning/decisions/0002-competency-criteria-model.rst>`_. |
| 20 | + |
| 21 | +``openedx_tagging`` is a generic tagging library with no knowledge of any specific taxonomy |
| 22 | +flavor built on top of it, CBE or otherwise, and must not gain any: a specific downstream |
| 23 | +applet's model or relation name has no business appearing in this library's source, since |
| 24 | +that reverses the intended dependency direction (CBE depends on ``openedx_tagging``, not |
| 25 | +the other way around) and would break for any Open edX install running ``openedx_tagging`` |
| 26 | +without CBE installed. |
| 27 | + |
| 28 | +Decision |
| 29 | +-------- |
| 30 | + |
| 31 | +Report a taxonomy's type entirely within **openedx-platform**, using the existing relation |
| 32 | +between ``Taxonomy`` and ``CompetencyTaxonomy`` established in ADR 0002, without adding any |
| 33 | +field, method, or enum value to ``openedx_tagging`` or the CBE app: |
| 34 | + |
| 35 | +.. image:: images/CompetencyTypeDetection.png |
| 36 | + :alt: Studio calls openedx-platform's serializer, which delegates to oel_tagging's pure |
| 37 | + base Taxonomy serializer for the Taxonomy row, and separately runs a hasattr check |
| 38 | + against the same already-fetched instance to detect a related CompetencyTaxonomy |
| 39 | + row, with no new API call and no changes to oel_tagging. |
| 40 | + |
| 41 | +- openedx-platform's REST layer adds a read-only ``taxonomy_type`` value to its taxonomy |
| 42 | + serializer, computed by checking whether a related ``CompetencyTaxonomy`` row exists for |
| 43 | + that ``Taxonomy``: ``"competency"`` if so, ``"tags"`` otherwise. |
| 44 | +- That same layer's queryset fetches the related ``CompetencyTaxonomy`` row alongside the |
| 45 | + ``Taxonomy`` list, so the check costs no extra query per row. |
| 46 | +- ``openedx_tagging``'s ``Taxonomy`` model, its base ``TaxonomySerializer``, and the CBE app |
| 47 | + stay fully unaware of each other for this purpose: no new field, no new enum value, no |
| 48 | + import. |
| 49 | +- No creation-time wiring is needed to keep this accurate: ADR 0002 Decision 1 already |
| 50 | + creates the ``CompetencyTaxonomy`` row in the same transaction as its parent ``Taxonomy`` |
| 51 | + row, so the existence check can never drift out of sync the way a separately-stored |
| 52 | + field could. |
| 53 | + |
| 54 | +**Known trade-off.** A future third taxonomy type needs another hardcoded branch in |
| 55 | +openedx-platform's shared serializer, the same cost a field-based approach would have |
| 56 | +avoided with a one-line enum addition. Accepted because keeping ``openedx_tagging`` and the |
| 57 | +CBE app free of any competency-specific reference, even an inert stored value, was judged |
| 58 | +more valuable than that extensibility, particularly given the project's move away from |
| 59 | +system-defined taxonomies, which makes a third taxonomy flavor unlikely soon. |
| 60 | + |
| 61 | +Rejected Alternatives |
| 62 | +---------------------- |
| 63 | + |
| 64 | +``taxonomy_type`` enum field on the base ``Taxonomy`` model |
| 65 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 66 | + |
| 67 | +A ``TaxonomyType(models.TextChoices)`` field (``TAGS``/``COMPETENCY``) added directly to |
| 68 | +``Taxonomy``, set by whichever code creates a ``CompetencyTaxonomy`` row, in the same |
| 69 | +transaction as ADR 0002 Decision 1's existing lifecycle rule. Although this requires no |
| 70 | +per-request check and was more extensible for a hypothetical third taxonomy flavor, it |
| 71 | +still named a CBE-specific concept, a ``COMPETENCY`` enum value, directly in |
| 72 | +``openedx_tagging``'s own schema and public API. Keeping ``openedx_tagging`` and the CBE |
| 73 | +app fully free of any competency-specific reference, even an inert one, is worth the lost |
| 74 | +extensibility. |
| 75 | + |
| 76 | +Check for a related ``CompetencyTaxonomy`` row directly inside ``openedx_tagging`` |
| 77 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 78 | + |
| 79 | +Using ``hasattr(instance, "competencytaxonomy")`` inside ``openedx_tagging``'s serializer. |
| 80 | +Needed no migration and, unlike the field-based approach above, could never drift out of |
| 81 | +sync since it read the relation directly instead of a separately-stored value. Rejected |
| 82 | +because it hardcodes a specific downstream applet's multi-table-inheritance relation name |
| 83 | +into a standalone, generic library, which is exactly the reverse-dependency problem this |
| 84 | +decision exists to avoid. It also risks an N+1 query on every taxonomy list request unless |
| 85 | +``openedx_tagging``'s own queryset adds a matching ``select_related``, a cost every |
| 86 | +consumer of that shared serializer would inherit, not just openedx-platform. |
| 87 | + |
| 88 | +Two-call frontend create flow |
| 89 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 90 | + |
| 91 | +Studio calls the plain create-taxonomy endpoint, then makes a second call to a separate, |
| 92 | +CBE-owned endpoint to convert it to a competency taxonomy, with no new REST surface needed. |
| 93 | +Rejected because the frontend must orchestrate both calls itself and handle the case where |
| 94 | +the first succeeds but the second fails, leaving a plain taxonomy behind with no competency |
| 95 | +conversion. |
| 96 | + |
| 97 | +New combined competency REST API |
| 98 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 99 | + |
| 100 | +A single new CBE-owned endpoint that creates both the ``Taxonomy`` and |
| 101 | +``CompetencyTaxonomy`` rows atomically. Rejected because it is a new endpoint to design, |
| 102 | +build, and maintain, and the frontend still needs both this call and the plain |
| 103 | +create-taxonomy call, choosing between them since nothing upstream tells it in advance |
| 104 | +whether a Tags or Competency taxonomy is being created; that branching just relocates the |
| 105 | +type-awareness into the frontend rather than removing it. |
| 106 | + |
| 107 | +An overridable ``Taxonomy.get_type()`` method |
| 108 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 109 | + |
| 110 | +A base ``Taxonomy.get_type()`` method returning ``"tags"``, overridden by |
| 111 | +``CompetencyTaxonomy`` to return ``"competency"``, mirroring the existing |
| 112 | +``Taxonomy.system_defined`` / ``SystemDefinedTaxonomy`` base/override shape. Rejected for |
| 113 | +two independent reasons: |
| 114 | + |
| 115 | +- That base/override shape is implemented via ``Taxonomy._taxonomy_class`` and |
| 116 | + ``Taxonomy.cast()``/``Taxonomy.copy()``, which are planned to be removed as a pattern. |
| 117 | +- Querying taxonomies the normal way (``Taxonomy.objects.all()``) returns plain ``Taxonomy`` |
| 118 | + instances, so a subclass method override is never reached without an explicit cast step |
| 119 | + first. The existing ``.cast()``/``.copy()`` implementation would not correctly perform |
| 120 | + that cast for a true multi-table-inheritance subclass in any case: it copies a hardcoded |
| 121 | + list of base ``Taxonomy`` field values in Python and never queries the subclass's own |
| 122 | + table, so it would silently produce a ``CompetencyTaxonomy`` instance with unset or wrong |
| 123 | + subclass-specific fields. |
0 commit comments