Commit Graph

76 Commits

Author SHA1 Message Date
Jacob Burroughs 3b7130c161 Remove a lot of settings
[ignore-stage-results=Flakey Spec Catcher]

refs AE-551

Change-Id: If7b5191c20cfadc438cdc2bc8b489eb2806582fe
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/334831
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
2024-01-09 21:32:17 +00:00
Chawn Neal 60f5f2b93a cancel_duplicate messages for cross-shard users
fixes VICE-3556
flag=none

requires MRA

Test PLAN:
 0)

 For SPEC Test
  b. use the below to run the spec
  RAILS_LOAD_CANCEL_PENDING_DUPLICATE_MESSAGES=1 bundle exec rspec spec/lib/notification_message_creator_spec.rb:746

 For UI Test
  a. go to: lib/notification_message_creator.rb:84
  replace immediate_policy?(user, channel) to true

 setup:
  Have two shards A,B.
  Have a courseA on shard A.
  Have a userB on shard B.
  Enroll the userB on courseA.
  Create an assignmentA on courseA.
  -- make it /3 or a number *important
  Create submit an answer on assignmentA.

 1) go to grade assignment A <= 3.
ex: /courses/1/gradebook/speed_grader?assignment_id=1&student_id=20000000000001
 -- if you go above it may trigger
 'Submission Grade Changed' which is the incorrect notification.
 a) change grade many times.

 2) go to /users/userB.user_id/messages
ex: /users/20000000000001/messages
 3) See that all are cancelled except 1 staged.
 -- 'Submission Graded'

Notes:
Why duplicate messages are being created.
1. Sharding, messages are not being searched on the users shard.
-- this ticket fixes.

2. graded_recently? is used incorrectly
- submission_graded should follow submission_grade_changed and use
!graded_recently? for whenever clause.

3. submission_graded && submission_grade_changed notifications are used incorrectly
submission_graded && submission_grade_changed notifications can both be
called during regrade process. submission_graded should only be called for the
first grade.

Change-Id: Idd6075527ec4a43b43f3e57b52601eb4715478aa
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/320275
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
2023-06-14 16:05:07 +00:00
Jacob Burroughs 7dcc507d0a Rubocop for ruby 3.1
[skip-stages=Flakey]

Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
2023-06-06 16:44:26 +00:00
Cody Cutrer c2cba46851 RuboCop: Style/StringLiterals, Style/StringLiteralsInInterpolation
[skip-stages=Flakey]

auto-corrected

Change-Id: I4a0145abfd50f126669b20f3deaeae8377bac24d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279535
Tested-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
2021-11-25 14:03:06 +00:00
Cody Cutrer e73cf9ddf4 RuboCop: Style/HashSyntax
[skip-stages=Flakey]

auto-corrected

Change-Id: I9371a61046aee6b148f89dd434114a8ba2b1188c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279533
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
2021-11-25 14:02:35 +00:00
Cody Cutrer bb548e0b2c RuboCop: Style/NilComparison, Style/NonNilCheck
auto-corrected

Change-Id: If8c067e5b9bd4d21131c6699900a7a14a988efeb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279000
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2021-11-19 22:38:10 +00:00
Cody Cutrer 4d43809cae RuboCop: Style/PercentLiteralDelimiters
[skip-stages=Flakey]

auto-corrected, with a post-review looking for multiline strings
to convert to heredocs

Change-Id: I7f7afb11edd63415cde10866822dd2ac5ba0d8be
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278669
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
2021-11-18 23:05:50 +00:00
Cody Cutrer c65d57737a RuboCop: Layout lib
Change-Id: I0655d9a9d750f2debd6378b03d8ddc1403ebc31b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274158
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2021-09-22 20:01:52 +00:00
Davis Hyer b542b5eda9 remove deprecate_sms flag
fixes VICE-1507
flag=deprecate_sms

test plan:
  - cursory testing around notifications:
    - account notifications preferences page should load without any
      sms channels
    - course notifications preferences page should load without any
      sms channels
    - notifications can still be sent for other channels

qa risk: low

Change-Id: Ibfa0c4f404f50e9a88960b26bc367a2ea8acee12
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272590
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
Reviewed-by: Caleb Guanzon <cguanzon@instructure.com>
2021-08-31 20:43:18 +00:00
Matthew Lemon fe3279b877 fix notification timezone bug
We need to get the account's timezone to be used as a default in
the event that the user has a null timezone set.

fixes VICE-1794
flag=none

Test Plan:
- Set the account timezone to something besides mountain time
  e.g. Alaska is what I used in testing
- Set a user's timezone to null. I just grabed the user in a rails
  console and updated User.find(<user_id>).update!(time_zone: nil)
- Enroll the user in a published course
- Create an assignment with a due date or another event that will
  trigger a notification with a specific time in the message body
- Check the users messages and note that the due date should reflect
  the accounts timezone
  /users/<user_id>/messages

Change-Id: Ib7787a424dc75c231da4f859e44f14a4a29f8914
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271904
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2021-08-23 15:48:53 +00:00
Omar Gerardo Soto-Fortuño 91ea3092ad Fix add_channel to use path_type
Test plan:
  - Spec pass

flag=deprecate_sms

Fixes VICE-1608

Change-Id: I61eb9dca5f8bbbbbb056cee139b21b8b9b88038b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/267124
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2021-06-15 15:16:07 +00:00
Omar Gerardo Soto-Fortuño 83886a6a24 No longer send sms messages
Test plan:
  - Spec pass

flag=deprecate_sms

Closes VICE-1505

Change-Id: Id616bdd984768745f6acf06943e118ccaae43eb8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266377
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2021-06-08 13:31:56 +00:00
Rob Orton c2002769ae don't error if partitions have not been created
Change-Id: I8689dcfce14402167b6ee271ff764e3c63a3c6b2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/265351
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
2021-05-20 16:35:42 +00:00
Rob Orton 99154a59d9 remove unused line of code
Change-Id: Ib6c2e588055d5d1bbb6978036fd25ac20c54e218
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/265348
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
2021-05-20 15:23:16 +00:00
Rob Orton 0653ff2f53 use root_account association when possible
when our context is an assignment we currently are loading a
rubric_association to identify the root_account

test plan
 - specs should pass

fixes VICE-1280
flag = none

Change-Id: I9d9493da1d80cb8208e999619700f657a8c00bca
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263328
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Tested-by: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
Reviewed-by: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
QA-Review: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
Product-Review: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
2021-04-21 16:18:57 +00:00
Rob Orton 3526466773 use loaded notification polices instead of query
we preload all the notification policies, so we want to use find instead
of a query to check for fallback channels

test plan
 - specs should pass

refs VICE-1280
flag = none

Change-Id: I963610d6cf76236e4a69f33f52932694c9751984
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263325
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Omar Soto-Fortuño <omar.soto@instructure.com>
QA-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
Product-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
2021-04-21 14:36:56 +00:00
Ethan Vizitei de07ac7b41 stop policy race condition
closes FOO-1405

TEST PLAN:
  1) get several jobs trying to send the same
    kind of notification to the same user at the same time
    when they don't currently have a policy for that user
  2) only one policy gets created, and all jobs use that policy
    without throwing any errors

Change-Id: Ib5b8b8348d5f4e928724aad9c4a49a844c19dbf8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/255975
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2021-01-04 20:15:17 +00:00
Rob Orton e2e1fbc00e add course_ids for appointments
appointments are actually calendar events that can and do belong to many
courses. If a user has a notification preference in any of the courses,
we will respect that preference.

test plan
 - it should set course_ids and respect notification preferences

fixes VICE-983
flag=none

Change-Id: I4ccaaab541eb92599f4e1ca1fb63f1930556a8d5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/253226
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Caleb Guanzon <cguanzon@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
2020-12-16 20:03:40 +00:00
Rob Orton 6c5379f10e remove notification_granular_course_preferences ff
test plan
 - specs should pass

flag = notification_granular_course_preferences
fixes VICE-999

Change-Id: Ie4ecde79c912538d6da3f7f3f1c1be93c8ad2bab
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254039
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Caleb Guanzon <cguanzon@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
2020-12-03 21:29:44 +00:00
Rob Orton 1240475b49 remove feature mute_notifications_by_course
test plan
 - specs should pass

fixes VICE-998
flag=mute_notifications_by_course

Change-Id: I2dc572a8013817bd1e3ae2ace54e52932e463cdd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254038
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
2020-12-02 18:14:53 +00:00
Cody Cutrer 06763dd519 add # frozen_string_literal: true for lib
Change-Id: I59b751cac52367a89e03f572477f0cf1d607b405
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251155
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2020-10-27 20:49:50 +00:00
Cody Cutrer c5227d3f1b shackles was renamed to guardrail
closes FOO-989, FOO-990

Change-Id: I49dfa130cb74c34dd0eb25952790176ae4951058
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249365
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2020-10-06 19:23:08 +00:00
Rob Orton b4d2f91c70 populate communication_channels root_account_ids
communication_channels only live on users shard and they only work on
users shard. There have been bugs that were incorrectly placing them on
a different shard, but they never worked.

users invited into a course get a user object created as pre-registered,
but they do not get root_account_ids set until after they have clicking
the email. the email can also cause a user merge that would put the
communication_channel on the merged user but again the root_account_ids
would not be present until after that process so before_create was not
enough to populate communication_channels.

this commit also makes it so the channels are just populated on a user
merge event, since they could change, this is also appropriate.

after a datafix up is run ids should be populated but log a stat when we
send a message to ensure it.

fixes VICE-895
flag=none

Change-Id: I4721a563988f8fa399fca9d23b73e17c2a6371e7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249227
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-10-05 22:42:55 +00:00
Rob Orton 24882a5aad spec: stop canceling messages in test
the schema may not exist that we are trying to query

Change-Id: I3ca987fd31a1e230b9fdec30cf04c7b8c05f01df
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/246496
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
2020-09-07 13:03:40 +00:00
Cody Cutrer d805bba4d3 target the correct partition(s) for messages queries with ranges
test plan:
 * current specs exercise both queries

Change-Id: I61d9f3bb6e6758fcc895332ccd9687a13b40b894
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/245975
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2020-08-27 15:34:16 +00:00
Rob Orton fd2d5323b5 use consistant methods for skipping messages
Change-Id: If82aee2f4c78cafc02d72bd3a8ca0eded4f9afe1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/244777
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
2020-08-12 21:22:20 +00:00
Drake Harper c337ad8a96 Fix not sending notification when adding an admin
Test Plan:
-navigate to /accounts/<user id with admin access>/settings
-add an admin by email
-navigate to /users/<new user id>/messages
-verify there is an message for

flag=none
fixes VICE-573

Change-Id: I5f3f5b4bae657a5449836cddc525310bef3e8682
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/244801
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2020-08-12 15:59:11 +00:00
Rob Orton 9425915cbf persist bounced messages
communication channels can be set to a bounced state for a temporary
problem and maybe restored after some time. When a communication_channel
gets in a bounced state, immediate messages to it would not get saved,
but delayed messages would. This commit changes immediate messages to
still be created in a bounced state, but not be sent. This is
informational only and is similar to a canceled message. When possible
we still create a new delayed message for the notification. Then when
the channel is no longer bouncing, they will get all the delayed
messages in the next summary. Messages regarding registration will not
create a delayed message, but they will still be in the bounced state to
easier verify if a message was sent or not.

test plan 
 - specs should pass

fixes VICE-654
flag=none

Change-Id: I4810636f0d82a372799942fd1c2f473587900c8f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243709
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
2020-07-30 17:22:45 +00:00
Rob Orton 4c795aa6fd check notification enabled once
test plan 
 - specs should pass

refs VICE-654

Change-Id: I6ec39d77e59c3be50b2501636d2a2ca3aad079bb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243708
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
2020-07-29 15:14:05 +00:00
Rob Orton f1464efac7 message creator refactor part 6
this changes how we process user_channels in the file, this will iterate
over all the applicable channels per user instead of iterate over users.
this also makes it so notification policies are looked up in the same
method for each type of message

test plan
 - specs should pass

flag=none
fixes VICE-414

Change-Id: I43416151567950a43da4b309f4a9fffb28a4a048
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242213
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
2020-07-16 19:13:52 +00:00
Rob Orton 42318b8770 message creator refactor part 7
This is just a few comments and logging some new statistics to see how
some of this is currently used.

test plan
 - specs should pass

flag=none
refs VICE-414

Change-Id: I6f003f9213722a9662c258e545f8afaf1ccdb650
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242211
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
2020-07-09 17:21:28 +00:00
Rob Orton 7da01fc5df message creator refactor part 5
this particular code was added to message creator when processing
messages was moved from notifcation.rb. When it was brought over it was
broken because it was assigning the message to daily instead of checking
for daily. This was later fixed because in a commit that was for
reducing memory usage.

delayed_messages is delayed_messages for all users passed into the
message_creator at that time, and this should not be used to determine
if a fallback message should or should not be sent.

Test plan
 - specs should pass

flag=none
refs VICE-414

Change-Id: I10f12d4d8a84984ddf46679b793f8e7db7cfb1f5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/240337
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
2020-07-09 17:21:14 +00:00
Rob Orton b31b9c4432 message creator refactor part 4
test plan
 - specs should pass

several tests were added and there are many tests for this file, we just
want them all to pass. There should be no change in functionality.

flag=none
refs VICE-414

Change-Id: I9c3cdd73706b780dc8578db7651e827e6a2e66f4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/240336
Reviewed-by: Davis Hyer <dhyer@instructure.com>
QA-Review: Davis Hyer <dhyer@instructure.com>
Product-Review: Davis Hyer <dhyer@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-07-02 23:35:55 +00:00
Rob Orton 2c3c869930 message creator refactor part 3
test plan
 - specs should pass

several tests were added and there are many tests for this file, we just
want them all to pass. There should be no change in functionality.

flag=none
refs VICE-414

Change-Id: Ic9fce60499826da7852a9d49f0c0e875ee950633
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/236242
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
2020-07-02 16:38:56 +00:00
Rob Orton e4042959c3 stop creating policies for secondary emails
test plan
 - send a message
 - message should send
 - specs should pass
 - should not send duplicate messages

flag=none
fixes VICE-506
fixes VICE-538

Change-Id: Ief7f09db4fdfc78864be9a537902e6863b1e0858
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/240227
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Matthew Lemon <mlemon@instructure.com>
2020-06-18 21:25:20 +00:00
Caleb Guanzon 4560b41a9b move course notification overrides feature flag to SiteAdmin
fixes KNO-526

test plan:
- feature only works when flag is turned on
  in the site admin level

Change-Id: I8e4f7669606498d61c327d3a352c4533c0c08179
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/239378
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2020-06-08 16:19:04 +00:00
Matthew Lemon 2fd60b2131 stop sending notifications on non-active channels
fixes KNO-502
flag=none

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- Create a user and add several communication channels to them
- Confirm all the communication channels but one
- Create a course and add the user to it
- Update the users notifications preferences to immediate for all
  preferences and channels
- Create a notification
- As an admin navigate to /users/<user_id>/messages
- Note that there should only be a new message sent to active
  communication channels

Change-Id: Ib5937526e881ea455addb3e4a173bea19e924d34
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/237910
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-05-19 21:42:07 +00:00
Matthew Lemon 40fe720649 respect notification preference overrides
Allows course context notification preference overrides to actually take
effect when creating notifications for a user.

fixes KNO-402
flag=notification_granular_course_preferences

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- Run the following migration
  bundle exec rake db:migrate:up VERSION=20200420211742
- Create two courses and add a student and a teacher to it
- configure a communication channel for the student
- As the student create a notification preference override for
  announcments with an immediate frequency for the first course
  - This can be done through graphiql using the following mutation

```
mutation MyMutation {
  __typename
  updateNotificationPreferences(
    input: {
      contextType: Course,
      communicationChannelId: <communication_channel_id>,
      courseId: <course_id>,
      frequency: immediate,
      notificationCategory: Announcement
    }
  ) {
    course {
      _id
      notificationPreferences {
        channels {
          _id
          path
          pathType
          notificationPolicies {
            communicationChannelId
            frequency
            notification {
              category
              categoryDisplayName
              name
            }
          }
          notificationPolicyOverrides(
            contextType: Course,
            courseId: <course_id>
          ) {
            communicationChannelId
            frequency
            notification {
              category
              categoryDisplayName
              name
            }
          }
        }
      }
      notificationPreferencesEnabled
    }
  }
}
```

- As the student navigate to /profile/communication and set all your
  Announcement policies to 'weekly'
- As the teacher navigate to the SECOND course and create an
  announcement
- Navigate to /users/<student_id>/messages and note that the
  announcement notification should not exist
- In a rails console check that the DelayedMessage was created
  n = Notification.where(category: 'Announcement').first
  delayed_messages = DelayedMessage.where(
    notification_id: n.id,
    communication_channel_id: <channel_id>
  )
  - The delayed_messages array should contain the notification for the
    announcment with a 'weekly' frequency

- As the teacher navigate to the FIRST course and create an announcement
- Navigate to /users/<student_id>/messages and note that the
  announcement notification should exist
- In a rails console validate that the DelayedMessage was not created
  using similar steps as detailed above

- Now as the student set your override policy to 'daily' using the same
  mutation provided above but changing the frequency
- navigate to /profile/communication and set all your Announcement
  policies to 'immediately'
- Run the same tests as above but now validate that an immediate message
  is created for the second course when an announcment is created and no
  delayed message is created
- Also verify that a delayed message with a 'daily' frequency is created
  for the first course when an announcement is created and no immediate
  message is created
- phew, that was a doozy of a test plan!

Change-Id: Idb5e95bf13762472c3fdd7aceef200a17f5cd9a0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234804
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Ahmad Amireh <ahmad@instructure.com>
Product-Review: Ahmad Amireh <ahmad@instructure.com>
2020-05-04 19:06:04 +00:00
Matthew Lemon 746bd9c29e fix extra query in notification_message_creator
The small refactor made to this file ended up introducing an unnecessary
query which we are removing here.

refs KNO-409
flag=none

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- Tests pass

Change-Id: Iee116f1d87c39b2c129b90cc7ed4bab964b9095b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234952
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2020-04-24 16:52:43 +00:00
Matthew Lemon 3ac4eb957c refactor notification_message_creator
Fixing up notification_message_creator a little bit in preparation for
adding notification_policy_overrides to the notification creation
process.

fixes KNO-409
flag=none

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- Tests pass

Change-Id: I83ce9aa58ebf9bd79c00578e0e24352d34f803c2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234413
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ahmad Amireh <ahmad@instructure.com>
QA-Review: Ahmad Amireh <ahmad@instructure.com>
Product-Review: Ahmad Amireh <ahmad@instructure.com>
2020-04-22 17:35:40 +00:00
Jacob Fugal 07d9174666 [OPTIMIZE] clean up recent message handling
localize recent message count management into one place, and populate it
beforehand to allow a single DB query for all users not already in the
cache. using a single DB query removes the need for the redis caching

Change-Id: Ic0d58f820628742cbb92fc8fab075d1c57b9fb66
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231282
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2020-03-25 15:13:47 +00:00
Simon Williams 8f188e3d80 remove repeat query in delayed message creation
DelayedMessage#set_send_at is called as a before save, and calls both:
- self.communication_channel
- self.communication_channel.user

Meaning that each time a delayed message is saved, it generates a query
for a communication channel and a user.

One common place delayed messages are created is in the
NotificationMessageCreator. at the time they are created the
communication channel and user are already in memory, we just need to
coax rails into using them.

Rails tries to set inverse_of associations automatically whenever
possible, but it appears that something (maybe the order argument) on
user.communication_channels is preventing that from happening. Once we
have inverse associations working properly, we can pass the object
instead of the id so everything is preloaded.

As noted in the inline comment, this is safe because notification_policy
is always loaded from a communication_channel association, whose
automatic inverse_of association is working correctly.

test plan:
- send a delayed notification to many people
- it should work, and not over-query communication_channels or users

Change-Id: Ia24282c9a24746701227781b4782b22b6c9f70f7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231352
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-03-25 01:49:16 +00:00
Rob Orton 5a060b2668 rename scope
flag=none

test plan
 - spec should pass

Change-Id: I4f7839410ed6f3878c6dc71ff74eb510d20b8142
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/229513
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ben Nelson <bnelson@instructure.com>
QA-Review: Ben Nelson <bnelson@instructure.com>
Product-Review: Ben Nelson <bnelson@instructure.com>
2020-03-11 22:17:23 +00:00
Rob Orton 427a7cf0f4 skip messages for disabled courses
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>
2020-02-29 00:23:14 +00:00
Rob Orton 763c7c9ffb remove some unused methods and scopes
test plan
 - specs should pass

fixes KNO-237

Change-Id: Ibea4ee22359fda8e3b11b50ce9ce7a2d6c939761
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/227682
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Davis Hyer <dhyer@instructure.com>
QA-Review: Davis Hyer <dhyer@instructure.com>
Product-Review: Davis Hyer <dhyer@instructure.com>
2020-02-25 21:48:06 +00:00
James Williams 2be249c487 fix sending immediate notifications for delayed policies
test plan:
* create a user in a course with a confirmed email
* as that user, view the notificatio settings and
 override a testable notification type
 (e.g. Announcements) to send later (daily/weekly)
* override another type to send immediately
* test the notification (e.g. add an announcement
 to the course)
* should not send the notification right away

closes #CORE-2815

Change-Id: I8930d1ff4142a5128224023bcdfdcf9c2458bcc0
Reviewed-on: https://gerrit.instructure.com/190165
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
2019-04-22 16:45:20 +00:00
Cody Cutrer 8d10ea6f68 fix a butt-ton of N+1 in DelayedNotification.process
Change-Id: I2adcced2aa8db70828c2943a004525eaef267a37
Reviewed-on: https://gerrit.instructure.com/185626
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2019-03-21 19:09:19 +00:00
James Williams 6f5944f84f add hidden setting to default observer notifications off
test plan:
* run the following in a rails console:
 Account.default.tap do |a|
   a.settings[default_notifications_disabled_for_observers] = true
   a.save!
 end
* use a sis batch import with user_observers.csv to create
 an observation link for a student and observer
* as the observer, view the "Notifications" section of their
 account settings
* they should all default to "Never"

closes #COMMS-1898

Change-Id: Iabf5bff42efa521bdad34ccf7de277cee0e1789c
Reviewed-on: https://gerrit.instructure.com/184022
Tested-by: Jenkins
Reviewed-by: Landon Gilbert-Bland <lbland@instructure.com>
Reviewed-by: Steven Burnett <sburnett@instructure.com>
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>
2019-03-07 22:30:16 +00:00
James Williams ab55549a6c run recent_messages_for_user on slave
Change-Id: Id6da71dd0655cded67b61315ef63a488abe0b579
Reviewed-on: https://gerrit.instructure.com/178808
Tested-by: Rob Orton <rob@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
2019-01-22 14:01:37 +00:00
James Williams 5ce50262cb add created_at scope to recent_messages query
Change-Id: Ib6dd3f6845fd865900361a203471371d5467ca95
Reviewed-on: https://gerrit.instructure.com/178656
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
2019-01-21 16:46:04 +00:00