Merge pull request #16008 from hellcp-work/refactor-decision-kind-2

Start using STI for decision model

src/api/app/assets/javascripts/webui/canned_responses.js

@@ -1,10 +1,15 @@
 function setupCannedResponses() { // jshint ignore:line
   $('[data-canned-controller]').on('click', '[data-canned-response]', function(e) {
     $(e.target).closest('[data-canned-controller]').find('textarea').val(e.target.dataset.cannedResponse);
-    // TODO: adapt the following line when Decision#kind is replaced with Decision#type
-    // Set `cleared` by default for canned responses with decision_kind nil
-    let kind = e.target.dataset.decisionKind === 'favored' ? 'favor' : 'cleared';
-    $(e.target).closest('[data-canned-controller]').find('#decision_kind').val(kind);
+    // let's make sure this exists first, to avoid errors
+    if ($(e.target).closest('[data-canned-controller]').find('#decision_type').length !== 0) {
+      // we gather up all the possible options, to make sure whatever we click doesn't wipe out the selection box
+      var optionsHtml = $(e.target).closest('[data-canned-controller]').find('#decision_type')[0].options;
+      var options = Array.from(optionsHtml).map(el => el.value);
+      // and if whatever we are trying to set exists within the options, we set it, otherwise we don't change the select box
+      if (options.includes(e.target.dataset.decisionType))
+        $(e.target).closest('[data-canned-controller]').find('#decision_type').val(e.target.dataset.decisionType);
+    }
     // we have to enable the submit button for the comments form
     $(e.target).closest('[class*="-comment-form"]').find('input[type="submit"]').prop('disabled', false);
   });

src/api/app/components/canned_responses_dropdown_component.html.haml

@@ -4,9 +4,10 @@
     Canned Responses
   %ul.dropdown-menu
     - @canned_responses_by_kind.each do |decision_kind, canned_responses|
-      .dropdown-header= decision_kind&.capitalize || 'Generic'
+      .dropdown-header= decision_kind&.humanize || 'Generic'
       - canned_responses.each do |canned_response|
-        %li.dropdown-item{ data: { 'canned-response': canned_response.content, 'decision_kind': decision_kind }, role: 'button' }
+        - decision_type = "Decision#{decision_kind.camelize}" if decision_kind.present?
+        %li.dropdown-item{ data: { 'canned-response': canned_response.content, 'decision-type': decision_type }, role: 'button' }
           = canned_response.title
     %hr.dropdown-divider/
     = link_to('Create and modify' , canned_responses_path, class: 'dropdown-item')

src/api/app/components/reports_modal_component.html.haml

@@ -21,8 +21,7 @@
               -# Decision-related canned responses exclusively:
               = render CannedResponsesDropdownComponent.new(canned_responses)
             = form.text_area(:reason, class: 'form-control mb-3', required: true, placeholder: 'Reason for the decision')
-            -# TODO: this should be adapted when we move to Decision#type
-            = form.select(:kind, DECISION_KIND_MAP.to_a, {}, class: 'form-select')
+            = form.select(:type, Decision.types(reportable), {}, class: 'form-select')
           .modal-footer
             - reports.each do |report|
               = form.hidden_field(:report_ids, multiple: true, value: report.id)

src/api/app/components/reports_modal_component.rb

@@ -1,9 +1,6 @@
 class ReportsModalComponent < ApplicationComponent
   attr_reader :reportable, :reportable_name, :user, :reports
 
-  # TODO: temporary solution until Decision#type replaces Decision#kind with new values
-  DECISION_KIND_MAP = { 'cleared' => 'cleared', 'favored' => 'favor' }.freeze
-
   def initialize(reportable:, reportable_name:, user:, reports:)
     super
 

src/api/app/controllers/webui/decisions_controller.rb

@@ -19,6 +19,8 @@ def create
   private
 
   def decision_params
-    params.require(:decision).permit(:reason, :kind, report_ids: [])
+    # TODO: remove merge and replace decision_kind with decision_type
+    kind = params[:decision][:type] == 'DecisionFavored' ? 'favor' : 'cleared'
+    params.require(:decision).permit(:reason, :type, report_ids: []).merge(kind: kind)
   end
 end

src/api/app/models/decision.rb

@@ -1,11 +1,14 @@
 class Decision < ApplicationRecord
+  TYPES = %w[DecisionFavored DecisionCleared].freeze
+
   validates :reason, presence: true, length: { maximum: 65_535 }
   validates :type, presence: true, length: { maximum: 255 }
 
   belongs_to :moderator, class_name: 'User', optional: false
 
   has_many :reports, dependent: :nullify
 
+  # TODO: Remove this after type is deployed
   enum kind: {
     cleared: 0,
     favor: 1
@@ -14,8 +17,31 @@ class Decision < ApplicationRecord
   after_create :create_event
   after_create :track_decision
 
+  def description
+    'The moderator decided on the report'
+  end
+
+  # List of all viable types for a reportable, used in the decision creation form
+  def self.types(reportable)
+    TYPES.filter_map do |decision_type_name|
+      decision_type = decision_type_name.constantize
+      [decision_type.display_name, decision_type.name] if decision_type.display?(reportable)
+    end.compact.to_h
+  end
+
+  # We use this to determine if the decision type should be displayed for reportable
+  def self.display?(_reportable)
+    true
+  end
+
+  # We display this in the decision creation form
+  def self.display_name
+    'unknown'
+  end
+
   private
 
+  # TODO: Replace this with `AbstractMethodCalled` after type is deployed
   def create_event
     case kind
     when 'cleared'
@@ -29,8 +55,9 @@ def event_parameters
     { id: id, moderator_id: moderator.id, reason: reason, report_last_id: reports.last.id, reportable_type: reports.first.reportable.class.name }
   end
 
+  # TODO: Remove kind after type is deployed
   def track_decision
-    RabbitmqBus.send_to_bus('metrics', "decision,kind=#{kind} hours_before_decision=#{hours_before_decision},count=1")
+    RabbitmqBus.send_to_bus('metrics', "decision,kind=#{kind},type=#{type} hours_before_decision=#{hours_before_decision},count=1")
   end
 
   def hours_before_decision

src/api/app/models/decision_cleared.rb

@@ -1,4 +1,19 @@
 class DecisionCleared < Decision
+  after_create :create_event
+
+  def description
+    'The moderator decided to clear the report'
+  end
+
+  def self.display_name
+    'cleared'
+  end
+
+  private
+
+  def create_event
+    Event::ClearedDecision.create(event_parameters)
+  end
 end
 
 # == Schema Information

src/api/app/models/decision_favored.rb

@@ -0,0 +1,38 @@
+class DecisionFavored < Decision
+  after_create :create_event
+
+  def description
+    'The moderator decided to favor the report'
+  end
+
+  def self.display_name
+    'favored'
+  end
+
+  private
+
+  def create_event
+    Event::FavoredDecision.create(event_parameters)
+  end
+end
+
+# == Schema Information
+#
+# Table name: decisions
+#
+#  id           :bigint           not null, primary key
+#  kind         :integer          default("cleared")
+#  reason       :text(65535)      not null
+#  type         :string(255)      not null, default("DecisionCleared")
+#  created_at   :datetime         not null
+#  updated_at   :datetime         not null
+#  moderator_id :integer          not null, indexed
+#
+# Indexes
+#
+#  index_decisions_on_moderator_id  (moderator_id)
+#
+# Foreign Keys
+#
+#  fk_rails_...  (moderator_id => users.id)
+#

src/api/app/policies/appeal_policy.rb

@@ -31,12 +31,12 @@ def create?
   def decision_cleared_report_from_user?
     return false unless record.appellant == user
 
-    record.decision.kind == 'cleared' && record.decision.reports.pluck(:user_id).include?(user.id)
+    record.decision.type == 'DecisionCleared' && record.decision.reports.pluck(:user_id).include?(user.id)
   end
 
   def decision_favored_report_of_action_from_user?
     return false unless record.appellant == user
 
-    record.decision.kind == 'favor' && "#{@report.reportable_type}Policy".constantize.new(user, @report.reportable).update?
+    record.decision.type == 'DecisionFavored' && "#{@report.reportable_type}Policy".constantize.new(user, @report.reportable).update?
   end
 end

src/api/app/policies/decision_favored_policy.rb

@@ -0,0 +1,2 @@
+class DecisionFavoredPolicy < DecisionPolicy
+end

src/api/app/views/webui/appeals/_list_decision.html.haml

@@ -2,7 +2,7 @@
   .card-body
     %h3 Decision
     %p
-      The moderator decided to
-      = decision.kind
-      the reports. The reason was:
+      = succeed '.' do
+        = decision.description
+      The reason was:
       = decision.reason

src/api/app/views/webui/users/canned_responses/edit.html.haml

@@ -13,5 +13,6 @@
       - if policy(Decision.new).create?
         .mb-3
           = f.label :decision_kind, "Choose 'cleared' or 'favored' if this is a decision's reason"
-          = f.select(:decision_kind, CannedResponse.decision_kinds.keys, { include_blank: 'none' }, class: 'form-select')
+          - kinds = CannedResponse.decision_kinds.keys.to_h { |k| [k.humanize, k] }
+          = f.select(:decision_kind, kinds, { include_blank: 'none' }, class: 'form-select')
       = f.submit 'Save', class: 'btn btn-primary'

src/api/app/views/webui/users/canned_responses/index.html.haml

@@ -13,7 +13,8 @@
       - if policy(Decision.new).create?
         .mb-3
           = f.label :decision_kind, "Choose 'cleared' or 'favored' if this is a decision's reason"
-          = f.select(:decision_kind, CannedResponse.decision_kinds.keys, { include_blank: 'none', selected: nil }, class: 'form-select')
+          - kinds = CannedResponse.decision_kinds.keys.to_h { |k| [k.humanize, k] }
+          = f.select(:decision_kind, kinds, { include_blank: 'none', selected: nil }, class: 'form-select')
       = f.submit 'Create', class: 'btn btn-primary'
 
 .card.mt-2
@@ -31,7 +32,7 @@
             .accordion-collapse.collapse{ id: "canned-response-#{canned_response.id}-collapse" }
               .accordion-body
                 .d-flex.justify-content-between
-                  %strong= canned_response.decision_kind.try(:capitalize) || 'Generic'
+                  %strong= canned_response.decision_kind.try(:humanize) || 'Generic'
                   .d-flex
                     = link_to(edit_canned_response_path(canned_response), class: 'btn py-0 px-1 btn-link') do
                       %i.fas.fa-edit

src/api/db/data/20240417122944_set_the_decision_type_from_decision_kind.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class SetTheDecisionTypeFromDecisionKind < ActiveRecord::Migration[7.0]
+  def up
+    Decision.in_batches do |batch|
+      batch.each do |decision|
+        decision.type = decision.favor? ? 'DecisionFavored' : 'DecisionCleared'
+      end
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

src/api/db/data_schema.rb

@@ -1 +1 @@
-DataMigrate::Data.define(version: 20240321103332)
+DataMigrate::Data.define(version: 20240417122944)

src/api/lib/tasks/dev/reports.rake

@@ -55,7 +55,7 @@ namespace :dev do
 
       Report.find_each do |report|
         # Reports with even id will be 'cleared' (0). Those with odd id will be 'favor' (1).
-        Decision.create!(reason: "Just because! #{report.id}", moderator: admin, kind: (report.id % 2), reports: [report])
+        Decision.create!(reason: "Just because! #{report.id}", moderator: admin, type: Decision::TYPES[(report.id % 2)], reports: [report])
       end
 
       # The same decision applies to more than one report about the same object/reportable.

src/api/spec/factories/appeal.rb

@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :appeal do
-    decision
+    decision { association :decision_cleared }
     appellant { association :confirmed_user }
     reason { 'Some random reason' }
   end

src/api/spec/factories/decisions.rb → src/api/spec/factories/decision_cleared.rb

@@ -1,18 +1,10 @@
 FactoryBot.define do
-  factory :decision do
+  factory :decision_cleared do
     moderator factory: [:user]
     reason { Faker::Markdown.emphasis }
 
     after(:build) do |decision|
       decision.reports << create(:report, reason: 'This is spam!') if decision.reports.empty?
     end
-
-    trait :cleared do
-      kind { 'cleared' }
-    end
-
-    trait :favor do
-      kind { 'favor' }
-    end
   end
 end

src/api/spec/factories/decision_favored.rb

@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :decision_favored do
+    moderator factory: [:user]
+    reason { Faker::Markdown.emphasis }
+
+    after(:build) do |decision|
+      decision.reports << create(:report, reason: 'This is spam!') if decision.reports.empty?
+    end
+  end
+end

src/api/spec/factories/notification.rb

@@ -94,7 +94,7 @@
 
     trait :cleared_decision do
       event_type { 'Event::ClearedDecision' }
-      notifiable { association(:decision, :cleared) }
+      notifiable { association(:decision_cleared) }
 
       after(:build) do |notification|
         notification.event_payload['reportable_type'] ||= notification.notifiable.reports.first.reportable.class.to_s
@@ -103,7 +103,7 @@
 
     trait :favored_decision do
       event_type { 'Event::FavoredDecision' }
-      notifiable { association(:decision, :favor) }
+      notifiable { association(:decision_favored) }
 
       after(:build) do |notification|
         notification.event_payload['reportable_type'] ||= notification.notifiable.reports.first.reportable.class.to_s

src/api/spec/features/beta/webui/appeals_spec.rb

@@ -10,7 +10,7 @@
     let(:user) { create(:confirmed_user) }
     let(:comment) { create(:comment_package, user: user) }
     let(:report) { create(:report, reportable: comment) }
-    let!(:decision) { Decision.create!(kind: Decision.kinds['favor'], reason: "It's spam indeed", reports: [report], moderator: moderator) }
+    let!(:decision) { Decision.create!(type: 'DecisionFavored', reason: "It's spam indeed", reports: [report], moderator: moderator) }
 
     before do
       EventSubscription.create!(eventtype: Event::FavoredDecision.name,

src/api/spec/features/beta/webui/decisions_spec.rb

@@ -11,7 +11,7 @@
   def fill_decisions_modal(reportable)
     within("#reports-modal-#{reportable.class.to_s.downcase}-#{reportable.id}") do
       fill_in id: 'decision_reason', with: 'Reason for reporting is correct.'
-      select('favor', from: 'decision[kind]')
+      select('favored', from: 'decision[type]')
       click_button('Submit')
     end
   end

src/api/spec/features/webui/canned_responses_spec.rb

@@ -58,7 +58,7 @@
       visit canned_responses_path
       fill_in(name: 'canned_response[title]', with: 'wow')
       fill_in(name: 'canned_response[content]', with: 'a decision-related canned response')
-      find(:id, 'canned_response_decision_kind').select('favored')
+      find(:id, 'canned_response_decision_kind').select('Favored')
       click_button('Create')
       find('.accordion-button').click
     end

src/api/spec/mailers/event_mailer_spec.rb

@@ -449,7 +449,7 @@
       let(:report) { create(:report, user: reporter) }
       let(:package) { report.reportable.commentable }
       let!(:subscription) { create(:event_subscription_cleared_decision, user: reporter) }
-      let(:decision) { create(:decision, :cleared, moderator: admin, reason: 'This is NOT spam.', reports: [report]) }
+      let(:decision) { create(:decision_cleared, moderator: admin, reason: 'This is NOT spam.', reports: [report]) }
       let(:event) { Event::ClearedDecision.last }
       let(:mail) { EventMailer.with(subscribers: event.subscribers, event: event).notification_email.deliver_now }
 
@@ -492,7 +492,7 @@
       let!(:reporter_subscription) { create(:event_subscription_favored_decision, user: reporter) }
       let!(:offender_subscription) { create(:event_subscription_favored_decision, user: offender, receiver_role: 'offender') }
 
-      let(:decision) { create(:decision, :favor, moderator: admin, reason: 'This is spam for sure.', reports: [report]) }
+      let(:decision) { create(:decision_favored, moderator: admin, reason: 'This is spam for sure.', reports: [report]) }
       let(:event) { Event::FavoredDecision.last }
       let(:mail) { EventMailer.with(subscribers: event.subscribers, event: event).notification_email.deliver_now }
 
@@ -535,7 +535,7 @@
 
       let!(:moderator_subscription) { create(:event_subscription_appeal_created, user: moderator) }
 
-      let(:decision) { create(:decision, :favor, moderator: moderator, reason: 'This is spam for sure.', reports: [report]) }
+      let(:decision) { create(:decision_favored, moderator: moderator, reason: 'This is spam for sure.', reports: [report]) }
       let(:appeal) { create(:appeal, appellant: appellant, decision: decision, reason: 'I strongly disagree!') }
       let(:event) { Event::AppealCreated.last }
       let(:mail) { EventMailer.with(subscribers: event.subscribers, event: event).notification_email.deliver_now }
@@ -579,7 +579,7 @@
 
       let!(:moderator_subscription) { create(:event_subscription_appeal_created, user: moderator) }
 
-      let(:decision) { create(:decision, :favor, moderator: moderator, reason: 'This is spam for sure.', reports: [report]) }
+      let(:decision) { create(:decision_favored, moderator: moderator, reason: 'This is spam for sure.', reports: [report]) }
       let(:appeal) { create(:appeal, appellant: appellant, decision: decision, reason: 'I strongly disagree!') }
       let(:event) { Event::AppealCreated.last }
       let(:mail) { EventMailer.with(subscribers: event.subscribers, event: event).notification_email.deliver_now }