message creator refactor part 8

small changes relating to the space

test plan
 - specs should pass

flag=none
refs VICE-414

Change-Id: I03f6d76df3b46f8040222192995197d61fe51e48
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242212
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>

app/models/notification.rb

@@ -211,19 +211,19 @@ class Notification < ActiveRecord::Base
   end
 
   def registration?
-    return self.category == "Registration"
+    self.category == "Registration"
   end
 
   def migration?
-    return self.category == "Migration"
+    self.category == "Migration"
   end
 
   def summarizable?
-    return !self.registration? && !self.migration?
+    !self.registration? && !self.migration?
   end
 
   def dashboard?
-    return ["Migration", "Registration", "Summaries", "Alert"].include?(self.category) == false
+    ["Migration", "Registration", "Summaries", "Alert"].exclude?(self.category)
   end
 
   def category_slug

app/models/notification_policy.rb

@@ -75,8 +75,8 @@ class NotificationPolicy < ActiveRecord::Base
       frequency = params[:frequency]
       cc = user.communication_channels.find(params[:channel_id])
 
-      # Find any existing NotificationPolicies for the category and the channel. If frequency is 'never', delete the
-      # entry. If other than that, create or update the entry.
+      # Find any existing NotificationPolicies for the category and the channel.
+      # create or update the entry.
       NotificationPolicy.transaction do
         notifications.each do |notification_id|
           scope = user.notification_policies.

spec/lib/notification_message_creator_spec.rb

@@ -431,12 +431,14 @@ describe NotificationMessageCreator do
       expect(messages.map(&:to).sort).to eq ['dashboard']
     end
 
-    it "should force certain categories to send immediately" do
+    it "should not force non immediate categories to be immediate" do
       notification_set(:notification_opts => { :name => "Thing 1", :category => 'Not Migration' })
       @notification_policy.frequency = 'daily'
       @notification_policy.save!
       expect { NotificationMessageCreator.new(@notification, @assignment, :to_list => @user).create_message }.to change(DelayedMessage, :count).by 1
+    end
 
+    it 'should force certain categories to send immediately' do
       notification_set(:notification_opts => { :name => "Thing 2", :category => 'Migration' })
       @notification_policy.frequency = 'daily'
       @notification_policy.save!

spec/models/assessment_request_spec.rb

@@ -93,6 +93,7 @@ describe AssessmentRequest do
       @request.send_reminder!
 
       expect(@request.messages_sent.keys).to include(notification_name)
+      expect(@request.messages_sent[notification_name].count).to eq 1
       message = @request.messages_sent[notification_name].first
       expect(message.body).to include(@assignment.title)
     end