Skip to content

Commit

Permalink
Merge pull request #43422 from code-dot-org/jamescodeorg/cached-level…
Browse files Browse the repository at this point in the history 
…-load-source

Update user_progress and user_app_options to support teacher view for cached levels
  • Loading branch information
@jamescodeorg
jamescodeorg committed Nov 10, 2021
2 parents 527cba6 + 7aa8d4d commit c8da451
Show file tree
Hide file tree
Showing 9 changed files with 337 additions and 291 deletions.
29 changes: 21 additions & 8 deletions apps/src/code-studio/initApp/loadApp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,14 +299,18 @@ async function loadAppAsync(appOptions) {
appOptions.disableSocialShare = true;
}

const userAppOptionsRequest = $.ajax(
`/api/user_app_options` +
const userAppOptionsRequest = $.ajax({
url:
`/api/user_app_options` +
`/${appOptions.scriptName}` +
`/${appOptions.lessonPosition}` +
`/${appOptions.levelPosition}` +
`/${appOptions.serverLevelId}` +
`?get_channel_id=${shouldGetChannelId}`
);
`/${appOptions.serverLevelId}`,
data: {
user_id: clientState.queryParams('user_id'),
get_channel_id: shouldGetChannelId
}
});

const sectionId = clientState.queryParams('section_id') || '';
const exampleSolutionsRequest = $.ajax(
Expand All @@ -324,9 +328,18 @@ async function loadAppAsync(appOptions) {
appOptions.exampleSolutions = exampleSolutions;
appOptions.disableSocialShare = data.disableSocialShare;

// We do not need to process data.progress here because labs do not use
// the level progress data directly. (The progress bubbles in the header
// of the level pages are rendered by header.build in header.js.)
if (data.isStarted) {
appOptions.level.isStarted = data.isStarted;
}
if (data.skipInstructionsPopup) {
appOptions.level.skipInstructionsPopup = data.skipInstructionsPopup;
}
if (data.readonlyWorkspace) {
appOptions.readonlyWorkspace = data.readonlyWorkspace;
}
if (data.callouts) {
appOptions.callouts = data.callouts;
}

if (data.lastAttempt) {
appOptions.level.lastAttempt = data.lastAttempt.source;
Expand Down
5 changes: 4 additions & 1 deletion apps/src/code-studio/progress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ function getLevelProgress(signedIn, progressData, scriptName) {
case null:
// We do not know if user is signed in or not, send a request to the server
// to find out if the user is signed in and retrieve progress information
return $.ajax(`/api/user_progress/${scriptName}`)
return $.ajax({
url: `/api/user_progress/${scriptName}`,
data: {user_id: clientState.queryParams('user_id')}
})
.then(data => {
if (data.signedIn) {
return {
Expand Down
79 changes: 28 additions & 51 deletions apps/test/unit/code-studio/initApp/loadAppTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('loadApp.js', () => {
sinon.stub(project, 'getSharingDisabled').returns(false);
});
beforeEach(() => {
sinon.stub(clientState, 'queryParams').returns(undefined);
sinon.stub($, 'ajax').callsFake(() => ({
done: successCallback => ({
fail: failureCallback => {
Expand Down Expand Up @@ -66,38 +67,26 @@ describe('loadApp.js', () => {
window.appOptions = oldAppOptions;
});
afterEach(() => {
clientState.queryParams.restore();
$.ajax.restore();
});

const stubQueryParams = (paramName, paramValue) => {
clientState.queryParams.restore(); // restore the default stub
sinon
.stub(clientState, 'queryParams')
.callsFake(param => (param === paramName ? paramValue : undefined));
};

const stubAppOptionsRequests = (
appOptions,
userAppOptionsResponse = {signedIn: false},
exampleSolutionsResponse = []
) => {
const {
scriptName,
lessonPosition,
levelPosition,
serverLevelId,
serverScriptLevelId,
levelRequiresChannel,
channel
} = appOptions;

$.ajax.restore();
const ajaxStub = sinon.stub($, 'ajax');
ajaxStub
.withArgs(
`/api/user_app_options/${scriptName}/${lessonPosition}/${levelPosition}/${serverLevelId}?get_channel_id=${!!levelRequiresChannel &&
!channel}`
)
.returns(userAppOptionsResponse);

ajaxStub
.withArgs(
`/api/example_solutions/${serverScriptLevelId}/${serverLevelId}?section_id=`
)
.returns(exampleSolutionsResponse);
ajaxStub.onCall(0).returns(userAppOptionsResponse);
ajaxStub.onCall(1).returns(exampleSolutionsResponse);
};

it('loads attempt stored under server level id', done => {
Expand Down Expand Up @@ -133,21 +122,13 @@ describe('loadApp.js', () => {
const appOptionsData = document.createElement('script');
appOptionsData.setAttribute('data-appoptions', JSON.stringify(appOptions));
document.body.appendChild(appOptionsData);
const queryParamsStub = sinon
.stub(clientState, 'queryParams')
.callsFake(param => {
if (param === 'solution') {
return 'true';
}
return undefined;
});
stubQueryParams('solution', 'true');

loadAppOptions().then(() => {
expect(window.appOptions.level.lastAttempt).to.be.undefined;
expect(readLevelId).to.be.undefined;

document.body.removeChild(appOptionsData);
queryParamsStub.restore();
done();
});
});
Expand All @@ -156,23 +137,17 @@ describe('loadApp.js', () => {
const appOptionsData = document.createElement('script');
appOptionsData.setAttribute('data-appoptions', JSON.stringify(appOptions));
document.body.appendChild(appOptionsData);
const queryParamsStub = sinon
.stub(clientState, 'queryParams')
.callsFake(param => {
if (param === 'user_id') {
return 'true';
}
return undefined;
});
stubQueryParams('user_id', '12345');

loadAppOptions().then(() => {
expect(window.appOptions.level.lastAttempt).to.be.undefined;
expect(readLevelId).to.be.undefined;
loadAppOptions()
.then(() => {
expect(window.appOptions.level.lastAttempt).to.be.undefined;
expect(readLevelId).to.be.undefined;

document.body.removeChild(appOptionsData);
queryParamsStub.restore();
done();
});
document.body.removeChild(appOptionsData);
done();
})
.catch(err => done(err));
});

it('passes get_channel_id true to load user progress if appOptions has levelRequiresChannel true and no channel', done => {
Expand All @@ -196,11 +171,13 @@ describe('loadApp.js', () => {
appOptionsData.setAttribute('data-appoptions', JSON.stringify(appOptions));
document.body.appendChild(appOptionsData);

loadAppOptions().then(() => {
expect(window.appOptions.channel).to.equal(responseChannel);
document.body.removeChild(appOptionsData);
done();
});
loadAppOptions()
.then(() => {
expect(window.appOptions.channel).to.equal(responseChannel);
document.body.removeChild(appOptionsData);
done();
})
.catch(err => done(err));
});

it('calls example_solutions endpoint and sets example solutions to appOptions', done => {
Expand Down
10 changes: 0 additions & 10 deletions dashboard/app/controllers/activities_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,6 @@ def milestone
user_level: @user_level
)

if solved
slog(
tag: 'activity_finish',
script_level_id: @script_level.try(:id),
level_id: @level.id,
user_agent: request.user_agent,
locale: locale
)
end

# log this at the end so that server errors (which might be caused by invalid input) prevent logging
log_milestone(@level_source, params)
end
Expand Down
115 changes: 75 additions & 40 deletions dashboard/app/controllers/api_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,14 @@ def script_standards
# Return a JSON summary of the user's progress for params[:script].
def user_progress
if current_user
if params[:user_id].present?
user = User.find(params[:user_id])
return head :forbidden unless user.student_of?(current_user)
else
user = current_user
end

script = Script.get_from_cache(params[:script])
user = params[:user_id].present? ? User.find(params[:user_id]) : current_user
teacher_viewing_student = !current_user.student? && current_user.students.include?(user)
render json: summarize_user_progress(script, user).merge(
{
Expand All @@ -444,66 +450,95 @@ def user_progress
# Returns app_options values that are user-specific. This is used on cached
# levels.
def user_app_options
response = user_summary(current_user)
response[:signedIn] = !current_user.nil?
response = {}

script = Script.get_from_cache(params[:script])
lesson = script.lessons[params[:lesson_position].to_i - 1]
script_level = lesson.cached_script_levels[params[:level_position].to_i - 1]
level = params[:level] ? Script.cache_find_level(params[:level].to_i) : script_level.oldest_active_level

if current_user
user_level = current_user.last_attempt(level, script)
level_source = user_level.try(:level_source).try(:data)

# Temporarily return the full set of progress so we can overwrite what the sessionStorage changed
response[:progress] = summarize_user_progress(script, current_user)[:progress]
response[:isHoc] = script.hoc?

if user_level
response[:lastAttempt] = {
timestamp: user_level.updated_at.to_datetime.to_milliseconds,
source: level_source
}
response[:signedIn] = true

# Set `user` to the user that we should use to calculate the app_options
# and note if the signed-in user is viewing another user (e.g. a teacher
# viewing a student's work).
if params[:user_id].present?
user = User.find(params[:user_id])
return head :forbidden unless can?(:view_as_user, script_level, user)
viewing_other_user = true
else
user = current_user
viewing_other_user = false
end

# Pairing info
is_navigator = user_level.navigator?
if is_navigator
driver = user_level.driver
driver_level_source_id = user_level.driver_level_source_id
end
if viewing_other_user
response[:isStarted] = level_started?(level, script, user)

response[:isNavigator] = is_navigator
if driver
response[:pairingDriver] = driver.name
if driver_level_source_id
response[:pairingAttempt] = edit_level_source_path(driver_level_source_id)
elsif level.channel_backed?
response[:pairingChannelId] = get_channel_for(level, script.id, driver)
end
end
# This is analogous to readonly_view_options
response[:skipInstructionsPopup] = true
response[:readonlyWorkspace] = true
response[:callouts] = []
end
end

if level.finishable?
slog(
tag: 'activity_start',
script_level_id: script_level.try(:id),
level_id: level.contained_levels.empty? ? level.id : level.contained_levels.first.id,
user_agent: request.user_agent&.valid_encoding? ? request.user_agent : 'invalid_encoding',
locale: locale
)
# TODO: There are many other user-specific values in app_options that may
# need to be sent down. See LP-2086 for a list of potential values.

response[:disableSocialShare] = !!user.under_13?
response.merge!(progress_app_options(script, level, user))
else
response[:signedIn] = false
viewing_other_user = false
end

if params[:get_channel_id] == "true"
response[:channel] = get_channel_for(level, script.id, current_user)
response[:channel] = viewing_other_user ?
get_channel_for(level, script.id, user) :
get_channel_for(level, script.id)
response[:reduceChannelUpdates] =
!Gatekeeper.allows("updateChannelOnSave", where: {script_name: script.name}, default: true)
end

render json: response
end

# Gets progress-related app_options for the given script and level for the
# given user. This code is analogous to parts of LevelsHelper#app_options.
# TODO: Eliminate this logic from LevelsHelper#app_options or refactor methods
# to share code.
def progress_app_options(script, level, user)
response = {}

user_level = user.last_attempt(level, script)
level_source = user_level.try(:level_source).try(:data)

if user_level
response[:lastAttempt] = {
timestamp: user_level.updated_at.to_datetime.to_milliseconds,
source: level_source
}

# Pairing info
is_navigator = user_level.navigator?
if is_navigator
driver = user_level.driver
driver_level_source_id = user_level.driver_level_source_id
end

response[:isNavigator] = is_navigator
if driver
response[:pairingDriver] = driver.name
if driver_level_source_id
response[:pairingAttempt] = edit_level_source_path(driver_level_source_id)
elsif level.channel_backed?
response[:pairingChannelId] = get_channel_for(level, script.id, driver)
end
end
end

response
end

# GET /api/example_solutions/:script_level_id/:level_id
def example_solutions
script_level = Script.cache_find_script_level params[:script_level_id].to_i
Expand Down

0 comments on commit c8da451

Please sign in to comment.