We do this often enough in our actual code that I don’t think it is
worth the noise we create complaining about it. I feel it conditions
people to ignore other stuff that does matter.
I also feel it is a valid thing to do and forcing people to use new
variable names just to appease this rule is not helpful.
Here’s the docs about why the rule exists:
https://eslint.org/docs/rules/no-param-reassign
What say ye’all? Can we axe this?
Change-Id: Ib9e3c54e5a02d3bbc892d875448d71ae87b5e6c1
Reviewed-on: https://gerrit.instructure.com/167894
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
Change-Id: I39e504382d4cc3e4161c578b7dbb70bb67deb0fd
Reviewed-on: https://gerrit.instructure.com/168309
Reviewed-by: Steven Burnett <sburnett@instructure.com>
Tested-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
fixes CORE-119
Test plan
- Load up the rich content editor
- Ensure you can still select the same
files and embed them into the editor that
you could previously
- Ensure you can see paginated items in folders
- Ensure you can upload a file to folders that
you have permission to upload to
- Ensure you can't access locked folders
- Ensure UI is accessible
Change-Id: Ief8c731e04b1ee2faf07a052720d5d63aab03118
Reviewed-on: https://gerrit.instructure.com/120527
Tested-by: Jenkins
Reviewed-by: Carl Kibler <ckibler@instructure.com>
QA-Review: Carl Kibler <ckibler@instructure.com>
Product-Review: Matthew Goodwin <mattg@instructure.com>
fixes CORE-1995
so we can protect canvas against a non-responsive server at an external
school
Change-Id: I748f0ee054eccd4420ea5c8bc4196a2245812e45
Reviewed-on: https://gerrit.instructure.com/168272
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
new items appear on the page are "displayed" and "visible" after
the first click of "New Activity" button. So, we can check the
scroll position of the page to verify we're showing the right items.
add become_between to check that scroll positions are in a range
add scroll_height
add wait_for_spinner after first page load
Change-Id: If7eca702c5597f0b49c7a270589c50f58e389517
refs: ADMIN-1449
Reviewed-on: https://gerrit.instructure.com/167819
Tested-by: Jenkins
Reviewed-by: Anju Reddy <areddy@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Carl Kibler <ckibler@instructure.com>
Test Plan
Jenkins Passes
Change-Id: I4f97008398dd15ed509888683ccdc65fc7b7c256
Reviewed-on: https://gerrit.instructure.com/168227
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
Closes PLAT-3820
Test Plan:
* Specify custom fields in the customization form
* Save the key & configuration
* create a tool from that configuration and
verify the custom fields have been added
Change-Id: I00a89f7a5f7daceacb4260465e4c099da418bcb9
Reviewed-on: https://gerrit.instructure.com/167702
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
fixes COMMS-1487
Test Plan:
- As the teacher create a Scheduler appointment for that course and
divide up time slots, check the box of
'Allow students to see who has signed up for time slots that are still available.'
as well as 'limit each time slot to 1 user'
- As student 1 go in and sign up for a time slot
- As student 2 go in and notice that the time slot that was filled by student 1 is not visible,
you cannot see any users as having signed up for it because little do you know that it even exists
Change-Id: Ibac74d161c586f48457a2653e81cb742419485a4
Reviewed-on: https://gerrit.instructure.com/168003
Tested-by: Jenkins
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>
Reviewed-by: Landon Gilbert-Bland <lbland@instructure.com>
fixes COMMS-1469
Test Plan:
- As a student go submit an assignment
- try to add a media comment to the submission
- notice after the modal pops up the comment boxes doesn't go away.
Change-Id: Ia022f9afa254d2d180ecc0713a4f199edaebc56c
Reviewed-on: https://gerrit.instructure.com/167253
Tested-by: Jenkins
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>
Reviewed-by: Aaron Hsu <ahsu@instructure.com>
test plan:
- Animated UI elements should jump around quickly
when Selenium tests are running
- Animated UI elements should move normally when
using Canvas
fixes QA-566
Change-Id: I81c19ffdc45da567b91d461e33a0bdd31e48c6e7
Reviewed-on: https://gerrit.instructure.com/167849
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Tucker Mcknight <tmcknight@instructure.com>
this is the commit that actually does the syntax conversion from
coffeescript to javascript.
test plan:
* automated builds, which have javascript/selenium tests that cover
the screenreader_gradebook should pass.
* look at the log output of the linters-and-js build and verify
That the tests for “ScreenReader Gradebook” ran. (Without a change
In this commit, it was only running .coffee specs. But I changed it
So this step is to just make sure that did the right thing)
* manually try out the "individual gradebook" page and click around
to see that everything works
Change-Id: I32aeb14270ce751df49a1c7ae87cc57dceab69f6
Reviewed-on: https://gerrit.instructure.com/168020
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
this will not pass jenkins since it renames these files to .js
without also converting the syntax to javaScript. However, this is
intentionally it's own commit so that when you `git log --follow`
a file, it can track its history across the name change.
the following commit actually changes the syntax.
test plan:
* make sure the next commit passes. This should just rename things
Change-Id: Ifb010d4d414e68dea96944f4ff2386e6850a4262
Reviewed-on: https://gerrit.instructure.com/168019
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
turns out external_tool_id comes over as a string
closes #ADMIN-1483
Change-Id: I7e02d99c5936e77101405cd534bef40f231618ba
Reviewed-on: https://gerrit.instructure.com/167658
Reviewed-by: Mysti Sadler <mysti@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
Tested-by: Jenkins
QA-Review: Han Yan <hyan@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
these are more leaf-nodes that aren’t required by any other amd/coffee
files. Meaning they can be safely converted to ES6 modules
...slowly churning through everything…
Test plan:
* automated builds should pass
Change-Id: I35fcf7f8df267ae8d77f0d41305082148971c335
Reviewed-on: https://gerrit.instructure.com/168055
Reviewed-by: Marc Phillips <mphillips@instructure.com>
Tested-by: Jenkins
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
this will not pass jenkins since it renames these files to .js
without also converting the syntax to javaScript. However, this is
intentionally it's own commit so that when you `git log --follow`
a file, it can track its history across the name change.
the following commit actually changes the syntax.
test plan:
* make sure the next commit passes. This should just rename things
Change-Id: Ida326a53904703c9782559f693a672cd6d363c1b
Reviewed-on: https://gerrit.instructure.com/168054
Reviewed-by: Marc Phillips <mphillips@instructure.com>
Tested-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
g/167863 will keep the default from getting stored in multicache
but it can still collide in the in-process cache
closes #CORE-1977
Change-Id: Ic29292ad1da69af9684862a934698ed73d001937
Reviewed-on: https://gerrit.instructure.com/167920
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
These objects were originally accessible via the node/legacyNode, but
these first-class fields will be a bit more convenient to use.
Test plan:
* retrieve objects from the top-level
course/assignment/assignmentGroup fields
Change-Id: I6f64a0841a36dabd753e8e91aa4145689d56fc1b
Reviewed-on: https://gerrit.instructure.com/167469
Reviewed-by: Carl Kibler <ckibler@instructure.com>
Tested-by: Jenkins
QA-Review: Cameron Matheson <cameron@instructure.com>
Product-Review: Cameron Matheson <cameron@instructure.com>
Change-Id: Ie883048a09d91b6a6acaaa2d1870eae1b30141d7
Reviewed-on: https://gerrit.instructure.com/167980
Reviewed-by: Matthew Edwards <medwards@instructure.com>
Tested-by: Jenkins
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Previously index queries were being run against all
the context tools for a devkey, which would mean that
the account would show as being installed with a tool
if any of the courses or subaccounts had installed it.
Now it searches from the context up to see if it is
installed, not the other way around.
Test Plan:
- install a tool in an account (1.3)
- note that it shows in a course and it can't be installed
- uninstall the tool, note that it is removed in teh course
- install that tool in the course
- note that it is not installed at the account
- install the tool at course then at account, note that there
are two tools now
- remove the course tool and note that there is now only one
Change-Id: I9985f80e68dbc99747f3467b1d69ef329713cf16
Reviewed-on: https://gerrit.instructure.com/167772
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
this is only needed to protect yourself in hostile environments
where someone might do something like Object.prototype.foobar = “blah”
Because we control all the javascript that is executed on our page,
That won’t ever happen. And if it did, there are a million and a half
other places in our code that will blow up before your code blows up.
Besides, we already catch that ^ in `no-extend-native`
So complaining to you and making you handle this case is just not needed
To read the docs of why this rule exists go here:
https://eslint.org/docs/rules/guard-for-in
What say ye’all? can we axe this?
Change-Id: Ie942578e0b8a3cce8130412f866556da7b9e984c
Reviewed-on: https://gerrit.instructure.com/167896
Tested-by: Jenkins
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
These are all the js files from slickgrid that are not used in any of
our webpack bundles. So technically, we don’t need them.
Gradebook people, what would you prefer? To get rid of them so we don’t
have files sitting around in our repo that are unused or keep them
so whatever is in /vendor/slickgrid is an accurate representation of
what was copypasta’ed into our codebase?
It is up to you, I am just making this commit because I was auditing
All our other unused stuff and so you are aware. I’m good either way.
If you want me to submit this, I’ll fix up the commitmessage
Also if we are going to delete these I’m sure there are .png, .gif and
.css files in public/javascripts/vendor/slickgrid that are also unused
that could be deleted if we wanted to
Test plan:
* all the automated builds should pass
* the gradebook, gradezilla and jobs pages should work
Change-Id: Ia0be29b959c7b264c828531b5a0d6e839e9a58a3
Reviewed-on: https://gerrit.instructure.com/167745
Tested-by: Jenkins
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
The missing @API tag caused the RubricsController to be missing
from the yard `options[:objects]` which are [transformed](https://github.com/instructure/canvas-lms/blob/master/doc/api/fulldoc/html/setup.rb#L159)
into the resources which are handed to the ApiScopeMappingWriter.
Since the ApiScopeMappingWriter didn't look at the actions
provided in the RubricsController, the scopes weren't written to
lib/api_scope_mapper.rb which means that they can't be enabled in
the developer keys.
closes gh-1341
Test Plan:
- Run either `rails doc:api` or `rails canvas:compile_assets`
- Make sure the feature option Developer key management and
scoping is enabled in your top level account
- Go the the admin of your top level account
- Create a new developer key
- Enable enforce scopes
- See if the url:POST|/api/v1/courses/:course_id/rubrics and
url:PUT|/api/v1/courses/:course_id/rubrics/:id scopes are
present under the rubrics expander
Change-Id: I9e5ea784f3c5c942005789cf1c884453d3a34dd9
Reviewed-on: https://gerrit.instructure.com/167963
Tested-by: Jenkins
Reviewed-by: Matthew Edwards <medwards@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
this allow differentiation and targeting of assignments on the syllabus
page based on workflow state
closes gh-1346
Change-Id: Id543db766579a88d86ee1e1450a9727807356c48
Reviewed-on: https://gerrit.instructure.com/167969
Tested-by: Jenkins
Reviewed-by: Matthew Edwards <medwards@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
When importing a gradebook CSV and and analyzing what changes have been
made, map changes to custom columns properly and do not attempt to
include deleted or hidden columns.
fixes GRADE-1616
Test plan:
- In a course with some students, set up four custom columns
(referred to as C1 through C4); for now, don't change any
settings on them
- In new Gradebook, set some values for the columns
- Adjust them as follows:
- C1: deleted
- C2: hidden
- C3: read-only
- C4: [leave as is]
- Export a CSV
- The CSV should contain C3 and C4 but not the other two
columns (this behavior has not changed)
- Twiddle some assignment scores and custom-column values in the CSV
- Import the edited CSV and check the following on the proposed
changes screen:
- Changes to C4 are shown as expected
- Any changes to assignment scores are shown as expected
- Changes to C3 are *not* shown, and the warning message about
preventing changing read-only columns is shown at the bottom
- Otherwise, everything is as you'd expect: the importer isn't
attempting to replace a custom column with the section name
or perpetrate any similar skulduggery
Change-Id: Ib1ebcec6820120288e67c1652cb5496a8b8240a7
Reviewed-on: https://gerrit.instructure.com/167149
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Tested-by: Jenkins
when it runs into classes that have fat arrow methods, decaffeinate
Adds a hack at the top of the generated constructor function to allow
it to do `this.fatArrowMethod = this.fatArrowMethod.bind(this)` BEFORE
it calls `super` so it can recreate exactly the behavior of what
coffeescript did.
That works fine and dandy but when you run with Istanbul it adds a bunch
Of generated code to track line execution and so when it was looking
for thisFn.indexOf(';') in the hack, it finds a semicolon inserted
by istanbul
The actual app code was fine, it was just when you enable istambule that
It would break.
Long term, the better thing to do would be to modify the actual code so
It doesn’t need the hack but that requires careful examination of
Execution order and human intervention and I didn’t want to do that as
Part of a “decaffeinate” commit.
Test plan:
Run
COVERAGE=true DISABLE_HAPPYPACK=1 yarn jspec-watch \
spec/coffeescripts/views/accounts/admin_tools
* it should work
* the master build should pass after this is merged
Change-Id: Ib29f9d57134f992561f082ba75edca5c745bc91e
Reviewed-on: https://gerrit.instructure.com/167915
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
This rule is just there to protect against a pretty esoteric edge
case that we never do. So in effect, in our code, we never will run
into the case where you’ll need it
Read here for why you would need it:
https://eslint.org/docs/rules/no-prototype-builtins
What say ye’all can we axe this?
Change-Id: I4b9124dd457e843e8429eb701e1ddc77fc86c182
Reviewed-on: https://gerrit.instructure.com/167895
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
This addresses "No such file or directory" for first-time users
of script/canvas_update and script/prepare/prepare .
Test Plan:
- qa-cr
Change-Id: I65373682830c7cfb169b6d0b5436dc24542cd446
Reviewed-on: https://gerrit.instructure.com/167770
Tested-by: Jenkins
Reviewed-by: Brian Watson <bwatson@instructure.com>
QA-Review: Michael Hargiss <mhargiss@instructure.com>
Product-Review: Michael Hargiss <mhargiss@instructure.com>
[ci no-cached-dist]
Change-Id: If1db111692727de45c599697e18767dc117bb0c9
Reviewed-on: https://gerrit.instructure.com/167863
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
closes #CORE-1770
Change-Id: Ic0907e2c6d7e01158ea9ad651bc676e1732aab73
Reviewed-on: https://gerrit.instructure.com/167143
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
To figure these out, I made a custom webpack plugin that deleted any
file webpack knew about in public/ or app/.
I committed the result of that, then ran:
rm public/javascripts app/jsx app/coffeescripts
And then by looking at the things that were still there it meant that
webpack never saw them so they are not used in any bundles
Test plan:
* see if you can think of anything that actually uses any of these
And let me know if you can find anything
* automated builds should pass
Change-Id: Id4584fedf96668ba9a5310f07611a8d7b78aaf62
Reviewed-on: https://gerrit.instructure.com/167744
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
Fixes: PLAT-3853
Test Plan:
1) Add global_navigation launch with visibility set to
'admins'.
a) Go to https://canvas.instructure.com/doc/api/file.tools_xml.html
Copy XML for 'Admin/Teacher/TA/Designer-only navigation', but
change "course_navigation" to "global_navigation".
b) Add the above xml at the account level.
2) As a student (not a teacher / admin in any context) verify:
[your instance]/api/v1/accounts/self/lti_apps/launch_definitions?placements[]=global_navigation&as_user_id=[your user]
returns the global navigation lti in results.
Change-Id: If1a6c78e9a7731f3a5fc62addd5cc418e8f94270
Reviewed-on: https://gerrit.instructure.com/167629
Tested-by: Jenkins
QA-Review: Aiona Rae Hernandez <ahernandez@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>
Reviewed-by: Nathan Mills <nathanm@instructure.com>
This commit renames the feature flag to HTML5 Media Recorder in
RCE; it previously included Arc because it was built by the Arc
team but does not directly relate to the Arc product. Also
updated the flag to allow visibility in beta.
REFS: COMMS-1486
Test plan:
- Log in to Canvas as an admin
- Open Account Settings
- In Account feature options, make sure you can view the
HTML5 Media Recorder in RCE feature option with the
development label and that the feature can be toggled on
Change-Id: I47d40263ba3dd91ad0bcca9b14ca7ba83409e323
Reviewed-on: https://gerrit.instructure.com/167716
Tested-by: Jenkins
Reviewed-by: Landon Gilbert-Bland <lbland@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: KC Naegle <knaegle@instructure.com>
We have no documentation in Confluence or this repo for running
Selenium tests natively. This commit adds a doc for that.
We're also migrating the "prepare" script from the qa_tools repo
to this one.
Test Plan:
- follow the instructions in doc/testing_with_selenium.md to
verify it's accurate
- run script/canvas_update to verify it still works
- follow the setup instructions in script/prepare/README.md and
verify it works
Change-Id: I11d63e60dc1faa8be1dfacfc9bebfcbea55c31f1
Reviewed-on: https://gerrit.instructure.com/167300
Tested-by: Jenkins
Reviewed-by: David Tan <dtan@instructure.com>
QA-Review: David Tan <dtan@instructure.com>
Product-Review: Michael Hargiss <mhargiss@instructure.com>
Marc Phillips