Skip to content
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

Progress tab: Initial work #19881

Merged
merged 14 commits into from
Jan 11, 2018
13 changes: 2 additions & 11 deletions apps/src/code-studio/progress.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { getStore } from './redux';
import { authorizeLockable } from './stageLockRedux';
import { setViewType, ViewType } from './viewAsRedux';
import { getHiddenStages } from './hiddenStageRedux';
import { LevelStatus } from '@cdo/apps/util/sharedConstants';
import { TestResults } from '@cdo/apps/constants';
import {
initProgress,
Expand All @@ -29,6 +28,7 @@ import {
setCurrentStageId,
setScriptCompleted,
setStageExtrasEnabled,
getLevelResult,
} from './progressRedux';
import { setVerified } from '@cdo/apps/code-studio/verifiedTeacherRedux';
import { renderTeacherPanel } from './teacher';
Expand Down Expand Up @@ -249,16 +249,7 @@ function queryUserProgress(store, scriptData, currentLevelId) {

// Merge progress from server (loaded via AJAX)
if (data.levels) {
const levelProgress = _.mapValues(data.levels, level => {
if (level.status === LevelStatus.locked) {
return TestResults.LOCKED_RESULT;
}
if (level.submitted || level.readonly_answers) {
return TestResults.SUBMITTED_RESULT;
}

return level.result;
});
const levelProgress = _.mapValues(data.levels, getLevelResult);
store.dispatch(mergeProgress(levelProgress));
if (data.peerReviewsPerformed) {
store.dispatch(mergePeerReviewProgress(data.peerReviewsPerformed));
Expand Down
16 changes: 16 additions & 0 deletions apps/src/code-studio/progressRedux.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,22 @@ function bestResultLevelId(levelIds, progressData) {
return bestId;
}

/**
* Given a level that we get from the server using either /api/user_progress or
* /dashboardapi/section_level_progress, extracts the result, appropriately
* discerning a locked/submitted result for certain levels.
*/
export const getLevelResult = level => {
if (level.status === LevelStatus.locked) {
return TestResults.LOCKED_RESULT;
}
if (level.submitted || level.readonly_answers) {
return TestResults.SUBMITTED_RESULT;
}

return level.result;
};

/**
* Does some processing of our passed in stages, namely
* - Removes 'hidden' field
Expand Down
76 changes: 44 additions & 32 deletions apps/src/sites/code.org/pages/public/teacher-dashboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import $ from 'jquery';
import React from 'react';
import ReactDOM from 'react-dom';
import SectionProjectsList from '@cdo/apps/templates/projects/SectionProjectsList';
import SectionProgress from '@cdo/apps/templates/teacherDashboard/SectionProgress';
import experiments from '@cdo/apps/util/experiments';
import firehoseClient from '@cdo/apps/lib/util/firehose';
import {
Expand Down Expand Up @@ -49,6 +50,16 @@ function renderSectionProjects(sectionId) {
});
}

function renderSectionProgress(section, validScripts) {
ReactDOM.render(
<SectionProgress
section={section}
validScripts={validScripts}
/>,
document.getElementById('section-progress-react')
);
}

// Everything below was copied wholesale from index.haml, where we had no linting.
// TODO (bjvanminnen): Fix remaining lint errors and re-enable rules.
/* eslint-disable eqeqeq, no-unused-vars */
Expand Down Expand Up @@ -729,32 +740,49 @@ function main() {
}
);

$scope.genericError = function (result) {
$window.alert("An unexpected error occurred, please try again. If this keeps happening, try reloading the page.");
};
$scope.tab = 'progress';
$scope.page = {zoom: false};
$scope.react_progress = false;

// We want to make our sections service request whether using react or angular
// because our tabs haml (in nav.haml) depends on the section being loaded. It
// could probably be cleaned up to behave differently without too much difficulty,
// which would make this unnecessary.
$scope.section = sectionsService.get({id: $routeParams.id});
// sections is used by our section dropdown, which at least initially will
// still be in angular
$scope.sections = sectionsService.query();
const paginatedPromise = paginatedSectionProgressService.get($routeParams.id)
.then(result => {
$scope.progress = result;
})
.catch($scope.genericError);

$scope.tab = 'progress';
$scope.page = {zoom: false};

// error handling
$scope.genericError = function (result) {
$window.alert("An unexpected error occurred, please try again. If this keeps happening, try reloading the page.");
};
$scope.section.$promise.catch($scope.genericError);
$scope.sections.$promise.catch($scope.genericError);

// the ng-select in the nav compares by reference not by value, so we can't just set
// selectedSection to section, we have to find it in sections.
$scope.sections.$promise.then(
function ( sections ){
$scope.selectedSection = $.grep(sections, function (section) { return (section.id == $routeParams.id);})[0];
}
);
$scope.sections.$promise.then(sections => {
$scope.selectedSection = sections.find(section => section.id.toString() === $routeParams.id);
});

if (experiments.isEnabled('sectionProgressRedesign')) {
$scope.react_progress = true;
$scope.$on('section-progress-rendered', () => {
$scope.section.$promise.then(script =>
renderSectionProgress(script, valid_scripts)
);
});
return;
}

// The below is not run if our experiment is not enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be more specific about the experiment here. There are a few going at a time in teacher dashboard.


const paginatedPromise = paginatedSectionProgressService.get($routeParams.id)
.then(result => {
$scope.progress = result;
})
.catch($scope.genericError);

$scope.progressLoadedFirst = false;
$scope.progressLoaded = false;
Expand Down Expand Up @@ -794,22 +822,6 @@ function main() {
return $scope.page.zoom ? Math.max(34 * $scope.progress.script.levels_count, 770) : 770;
};

// refresh progress every 30s
// TODO: 'update' progress instead of replacing it
//$interval(function() {
// if (!$scope.progressLoaded) { return; } // don't refresh if loading
// $scope.progressLoaded = false;
// if ($scope.scriptId) {
// $scope.progress = sectionsService.progress({id: $routeParams.id, script_id: $scope.scriptId});
// } else {
// $scope.progress = sectionsService.progress({id: $routeParams.id});
// }
// $q.all([$scope.progress.$promise, $scope.section.$promise]).then(function(data){
// $scope.mergeProgress();
// $scope.progressLoaded = true;
// });
// }, 30 * 1000);

$scope.scrollToStage = function ($event){
var doScroll = function () {
var element = $( $event.currentTarget );
Expand Down
62 changes: 62 additions & 0 deletions apps/src/templates/teacherDashboard/ScriptSelector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import React, { Component, PropTypes } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed this is a js file not jsx, on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either is fine. I think I usually use .js

import _ from 'lodash';

// TODO: Can/should we share any logic with AssignmentSelector?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer might be that they're just different.

This is a little confusing in that they use slightly difference sources of data for their list of scripts. AssignmentSelector uses a set of assignments that have been slightly processed, to be able to differentiate scripts and courses. In part this involves giving each one an "assignment id". That said, both have the same set of fields used for grouping I believe.


/**
* Group our assignments into categories for our dropdown
*/
const groupedAssignments = assignments => (
_(assignments)
.values()
.orderBy(['category_priority', 'category', 'position', 'name'])
.groupBy('category')
.value()
);

export default class ScriptSelector extends Component {
static propTypes = {
validScripts: PropTypes.arrayOf(PropTypes.shape({
// This shape is similar to that used by AssignmentSelector, but in that
// case they've been semi-processed and given assignIds to diferentiate
// courses and scripts
category: PropTypes.string.isRequired,
category_priority: PropTypes.number.isRequired,
id: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
position: PropTypes.number,
})).isRequired,
scriptId: PropTypes.string,
onChange: PropTypes.func.isRequired,
};

render() {
const { validScripts, scriptId, onChange } = this.props;

const grouped = groupedAssignments(validScripts);

return (
<div>
<select
value={scriptId}
onChange={event => onChange(event.target.value)}
>
<option key="default" value={''}/>
{Object.keys(grouped).map((groupName, index) => (
<optgroup key={index} label={groupName}>
{grouped[groupName].map((assignment) => (
(assignment !== undefined) &&
<option
key={assignment.id}
value={assignment.id}
>
{assignment.name}
</option>
))}
</optgroup>
))}
</select>
</div>
);
}
}
104 changes: 104 additions & 0 deletions apps/src/templates/teacherDashboard/SectionProgress.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import React, { PropTypes, Component } from 'react';
import ScriptSelector from './ScriptSelector';
import { getLevelResult } from '@cdo/apps/code-studio/progressRedux';
import SectionScriptProgress from './SectionScriptProgress';
import _ from 'lodash';

/**
* Given a particular section, this component owns figuring out which script to
* show progress for (selected via a dropdown), and querying the server for
* student progress. Child components then have the responsibility for displaying
* that progress.
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to separate the data gathering component from the UI component for the most part. That said, this component does still include the dropdown.

export default class SectionProgress extends Component {
static propTypes = {
// The section we get directly from angular right now. This gives us a
// different shape than some other places we use sections. For now, I'm just
// going to document the parts of section that we use here
section: PropTypes.shape({
id: PropTypes.number.isRequired,
students: PropTypes.arrayOf(PropTypes.shape({
id: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
})).isRequired
}).isRequired,
validScripts: PropTypes.arrayOf(PropTypes.shape({
category: PropTypes.string.isRequired,
category_priority: PropTypes.number.isRequired,
id: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
position: PropTypes.number,
})).isRequired,
};

state = {
// TODO: default to what is assigned to section, or at least come up with
// some heuristic so that we have a default
scriptId: "112",
scriptData: null,
studentLevelProgress: null,
}

componentDidMount() {
this.loadScript(this.state.scriptId);
}

onChangeScript = scriptId => {
this.setState({
scriptId,
scriptData: null,
studentLevelProgress: null,
});
this.loadScript(scriptId);
}

/**
* Query the server for script data (info about the levels in the script) and
* also for user progress on that script
*/
loadScript(scriptId) {
$.getJSON(`/dashboardapi/script_structure/${scriptId}`, scriptData => {
this.setState({
scriptData
});
});

$.getJSON(`/dashboardapi/section_level_progress/${this.props.section.id}?script_id=${scriptId}`, dataByStudent => {
// dataByStudent is an object where the keys are student.id and the values
// are a map of levelId to status
let studentLevelProgress = {};
Object.keys(dataByStudent).forEach(studentId => {
studentLevelProgress[studentId] = _.mapValues(dataByStudent[studentId], getLevelResult);
});

this.setState({
studentLevelProgress: studentLevelProgress
});
});
}

render() {
const { section, validScripts } = this.props;
const { scriptId, scriptData, studentLevelProgress } = this.state;

let levelDataInitialized = scriptData && studentLevelProgress;

return (
<div>
<ScriptSelector
validScripts={validScripts}
scriptId={scriptId}
onChange={this.onChangeScript}
/>
{!levelDataInitialized && <div>Loading...</div>}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: this is the spinner we normally use
<FontAwesome icon="spinner" className="fa-pulse fa-3x"/>

{levelDataInitialized &&
<SectionScriptProgress
section={section}
scriptData={scriptData}
studentLevelProgress={studentLevelProgress}
/>
}
</div>
);
}
}