Skip to content

Commit

Permalink
Merge pull request #33954 from code-dot-org/batch-fetch-teacher-scores
Browse files Browse the repository at this point in the history
Progress Performance: load teacher_scores in batches of 50 students at a time
  • Loading branch information
Erin007 committed Apr 3, 2020
2 parents bcd3458 + 5edc133 commit e9455d0
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ export const setSelectedLessons = selected => ({
type: SET_SELECTED_LESSONS,
selected
});
export const setStudentLevelScores = scoresData => ({
export const setStudentLevelScores = (scriptId, lessonId, scoresData) => ({
type: SET_STUDENT_LEVEL_SCORES,
scriptId,
lessonId,
scoresData
});

Expand Down Expand Up @@ -65,9 +67,23 @@ export default function sectionStandardsProgress(state = initialState, action) {
};
}
if (action.type === SET_STUDENT_LEVEL_SCORES) {
const prevLevelScoreByStage = state.studentLevelScoresByStage[
action.scriptId
]
? state.studentLevelScoresByStage[action.scriptId][action.lessonId]
: {};
return {
...state,
studentLevelScoresByStage: action.scoresData
studentLevelScoresByStage: {
...state.studentLevelScoresByStage,
[action.scriptId]: {
...state.studentLevelScoresByStage[action.scriptId],
[action.lessonId]: {
...prevLevelScoreByStage,
...action.scoresData[action.scriptId][action.lessonId]
}
}
}
};
}
return state;
Expand Down Expand Up @@ -332,19 +348,29 @@ export function fetchStandardsCoveredForScript(scriptId) {
export function fetchStudentLevelScores(scriptId, sectionId) {
return (dispatch, getState) => {
let state = getState();
$.ajax({
method: 'GET',
dataType: 'json',
url: `/dashboardapi/v1/teacher_scores/${sectionId}/${scriptId}`
}).then(data => {
const scoresData = data;
dispatch(setStudentLevelScores(scoresData));
const numStudents =
state.teacherSections.sections[state.teacherSections.selectedSectionId]
.studentCount;
let unpluggedLessonList = getUnpluggedLessonsForScript(state);
const unpluggedLessonIds = _.map(unpluggedLessonList, 'id');
const NUM_STUDENTS_PER_PAGE = 50;
const numPages = Math.ceil(numStudents / NUM_STUDENTS_PER_PAGE);
const requests = _.range(1, numPages + 1).map(currentPage => {
const url = `/dashboardapi/v1/teacher_scores/${sectionId}/${scriptId}?page=${currentPage}`;
return fetch(url, {credentials: 'include'})
.then(response => response.json())
.then(data => {
const scoresData = data;
unpluggedLessonIds.forEach(lessonId =>
dispatch(setStudentLevelScores(scriptId, lessonId, scoresData))
);
});
});
Promise.all(requests).then(function() {
let initialCompletedUnpluggedLessons = getInitialUnpluggedLessonCompletionStatus(
state,
scriptId,
scoresData
getState(),
scriptId
);
let unpluggedLessonList = getUnpluggedLessonsForScript(state);
const lessonsToSelect = _.filter(unpluggedLessonList, function(lesson) {
if (initialCompletedUnpluggedLessons.includes(lesson.id)) {
return lesson;
Expand All @@ -355,15 +381,15 @@ export function fetchStudentLevelScores(scriptId, sectionId) {
};
}

function getInitialUnpluggedLessonCompletionStatus(
state,
scriptId,
scoresData
) {
function getInitialUnpluggedLessonCompletionStatus(state, scriptId) {
let completedLessonIds = [];

if (scoresData[scriptId]) {
const levelScoresByStudentForScript = scoresData[scriptId];
if (
state.sectionStandardsProgress.studentLevelScoresByStage &&
state.sectionStandardsProgress.studentLevelScoresByStage[scriptId]
) {
const levelScoresByStudentForScript =
state.sectionStandardsProgress.studentLevelScoresByStage[scriptId];

Object.keys(levelScoresByStudentForScript).forEach(function(item) {
const studentScoresComplete = _.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ def score_stages_for_section
# GET /teacher_scores/<:section_id>/<:script_id>
def get_teacher_scores_for_script
section = Section.find(params[:section_id])
page = [params[:page].to_i, 1].max
if section.user_id == current_user.id
@teacher_scores = TeacherScore.get_level_scores_for_script_for_section(
params[:script_id],
params[:section_id]
params[:section_id],
page
)
render json: @teacher_scores, each_serializer: Api::V1::TeacherScoreSerializer
else
Expand Down
11 changes: 6 additions & 5 deletions dashboard/app/models/teacher_score.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,14 @@ def self.score_level_for_student(
)
end

def self.get_level_scores_for_script_for_section(script_id, section_id)
def self.get_level_scores_for_script_for_section(script_id, section_id, page)
level_scores_by_student_by_stage_by_script = {}
stages = Script.find(script_id).stages
# Teacher scores are currently only relevant for unplugged lessons
stages = Script.find(script_id).stages.select(&:display_as_unplugged)
student_ids = Section.find(section_id).students.page(page).per(50).pluck(:id)
stage_student_level_scores = {}
stages.each do |stage|
level_scores = get_level_scores_for_stage_for_section(stage, section_id)
level_scores = get_level_scores_for_stage_for_students(stage, student_ids)
unless level_scores.empty?
stage_student_level_scores[stage.id] = level_scores
end
Expand All @@ -70,9 +72,8 @@ def self.get_level_scores_for_script_for_section(script_id, section_id)
level_scores_by_student_by_stage_by_script
end

def self.get_level_scores_for_stage_for_section(stage, section_id)
def self.get_level_scores_for_stage_for_students(stage, student_ids)
script_id = stage.script_id
student_ids = Section.find(section_id).students.pluck(:id)
level_ids = stage.script_levels.map(&:level_id)
user_levels = UserLevel.select(:id, :level_id, :user_id).where(user_id: student_ids, level_id: level_ids, script_id: script_id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class Api::V1::TeacherScoresControllerTest < ActionDispatch::IntegrationTest
:script_level,
script: script,
levels: [
create(:maze, name: 'test level 1')
create(:unplugged, name: 'test level 1')
]
)
level = script_level.levels[0]
Expand Down Expand Up @@ -128,7 +128,7 @@ class Api::V1::TeacherScoresControllerTest < ActionDispatch::IntegrationTest
:script_level,
script: script,
levels: [
create(:maze, name: 'test level 1')
create(:unplugged, name: 'test level 1')
]
)
level = script_level.levels[0]
Expand All @@ -141,7 +141,7 @@ class Api::V1::TeacherScoresControllerTest < ActionDispatch::IntegrationTest

post '/dashboardapi/v1/teacher_scores', params: {section_id: section.id, stage_scores: [{stage_id: stage.id, score: score}]}

assert_queries 11 do
assert_queries 13 do
get "/dashboardapi/v1/teacher_scores/#{section.id}/#{script.id}"
end

Expand All @@ -153,7 +153,7 @@ class Api::V1::TeacherScoresControllerTest < ActionDispatch::IntegrationTest

assert_equal section.students.count, 11

assert_queries 11 do
assert_queries 13 do
get "/dashboardapi/v1/teacher_scores/#{section.id}/#{script.id}"
end
end
Expand Down
15 changes: 8 additions & 7 deletions dashboard/test/models/teacher_score_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class TeacherScoreTest < ActiveSupport::TestCase
:script_level,
script: @script,
levels: [
create(:maze, name: 'test level 1')
create(:unplugged, name: 'test level 1')
]
)
@stage = @script_level.stage
Expand Down Expand Up @@ -106,18 +106,18 @@ class TeacherScoreTest < ActiveSupport::TestCase
)
end

assert_equal(TeacherScore.get_level_scores_for_stage_for_section(@stage, @section.id), {@student_1.id => {@level_1.id => @score_2}})
assert_equal(TeacherScore.get_level_scores_for_stage_for_students(@stage, @section.students.pluck(:id)), {@student_1.id => {@level_1.id => @score_2}})
end

test 'get scores for stage for section' do
test 'get scores for stage for students' do
TeacherScore.score_stage_for_section(
@section.id, @stage.id, @score
)

assert_equal(
TeacherScore.get_level_scores_for_stage_for_section(
TeacherScore.get_level_scores_for_stage_for_students(
@stage,
@section.id
@section.students.pluck(:id)
),
{
@student_1.id => {@level_1.id => @score},
Expand All @@ -131,11 +131,12 @@ class TeacherScoreTest < ActiveSupport::TestCase
TeacherScore.score_stage_for_section(
@section.id, @stage.id, @score
)

page = 1
assert_equal(
TeacherScore.get_level_scores_for_script_for_section(
@script.id,
@section.id
@section.id,
page
),
{
@script.id => {
Expand Down

0 comments on commit e9455d0

Please sign in to comment.