Skip to content

Allow cross-book doenet exercises - #1404

Merged
bnmnetp merged 2 commits into
RunestoneInteractive:mainfrom
oscarlevin:doenet-external
Aug 24, 2026
Merged

Allow cross-book doenet exercises#1404
bnmnetp merged 2 commits into
RunestoneInteractive:mainfrom
oscarlevin:doenet-external

Conversation

@oscarlevin

Copy link
Copy Markdown
Collaborator

Loading a doenet exercise that lives in another book redirected the iframe to the change course page. This adds the "?mode=browsing" param to the iframe url so the exercise appears. I tested it locally and seems to work.

Claude also provided a migration for existing books. I put that on a second commit in case you don't want to deal with it and just let the books rebuild.

@oscarlevin
oscarlevin requested a review from bnmnetp as a code owner August 21, 2026 22:23
Copilot AI lite review requested due to automatic review settings August 21, 2026 22:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Doenet exercises failing to load when embedded from a different (base) course by ensuring embedded iframe URLs are rendered in anonymous “browsing” mode, avoiding the book server’s active-course mismatch redirect.

Changes:

  • Append ?mode=browsing to baked Doenet iframe src URLs during question build (_process_single_question).
  • Add an Alembic migration to backfill existing stored Doenet questions’ iframe URLs to include ?mode=browsing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
components/rsptx/build_tools/core.py Updates Doenet iframe URL rewriting to append ?mode=browsing at build time.
migrations/versions/5c11e34a2611_fix_doenet_iframe_course_mismatch.py Backfills existing doenet questions’ stored HTML to add (and remove on downgrade) the browsing-mode query param.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 1116 to 1120
dbtext = re.sub(
r'(<iframe.*?)src="(.*?.html)"',
rf'\1 src="/ns/books/published/{course_name}/\2"',
rf'\1 src="/ns/books/published/{course_name}/\2?mode=browsing"',
dbtext,
)
Comment on lines +45 to +47
# The negative lookahead on the closing quote keeps this idempotent -- a row
# already carrying ?mode=browsing (e.g. rebuilt after this fix shipped) is
# left untouched rather than getting a second query string appended.
@bnmnetp

bnmnetp commented Aug 24, 2026

Copy link
Copy Markdown
Member

My concern with this approach is that mode=browsing implies that the user is not logged in to the book they are looking at. This may not have any impact on the Doenet questions and their behavior to save and restore.

It probably doesn't since I don't think the Doenet code looks at eBookConfig, but I'd like to verify that before merging.

I think a rebuild of the book is probably sufficient since there are few enough doenet books and I can do a rebuild almost immediately after merging/releasing this PR.

@bnmnetp

bnmnetp commented Aug 24, 2026

Copy link
Copy Markdown
Member

Now that I've looked, and remember that we have these *-if.html that are loaded as part of the iframe I see that they don't even have any eBookConfig part to them, so this should work fine.

@oscarlevin

Copy link
Copy Markdown
Collaborator Author

It does seem to work as expected in my testing. And of course, the user is NOT logged into the other book in that scenario. :)

@bnmnetp
bnmnetp merged commit ba7eb4b into RunestoneInteractive:main Aug 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants