-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
base: master
Are you sure you want to change the base?
Changes from all commits
f03594d
c3ef120
9cae873
17989ee
4472b6c
a168d58
57b7977
599bcaa
5e3b195
e7aac7c
30f8418
1d2970d
9ad3163
c7e8816
e64bd7e
5541aae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase | ||
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory | ||
|
||
import openedx.core.djangoapps.content.block_structure.api as bs_api | ||
from common.djangoapps.course_modes.models import CourseMode | ||
from common.djangoapps.student.roles import ( | ||
CourseBetaTesterRole, | ||
|
@@ -2228,6 +2229,9 @@ def test_get_override_for_unreleased_block(self): | |
display_name='Unreleased Section', | ||
) | ||
|
||
# We need to update the course in the cache after we create the new block. | ||
bs_api.update_course_in_cache(self.course_data.course_key) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I wouldn't put it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, I think I'll leave it as is then. |
||
|
||
resp = self.client.get( | ||
self.get_url(subsection_id=unreleased_subsection.location) | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
|
||
from crum import set_current_request | ||
|
||
import openedx.core.djangoapps.content.block_structure.api as bs_api | ||
from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory | ||
from common.djangoapps.student.models import CourseEnrollment | ||
from common.djangoapps.student.tests.factories import UserFactory | ||
|
@@ -76,44 +77,45 @@ def setUp(self): | |
CourseEnrollment.enroll(self.student, self.course.id) | ||
self.instructor = UserFactory.create(is_staff=True, username='test_instructor', password=self.TEST_PASSWORD) | ||
self.refresh_course() | ||
# Since this doesn't happen automatically and we don't want to run all the publish signal handlers | ||
# Just make sure we have the latest version of the course in cache before we test the problem. | ||
bs_api.update_course_in_cache(self.course.id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. So this class already has the |
||
|
||
@patch('lms.djangoapps.grades.events.tracker') | ||
def test_submit_answer(self, events_tracker): | ||
self.submit_question_answer('p1', {'2_1': 'choice_choice_2'}) | ||
course = self.store.get_course(self.course.id, depth=0) | ||
|
||
event_transaction_id = events_tracker.emit.mock_calls[0][1][1]['event_transaction_id'] | ||
events_tracker.emit.assert_has_calls( | ||
[ | ||
mock_call( | ||
events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
{ | ||
'user_id': str(self.student.id), | ||
'event_transaction_id': event_transaction_id, | ||
'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
'course_id': str(self.course.id), | ||
'problem_id': str(self.problem.location), | ||
'weighted_earned': 2.0, | ||
'weighted_possible': 2.0, | ||
}, | ||
), | ||
mock_call( | ||
events.COURSE_GRADE_CALCULATED, | ||
{ | ||
'course_version': str(course.course_version), | ||
'percent_grade': 0.02, | ||
'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=', | ||
'user_id': str(self.student.id), | ||
'letter_grade': '', | ||
'event_transaction_id': event_transaction_id, | ||
'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
'course_id': str(self.course.id), | ||
'course_edited_timestamp': str(course.subtree_edited_on), | ||
} | ||
), | ||
], | ||
any_order=True, | ||
) | ||
expected_calls = [ | ||
mock_call( | ||
events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
{ | ||
'user_id': str(self.student.id), | ||
'event_transaction_id': event_transaction_id, | ||
'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
'course_id': str(self.course.id), | ||
'problem_id': str(self.problem.location), | ||
'weighted_earned': 2.0, | ||
'weighted_possible': 2.0, | ||
}, | ||
), | ||
mock_call( | ||
events.COURSE_GRADE_CALCULATED, | ||
{ | ||
'course_version': str(self.course.course_version), | ||
'percent_grade': 0.02, | ||
'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=', | ||
'user_id': str(self.student.id), | ||
'letter_grade': '', | ||
'event_transaction_id': event_transaction_id, | ||
'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, | ||
'course_id': str(self.course.id), | ||
'course_edited_timestamp': str(self.course.subtree_edited_on), | ||
} | ||
), | ||
] | ||
|
||
events_tracker.emit.assert_has_calls(expected_calls, any_order=True) | ||
|
||
@ddt.data(True, False) | ||
def test_delete_student_state(self, emit_signals): | ||
|
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'm really surprised that this didn't come up as an issue before. I guess because this management command is so little-used, and the vast majority of imports happen through the Studio UI?
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.
Yea, that was my guess as well.