-
Notifications
You must be signed in to change notification settings - Fork 4
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
Lms/updated parallel write responses #9393
Conversation
activity_session.reload | ||
expect(activity_session.old_concept_results.size).to eq 7 | ||
# Run Sidekiq jobs immediately instead of queuing them | ||
Sidekiq::Testing.inline! do | ||
expect do | ||
put :update, params: { id: activity_session.uid, concept_results: concept_results }, as: :json | ||
end.to change { activity_session.reload.old_concept_results.size }.by(3) | ||
.and change { activity_session.reload.concept_results.size }.by(3) | ||
end |
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.
The original version of those code with expect ... to eq 7
was counting the items that were created as part of setup as well as the ones created by the function call. This cleans things up slightly to only count items new from the call.
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.
A couple of minor comments/questions but nothing holding up approval. Nice sleuthing on the bulk_insert
bug.
services/QuillLMS/spec/workers/save_activity_session_concept_results_worker_spec.rb
Outdated
Show resolved
Hide resolved
services/QuillLMS/spec/controllers/api/v1/activity_sessions_controller_spec.rb
Show resolved
Hide resolved
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.
One note about naming, but looks good to me.
Don't give it the same name as the original gem's name so that we can avoid confusion
WHAT
Begin writing
ConceptResult
records whenever we writeOldConceptResult
records so that both tables keep up to date with any new data from student activitiesWHY
We eventually want to replace
OldConceptResult
withConceptResult
, so we need to make sure we can write to both at the same time. Once we're doing that, we can confirm that the data being written is complete in both places to build confidence for a full switch-overHOW
Add a worker that writes
ConceptResult
records, and enqueue that job during theActivitySessionController
'shandle_concept_results
function which is the place where that data gets written.Notion Card Links
https://www.notion.so/quill/Write-Response-records-in-parallel-with-ConceptResults-977206a533da442c9c3cd9fa9ad97be7