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

Call API to import a Google Classroom on "Import Section" click #16385

Merged
merged 6 commits into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
"reactabular": "^2.0.3",
"redux": "^3.3.1",
"redux-logger": "^2.6.1",
"redux-mock-store": "^1.2.3",
"redux-thunk": "^2.0.1",
"rgbcolor": "0.0.4",
"sanitize-html": "^1.11.3",
Expand Down
27 changes: 16 additions & 11 deletions apps/src/templates/teacherDashboard/RosterDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ LoadError.propTypes = {

export default class RosterDialog extends React.Component {
static propTypes = {
handleClose: React.PropTypes.func,
handleImport: React.PropTypes.func,
handleCancel: React.PropTypes.func,
isOpen: React.PropTypes.bool,
classrooms: React.PropTypes.arrayOf(classroomShape),
loadError: loadErrorShape,
Expand All @@ -118,24 +119,27 @@ export default class RosterDialog extends React.Component {
constructor(props) {
super(props);
this.state = {};
this.handleClose = this.handleClose.bind(this);
this.onClassroomSelected = this.onClassroomSelected.bind(this);
}

handleClose() {
this.props.handleClose(this.state.selectedId);
}
importClassroom = () => {
this.props.handleImport(this.state.selectedId);
this.setState({selectedId: null});
};

cancel = () => {
this.props.handleCancel();
};

onClassroomSelected(id) {
onClassroomSelected = id => {
this.setState({selectedId: id});
}
};

render() {
return (
<BaseDialog
useUpdatedStyles
isOpen={this.props.isOpen}
handleClose={this.handleClose}
handleClose={this.cancel}
assetUrl={() => ''}
{...this.props}
>
Expand All @@ -159,17 +163,18 @@ export default class RosterDialog extends React.Component {
</div>
<div style={styles.footer}>
<button
onClick={this.handleClose}
onClick={this.cancel}
style={{...styles.buttonPrimary, ...styles.buttonSecondary}}
>
{locale.dialogCancel()}
</button>
<button
onClick={this.handleClose}
onClick={this.importClassroom}
style={Object.assign({},
styles.buttonPrimary,
!this.state.selectedId && {opacity: 0.5}
)}
disabled={!this.state.selectedId}
>
{locale.chooseSection()}
</button>
Expand Down
3 changes: 2 additions & 1 deletion apps/src/templates/teacherDashboard/RosterDialog.story.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ const ExampleDialogButton = React.createClass({
<div>
<RosterDialog
isOpen={!!this.state && this.state.open}
handleClose={() => this.setState({open: false})}
handleImport={() => this.setState({open: false})}
handleCancel={() => this.setState({open: false})}
studioUrl=""
{...this.props}
/>
Expand Down
25 changes: 19 additions & 6 deletions apps/src/templates/teacherDashboard/SectionsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import SectionTable from './SectionTable';
import RosterDialog from './RosterDialog';
import ProgressButton from '@cdo/apps/templates/progress/ProgressButton';
import { setSections, setValidAssignments, newSection } from './teacherSectionsRedux';
import { loadClassroomList } from './googleClassroomRedux';
import { loadClassroomList, importClassroomStarted } from './googleClassroomRedux';
import { classroomShape, loadErrorShape } from './shapes';
import i18n from '@cdo/locale';
import experiments from '@cdo/apps/util/experiments';
Expand Down Expand Up @@ -35,6 +35,7 @@ class SectionsPage extends Component {
setSections: PropTypes.func.isRequired,
setValidAssignments: PropTypes.func.isRequired,
loadClassroomList: PropTypes.func.isRequired,
importClassroomStarted: PropTypes.func.isRequired,
};

state = {
Expand Down Expand Up @@ -71,10 +72,21 @@ class SectionsPage extends Component {
this.props.loadClassroomList();
};

handleImportClose = (selectedId) => {
handleImportCancel = () => {
this.setState({rosterDialogOpen: false});
// TODO (josh): use `selectedId`.
console.log(selectedId);
};

handleImport = courseId => {
this.props.importClassroomStarted();

$.getJSON('/dashboardapi/import_google_classroom', { courseId }).then(() => {
this.setState({rosterDialogOpen: false, sectionsLoaded: false});

$.getJSON("/v2/sections/").done(results => {
this.props.setSections(results, true);
this.setState({sectionsLoaded: true});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Both of these APIs set sectionsLoaded. If the call to /v2/sections finishes first, we end up with sectionsLoaded = true. If import_google_classroom finishes first, we end up with sectionsLoaded = false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I follow. The JSON calls are nested, so the /v2/sections request won't start until after the first request resolves.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad. Didn't realize they were nested.

});
};

addSection = () => this.props.newSection();
Expand Down Expand Up @@ -120,7 +132,8 @@ class SectionsPage extends Component {
{sectionsLoaded && numSections > 0 && <SectionTable/>}
<RosterDialog
isOpen={this.state.rosterDialogOpen}
handleClose={this.handleImportClose}
handleImport={this.handleImport}
handleCancel={this.handleImportCancel}
classrooms={this.props.classrooms}
loadError={this.props.loadError}
studioUrl={this.props.studioUrl}
Expand All @@ -136,4 +149,4 @@ export default connect(state => ({
studioUrl: state.teacherSections.studioUrl,
classrooms: state.googleClassroom.classrooms,
loadError: state.googleClassroom.loadError,
}), { newSection, setSections, setValidAssignments, loadClassroomList })(SectionsPage);
}), { newSection, setSections, setValidAssignments, loadClassroomList, importClassroomStarted })(SectionsPage);
12 changes: 12 additions & 0 deletions apps/src/templates/teacherDashboard/googleClassroomRedux.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import $ from 'jquery';

const SET_CLASSROOM_LIST = 'teacherDashboard/SET_CLASSROOM_LIST';
const IMPORT_CLASSROOM_STARTED = 'teacherDashboard/IMPORT_CLASSROOM_STARTED';
const FAILED_LOAD = 'teacherDashboard/FAILED_LOAD';

export const loadClassroomList = () => {
Expand All @@ -14,6 +17,8 @@ export const loadClassroomList = () => {

export const setClassroomList = classrooms => ({ type: SET_CLASSROOM_LIST, classrooms });

export const importClassroomStarted = () => ({ type: IMPORT_CLASSROOM_STARTED });

export const failedLoad = (status, message) => ({ type: FAILED_LOAD, status, message });

const initialState = {
Expand All @@ -28,6 +33,13 @@ export default function googleClassroom(state = initialState, action) {
};
}

if (action.type === IMPORT_CLASSROOM_STARTED) {
return {
...state,
classrooms: null,
};
}

if (action.type === FAILED_LOAD) {
return {
...state,
Expand Down
3 changes: 2 additions & 1 deletion apps/src/templates/teacherDashboard/shapes.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { PropTypes } from 'react';
import { SectionLoginType } from '@cdo/apps/util/sharedConstants';

export const sectionShape = PropTypes.shape({
id: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
// Though we validate valid login types here, the server actually owns the
// canonical list, and passes us the list of valid login types.
loginType: PropTypes.oneOf(['word', 'email', 'picture']).isRequired,
loginType: PropTypes.oneOf(Object.keys(SectionLoginType)).isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

stageExtras: PropTypes.bool.isRequired,
pairingAllowed: PropTypes.bool.isRequired,
studentNames: PropTypes.arrayOf(PropTypes.string).isRequired,
Expand Down
12 changes: 10 additions & 2 deletions apps/src/templates/teacherDashboard/teacherSectionsRedux.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ export const setValidAssignments = (validCourses, validScripts) => ({
validCourses,
validScripts
});
export const setSections = sections => ({ type: SET_SECTIONS, sections });

/**
* Set the list of sections to display. If `reset` is true, first clear the
* existing list.
* @param sections
* @param reset
*/
export const setSections = (sections, reset = false) => ({ type: SET_SECTIONS, sections, reset });
export const updateSection = (sectionId, serverSection) => ({
type: UPDATE_SECTION,
sectionId,
Expand Down Expand Up @@ -114,9 +121,10 @@ export default function teacherSections(state=initialState, action) {
if (action.type === SET_SECTIONS) {
const sections = action.sections.map(section =>
sectionFromServerSection(section));
const prevSectionIds = action.reset ? [] : state.sectionIds;
return {
...state,
sectionIds: state.sectionIds.concat(sections.map(section => section.id)),
sectionIds: prevSectionIds.concat(sections.map(section => section.id)),
Copy link
Contributor

Choose a reason for hiding this comment

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

might be good to get some test coverage of this reset behavior in teacherSectionsReduxTest.js

sections: {
...state.sections,
..._.keyBy(sections, 'id')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const defaultProps = {
setValidAssignments: () => {},
loadClassroomList: () => {},
studioUrl: '',
importClassroomStarted: () => {},
};

describe('SectionsPage', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { assert } from '../../../util/configuredChai';
import sinon from 'sinon';
import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';

import reducer, {
loadClassroomList,
setClassroomList,
failedLoad,
importClassroomStarted,
} from '@cdo/apps/templates/teacherDashboard/googleClassroomRedux';

describe('googleClassroomRedux', () => {
const initialState = reducer(undefined, {});

describe('loadClassroomList', () => {
it('loads the classroom list from the API', finish => {
const data = {
courses: [{id: "6949959535", section: "123", name: "Sample class", enrollment_code: "gc12wb"}]
};

const xhr = sinon.useFakeXMLHttpRequest();
xhr.onCreate = request => {
setTimeout(() => {
const mockResponse = JSON.stringify(data);
request.respond(200, {'Content-Type': 'application/json'}, mockResponse);

assert.deepEqual([setClassroomList(data.courses)], store.getActions());
finish();
}, 0);
};

const mockStore = configureStore([thunk]);
const store = mockStore({});

store.dispatch(loadClassroomList());
});

it('fails to load the classroom list from the API', finish => {
const failureStatus = 403;
const failureMessage = 'Sample error message';

const xhr = sinon.useFakeXMLHttpRequest();
xhr.onCreate = request => {
setTimeout(() => {
const mockResponse = JSON.stringify({error: failureMessage});
request.respond(failureStatus, {'Content-Type': 'application/json'}, mockResponse);

assert.deepEqual([failedLoad(failureStatus, failureMessage)], store.getActions());
finish();
}, 0);
};

const mockStore = configureStore([thunk]);
const store = mockStore({});

store.dispatch(loadClassroomList());
});
});

describe('importClassroomStarted', () => {
it('starting import sets the list of classrooms to null', () => {
const state = reducer(initialState, setClassroomList(['fake classrooms']));
assert.deepEqual(state.classrooms, ['fake classrooms']);

const nextState = reducer(state, importClassroomStarted());
assert.equal(nextState.classrooms, null);
});
});
});
4 changes: 4 additions & 0 deletions apps/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7816,6 +7816,10 @@ redux-logger@^2.6.1:
dependencies:
deep-diff "0.3.4"

redux-mock-store@^1.2.3:
version "1.2.3"
resolved "https://registry.yarnpkg.com/redux-mock-store/-/redux-mock-store-1.2.3.tgz#1b3ad299da91cb41ba30d68e3b6f024475fb9e1b"

redux-thunk@^2.0.1:
version "2.1.0"
resolved "https://registry.yarnpkg.com/redux-thunk/-/redux-thunk-2.1.0.tgz#c724bfee75dbe352da2e3ba9bc14302badd89a98"
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/controllers/api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def google_classrooms
end

def import_google_classroom
course_id = params[:course_id].to_s
course_id = params[:courseId].to_s

query_google_classroom_service do |service|
students = service.list_course_students(course_id).students.map do |student|
Expand Down
1 change: 1 addition & 0 deletions lib/cdo/shared_constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ module SharedConstants
word: 'word',
picture: 'picture',
email: 'email',
google_classroom: 'google_classroom',
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ content-type: text/ng-template
-# Just an invisible element for some padding to keep things looking good when we're not showing tabs.
&nbsp;

.managebuttons{style: "float:right; margin-bottom: 9px;", "ng-if" => "tab == 'manage'"}
.managebuttons{style: "float:right; margin-bottom: 9px;", "ng-if" => "tab == 'manage' && section.login_type !== 'google_classroom'"}
%span{"ng-if" => "section.login_type !== 'email'"}
%button.btn.btn-white{"ng-click" => "new_student()", "ng-disabled" => "bulk_import.editing"}= I18n.t('dashboard_students_add_student')
%button.btn.btn-white{"ng-click" => "bulk_import.editing = true", "ng-disabled" => "bulk_import.editing"}= I18n.t('dashboard_students_add_students')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ content-type: text/ng-template
%th.manage-th{style: "padding-top:5px;padding-bottom:5px; height:45px"}
%button.btn.btn-primary{"ng-click" => "save(section.students)", "ng-show" => "editingAny(section.students)", style: "float:left", "ng-disabled" => "allForm.$invalid"}= I18n.t('dashboard_action_save_all')

%div{"ng-if" => "section.login_type !== 'email'"}
%div{"ng-if" => "section.login_type === 'picture' || section.login_type === 'word'"}
%h3{"ng-show" => 'section.students.length > 0'}
= I18n.t 'dashboard_students_share_section', section_code: '{{section.code}}', join_url: CDO.code_org_url('/join', 'http:')
%br/
Expand Down