Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat!: Drop the block_structure.storage_backing_for_cache WaffleSwitch #35185
feat!: Drop the block_structure.storage_backing_for_cache WaffleSwitch #35185
Changes from all commits
16d440a
57cdbfa
261b498
fab0267
9164e92
e52253d
e5f698a
db266e2
cf37ca4
f0f18f8
8741417
80ed5ee
d58dea3
a6e5b81
398160c
4768568
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think doing it here manually is okay. I'd probably just enable the
published
signal for this test case and let it build everything–to test part of the machinery as well, since the triggers for that may change over time. It might involve splitting off this test from the rest of the test case though.I wouldn't put it in
xmodule/modulestore/tests/factories.py
though, since that's called by a lot of things that may not require this functionality.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I think I'll leave it as is then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I'd probably emit the signal and let everything run (I think this is the most expensive part of it anyhow), but I think either way is okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this class already has the
course_published
signal enabled and so I thought he issue might be that theCourseFactor
needed theemit_signals
parameter set to true, but even with that this did not work. How important is it that this be figured out for this PR? I can dig into it further if needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work when you test the functionality locally via browser? If it doesn't happen automatically, it's possible that something is going wrong (e.g. it's going to trigger later on lazy-load). I'm really sorry to keep dragging this PR out, but so many things rely on block structures that I think it's worth investigating why it doesn't refresh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ormsbee I've tested it locally and created a new course and made various changes to the course sections/subsections and I can see them all propagate correctly to the course outline page. Let me know if there is anything else you want me to test.