build: add an isolated_apps import-linter contract for openedx_catalog (#860)

[ufedaseyeuconsultant]

Summary

Closes #860.

• Adds an isolated_apps import-linter contract, mirroring openedx-platform's own, so a private-module import into openedx_catalog (like from openedx_catalog.models import CourseRun instead of .models_api) fails here, not only on the platform's CI. Follow-up to #853's review, deliberately split out as its own PR per your preference there.
• Scoped to openedx_catalog only for now. Extending it to openedx_content, openedx_learning, or openedx_tagging surfaces pre-existing violations (confirmed by testing the broader scope locally and reverting), which is out of scope for this change; happy to open a separate follow-up for that if useful.
• IsolatedAppsContract (test_utils/isolated_apps_contract.py) is ported verbatim from openedx.testing.importlinter.isolated_apps_contract in openedx-platform apart from the module docstring (rewritten; nothing copies this file back to openedx-platform, so the docstring shouldn't imply that it does) and the list() workaround in check(): self.isolated_apps/self.allowed_modules are copied through list() before iterating, to work around a pylint false positive (E1133 not-an-iterable) caused by pylint not understanding that Contract._populate_fields() replaces these class-level Field descriptors with their parsed list values at the instance level. Tried typing.cast and an annotated local first; neither satisfied pylint, list() did, with no suppression directive needed.
• Lives in test_utils/, not src/, since it's a dev/CI-only tool and shouldn't ship in the installed package.

The contract catches a real violation

Temporarily reverting criteria.py's import back to from openedx_catalog.models import CourseRun (the pre-#853 line) and running lint-imports on this branch:

"top-level source folders are layered correctly" KEPT
"openedx_content's internal applets are layered correctly" KEPT
"code outside an isolated app only reaches its public API" BROKEN

Contracts: 2 kept, 1 broken.

----------------
Broken contracts
----------------

"code outside an isolated app only reaches its public API"
----------------------------------------------------------

openedx_learning.applets.cbe.models.criteria:18: imported from non-public API of
openedx_catalog:
    from openedx_catalog.models import CourseRun

Reverted back to .models_api afterward; the branch is otherwise unchanged from main.

Test plan

• lint-imports on main plus this change — 3/3 contracts kept, 237 files analyzed (all of src/; the top-level tests/ package isn't in root_packages so it's out of scope, same as today)
• lint-imports with criteria.py's import reverted to the pre-#853 line — 1 contract broken, naming exactly that line (see above)
• pytest tests/openedx_learning/applets/cbe/ — 142 passed
• pylint / pycodestyle / isort / mypy clean on test_utils/isolated_apps_contract.py

🤖 Generated with Claude Code

[openedx-webhooks]

Thanks for the pull request, @ufedaseyeuconsultant!

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:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

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.

[mgwozdz-unicon]

This comment is a collaboration between Claude and me.

@ufedaseyeuconsultant Thanks, this does what #860 asks for. I ran lint-imports on this branch and all three contracts are kept, and reverting the criteria.py import to openedx_catalog.models breaks the new contract on that line, as your description shows. A few small changes before I approve, mostly trimming text so the copied file stays close to openedx-platform's original:

1. Module docstring in test_utils/isolated_apps_contract.py. Please drop the commit hash and the last sentence ("Kept dependency-free beyond import-linter itself..."). Nothing copies this file back to openedx-platform, so that sentence describes a process that doesn't exist. Something like:

An import-linter contract that flags imports of an isolated app's private modules.

Ported from openedx-platform's ``openedx.testing.importlinter.isolated_apps_contract``,
since this library can't depend on openedx-platform.

2. Method docstrings on check() and render_broken_contract(). Please remove these two. pylint passes without them, and leaving them out keeps the file closer to openedx-platform's copy.

3. Comment above the new contract in .importlinter. Please drop the clause starting ", which is what let a foreign-key import reach into openedx_catalog.models directly..." The history of how the bad import happened belongs in #853 and #860, not in the config. With that gone, the rest can be shorter too, for example:

# Mirrors openedx-platform's isolated-apps contract (see test_utils/isolated_apps_contract.py):
# code outside an isolated app may only import its public modules (api, models_api, data,
# tests). src_layering above only controls which top-level packages may depend on which.

Nit: the list() workaround in check(). The : list[str] annotations on the two list() lines don't do anything: mypy skips the body of check() because the method has no type hints, and pylint passes without them. I'd drop them and cut the three-line comment above them down to one line, e.g. # pylint can't tell import-linter replaces these Field descriptors with lists.

Nit: PR description. It says the file is ported verbatim with one deliberate deviation. Once 1 and 2 are done that will be accurate apart from the docstring, but it would help to say "verbatim apart from the module docstring and the list() workaround." Please also link #860 in the description so the PR is tied to its ticket.

[ufedaseyeuconsultant]

@mgwozdz-unicon Done

[mgwozdz-unicon]

Thank you!

[ormsbee]

FYI @kdmccormick, @bradenmacdonald

I'm merging this as a stopgap to keep us from breaking openedx-platform's build, but I realize that this redundancy isn't a long term solution. I'm not entirely sure where we should be putting this to keep consistency across repos.

[kdmccormick]

@ormsbee @mgwozdz-unicon This looks good for now, thanks for merging it in.

I think the long term solution would be to move the IsolatedAppsContract into edx-lint, and hard-coding it with a default list of allowed_modules (probably just api, models_api, and data). Then, any other repo could make use if it with just a few lines in .importlinter

[bradenmacdonald]

Nice, thanks! I agree it would be nice to have this in edx-lint so it's reusable across repos. We should also fix those existing violations within this repo ASAP.