Skip to content

Commit

Permalink
modernize canvas_quizzes
Browse files Browse the repository at this point in the history
fixes FOO-1409
flag  = none

no more client_apps, canvas_quizzes now lives as part of canvas-lms
proper inside app/jsx/, which makes the build leaner and leaves us with
one less thing to reason about

logical changes:

- converted from AMD to ES modules
- upgraded to recent react + react-router
- dropped RSVP in favor of native Promises
- used CanvasModal instead of home-grown Dialog
- removed dead code; notifications in particular were fishy as there had
  no dependents at all and did not even show up in the graph
- ported tests to Jest, added more unit ones and two integration ones
- removed "config.onError" and now throws errors where appropriate
- disabled console statements in non-dev

:: test plan ::

- create a (old-school) quiz containing all types of questions
- as 3 distinct students, take the quiz and try to randomize your
  answers

at this point it's helpful to have a reference to compare the screens; I
replicated the quiz on my production sandbox for this

- go to /courses/:id/quizzes/:id/submissions/:id/log
  - verify it looks OK
  - click on a specific question in the stream and verify the question
    inspector widget works OK
  - go back to stream and push "View table"
  - verify the table and its controls are OK

- go to /courses/:id/quizzes/:id/statistics
  - verify it looks OK
  - click on ? in the discrimination index chart and verify it displays
    a dialog with help content
  - click on "X respondents" in one of the charts and verify it displays
    a dialog with the respondent names
  - verify the interactive charts do interact as expected (no logic
    changed here so just a quick glance)
  - link to "View in SpeedGrader" for essay-like questions works

Change-Id: I79af5ff4f1479503b5e2528b613255dde5bc45d3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256118
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Simon Williams <[email protected]>
QA-Review: Simon Williams <[email protected]>
Product-Review: Simon Williams <[email protected]>
  • Loading branch information
amireh committed Jan 14, 2021
1 parent a4e1da3 commit 46f8efd
Show file tree
Hide file tree
Showing 461 changed files with 10,670 additions and 46,755 deletions.
4 changes: 0 additions & 4 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ docker-compose/
docker-compose.local.*.yml
docker-compose.override.yml

client_apps/canvas_quizzes/dist/
client_apps/canvas_quizzes/node_modules/
client_apps/canvas_quizzes/tmp/
coverage/
coverage-js/
gems/*/node_modules/
Expand All @@ -32,7 +29,6 @@ packages/*/node_modules/
packages/canvas-planner/lib
public/dist/
public/doc/
public/javascripts/client_apps/
public/javascripts/translations/
!public/javascripts/translations/_core_en.js
tmp/*
Expand Down
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/app/jsx/canvas_quizzes/**/*.test.js
/gems/canvas_i18nliner/*
/doc/*
/public/javascripts/translations/*
/public/javascripts/vendor/*
/public/javascripts/bower/*
public/javascripts/react-dnd-test-backend.js
public/javascripts/mathquill.js
public/javascripts/symlink_to_node_modules/*

spec/selenium/helpers/jquery.simulate.js
spec/javascripts/support/jquery.mockjax.js
Expand Down
60 changes: 59 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,55 @@ module.exports = {
'react/no-render-return-value': 'warn', // In future versions of react this will fail
'react/state-in-constructor': 'off',
'react/static-property-placement': 'off',

// don't restrict Math.pow for ** operator
// ref: https://github.com/airbnb/javascript/blob/1f786e154f6c32385607e1688370d7f2d053f88f/packages/eslint-config-airbnb-base/rules/best-practices.js#L225
'no-restricted-properties': ['error',
{
object: 'arguments',
property: 'callee',
message: 'arguments.callee is deprecated',
},
{
object: 'global',
property: 'isFinite',
message: 'Please use Number.isFinite instead',
},
{
object: 'self',
property: 'isFinite',
message: 'Please use Number.isFinite instead',
},
{
object: 'window',
property: 'isFinite',
message: 'Please use Number.isFinite instead',
},
{
object: 'global',
property: 'isNaN',
message: 'Please use Number.isNaN instead',
},
{
object: 'self',
property: 'isNaN',
message: 'Please use Number.isNaN instead',
},
{
object: 'window',
property: 'isNaN',
message: 'Please use Number.isNaN instead',
},
{
property: '__defineGetter__',
message: 'Please use Object.defineProperty instead.',
},
{
property: '__defineSetter__',
message: 'Please use Object.defineProperty instead.',
},
],

'no-restricted-syntax': [
// This is here because we are turning off 2 items from what AirBnB cares about.
'error',
Expand Down Expand Up @@ -183,6 +232,15 @@ module.exports = {
'import/no-unresolved': 'off',
'import/no-webpack-loader-syntax': 'off'
}
}
},
{
files: ['app/jsx/canvas_quizzes/**/*'],
rules: {
'react/prop-types': 'off',
'prefer-const': 'warn',
'prettier/prettier': 'off',
'react/no-string-refs': 'warn',
}
},
]
}
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ yarn-error.log
/spec/coffeescripts/plugins/
/spec/plugins/

# generated client app stuff
/public/javascripts/client_apps/

# canvas-gems
/gems/*/coverage
/gems/*/tmp
Expand Down
1 change: 0 additions & 1 deletion .i18nrc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

"include": [
"app/coffeescripts/.i18nrc",
"client_apps/canvas_quizzes/.i18nrc",
"gems/plugins/.i18nrc",
"public/javascripts/.i18nrc"
]
Expand Down
1 change: 0 additions & 1 deletion .stylelintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"ignoreFiles": [
"app/stylesheets/vendor/**",
"client_apps/canvas_quizzes/vendor/**",
"public/**",
],
"rules": {
Expand Down
4 changes: 0 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ RUN set -eux; \
.yardoc \
app/stylesheets/brandable_css_brands \
app/views/info \
client_apps/canvas_quizzes/dist \
client_apps/canvas_quizzes/node_modules \
client_apps/canvas_quizzes/tmp \
config/locales/generated \
gems/canvas_i18nliner/node_modules \
log \
Expand All @@ -92,7 +89,6 @@ RUN set -eux; \
pacts \
public/dist \
public/doc/api \
public/javascripts/client_apps \
public/javascripts/compiled \
public/javascripts/translations \
reports \
Expand Down
3 changes: 0 additions & 3 deletions Dockerfile.jenkins-cache
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ RUN --mount=target=/tmp/src \
\
/tmp/dst && \
find \
client_apps/* \
gems/canvas_i18nliner \
gems/plugins/* \
packages/* \
Expand Down Expand Up @@ -76,7 +75,6 @@ RUN --mount=target=/tmp/src \
app/stylesheets \
app/views/jst \
bin \
client_apps \
config/environments \
config/locales \
frontend_build \
Expand All @@ -92,7 +90,6 @@ RUN --mount=target=/tmp/src \
config/canvas_rails_switcher.rb \
config/environment.rb \
config/initializers/plugin_symlinks.rb \
config/initializers/client_app_symlinks.rb \
config/initializers/json.rb \
config/initializers/revved_asset_urls.rb \
db/migrate/*_regenerate_brand_files_based_on_new_defaults_*.rb \
Expand Down
4 changes: 0 additions & 4 deletions Jenkinsfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ pipeline {
}
}

tests['canvas_quizzes'] = {
sh 'build/new-jenkins/js/tests-quizzes.sh'
}

for(int i = 0; i < JSG_NODE_COUNT; i++) {
tests["Karma - Spec Group - jsg${i}"] = makeKarmaStage('jsg', i, JSG_NODE_COUNT)
}
Expand Down
1 change: 1 addition & 0 deletions app/jsx/.i18nignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/canvas_quizzes/test
15 changes: 14 additions & 1 deletion app/jsx/bundles/quiz_statistics_cqs.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,17 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import 'quiz_statistics_cqs'
import $ from 'jquery'
import { configure, mount } from '../canvas_quizzes/statistics/main.js'

configure({
ajax: $.ajax,
loadOnStartup: true,
quizStatisticsUrl: ENV.quiz_statistics_url,
quizReportsUrl: ENV.quiz_reports_url,
courseSectionsUrl: ENV.course_sections_url
})

mount(document.body.querySelector('#content')).then(() => {
console.log('Yeah!!!')
})
10 changes: 5 additions & 5 deletions app/jsx/bundles/quiz_submission_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
*/

import $ from 'jquery'
import app from 'canvas_quizzes/apps/events'
import { configure, mount } from '../canvas_quizzes/events/main.js'
import ready from '@instructure/ready'

ready(() => {
app.configure({
configure({
ajax: $.ajax,
loadOnStartup: true,
quizUrl: ENV.quiz_url,
Expand All @@ -31,7 +31,7 @@ ready(() => {
allowMatrixView: ENV.can_view_answer_audits
})

app
.mount(document.body.querySelector('#content'))
.then(() => console.log('Yeah, a canvas quiz app has been loaded!!!'))
mount(document.body.querySelector('#content')).then(() =>
console.log('Yeah, a canvas quiz app has been loaded!!!')
)
})
100 changes: 100 additions & 0 deletions app/jsx/canvas_quizzes/events/__tests__/main.test.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2014 - present Instructure, Inc.
* Copyright (C) 2021 - present Instructure, Inc.
*
* This file is part of Canvas.
*
Expand All @@ -16,23 +16,21 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/

define(function(require) {
var $ = require('jquery');
import Dispatcher from './core/dispatcher'
import EventStore from './stores/events'

// We're already logging errors in config/initializers/rsvp.js
if(typeof(jasmine) !== "undefined" && typeof(jasmine !== undefined)){
jasmine.RSVP.logRSVPErrors = false;
}
const Actions = {}

return {
ajax: $.ajax,
Actions.dismissNotification = function(key) {
return Dispatcher.dispatch('notifications:dismiss', key)
}

xhr: {
timeout: 25
},
Actions.reloadEvents = function() {
EventStore.load()
}

onError: function(message) {
throw new Error(message);
}
};
});
Actions.setActiveAttempt = function(attempt) {
EventStore.setActiveAttempt(attempt)
}

export default Actions
64 changes: 64 additions & 0 deletions app/jsx/canvas_quizzes/events/bundles/routes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (C) 2021 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react'
import {
BrowserRouter,
HashRouter,
Switch,
Route,
} from 'react-router-dom'

import AnswerMatrixRoute from '../routes/answer_matrix'
import AppRoute from '../routes/app'
import EventStreamRoute from '../routes/event_stream'
import QuestionRoute from '../routes/question'

export default function App(props) {
const matches = window.location.pathname.match(/(.*\/log)/)
const baseUrl = (matches && matches[0]) || ''
const Router = props.useHashRouter ? HashRouter : BrowserRouter

return (
<Router basename={baseUrl}>
<Switch>
<Route path="/questions/:id">
<AppRoute>
<QuestionRoute {...props} />
</AppRoute>
</Route>

<Route path="/answer_matrix">
<AppRoute>
<AnswerMatrixRoute {...props} />
</AppRoute>
</Route>

<Route path="/">
<AppRoute>
<EventStreamRoute {...props} />
</AppRoute>
</Route>

<Route path="*">
<AppRoute />
</Route>
</Switch>
</Router>
)
}
40 changes: 40 additions & 0 deletions app/jsx/canvas_quizzes/events/collections/events.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (C) 2021 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import Backbone from 'backbone'
import Event from '../models/event'
import fromJSONAPI from '../../shared/models/common/from_jsonapi'
import config from '../config'
import PaginatedCollection from '../mixins/paginated_collection'

export default Backbone.Collection.extend({
model: Event,
// eslint-disable-next-line object-shorthand
constructor: function() {
PaginatedCollection(this)
return Backbone.Collection.apply(this, arguments)
},

url() {
return config.eventsUrl
},

parse(payload) {
return fromJSONAPI(payload, 'quiz_submission_events')
}
})
Loading

0 comments on commit 46f8efd

Please sign in to comment.