REFACTOR: Replace score bonuses with low/med/high priorities

We removed score from the UX so it makes more sense to have sites set
priorities instead of score bonuses.

app/assets/javascripts/discourse/controllers/review-settings.js.es6

@@ -1,19 +1,32 @@
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import computed from "ember-addons/ember-computed-decorators";
 
 export default Ember.Controller.extend({
   saving: false,
   saved: false,
 
+  @computed
+  reviewablePriorities() {
+    return [
+      { id: 0, name: "low" },
+      { id: 5, name: "medium" },
+      { id: 10, name: "high" }
+    ];
+  },
+
   actions: {
     save() {
-      let bonuses = {};
+      let priorities = {};
       this.get("settings.reviewable_score_types").forEach(st => {
-        bonuses[st.id] = parseFloat(st.score_bonus);
+        priorities[st.id] = parseFloat(st.reviewable_priority);
       });
 
       this.set("saving", true);
-      ajax("/review/settings", { method: "PUT", data: { bonuses } })
+      ajax("/review/settings", {
+        method: "PUT",
+        data: { reviewable_priorities: priorities }
+      })
         .then(() => {
           this.set("saved", true);
         })

app/assets/javascripts/discourse/templates/review-settings.hbs

@@ -1,12 +1,11 @@
 <div class='reviewable-settings'>
-  <h4>{{i18n "review.settings.score_bonuses.title"}}</h4>
+  <h4>{{i18n "review.settings.priorities.title"}}</h4>
-  <p class='description'>{{i18n "review.settings.score_bonuses.description"}}</p>
 
   {{#each settings.reviewable_score_types as |rst|}}
     <div class='reviewable-score-type'>
       <div class='title'>{{rst.title}}</div>
       <div class='field'>
-        {{input value=rst.score_bonus}}
+        {{combo-box value=rst.reviewable_priority content=settings.reviewable_priorities}}
       </div>
     </div>
   {{/each}}

app/assets/stylesheets/common/base/reviewables.scss

@@ -51,8 +51,9 @@
 }
 
 .reviewable-settings {
-  p.description {
+  h4 {
-    margin-bottom: 2em;
+    margin-top: 1em;
+    margin-bottom: 1em;
   }
 
   .saved {
@@ -60,7 +61,11 @@
   }
   .reviewable-score-type {
     display: flex;
+    margin-bottom: 0.5em;
 
+    .select-kit {
+      min-width: 10em;
+    }
     .title {
       width: 30%;
     }

app/controllers/reviewables_controller.rb

@@ -188,14 +188,21 @@ class ReviewablesController < ApplicationController
     raise Discourse::InvalidAccess.new unless current_user.admin?
 
     post_action_types = PostActionType.where(id: PostActionType.flag_types.values).order('id')
-    data = { reviewable_score_types: post_action_types }
 
     if request.put?
-      params[:bonuses].each do |id, bonus|
+      params[:reviewable_priorities].each do |id, priority|
-        PostActionType.where(id: id).update_all(score_bonus: bonus.to_f)
+        if !priority.nil? && Reviewable.priorities.has_value?(priority.to_i)
+          # For now, the score bonus is equal to the priority. In the future we might want
+          # to calculate it a different way.
+          PostActionType.where(id: id).update_all(
+            reviewable_priority: priority.to_i,
+            score_bonus: priority.to_f
+          )
+        end
       end
     end
 
+    data = { reviewable_score_types: post_action_types }
     render_serialized(data, ReviewableSettingsSerializer, rest_serializer: true)
   end
 

app/jobs/scheduled/reviewable_priorities.rb

@@ -12,6 +12,8 @@ class Jobs::ReviewablePriorities < Jobs::Scheduled
         FROM reviewables
     SQL
 
-    Reviewable.set_priorities(medium: res[0], high: res[1])
+    medium, high = res
+
+    Reviewable.set_priorities(medium: medium, high: high)
   end
 end

app/models/post_action_type.rb

@@ -127,12 +127,12 @@ end
 #
 # Table name: post_action_types
 #
-#  name_key    :string(50)       not null
+#  name_key            :string(50)       not null
-#  is_flag     :boolean          default(FALSE), not null
+#  is_flag             :boolean          default(FALSE), not null
-#  icon        :string(20)
+#  icon                :string(20)
-#  created_at  :datetime         not null
+#  created_at          :datetime         not null
-#  updated_at  :datetime         not null
+#  updated_at          :datetime         not null
-#  id          :integer          not null, primary key
+#  id                  :integer          not null, primary key
-#  position    :integer          default(0), not null
+#  position            :integer          default(0), not null
-#  score_bonus :float            default(0.0), not null
+#  reviewable_priority :integer
 #

app/models/reviewable.rb

@@ -41,6 +41,15 @@ class Reviewable < ActiveRecord::Base
     Jobs.enqueue(:notify_reviewable, reviewable_id: self.id) if pending?
   end
 
+  # The gaps are in case we want more accuracy in the future
+  def self.priorities
+    @priorities ||= Enum.new(
+      low: 0,
+      medium: 5,
+      high: 10
+    )
+  end
+
   def self.statuses
     @statuses ||= Enum.new(
       pending: 0,
@@ -157,15 +166,18 @@ class Reviewable < ActiveRecord::Base
     rs
   end
 
-  def self.set_priorities(medium: nil, high: nil)
+  def self.set_priorities(values)
-    PluginStore.set('reviewables', 'priority_medium', medium) if medium
+    values.each do |k, v|
-    PluginStore.set('reviewables', 'priority_high', high) if high
+      id = Reviewable.priorities[k]
+      PluginStore.set('reviewables', "priority_#{id}", v) unless id.nil?
+    end
   end
 
   def self.min_score_for_priority(priority = nil)
     priority ||= SiteSetting.reviewable_default_visibility
-    return 0.0 unless ['medium', 'high'].include?(priority)
+    id = Reviewable.priorities[priority.to_sym]
-    return PluginStore.get('reviewables', "priority_#{priority}").to_f
+    return 0.0 if id.nil?
+    return PluginStore.get('reviewables', "priority_#{id}").to_f
   end
 
   def history
@@ -502,6 +514,7 @@ end
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null
 #  reviewable_by_group_id  :integer
+#  claimed_by_id           :integer
 #  category_id             :integer
 #  topic_id                :integer
 #  score                   :float            default(0.0), not null

app/models/reviewable_flagged_post.rb

@@ -296,6 +296,7 @@ end
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null
 #  reviewable_by_group_id  :integer
+#  claimed_by_id           :integer
 #  category_id             :integer
 #  topic_id                :integer
 #  score                   :float            default(0.0), not null

app/models/reviewable_queued_post.rb

@@ -147,6 +147,7 @@ end
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null
 #  reviewable_by_group_id  :integer
+#  claimed_by_id           :integer
 #  category_id             :integer
 #  topic_id                :integer
 #  score                   :float            default(0.0), not null

app/models/reviewable_user.rb

@@ -103,6 +103,7 @@ end
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null
 #  reviewable_by_group_id  :integer
+#  claimed_by_id           :integer
 #  category_id             :integer
 #  topic_id                :integer
 #  score                   :float            default(0.0), not null

app/models/user_custom_field.rb

@@ -17,6 +17,5 @@ end
 #
 # Indexes
 #
-#  idx_user_custom_fields_last_reminded_at       (name,user_id) UNIQUE WHERE ((name)::text = 'last_reminded_at'::text)
 #  index_user_custom_fields_on_user_id_and_name  (user_id,name)
 #

app/serializers/reviewable_score_bonus_serializer.rb

@@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-class ReviewableScoreBonusSerializer < ApplicationSerializer
-  attributes :id, :name, :score_bonus
-
-  def name
-    I18n.t("post_action_types.#{object.name_key}.title")
-  end
-end

app/serializers/reviewable_score_type_serializer.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class ReviewableScoreTypeSerializer < ApplicationSerializer
-  attributes :id, :title, :score_bonus, :icon
+  attributes :id, :title, :reviewable_priority, :icon
 
   # Allow us to share post action type translations for backwards compatibility
   def title
@@ -9,12 +9,12 @@ class ReviewableScoreTypeSerializer < ApplicationSerializer
       I18n.t("reviewable_score_types.#{ReviewableScore.types[id]}.title")
   end
 
-  def score_bonus
+  def reviewable_priority
-    object.score_bonus.to_f
+    object.reviewable_priority.to_i
   end
 
-  def include_score_bonus?
+  def include_reviewable_priority?
-    object.respond_to?(:score_bonus)
+    object.respond_to?(:reviewable_priority)
   end
 
   def icon

app/serializers/reviewable_settings_serializer.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class ReviewableSettingsSerializer < ApplicationSerializer
-  attributes :id
+  attributes :id, :reviewable_priorities
 
   has_many :reviewable_score_types, serializer: ReviewableScoreTypeSerializer
 
@@ -12,4 +12,10 @@ class ReviewableSettingsSerializer < ApplicationSerializer
   def reviewable_score_types
     object[:reviewable_score_types]
   end
+
+  def reviewable_priorities
+    Reviewable.priorities.map do |p|
+      { id: p[1], name: I18n.t("reviewables.priorities.#{p[0]}") }
+    end
+  end
 end

config/locales/client.en.yml

@@ -377,9 +377,8 @@ en:
         saved: "Saved"
         save_changes: "Save Changes"
         title: "Settings"
-        score_bonuses:
+        priorities:
-          title: "Score Bonuses"
+          title: "Reviewable Priorities"
-          description: "Bonuses allow certain types to be scored higher than others so they can be prioritized. Note: changing these values will not apply to previously scored items."
 
       moderation_history: "Moderation History"
       view_all: "View All"

config/locales/server.en.yml

@@ -4414,6 +4414,11 @@ en:
     webhook_deactivation_reason: "Your webhook has been automatically deactivated. We received multiple '%{status}' HTTP status failure responses."
 
   reviewables:
+    priorities:
+      low: "Low"
+      medium: "Medium"
+      high: "High"
+
     must_claim: "You must claim items before acting on them."
     user_claimed: "This item has been claimed by another user."
     missing_version: "You must supply a version parameter"

db/migrate/20190522194332_add_priority_to_post_action_types.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddPriorityToPostActionTypes < ActiveRecord::Migration[5.2]
+  def up
+    add_column :post_action_types, :reviewable_priority, :integer, default: 0, null: false
+    execute(<<~SQL)
+      UPDATE post_action_types
+      SET reviewable_priority = CASE
+        WHEN score_bonus > 5 THEN 10
+        WHEN score_bonus > 0 THEN 5
+        ELSE 0
+      END
+    SQL
+  end
+
+  def down
+    remove_column :post_action_types, :reviewable_priority
+  end
+end

spec/jobs/reviewable_priorities_spec.rb

@@ -8,16 +8,17 @@ describe Jobs::ReviewablePriorities do
     (1..6).each { |i| Fabricate(:reviewable, score: i) }
     Jobs::ReviewablePriorities.new.execute({})
 
-    expect(Reviewable.min_score_for_priority('low')).to eq(0.0)
+    expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
+    expect(Reviewable.min_score_for_priority(:medium)).to eq(3.0)
     expect(Reviewable.min_score_for_priority('medium')).to eq(3.0)
-    expect(Reviewable.min_score_for_priority('high')).to eq(6.0)
+    expect(Reviewable.min_score_for_priority(:high)).to eq(6.0)
   end
 
   it "will return 0 if no reviewables exist" do
     Jobs::ReviewablePriorities.new.execute({})
 
-    expect(Reviewable.min_score_for_priority('low')).to eq(0.0)
+    expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
-    expect(Reviewable.min_score_for_priority('medium')).to eq(0.0)
+    expect(Reviewable.min_score_for_priority(:medium)).to eq(0.0)
-    expect(Reviewable.min_score_for_priority('high')).to eq(0.0)
+    expect(Reviewable.min_score_for_priority(:high)).to eq(0.0)
   end
 end

spec/models/reviewable_spec.rb

@@ -284,20 +284,20 @@ RSpec.describe Reviewable, type: :model do
 
   context "priorities" do
     it "returns 0 for unknown priorities" do
-      expect(Reviewable.min_score_for_priority('wat')).to eq(0.0)
+      expect(Reviewable.min_score_for_priority(:wat)).to eq(0.0)
     end
 
     it "returns 0 for all by default" do
-      expect(Reviewable.min_score_for_priority('low')).to eq(0.0)
+      expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
-      expect(Reviewable.min_score_for_priority('medium')).to eq(0.0)
+      expect(Reviewable.min_score_for_priority(:medium)).to eq(0.0)
-      expect(Reviewable.min_score_for_priority('high')).to eq(0.0)
+      expect(Reviewable.min_score_for_priority(:high)).to eq(0.0)
     end
 
     it "can be set manually with `set_priorities`" do
       Reviewable.set_priorities(medium: 12.5, high: 123.45)
-      expect(Reviewable.min_score_for_priority('low')).to eq(0.0)
+      expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
-      expect(Reviewable.min_score_for_priority('medium')).to eq(12.5)
+      expect(Reviewable.min_score_for_priority(:medium)).to eq(12.5)
-      expect(Reviewable.min_score_for_priority('high')).to eq(123.45)
+      expect(Reviewable.min_score_for_priority(:high)).to eq(123.45)
     end
 
     it "will return the default priority if none supplied" do

spec/requests/reviewables_controller_spec.rb

@@ -387,9 +387,23 @@ describe ReviewablesController do
       end
 
       it "allows the settings to be updated" do
-        put "/review/settings.json", params: { bonuses: { 8 => 3.45 } }
+        put "/review/settings.json", params: { reviewable_priorities: { 8 => Reviewable.priorities[:medium] } }
         expect(response.code).to eq("200")
-        expect(PostActionType.find_by(id: 8).score_bonus).to eq(3.45)
+        pa = PostActionType.find_by(id: 8)
+        expect(pa.reviewable_priority).to eq(Reviewable.priorities[:medium])
+        expect(pa.score_bonus).to eq(5.0)
+
+        put "/review/settings.json", params: { reviewable_priorities: { 8 => Reviewable.priorities[:low] } }
+        expect(response.code).to eq("200")
+        pa = PostActionType.find_by(id: 8)
+        expect(pa.reviewable_priority).to eq(Reviewable.priorities[:low])
+        expect(pa.score_bonus).to eq(0.0)
+
+        put "/review/settings.json", params: { reviewable_priorities: { 8 => Reviewable.priorities[:high] } }
+        expect(response.code).to eq("200")
+        pa = PostActionType.find_by(id: 8)
+        expect(pa.reviewable_priority).to eq(Reviewable.priorities[:high])
+        expect(pa.score_bonus).to eq(10.0)
       end
     end
 

test/javascripts/helpers/review-pretender.js.es6

@@ -95,17 +95,22 @@ export default function(helpers) {
         {
           id: 3,
           title: "Off-Topic",
-          score_bonus: 0.0
+          reviewable_priority: 0
         },
         {
           id: 4,
           title: "Inappropriate",
-          score_bonus: 0.0
+          reviewable_priority: 5
         }
       ],
       reviewable_settings: {
         id: 13870,
-        reviewable_score_type_ids: [3, 4]
+        reviewable_score_type_ids: [3, 4],
+        reviewable_priorities: [
+          { id: 0, name: "Low" },
+          { id: 5, name: "Medium" },
+          { id: 10, name: "High" }
+        ]
       },
       __rest_serializer: "1"
     });