Skip to content

docs and fix: Clarify that yaml priority only applies to role_extensions - #540

Open
rodmgwgu wants to merge 3 commits into
openedx:mainfrom
WGU-Open-edX:rod/priority-resolution
Open

rodmgwgu wants to merge 3 commits into
openedx:mainfrom
WGU-Open-edX:rod/priority-resolution

Conversation

@rodmgwgu

Copy link
Copy Markdown
Contributor

Closes #525

Problem

priority is meant to resolve conflicts between role_extensions only. The compiler also applied it to base definitions: when two files declared the same category, permission, or role differently, the higher-priority file silently won. This was never in an ADR and contradicted validation, which already rejects such conflicts regardless of priority.

Approach

  • Added ADR 0031 stating priority resolves role extensions only, never base definitions.
  • Compiler: differing base definitions now raise SchemaCompileError regardless of priority (identical ones still merge). Extension resolution is unchanged.
  • Clarified the priority docs in ADR 0017, ADR 0023, the schema reference, and the JSON Schema.

Manual testing instructions

In a tutor dev env with this repo mounted:

  1. tutor dev run cms ./manage.py cms load_authz_schema --dry-run — applies cleanly (shipped schema has no conflicts).
  2. Declare the same role ID in two files with different content and priorities, then run load_authz_schema — expect a CommandError reporting the conflict; DB untouched.
  3. Replace the second definition with a role_extensions entry — expect success, confirming priority still resolves extensions.

Automated: TestBaseDefinitionConflicts covers the new behavior; full schema suite (278 tests) passes.

Rollback plan

Revert this PR. Contained to the compiler, its tests, and docs. No migration.

Retro compatibility

No behavior change for supported deployments: validation already rejected these conflicts before compile. Only a direct compiler call with conflicting base definitions now raises instead of picking a winner.

AI Usage

Kiro assisted with investigation, ADR authoring, and implementation, with human guidance on the decision and validation against the ADRs.


Merge checklist:

  • Changelog record added — Fixed entry under Unreleased
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets — no migration

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Oct 16, 2026
@openedx-webhooks

openedx-webhooks commented Oct 16, 2026 •

Copy link
Copy Markdown

Thanks for the pull request, @rodmgwgu!

This repository is currently maintained by @openedx/committers-openedx-authz.

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.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Oct 16, 2026
@rodmgwgu
rodmgwgu marked this pull request as ready for review October 16, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

Priority resolution for category, permission, and role definitions

2 participants