Skip to content

canvas - separate courses per session #2347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

shanbady
Copy link
Contributor

@shanbady shanbady commented Jul 10, 2025

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/7772

Description (What does it do?)

This PR makes it so that the course folder is included in the readable id when creating canvas learning resource objects.

How can this be tested?

  1. Checkout this branch
  2. restart celery
  3. ingest canvas courses via python manage.py backpopulate_canvas_courses
  4. once finished, inspect the readable ids of the learning resource objects where etl_source="canvas"
from learning_resources.models import *
list(LearningResource.objects.filter(etl_source="canvas").values_list("readable_id"))

or check via admin: http://open.odl.local:8063/admin/learning_resources/learningresource/?etl_source=canvas
5. note that the readable ids should look like "{folder_name}-{course_id}"

Additional Context

In the initial implementation we expected the course id in the course_settings.xml to be unique which turned out to not be the case between sessions. This pr resolves that issue.

@shanbady shanbady marked this pull request as ready for review July 10, 2025 17:26
@shanbady shanbady added the Needs Review An open Pull Request that is ready for review label Jul 10, 2025
@shanbady shanbady changed the title Shanbady/canvas separate courses per session canvas - separate courses per session Jul 10, 2025
@mbertrand mbertrand self-assigned this Jul 10, 2025
run_id=f"{readable_id}+canvas", learning_resource=resource, published=True
run_id=f"{readable_id}+canvas",
learning_resource=resource,
published=True,
Copy link
Member

Choose a reason for hiding this comment

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

New session-specific courses were created, but the 2 older courses from an earlier command run are still present with test_mode=True. I think any courses not found in the archives during a command run should probably be deleted and/or have test_mode set to False (in addition to published=False), with all their contentfiles deleted or unpublished from regular and vector search.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the deletion of stale canvas courses. Retrieving the current course ids required going into the archive so i funnel up the relevant readable ids and at the end of the command do a deletion+deindex. This also only applies when running the management command since the webhook just tells us to add a new entry.

@mbertrand mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Jul 10, 2025
@shanbady shanbady requested a review from mbertrand July 11, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants