Skip to content

Commit

Permalink
Merge pull request #27900 from code-dot-org/student-expand-bug
Browse files Browse the repository at this point in the history 
Teacher Feedback: Don't load tab until we have all the content from network
  • Loading branch information
@dmcavoy
dmcavoy committed Apr 9, 2019
2 parents b27b430 + 4e816f4 commit 2647fb3
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 166 deletions.
57 changes: 16 additions & 41 deletions apps/src/templates/instructions/TeacherFeedback.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ export class TeacherFeedback extends Component {
viewAs: PropTypes.oneOf(['Teacher', 'Student']).isRequired,
serverLevelId: PropTypes.number,
teacher: PropTypes.number,
displayKeyConcept: PropTypes.bool
displayKeyConcept: PropTypes.bool,
latestFeedback: PropTypes.array,
token: PropTypes.string
};

constructor(props) {
Expand All @@ -109,57 +111,30 @@ export class TeacherFeedback extends Component {
this.onRubricChange = this.onRubricChange.bind(this);

this.state = {
comment: '',
performance: null,
comment:
this.props.latestFeedback[0] && this.props.latestFeedback[0].comment
? this.props.latestFeedback[0].comment
: '',
performance:
this.props.latestFeedback[0] && this.props.latestFeedback[0].performance
? this.props.latestFeedback[0].performance
: null,
studentId: studentId,
latestFeedback: [],
latestFeedback: this.props.latestFeedback
? this.props.latestFeedback
: [],
submitting: false,
errorState: ErrorType.NoError,
token: null
errorState: ErrorType.NoError
};
}

componentDidMount = () => {
const {user, serverLevelId, teacher} = this.props;
const {studentId} = this.state;

window.addEventListener('beforeunload', event => {
if (!this.feedbackIsUnchanged()) {
event.preventDefault();
event.returnValue = i18n.feedbackNotSavedWarning();
}
});

if (this.props.viewAs === ViewType.Student) {
$.ajax({
url: `/api/v1/teacher_feedbacks/get_feedbacks?student_id=${user}&level_id=${serverLevelId}`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done(data => {
this.setState({
latestFeedback: data,
comment: data[0].comment,
performance: data[0].performance
});
});
} else if (!this.props.disabledMode) {
$.ajax({
url: `/api/v1/teacher_feedbacks/get_feedback_from_teacher?student_id=${studentId}&level_id=${serverLevelId}&teacher_id=${teacher}`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
})
.done((data, textStatus, request) => {
this.setState({
latestFeedback: request.status === 204 ? [] : [data],
token: request.getResponseHeader('csrf-token'),
comment: request.status === 204 ? '' : data.comment,
performance: request.status === 204 ? null : data.performance
});
})
.fail((jqXhr, status) => {
this.setState({errorState: ErrorType.Load});
});
}
};

componentWillUnmount() {
Expand Down Expand Up @@ -195,7 +170,7 @@ export class TeacherFeedback extends Component {
contentType: 'application/json;charset=UTF-8',
dataType: 'json',
data: JSON.stringify({teacher_feedback: payload}),
headers: {'X-CSRF-Token': this.state.token}
headers: {'X-CSRF-Token': this.props.token}
})
.done(data => {
this.setState({
Expand Down
74 changes: 53 additions & 21 deletions apps/src/templates/instructions/TopInstructionsCSP.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import HeightResizer from './HeightResizer';
import msg from '@cdo/locale';
import {ViewType} from '@cdo/apps/code-studio/viewAsRedux';
import experiments from '@cdo/apps/util/experiments';
import queryString from 'query-string';

const HEADER_HEIGHT = styleConstants['workspace-headers-height'];
const RESIZER_HEIGHT = styleConstants['resize-bar-width'];
Expand Down Expand Up @@ -127,6 +128,8 @@ class TopInstructionsCSP extends Component {

constructor(props) {
super(props);
//Pull the student id from the url
const studentId = queryString.parse(window.location.search).user_id;

const teacherViewingStudentWork =
this.props.viewAs === ViewType.Teacher &&
Expand All @@ -139,14 +142,20 @@ class TopInstructionsCSP extends Component {
: TabType.INSTRUCTIONS,
feedbacks: [],
rubric: null,
teacherViewingStudentWork: teacherViewingStudentWork
studentId: studentId,
teacherViewingStudentWork: teacherViewingStudentWork,
fetchingData: true,
token: null
};
}

/**
* Calculate our initial height (based off of rendered height of instructions)
*/
componentDidMount() {
const {user, serverLevelId} = this.props;
const {studentId} = this.state;

window.addEventListener('resize', this.adjustMaxNeededHeight);

const maxNeededHeight = this.adjustMaxNeededHeight();
Expand All @@ -155,31 +164,52 @@ class TopInstructionsCSP extends Component {
// adjusts max height.
this.props.setInstructionsRenderedHeight(Math.min(maxNeededHeight, 300));

const promises = [];

if (this.props.viewAs === ViewType.Student) {
$.ajax({
url:
'/api/v1/teacher_feedbacks/get_feedbacks?student_id=' +
this.props.user +
'&level_id=' +
this.props.serverLevelId,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done(data => {
this.setState({feedbacks: data}, this.forceTabResizeToMaxHeight);
});
promises.push(
$.ajax({
url: `/api/v1/teacher_feedbacks/get_feedbacks?student_id=${user}&level_id=${serverLevelId}`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done(data => {
this.setState({feedbacks: data});
})
);
}
//While this is behind an experiment flag we will only pull the rubric
//if the experiment is enable. This should prevent us from showing the
//rubric if not in the experiment.
if (experiments.isEnabled(experiments.MINI_RUBRIC_2019)) {
$.ajax({
url: `/levels/${this.props.serverLevelId}/get_rubric/`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done(data => {
this.setState({rubric: data}, this.forceTabResizeToMaxHeight);
});
promises.push(
$.ajax({
url: `/levels/${serverLevelId}/get_rubric/`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done(data => {
this.setState({rubric: data});
})
);
}

if (this.state.teacherViewingStudentWork) {
promises.push(
$.ajax({
url: `/api/v1/teacher_feedbacks/get_feedback_from_teacher?student_id=${studentId}&level_id=${serverLevelId}&teacher_id=${user}`,
method: 'GET',
contentType: 'application/json;charset=UTF-8'
}).done((data, textStatus, request) => {
this.setState({
feedbacks: request.status === 204 ? [] : [data],
token: request.getResponseHeader('csrf-token')
});
})
);
}

Promise.all(promises).then(() => {
this.setState({fetchingData: false}, this.forceTabResizeToMaxHeight);
});
}

componentWillUnmount() {
Expand Down Expand Up @@ -382,7 +412,7 @@ class TopInstructionsCSP extends Component {
teacherOnly={teacherOnly}
/>
)}
{displayFeedback && (
{displayFeedback && (!this.state.fetchingData || teacherOnly) && (
<InstructionsTab
className="uitest-feedback"
onClick={this.handleCommentTabClick}
Expand Down Expand Up @@ -431,7 +461,7 @@ class TopInstructionsCSP extends Component {
referenceLinks={this.props.referenceLinks}
/>
)}
{displayFeedback && (
{displayFeedback && !this.state.fetchingData && (
<TeacherFeedback
user={this.props.user}
visible={this.state.tabSelected === TabType.COMMENTS}
Expand All @@ -442,6 +472,8 @@ class TopInstructionsCSP extends Component {
}
rubric={this.state.rubric}
ref="commentTab"
latestFeedback={this.state.feedbacks}
token={this.state.token}
/>
)}
</div>
Expand Down

0 comments on commit 2647fb3

Please sign in to comment.