Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start using STI for decision model #16008

Merged
merged 8 commits into from Apr 22, 2024
13 changes: 9 additions & 4 deletions 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);
});
Expand Down
Expand Up @@ -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')
Expand Down
3 changes: 1 addition & 2 deletions src/api/app/components/reports_modal_component.html.haml
Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions 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

Expand Down
4 changes: 3 additions & 1 deletion src/api/app/controllers/webui/decisions_controller.rb
Expand Up @@ -19,6 +19,8 @@
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)

Check warning on line 24 in src/api/app/controllers/webui/decisions_controller.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/webui/decisions_controller.rb#L23-L24

Added lines #L23 - L24 were not covered by tests
end
end
29 changes: 28 additions & 1 deletion 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 }
hellcp-work marked this conversation as resolved.
Show resolved Hide resolved

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
Expand All @@ -14,8 +17,31 @@
after_create :create_event
after_create :track_decision

def description
'The moderator decided on the report'

Check warning on line 21 in src/api/app/models/decision.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision.rb#L21

Added line #L21 was not covered by tests
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)

Check warning on line 28 in src/api/app/models/decision.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision.rb#L26-L28

Added lines #L26 - L28 were not covered by tests
end.compact.to_h
end

# We use this to determine if the decision type should be displayed for reportable
def self.display?(_reportable)
true

Check warning on line 34 in src/api/app/models/decision.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision.rb#L34

Added line #L34 was not covered by tests
end

# We display this in the decision creation form
def self.display_name
'unknown'

Check warning on line 39 in src/api/app/models/decision.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision.rb#L39

Added line #L39 was not covered by tests
end

private

# TODO: Replace this with `AbstractMethodCalled` after type is deployed
def create_event
case kind
when 'cleared'
Expand All @@ -29,8 +55,9 @@
{ 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
Expand Down
15 changes: 15 additions & 0 deletions 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'

Check warning on line 5 in src/api/app/models/decision_cleared.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision_cleared.rb#L5

Added line #L5 was not covered by tests
end

def self.display_name
'cleared'

Check warning on line 9 in src/api/app/models/decision_cleared.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision_cleared.rb#L9

Added line #L9 was not covered by tests
end

private

def create_event
Event::ClearedDecision.create(event_parameters)
end
end

# == Schema Information
Expand Down
38 changes: 38 additions & 0 deletions 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'

Check warning on line 5 in src/api/app/models/decision_favored.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision_favored.rb#L5

Added line #L5 was not covered by tests
end

def self.display_name
'favored'

Check warning on line 9 in src/api/app/models/decision_favored.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/models/decision_favored.rb#L9

Added line #L9 was not covered by tests
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)
#
4 changes: 2 additions & 2 deletions src/api/app/policies/appeal_policy.rb
Expand Up @@ -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
2 changes: 2 additions & 0 deletions src/api/app/policies/decision_favored_policy.rb
@@ -0,0 +1,2 @@
class DecisionFavoredPolicy < DecisionPolicy
end
6 changes: 3 additions & 3 deletions src/api/app/views/webui/appeals/_list_decision.html.haml
Expand Up @@ -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
Expand Up @@ -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'
Expand Up @@ -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
Expand All @@ -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
Expand Down
@@ -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
2 changes: 1 addition & 1 deletion src/api/db/data_schema.rb
@@ -1 +1 @@
DataMigrate::Data.define(version: 20240321103332)
DataMigrate::Data.define(version: 20240417122944)
2 changes: 1 addition & 1 deletion src/api/lib/tasks/dev/reports.rake
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
@@ -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
10 changes: 10 additions & 0 deletions 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
4 changes: 2 additions & 2 deletions src/api/spec/factories/notification.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/beta/webui/appeals_spec.rb
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/beta/webui/decisions_spec.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/api/spec/features/webui/canned_responses_spec.rb
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/api/spec/mailers/event_mailer_spec.rb
Expand Up @@ -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 }

Expand Down Expand Up @@ -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 }

Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down