[RawanMatar89]

Base branch
infra/03-remote-config-fetch

Description

Routes outgoing requests through the currently selected instance's host and credentials instead of the app-level config, and refreshes tokens against that instance too. This is where the instance model (PR-1) and instance selection (PR-2) actually start affecting network traffic — before this PR, InstanceStore tracked a selection but nothing used it.

What's in this PR

RequestInterceptor

• Takes a new instanceStore: InstanceProvider dependency (already registered by PR-2 — no new DI wiring needed).
• adapt(...) now rewrites the host/scheme/port of any request built against the app's base URL to the selected instance's baseURL, via a new rewriteHostIfNeeded(_:). Path and query are left untouched. A request with no selected instance, or one not built against the app base URL in the first place (e.g. an SSO webview, a third-party SDK call), passes through unmodified.
• refreshToken(...) now prefers the selected instance's baseURL/oAuthClientId over the app-level config, falling back to app config when no instance is selected. token_type stays app-level (unchanged) — no instance-level override for that field exists yet.

DI

• NetworkAssembly.swift: RequestInterceptor registration now resolves and passes InstanceProvider.

Tests

• RequestInterceptorTests.swift (new): selected-instance host rewrite (host/path/query preserved correctly), no-instance-selected passthrough, and non-app-base-URL requests left untouched.

Out of scope

• Instance-scoped local storage / CoreData / downloads.
• Session lifecycle across instance switches.

[openedx-webhooks]

Thanks for the pull request, @RawanMatar89!

This repository is currently maintained by @openedx/openedx-mobile-maintainers.

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.

[IvanStepanok]

@RawanMatar89 Stop, nooooooo...
All we need is just add a few new values to yaml file. Something like this:
LMS_DIRECTORY:
ENABLED: true
DIRECTORY_URL: "https://providers.openedx-lms.stepanok.com/p/northwind-education-group/directory.json"
DIRECTORY_FILE: ""

And inside a JSON you are setup all the instances. As example something like this:
{
"format": "v1",
"provider": {
"name": "Northwind Education Group",
"tagline": "Five campuses, one app",
"logo": null
},
"include": [
{
"name": "Northwind College",
"description": "Main campus",
"url": "https://learn.northwind.edu",
"logo": "https://cdn.northwind.edu/logo.png",
"accent_color": "#002545",
"api": {
"feedback_email": "support@northwind.edu"
},
"theme": {
"accent_color_dark": "#4989bf",
"login_background": "https://cdn.northwind.edu/signin.png"
},
"ui_components": {
"course_unit_progress_enabled": true,
"course_dropdown_navigation_enabled": true
},
"dashboard": { "type": "list" }
}
]
}

We need to support JSON file by URL or locally by DIRECTORY_FILE value. We need them both. Please, do not remove config.yaml🙌

[RawanMatar89]

@RawanMatar89 Stop, nooooooo... All we need is just add a few new values to yaml file. Something like this: LMS_DIRECTORY: ENABLED: true DIRECTORY_URL: "https://providers.openedx-lms.stepanok.com/p/northwind-education-group/directory.json" DIRECTORY_FILE: ""

And inside a JSON you are setup all the instances. As example something like this: { "format": "v1", "provider": { "name": "Northwind Education Group", "tagline": "Five campuses, one app", "logo": null }, "include": [ { "name": "Northwind College", "description": "Main campus", "url": "https://learn.northwind.edu", "logo": "https://cdn.northwind.edu/logo.png", "accent_color": "#002545", "api": { "feedback_email": "support@northwind.edu" }, "theme": { "accent_color_dark": "#4989bf", "login_background": "https://cdn.northwind.edu/signin.png" }, "ui_components": { "course_unit_progress_enabled": true, "course_dropdown_navigation_enabled": true }, "dashboard": { "type": "list" } } ] }

We need to support JSON file by URL or locally by DIRECTORY_FILE value. We need them both. Please, do not remove config.yaml🙌

@ivan-stepanok 😄 I don't think we're actually disagreeing on the shape here, let me untangle it:

The URL-or-local-file JSON directory you're describing is exactly what's already built on the branch this PR stacks on — InstanceConfigLoader fetches a JSON catalog from a URL, falls back to a bundled JSON catalog if the fetch fails, same schema either way. So that part isn't going anywhere.

The one thing that did change is the outer local app config — instead of keeping that as YAML and adding a separate JSON directory format on top, it's now JSON too (config.json), so the build script, the bundled catalog, and the remote catalog all read the same format instead of two. That's not about disliking YAML 😅 — it's cutting the second parser (and the two-schemas-to-keep-in-sync problem) out of the pipeline. It's also just finishing the direction we were already in — this is basically what I did with the instances block a year ago (only yaml was used), just now split cleanly into local + remote with one shared format instead of mixed in.

If there's something specific relying on config.yaml staying YAML, flag it and we'll account for it — but as a format choice on its own I'd rather keep the one-format pipeline. Let me know if that resolves it or if I'm missing something on your side. 🤝

[IvanStepanok]

Thanks, @RawanMatar89 – the remote catalog looks good, and I'm fine with JSON becoming the build-time config source. The plist output is unchanged, so nothing shifts at runtime. A few things I'd like in before this merges, though:

A migration path. Anyone upgrading with an existing YAML config in their config_directory will just get a fatalError on launch with no hint why. process_config.py should detect a config.yaml with no config.json next to it and fail with a clear message – ideally shipping a small converter script alongside it.

Docs in the same PR. Documentation/CONFIGURATION_MANAGEMENT.md describes the YAML flow end to end, and config.yaml's inline comments are currently the only documentation several of these keys have. If the comments go away, that content needs to land in the doc here, not in a follow-up.

Pick one format. config_settings.yaml and file_mappings.yaml staying YAML leaves two formats side by side in the same folder, which is worse for whoever configures this than either option on its own. Either convert them too, or keep config.yaml and just add the catalog pointer key to it.

None of this is a blocker on the approach – just don't want to hand operators a silent break and stale docs.

[RawanMatar89]

Thank you @IvanStepanok 🙏🏻 all three are fair, and none of them are done yet, so no pushback:

Migration path: confirmed the gap. I'll add an explicit check up front and ship a small converter script alongside it, since the schema didn't reshape anything moving formats.
Docs: you're right, the doc in this PR is still the pre-instance-model version. I have the rewritten one (JSON-only pipeline, remote catalog, schema conventions) but never actually pushed it — that's on me. Landing it in this PR, not a follow-up.
Format consolidation: also confirmed 👍🏻, config_settings.yaml and file_mappings.yaml are still YAML. I'll convert both to JSON rather than growing config.yaml back — both are small (a couple of keys / a one-item list), so it's a quick conversion, not a redesign. 🤝

Will push all three as part of this PR before asking for another look. 🙌

[RawanMatar89]

Hi @IvanStepanok, the three changes from your last review are in. Pushed and ready for another look. 👀

One thing worth flagging before you dig in: these are stacked PRs, so this one's diff only makes sense on top of the branches below it. Branch names show the order — infra/01-tenant-model → infra/02-tenant-store → infra/03-remote-config-fetch → infra/04-instance-aware-networking (this one). Could we review and approve/merge in that order, starting from infra/01-tenant-model? Reviewing this one in isolation will show you everything from the branches underneath it too, which makes it harder to tell what's actually new here.

One naming note: any branch still says "tenant" was created before we agreed on the tenant → instance rename, and its content is already renamed to instance throughout. Just a stale branch name, not a leftover reference.

The merge-base changed after approval.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (0c72bd4) to head (a724c06).
⚠️ Report is 19 commits behind head on develop.

Additional details and impacted files

@@      Coverage Diff       @@
##   develop   #677   +/-   ##
==============================
==============================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The merge-base changed after approval.

The merge-base changed after approval.

The merge-base changed after approval.

The merge-base changed after approval.