Change-Id: Ib49bc8939cf1706e758429e531a87c57d0231a37
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251156
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
so that the canvas doesn't have to explicitly
Change-Id: I19c2df3464fdd10b8b09d97139ea3f973663d5a4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/250803
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
there have been many improvements down the line from here, but we can
still send a notification to 20k people at once by making an
announcement. This commit breaks up the to_list to chunks of 500.
flag=none
qa risk: low, very targeted change
closes KNO-313
Change-Id: Id39303536a5093ddab36b7e52aec0c5f09f313eb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/228317
Reviewed-by: Johnny Le <jle@instructure.com>
Product-Review: Johnny Le <jle@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Johnny Le <jle@instructure.com>
when a user disables notifications for a course, we should not send them
notifications. this commit checks for the policies set course_id and
skips when the user has disabled notifications for the course.
Other commit will verify the set_policy blocks are all setting the
course_id
test plan
- set a notification preference to disabled for a course
- trigger notification
- the user should not get the notification
flag=mute_notifications_by_course
qa risk: low, it's a new controller also, it is behind ff
closes KNO-224
Change-Id: I1c8de3060f31b2bc4914ed4b729bd494f9059674
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/228293
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Johnny Le <jle@instructure.com>
Reviewed-by: Johnny Le <jle@instructure.com>
Product-Review: Johnny Le <jle@instructure.com>
add flags for 6.0, even though it doesn't actually do anything yet
Change-Id: If8aba4d9f92e8a8ec890deadba7a94e21e01a804
Reviewed-on: https://gerrit.instructure.com/202686
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
name did not match usage, and all remaining usage can be satisfied in
other ways.
closes CNVS-38407
test plan: regression test message sending (immediate, delayed, and
dashboard notifications w/ stream items)
Change-Id: Ibe110527b4644cbaa417c53b2b53c2fd7e18dc9e
Reviewed-on: https://gerrit.instructure.com/120520
Tested-by: Jenkins
QA-Review: Heath Hales <hhales@instructure.com>
Reviewed-by: Matt Smith <msmith@instructure.com>
Reviewed-by: Benjamin Christian Nelson <bcnelson@instructure.com>
Reviewed-by: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: Simon Williams <simon@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>
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>
as of this commit, all canvas gems should be on rspec 3.5, and pass
without deprecation warnings.
closes CNVS-34040
test plan: specs should pass without deprecation warnings
Change-Id: I556b1a4a5aeb791c6ddd50ee35b51c513e025019
Reviewed-on: https://gerrit.instructure.com/98414
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Tested-by: Jenkins
except for CleanseTheSyckness, where serialized_attributes has been
removed without replacement
Change-Id: Ie0d81c17effa4b40b285f5d7108fa2586a8ce9ab
Reviewed-on: https://gerrit.instructure.com/89330
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Make most autoloadable files selinimizable. That means that you can edit
most models/helpers/gems/lib files and only have the relevant selenium
specs run \o/
How does it work?
=================
As with the existing selinimized stuff (controllers/views/css/js/etc), we
capture the dependencies of each spec file in a post-merge build. To
accomplish this, we hook into Rails' autoloading mechanism and give it
some hide/restore magic. This way we can hide all autoloaded constants
from one spec file to the next (technically, top-level example group).
As specs run, constants are loaded (or restored) via autoloading, and
we track all of them.
Then on your patchset build, we synthesize this information with the list
of files you changed. If all files are selinimizable, we can safely run
just a subset of specs.
What things are still not selinimizable?
========================================
* config/* (including routes.rb)
* models loaded when the app starts up (~20%, biggies like User, etc.)
* observers and their models (cuz they really like to load :allthethings:)
* things that are `require`d (`require_dependency` is a-ok though)
Test Plan:
* Normal patchset builds should pass
* Build with capturing enabled should pass and publish to S3
* Test commits based on this one should:
1. Be correctly selinimized if limited to captured autoloads, etc.
2. Not be selinimized otherwise
[selinimum-capture]
Change-Id: I04cea1fa6055d201d72d210a3e9bd770f26769a5
Reviewed-on: https://gerrit.instructure.com/76931
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Since Notification and NotificationPolicy can be reloaded,
BroadcastPolicy needs to re-cache the references on reload.
To test:
You can experience a reloading error before applying this patch
by triggering a notification after a reload.
- rails console
- reload!
- Course.last.enroll_student(User.last).accept!
This will cause `ArgumentError: A copy of Notifier has been removed from the module tree but is still active!`
With this patch applied, there should be no error (assuming
that student can be enrolled in that class).
rely on test_all_gems.sh to output header and trailer,
and use `set -e` in each test.sh to simplify passing
through errors
Change-Id: I3ba724ad2539ddfe31195394c43f646acfc73920
Reviewed-on: https://gerrit.instructure.com/70469
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
refs #CNVS-26056
Change-Id: Idee28041d97a6e8a7566ef922f3001906c617864
Reviewed-on: https://gerrit.instructure.com/69500
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
we are swallowing a bunch of errors in broadcast policy methods that
check differences in workflow state between the old and new objects.
instead, let's let these fail and fix bugs that may come from using
these methods incorrectly.
fixes CNVS-21212
test plan:
- this would manifest itself as a error when trying to check the
conditions for whether or not to send a notification on the following
models:
* announcement
* assignment
* calendar
* content_export
* course
* discussion
* enrollment
* group_membership
* quiz_submission
* submission
* wiki
- so the best way to test is to get notifications to send for these
models under different conditions and make sure they work correctly
Change-Id: Ia9d6d0c37101b2d0859d5753835227d314955c3d
Reviewed-on: https://gerrit.instructure.com/54541
Tested-by: Jenkins
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Steven Shepherd <sshepherd@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
* add test runner for broadcast_policy and fix a couple of tests that
had been broken by accident
* add test runner for canvas_quiz_statistics and add a reference to
iconv to it's Gemfile to allow tests to run in ruby 2
* add a test runner for canvas_sanitize
* remove leftover cruft from running gem builds in both rails 2 and 3
Change-Id: Iab2a0986f277a82c096e1fff2dab1d3a55b91733
Reviewed-on: https://gerrit.instructure.com/54518
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
closes CNVS-18909
test plan:
- trigger as many different permutations of notifications as possible
* for example, discussion announcements for a course and a group,
graded and ungraded
- none of them should raise an error
Change-Id: I35ea692bd8510a07be73c571f1e862b5bc0faa38
Reviewed-on: https://gerrit.instructure.com/49640
Tested-by: Jenkins
Reviewed-by: Joel Hough <joel@instructure.com>
QA-Review: Adam Stone <astone@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
closes CNVS-6016
No more error reports! (soon)
this commit builds up sentry integration through the new
Canvas::Errors module, along with other things that need
to happen on every exception. ErrorReports
should now get pushed towards just being used for representing
a complaint a user filed via the get help form.
I fixed about half the things that got linted as well
while I was in here, but because this touches to much
I fear divergence from tackling too many (I think we
can safely say it's "better than we found it")
I left a lot of the infrastructure for error reports in place
until other commits for plugins can be merged
TEST PLAN:
1) setup your raven.yml config file with the dsn for our
sentry install
2) force an error to happen in a request response cycle.
3) see the error in sentry
4) force an error to happen in a job
5) see the error in sentry
6) statsd increments shoudl still fire
7) for the moment, an error report should still get created.
Change-Id: I5a9dc7214598f8d5083451fd15f0423f8f939034
Reviewed-on: https://gerrit.instructure.com/51621
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
test plan:
* re-create your test db using
RAILS_ENV=test rake db:initial_setup
* should work
Change-Id: Iec245880490a43f1749c68bd2cc8869c6aba4d96
Reviewed-on: https://gerrit.instructure.com/47310
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
fixes #CNVS-14270
move broadcast policy into a gem of its own
to remove plugin deprecation warnings.
TEST PLAN:
-no behavior changes
-regression test notifications
(no need to hit them all, confirm that 3 or
4 separate notifications flow and
that will prove the pipeline)
Change-Id: If8445653ec09ab4d221124d85f9674d1cedd3751
Reviewed-on: https://gerrit.instructure.com/40899
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Matthew Wheeler <mwheeler@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Cody Cutrer