Skip to content

docs: add ADR for competency taxonomy detection - #662

Merged
ormsbee merged 6 commits into
openedx:mainfrom
mgwozdz-unicon:cbe-taxonomy-type-field-adr
Jul 29, 2026
Merged

docs: add ADR for competency taxonomy detection#662
ormsbee merged 6 commits into
openedx:mainfrom
mgwozdz-unicon:cbe-taxonomy-type-field-adr

Conversation

@mgwozdz-unicon

@mgwozdz-unicon mgwozdz-unicon commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

openedx_tagging's taxonomy Get endpoints need to report whether a taxonomy is a Competency Taxonomy (#618), symmetrically with the Create/Import endpoint's taxonomy_type field (#614).

This PR adds ADR 0013 (openedx_tagging), deciding to report a taxonomy's type entirely within openedx-platform: its serializer and view, which already override the base ones, check whether a related CompetencyTaxonomy row exists for a given Taxonomy and fix the resulting query cost, with no new field, method, or enum value added to openedx_tagging or the CBE app.

An earlier draft of this ADR instead proposed a taxonomy_type field directly on Taxonomy. Following architecture-team review, that approach and four others considered along the way are documented as Rejected Alternatives in the ADR itself.

Context

openedx_tagging needs a layering-safe way to report whether a Taxonomy
is competency-enabled (openedx#618), without gaining any knowledge of the
CompetencyTaxonomy subclass CBE owns. Adds a taxonomy_type field to
the base Taxonomy model set by the caller that creates a
CompetencyTaxonomy row, and documents why two prior approaches on
openedx#618 (an openedx_tagging-side hasattr check, an overridable get_type()
method) were unsound. Amends ADR 0002 to note the corresponding
creation-time invariant and that its rejection of a flat taxonomy_type
column still stands, unaffected by this narrower field.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 16, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @mgwozdz-unicon!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

Comment thread docs/openedx_tagging/decisions/0013-taxonomy-type-field.rst Outdated
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Jul 20, 2026
ADR 0013 no longer decides to add a taxonomy_type field to Taxonomy:
an architecture-meeting review moved that decision to detecting
competency taxonomies entirely within openedx-platform instead. The
lifecycle-rule note and Rejected Alternatives cross-reference this
amendment added to describe that field's creation-time invariant no
longer describe anything that happens, so revert ADR 0002 to its
state from before the amendment.
The next commit changes this ADR's decision away from a taxonomy_type
field, so the filename no longer describes it. Renamed on its own,
ahead of that content change, so the rename and the content diff stay
independently reviewable.
Reversed at the 2026-07-21 architecture meeting: instead of a
taxonomy_type field on the base Taxonomy model, report a taxonomy's
type entirely within openedx-platform (a hasattr check on the same
already-fetched instance, plus a matching select_related), so
openedx_tagging and the CBE app stay fully free of any
competency-specific reference. The field-based approach is demoted to
Rejected Alternatives, alongside two more precise variants (two-call
create flow, combined REST API) that were previously folded into a
single vaguer entry. Adds a diagram of the chosen approach.
Replace references to openedx-platform's specific serializer/view
class and method names, and the literal hasattr()/select_related()
calls, with a description of the mechanism itself. Those identifiers
belong to a repo this ADR doesn't govern, so naming them risked the
ADR reading as stale if openedx-platform's internal structure changes,
even though the decision itself wouldn't have.
Drop the TaxonomySerializer parenthetical (an openedx_tagging class
name that isn't needed to state the requirement), the redundant
CompetencyTaxonomy(Taxonomy) Python-syntax notation (already said in
prose just before it), and the parenthetical GitHub issue references,
which read oddly in an ADR meant to stand on its own.
@mgwozdz-unicon

Copy link
Copy Markdown
Contributor Author

We discussed this at today's architecture meeting and decided to go with option 4: keep the type-detection check, and its select_related fix, entirely inside openedx-platform's existing serializer and view overrides, rather than adding a taxonomy_type field to the base Taxonomy model. The tradeoff we're accepting is that a future third taxonomy type would need another hardcoded branch in openedx-platform's serializer instead of a one-line enum addition here, in exchange for keeping openedx_tagging and the CBE app completely free of any competency-specific reference, even an inert one.

I've updated ADR 0013 to reflect that: status is now Accepted, the Decision section describes the openedx-platform-side approach with a diagram, and the original field-based approach is now the first Rejected Alternative with the reasoning above. I also split the "frontend-only" alternative into the two more precise variants we actually compared in the table above (the two-call create flow and the combined REST API), and reverted the ADR 0002 amendment since it no longer describes anything that happens now that there's no new field.

The changes are spread across five commits so the diff stays easy to review: a revert of the ADR 0002 amendment, a pure rename of the ADR file with zero content diff, the actual decision rewrite, and two small follow-up commits trimming implementation detail (specific class/method names, and parenthetical issue references) out of the Context and Decision sections.

@bradenmacdonald, could you take another look when you get a chance? Let me know if I've mischaracterized anything about option 4 above.

@mgwozdz-unicon mgwozdz-unicon changed the title docs: add ADR for taxonomy_type field, amend ADR 0002 docs: add ADR for competency taxonomy detection Jul 21, 2026
@ormsbee
ormsbee merged commit 845dc1f into openedx:main Jul 29, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Contributions Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants