also prevent them in the future. our test-queue fork currently has a bug
where only the last duplicate spec will run, but this is a good idea anyway
most of these are just rewordings, but in some cases delete redundant/
identical specs
test plan: specs
Change-Id: I8f51be55e750cf87ffa8150822f4b80dc99e1eb4
Reviewed-on: https://gerrit.instructure.com/103993
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
refs CNVS-34862
in rails 5, we can't detect a transaction that was elided in favor
of joining a parent transaction, so don't try to. instead, always
call attachment callbacks immediately in the transaction, except
for the one spec that's actually testing that we don't do it in
a transaction
Change-Id: Ia53730586e305d95c41269b3be643194392a141d
Reviewed-on: https://gerrit.instructure.com/103757
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Drop computed_current_score and computed_final_score from the
enrollments table. The introduction of the scores table has
led to these columns being unnecessary.
closes CNVS-33574
test plan:
1. Grade a student in the gradebook. Note their current grade.
2. Log in as the student and go to the global grades page (you will
need to be enrolled in at least 2 courses). Verify the current grade
on the global grades page matches the current grade in the gradebook.
Change-Id: Id32cc083008d6582291bb09bde7761828370da4d
Reviewed-on: https://gerrit.instructure.com/103179
Tested-by: Jenkins
Reviewed-by: Neil Gupta <ngupta@instructure.com>
QA-Review: Neil Gupta <ngupta@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
also don't rely on plugin symlinks, as they are going away
test plan:
* be on canvas master
* have canvas analytics installed
* remove all compiled js in canvas and plugins
* run `rake i18n:generate` and save en.yml
* run `rake i18n:generate_js` and save the translations
* check out this commit
* run `rake i18n:generate` again and diff it:
* confirm you get its missing jsx strings
* confirm nothing else has changed
* run `rake i18n:generate_js` and diff the directory:
* confirm public/javascripts/translations/analytics.js has all the
missing strings
* confirm no other translation files have changed
Change-Id: I274aecba899367b8a26bc6f8791adbddd1cec06b
Reviewed-on: https://gerrit.instructure.com/103683
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
don't create extra AR objects or incur unnecessary write_attribute taxes
(super expensive for serialized columns). this speeds up saving any AR
model that has a broadcast policy (whether or not messages are sent), in
particular ones having serialized columns (e.g. quiz submissions), since
we avoid an unnecessary type cast on each attribute.
while it's a little hard to quantify the perf boost we'll see in the app,
here is a sampling of numbers we see in the specs:
spec/messages :
0:48 -> 0:45 (6.6% reduction)
spec/models/a* :
5:59 -> 5:17 (12% reduction)
spec/selenium/grades/speedgrader
14:09 -> 13:13 (6.7% reduction),
slowest spec went from 0:26 -> 0:19
speed gains in the app will likely be more modest, but performance should
noticeably improve in places like the speedgrader (e.g. grade quiz by
question)
test plan:
* specs
* regression test of notifications throughout the app, in particular:
* discussion topics
* assignments
* submissions
Change-Id: I1ccf6f7f293a35763fde077d612505069823390e
Reviewed-on: https://gerrit.instructure.com/102868
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Cemal Aktas <caktas@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
Closes: CNVS-35152
This changes all our tooling that used `npm` to
Run scripts to use `yarn`. `npm` will still work
for now but if you have yarn installed, it will use
that.
Cheat Sheet:
old command -> new command
npm install -> yarn install (or just `yarn`)
npm run webpack -> yarn run webpack
npm test -> yarn test
See more at https://yarnpkg.com
Test plan:
* all Jenkins builds should pass
* without yarn installed:
* run script/nuke_node.sh
* it should work and warn you about how `npm` is deprecated
* now install yarn, e.g.: `brew install yarn`
* run script/nuke_node.sh again
* it should work and you should see silly yarn emoji
in the output and it should be a lot faster
* the docker changes I made should work and docker should
build correctly and use yarn.
Change-Id: I4aa31eeae3ecc504634a7c72a1ea0d3396f445e3
Reviewed-on: https://gerrit.instructure.com/102969
Tested-by: Jenkins
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
also speed it up a hair by skipping coffeescript on non-i18ny files
test plan:
* i18n stuff should all pass jenkins
* g/100536 should be happier
Change-Id: I4a2dd70065dc80e82b062a93c662fc10c1b3cabf
Reviewed-on: https://gerrit.instructure.com/103460
Reviewed-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Jensen <jon@instructure.com>
speed up require_js and webpack builds. here's how:
* bump i18nliner
* it uses babel
* it is much faster
* tweak canvas_i18nliner to not need to precompile anything
* run i18n:generate_js without having to precompile anything
* webpack can avoid running js:generate altogether
test plan:
* tests pass
* i18n works as per usual under requirejs
* i18n works as per usual under webpack
* `rake i18n:generate`:
* builds identical yml before/after
* runs faster now
* works even if you nuke compiled coffee/jsx
* `rake i18n:generate_js`:
* builds identical translation files before/after
* runs faster now
* works even if you nuke compiled coffee/jsx
Change-Id: I2682e68ff87168d9f76e20deb93713aa9a50477a
Reviewed-on: https://gerrit.instructure.com/103285
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
refs CNVS-34862
well, the BlankSlateProtection fixes aren't technically Rails 5 -
it was also broken in Rails 4, if you have persistent test shards
and, this concludes spec/models/a*/**/*_spec.rb and spec/models/a*_spec.rb
Change-Id: If7a75a561a96e50b291dc84635bbe52f6445a035
Reviewed-on: https://gerrit.instructure.com/102986
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
test plan: ensure the essay questions in the question bank
attached to the ticket import as essay questions and
not as fill-in-multiple-blanks questions
fixes CNVS-35028
Change-Id: I8b7303248efc0aa258ae3d3125f645fca005e868
Reviewed-on: https://gerrit.instructure.com/102662
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: David Tan <dtan@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
fix a newly exposed autoloading edge case, that depends on spec run order.
given:
ConversationParticipant which autoloads:
SimpleTags and
Conversation (which would normally autoload SimpleTags),
when we reset autoloads and then resolve Conversation, ensure it restore
SimpleTags
also remove a weird self-require_dependency ... it's not a problem for AS,
but it is for selinimum
[ci selinimum capture]
test plan:
n/a, specs
Change-Id: I6b0193c9162a70554c997f4ed0c76690c86bf0e0
Reviewed-on: https://gerrit.instructure.com/102628
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
fixes CNVS-34170
test plan:
* configure statsd.yml
* use the app
* observe data sent to statsd, and logged, re: permissions
Change-Id: I1ff35f4679fffcac49774e1bad0cf2540b7f17b8
Reviewed-on: https://gerrit.instructure.com/99727
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Fixes: CNVS-34067
Test plan:
* with webpack turned on and in prod mode
* run RAILS_ENV=production bundle exec rake canvas:compile_assets
* pick a different language on your user settings page (e.g.maori)
* go to /calendar
* the “week, Month, Agenda” buttons and everything
else on the page should be translated into your language
* go to other pages, e.g. /courses/x/assignments/new
* all of the strings on those pages should be translated too
(The broken behavior was that any strings that came from handlebars
Files would not be translated but ones coming from js, erb or
Coffeescript were)
Change-Id: I9a4b76e857b6c809b663d18abd7196ca10ef8564
Reviewed-on: https://gerrit.instructure.com/102534
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
our pull requests have all been merged
closes CNVS-34066
test plan:
- things that require xml parsing should work, like:
- a course export / import
- lti grade passback
- a quiz multiple dropdowns question
- viewing a discussion
- the course link validator
Change-Id: I2ddeee0f252d6cce46bd82e144832e3eec85ecb0
Reviewed-on: https://gerrit.instructure.com/95475
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Find/fix/prevent weird stuff we are doing (or not doing) with `expect`.
Chapter I:
"I have been bent and broken, but - I hope - into a better shape."
Disallow `expect`s in a `before`. `before` is by definition before the
test, so it's not the place to be asserting anything. You can of course
still set up mocks there.
Chapter II:
"Take nothing on its looks; take everything on evidence. There's no
better rule."
Ensure every `expect` actually runs. i.e. it needs `to` or `not_to` in
order to actually do anything.
Chapter III:
"Ask no questions, and you'll be told no lies."
In selenium land, ensure every spec has at least one expecation. In
regular rspec land we just warn to stderr, but we may get more strict
in the future.
Test Plan: Specs
Change-Id: I5fc353ee8171e5191853d45f42ae42478f9220b4
Reviewed-on: https://gerrit.instructure.com/101224
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
fixes CNVS-34832
the arguments to these methods have not changed, just their names. and the
new methods became available in Rails 4, so we can start using them now
and prevent someone from doing old-style in the future
Change-Id: I61aa5512995dc8f25f3f7bd009a6cfa0a030e274
Reviewed-on: https://gerrit.instructure.com/101401
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
closes CNVS-34574
test plan:
- try to create rich content with the abbr tag
- it should work
Change-Id: I28ae285524fb671fc69709732976df2edd0d5d6d
Reviewed-on: https://gerrit.instructure.com/101164
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
fixes CNVS-34785
test plan:
- try to create a math tag with a valid link, and with a stored script
- the valid link should work, but the stored script should be stripped
Change-Id: I1bec604d673d5ece23b238e1d81a292baa68fa6f
Reviewed-on: https://gerrit.instructure.com/101202
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
previously, if run a sub-account, the report would only return groups
directly for that sub-account, not descendant accounts.
fixes CNVS-34604
test plan:
- create account structure A -> B -> C (A is root)
- create a group in account C
- run a provisioning report in account B
- it should include the group
Change-Id: I38f217e43bf59bb45e06be35c268236978cad6fa
Reviewed-on: https://gerrit.instructure.com/100759
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Because of some changes in YAML serialization we're seeing sadness with
content migrations, this stems from the converter being passed as a
constant rather than a full object.
Refs: CNVS-34329
Test Plan:
- Job based things still work
Change-Id: I592045db8450678644ff441a4c74a359c93651cc
Reviewed-on: https://gerrit.instructure.com/99930
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
Tested-by: Jenkins
The grade export report has been updated to consider the scores table
for the fresh scores. It falls back to the enrollment scores in case
the datafix hasn't migrated the data yet for the user.
fixes CNVS-34670
test plan:
- Create a course
- Add a student
- Create an assignment
- As the student, submit something
- As the teacher, grade the student
- Generate an account report for your account
- Check to make sure the account report has the right grade for the
student.
- Grade the student again and change their score
- Generate the account report again and make sure the grade changed
Change-Id: Ia43ce86d83c69c523a75c71eccdf90df785a2f08
Reviewed-on: https://gerrit.instructure.com/100967
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
i.e. if a plugin also calls has_broadcast_policy, don't wipe out
existing notifications
Change-Id: I5df3a2fd25b9050f678d63f2729e57aa2044f26a
Reviewed-on: https://gerrit.instructure.com/100975
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
closes CNVS-34657
test plan: rubocop should still work
Change-Id: Ibdd340dd7ba516639ee5b7225449422b324bd33a
Reviewed-on: https://gerrit.instructure.com/100831
Tested-by: Jenkins
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
closes CNVS-34065
test plan:
- do something that sends an email
- it should work
Change-Id: I21c40bd414bd43580faf7423a829fca85c5625ad
Reviewed-on: https://gerrit.instructure.com/95345
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Pedro Fajardo <pfajardo@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
Test Plan: Run the group and group_categories reports, make
sure you get all group categories, with course and account contexts
and make sure you get context information for the group report
Change-Id: Ie769a8679edf1240db571a6acaaa40009c060a22
refs: PFS-6415
Reviewed-on: https://gerrit.instructure.com/100180
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
QA-Review: Trevor Byington <tbyington@instructure.com>
Product-Review: Chad McGuire <cmcguire@instructure.com>
the thread_safe gem has deprecations too, but they'll require Rails 5.1
until they're fixed (when both activesupport and tzinfo no longer
depend on it)
Change-Id: Ic53839d911ba8ed4d463d17f9dd7207673510f3a
Reviewed-on: https://gerrit.instructure.com/100499
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
test plan:
* import the packages referenced in the tickets
* the questions (in the question banks) should come over
in some useful fashion
closes #CNVS-34360
Change-Id: I5766ae856de9f3e6a1551109d4cb69f6c6ae7dfd
Reviewed-on: https://gerrit.instructure.com/100222
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
closes CNVS-33615
test plan:
* upload and download files using S3
* submit an assignment
* do an SIS import with parallel
* do a course copy
* export and import a course
* upload some custom css file in theme editor
* test a canvadoc and crocodoc preview
Change-Id: I9145b39728938e7e5903d23c4a4598fc8df4ef45
Reviewed-on: https://gerrit.instructure.com/93002
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
this was lost in https://gerrit.instructure.com/#/c/90657/33
Change-Id: Ie3a0c38cb7df0b627954ebc00ec9b0fc99bf43c7
Reviewed-on: https://gerrit.instructure.com/100330
Tested-by: Jenkins
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
correctly whitelist things that chain off of whitelisted methods, e.g.
connection
Change-Id: I38ec4699b324d0e5d26de6bde36d841fa696d7e5
Reviewed-on: https://gerrit.instructure.com/100186
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
fixes SD-2006
mixing `singleton_class.prepend` and `extend` appears to cause `super`
to misbehave every once in a while (~1/250 workers in ~20% of builds)
... perhaps a ruby 2.3.1 bug?
just use `singleton_class.prepend` like autoextend does. 30+ test builds
with this tweak, and zero failures :yuss:
also change a couple `include` calls to `prepend`
test plan:
n/a, specs
Change-Id: I35629d63ed3855d95ae427c1cd992d12b950880c
Reviewed-on: https://gerrit.instructure.com/99613
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
also add a cop to prevent it in the future
Change-Id: Ic949cc4448f546736aa3aeee17fc1fc33a1f2808
Reviewed-on: https://gerrit.instructure.com/99847
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
Before this fix, there was not a way to determine the workflow_state of the
course without accessing API. This fix adds a way to send the course
workflow_state during an LTI launch.
Test plan:
1) create a course
2) install a tool with the $Canvas.course.workflowState custom field
3) launch the tool and verify that the course workflow_state is passed in
the launch parameters
Fixes PLAT-1956
Change-Id: I933ddcd45c689ecd054e692c7e4dc993874dc995
Reviewed-on: https://gerrit.instructure.com/94987
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Andrew Butterfield <abutterfield@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>
cleaner interface, with the bonus of making it easier to avoid mocking
`any_instance` on this class.
closes CNVS-33856
test plan:
- register linked in as a user service
- it should work
Change-Id: I158e132314569dd960e04c6c863db90146b8062a
Reviewed-on: https://gerrit.instructure.com/97542
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
in aws-sdk v2, this `.read` doesn't take a block, so the files were empty
verified manually by (re-)finalizing 42613b97
Change-Id: I07fecc2fba31b46de891a000ab16c298cf9a9cc1
Reviewed-on: https://gerrit.instructure.com/99250
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
closes CNVS-34242
This makes dr_diff have an option to avoid checking
lines surrounding changes. It also sets up eslint
to take advantage of this rule.
Test Plan:
- Automated Tests Pass
- Eslint only flags errors on changed lines
(when run through Gerrit/Jenkins/Gergich or
via script/eslint)
Change-Id: I900430f21c4c925e8fd87bd62e75b271fa84d08e
Reviewed-on: https://gerrit.instructure.com/99048
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
refs #CNVS-32574
Change-Id: If0a354295f9433fcf4dc2a776d93ae45833a44bd
Reviewed-on: https://gerrit.instructure.com/94683
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
refs #CNVS-32574
Change-Id: Ib1fc9c81dbfa4ece200a15a23105dbfa6f84d0c6
Reviewed-on: https://gerrit.instructure.com/94677
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
cleaner interface, with the bonus of making it easier to avoid mocking
`any_instance` on this class.
closes CNVS-33854
test plan:
- register twitter as a user service
- it should work
Change-Id: I9dd2c7b85dd9937ebe2653a5e1309757812b6bf2
Reviewed-on: https://gerrit.instructure.com/97537
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Benjamin Christian Nelson <bcnelson@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Cody Cutrer