[UserFeedback] Add option to send dmail on update

app/controllers/user_feedbacks_controller.rb

@@ -32,7 +32,11 @@ class UserFeedbacksController < ApplicationController
   def update
     @user_feedback = UserFeedback.find(params[:id])
     check_privilege(@user_feedback)
-    @user_feedback.update(user_feedback_params(:update))
+    params_update = user_feedback_params(:update)
+
+    @user_feedback.update(params_update)
+    not_changed = params_update[:send_update_dmail].to_s.truthy? && !@user_feedback.saved_change_to_body?
+    flash[:notice] = "Not sending update, body not changed" if not_changed
     respond_with(@user_feedback)
   end
 
@@ -52,6 +56,7 @@ class UserFeedbacksController < ApplicationController
   def user_feedback_params(context)
     permitted_params = %i[body category]
     permitted_params += %i[user_id user_name] if context == :create
+    permitted_params += [:send_update_dmail] if context == :update
 
     params.fetch(:user_feedback, {}).permit(permitted_params)
   end

app/models/user_feedback.rb

@@ -2,11 +2,11 @@ class UserFeedback < ApplicationRecord
   self.table_name = "user_feedback"
   belongs_to :user
   belongs_to_creator
-  validates :user, :creator, :body, :category, presence: true
-  validates :category, inclusion: { :in => %w(positive negative neutral) }
+  validates :body, :category, presence: true
+  validates :category, inclusion: { in: %w[positive negative neutral] }
   validate :creator_is_moderator, on: :create
   validate :user_is_not_creator
-  after_create :create_dmail
+  after_save :create_dmail
   after_create do |rec|
     ModAction.log(:user_feedback_create, { user_id: rec.user_id, reason: rec.body, type: rec.category, record_id: rec.id })
   end
@@ -17,9 +17,11 @@ class UserFeedback < ApplicationRecord
     ModAction.log(:user_feedback_delete, { user_id: rec.user_id, reason: rec.body, type: rec.category, record_id: rec.id })
     deletion_user = "\"#{CurrentUser.name}\":/users/#{CurrentUser.id}"
     creator_user = "\"#{creator.name}\":/users/#{creator.id}"
-    StaffNote.create(body:  "#{deletion_user} deleted #{rec.category} feedback, created #{created_at.to_date} by #{creator_user}: #{rec.body}", user_id: rec.user_id, creator: User.system)
+    StaffNote.create(body: "#{deletion_user} deleted #{rec.category} feedback, created #{created_at.to_date} by #{creator_user}: #{rec.body}", user_id: rec.user_id, creator: User.system)
   end
 
+  attr_accessor :send_update_dmail
+
   module SearchMethods
     def positive
       where("category = ?", "positive")
@@ -55,7 +57,7 @@ class UserFeedback < ApplicationRecord
       end
 
       if params[:creator_id].present?
-         q = q.where("creator_id = ?", params[:creator_id].to_i)
+        q = q.where("creator_id = ?", params[:creator_id].to_i)
       end
 
       if params[:creator_name].present?
@@ -81,29 +83,23 @@ class UserFeedback < ApplicationRecord
   end
 
   def create_dmail
-    body = %{@#{creator_name} created a "#{category} record":/user_feedbacks?search[user_id]=#{user_id} for your account:\n\n#{self.body}}
-    Dmail.create_automated(:to_id => user_id, :title => "Your user record has been updated", :body => body)
+    should_send = saved_change_to_id? || (send_update_dmail.to_s.truthy? && saved_change_to_body?)
+    return unless should_send
+
+    action = saved_change_to_id? ? "created" : "updated"
+    body = %(#{creator_name} #{action} a "#{category} record":/user_feedbacks?search[user_id]=#{user_id} for your account:\n\n#{self.body})
+    Dmail.create_automated(to_id: user_id, title: "Your user record has been updated", body: body)
   end
 
   def creator_is_moderator
-    if !creator.is_moderator?
-      errors.add(:creator, "must be moderator")
-      return false
-    else
-      return true
-    end
+    errors.add(:creator, "must be moderator") unless creator.is_moderator?
   end
 
   def user_is_not_creator
-    if user_id == creator_id
-      errors.add(:creator, "cannot submit feedback for yourself")
-      return false
-    else
-      return true
-    end
+    errors.add(:creator, "cannot submit feedback for yourself") if user_id == creator_id
   end
 
   def editable_by?(editor)
-    (editor.is_moderator? && editor != user)
+    editor.is_moderator? && editor != user
   end
 end

app/views/user_feedbacks/_form.html.erb

@@ -3,6 +3,9 @@
     <%= f.input :user_name, label: "User", autocomplete: "user", input_html: { value: user_feedback.user.try(:name) } %>
   <% end %>
   <%= f.input :category, as: :button_select, collection: ["positive", "neutral", "negative"].map { |e| [e.titleize, e] } %>
+  <% unless user_feedback.new_record? %>
+    <%= f.input :send_update_dmail, label: "Send update DMail", as: :boolean %>
+  <% end %>
   <%= f.input :body, as: :dtext %>
   <%= f.button :submit, "Submit" %>
 <% end %>

app/views/user_feedbacks/edit.html.erb

@@ -1,6 +1,6 @@
 <div id="c-user-feedbacks">
   <div id="a-edit">
-    <h1>Edit User Feedback</h1>
+    <h1>Edit User Feedback - <%= link_to_user @user_feedback.user %> </h1>
 
     <%= error_messages_for "user_feedback" %>
     <%= render "form", user_feedback: @user_feedback %>

test/functional/user_feedbacks_controller_test.rb

@@ -1,4 +1,4 @@
-require 'test_helper'
+require "test_helper"
 
 class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
   context "The user feedbacks controller" do
@@ -42,7 +42,7 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
 
       context "with search parameters" do
         should "render" do
-          get_auth user_feedbacks_path, @critic, params: {:search => {:user_id => @user.id}}
+          get_auth user_feedbacks_path, @critic, params: { search: { user_id: @user.id } }
           assert_response :success
         end
       end
@@ -50,8 +50,8 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
 
     context "create action" do
       should "create a new feedback" do
-        assert_difference("UserFeedback.count", 1) do
-          post_auth user_feedbacks_path, @critic, params: {:user_feedback => {:category => "positive", :user_name => @user.name, :body => "xxx"}}
+        assert_difference([-> { UserFeedback.count }, -> { Dmail.count }], 1) do
+          post_auth user_feedbacks_path, @critic, params: { user_feedback: { category: "positive", user_name: @user.name, body: "xxx" } }
         end
       end
     end
@@ -61,10 +61,24 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
         as(@critic) do
           @feedback = create(:user_feedback, user: @user, category: "negative")
         end
-        put_auth user_feedback_path(@feedback), @critic, params: { id: @feedback.id, user_feedback: { category: "positive" }}
+
+        assert_no_difference(-> { Dmail.count }) do
+          put_auth user_feedback_path(@feedback), @critic, params: { id: @feedback.id, user_feedback: { category: "positive" } }
+        end
 
         assert_redirected_to(@feedback)
         assert("positive", @feedback.reload.category)
+        assert_match(/created a/, Dmail.last.body)
+      end
+
+      should "send a new dmail" do
+        as(@critic) do
+          @feedback = create(:user_feedback, user: @user, category: "negative")
+        end
+        assert_difference(-> { Dmail.count }, 1) do
+          put_auth user_feedback_path(@feedback), @critic, params: { id: @feedback.id, user_feedback: { body: "changed", send_update_dmail: true } }
+        end
+        assert_match(/updated a/, Dmail.last.body)
       end
     end
 
@@ -76,14 +90,14 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
       end
 
       should "delete a feedback" do
-        assert_difference "UserFeedback.count", -1 do
+        assert_difference(-> { UserFeedback.count }, -1) do
           delete_auth user_feedback_path(@user_feedback), @critic
         end
       end
 
       context "by a moderator" do
         should "allow deleting feedbacks given to other users" do
-          assert_difference "UserFeedback.count", -1 do
+          assert_difference(-> { UserFeedback.count }, -1) do
             delete_auth user_feedback_path(@user_feedback), @mod
           end
         end
@@ -93,7 +107,7 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
             @user_feedback = create(:user_feedback, user: @mod)
           end
 
-          assert_difference "UserFeedback.count", 0 do
+          assert_no_difference(-> { UserFeedback.count }) do
             delete_auth user_feedback_path(@user_feedback), @mod
           end
         end

test/unit/user_feedback_test.rb

@@ -10,7 +10,7 @@ class UserFeedbackTest < ActiveSupport::TestCase
 
     should "create a dmail" do
       dmail = <<~EOS.chomp
-        @#{@mod.name} created a "positive record":/user_feedbacks?search[user_id]=#{@user.id} for your account:
+        #{@mod.name} created a "positive record":/user_feedbacks?search[user_id]=#{@user.id} for your account:
 
         good job!
       EOS