diff --git a/app/assets/javascripts/discourse/controllers/review-settings.js.es6 b/app/assets/javascripts/discourse/controllers/review-settings.js.es6 index 339b0055949..f832f77d7ee 100644 --- a/app/assets/javascripts/discourse/controllers/review-settings.js.es6 +++ b/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); }) diff --git a/app/assets/javascripts/discourse/templates/review-settings.hbs b/app/assets/javascripts/discourse/templates/review-settings.hbs index 142562cf2d6..2669a442d89 100644 --- a/app/assets/javascripts/discourse/templates/review-settings.hbs +++ b/app/assets/javascripts/discourse/templates/review-settings.hbs @@ -1,12 +1,11 @@
-

{{i18n "review.settings.score_bonuses.title"}}

-

{{i18n "review.settings.score_bonuses.description"}}

+

{{i18n "review.settings.priorities.title"}}

{{#each settings.reviewable_score_types as |rst|}}
{{rst.title}}
- {{input value=rst.score_bonus}} + {{combo-box value=rst.reviewable_priority content=settings.reviewable_priorities}}
{{/each}} diff --git a/app/assets/stylesheets/common/base/reviewables.scss b/app/assets/stylesheets/common/base/reviewables.scss index 8bdf2f338eb..8e78aed24fe 100644 --- a/app/assets/stylesheets/common/base/reviewables.scss +++ b/app/assets/stylesheets/common/base/reviewables.scss @@ -51,8 +51,9 @@ } .reviewable-settings { - p.description { - margin-bottom: 2em; + h4 { + 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%; } diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index 63c4bb56332..8d6818d8232 100644 --- a/app/controllers/reviewables_controller.rb +++ b/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| - PostActionType.where(id: id).update_all(score_bonus: bonus.to_f) + params[:reviewable_priorities].each do |id, priority| + 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 diff --git a/app/jobs/scheduled/reviewable_priorities.rb b/app/jobs/scheduled/reviewable_priorities.rb index f67698a21d1..49f4aa2cd84 100644 --- a/app/jobs/scheduled/reviewable_priorities.rb +++ b/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 diff --git a/app/models/post_action_type.rb b/app/models/post_action_type.rb index 121db590aa3..a1c8ea8f8ba 100644 --- a/app/models/post_action_type.rb +++ b/app/models/post_action_type.rb @@ -127,12 +127,12 @@ end # # Table name: post_action_types # -# name_key :string(50) not null -# is_flag :boolean default(FALSE), not null -# icon :string(20) -# created_at :datetime not null -# updated_at :datetime not null -# id :integer not null, primary key -# position :integer default(0), not null -# score_bonus :float default(0.0), not null +# name_key :string(50) not null +# is_flag :boolean default(FALSE), not null +# icon :string(20) +# created_at :datetime not null +# updated_at :datetime not null +# id :integer not null, primary key +# position :integer default(0), not null +# reviewable_priority :integer # diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index bbbd3a626c7..10cb0242115 100644 --- a/app/models/reviewable.rb +++ b/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) - PluginStore.set('reviewables', 'priority_medium', medium) if medium - PluginStore.set('reviewables', 'priority_high', high) if high + def self.set_priorities(values) + values.each do |k, v| + 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) - return PluginStore.get('reviewables', "priority_#{priority}").to_f + id = Reviewable.priorities[priority.to_sym] + 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 diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index 5c00228f8e3..75769eee8ce 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/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 diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb index 2255452a29d..97a3f6ed6c1 100644 --- a/app/models/reviewable_queued_post.rb +++ b/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 diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 31b8776ef8a..90a038860df 100644 --- a/app/models/reviewable_user.rb +++ b/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 diff --git a/app/models/user_custom_field.rb b/app/models/user_custom_field.rb index b64865386f1..0fb77e5f634 100644 --- a/app/models/user_custom_field.rb +++ b/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) # diff --git a/app/serializers/reviewable_score_bonus_serializer.rb b/app/serializers/reviewable_score_bonus_serializer.rb deleted file mode 100644 index 45ea585c803..00000000000 --- a/app/serializers/reviewable_score_bonus_serializer.rb +++ /dev/null @@ -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 diff --git a/app/serializers/reviewable_score_type_serializer.rb b/app/serializers/reviewable_score_type_serializer.rb index 9320e751212..60368ebcbce 100644 --- a/app/serializers/reviewable_score_type_serializer.rb +++ b/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 - object.score_bonus.to_f + def reviewable_priority + object.reviewable_priority.to_i end - def include_score_bonus? - object.respond_to?(:score_bonus) + def include_reviewable_priority? + object.respond_to?(:reviewable_priority) end def icon diff --git a/app/serializers/reviewable_settings_serializer.rb b/app/serializers/reviewable_settings_serializer.rb index 96e5573149c..f2a34e4e962 100644 --- a/app/serializers/reviewable_settings_serializer.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9f81f579ce8..a2152abe19d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -377,9 +377,8 @@ en: saved: "Saved" save_changes: "Save Changes" title: "Settings" - score_bonuses: - title: "Score Bonuses" - 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." + priorities: + title: "Reviewable Priorities" moderation_history: "Moderation History" view_all: "View All" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3cd45e95b72..905c77a3c5c 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/db/migrate/20190522194332_add_priority_to_post_action_types.rb b/db/migrate/20190522194332_add_priority_to_post_action_types.rb new file mode 100644 index 00000000000..7cbc5ade41d --- /dev/null +++ b/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 diff --git a/spec/jobs/reviewable_priorities_spec.rb b/spec/jobs/reviewable_priorities_spec.rb index b3962a24014..2f53a6b9479 100644 --- a/spec/jobs/reviewable_priorities_spec.rb +++ b/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('medium')).to eq(0.0) - expect(Reviewable.min_score_for_priority('high')).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(:high)).to eq(0.0) end end diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb index e27e6ecdf4e..bd0d40233d5 100644 --- a/spec/models/reviewable_spec.rb +++ b/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('medium')).to eq(0.0) - expect(Reviewable.min_score_for_priority('high')).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(: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('medium')).to eq(12.5) - expect(Reviewable.min_score_for_priority('high')).to eq(123.45) + 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(:high)).to eq(123.45) end it "will return the default priority if none supplied" do diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb index 0a32a3f1283..d83135e79ff 100644 --- a/spec/requests/reviewables_controller_spec.rb +++ b/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 diff --git a/test/javascripts/helpers/review-pretender.js.es6 b/test/javascripts/helpers/review-pretender.js.es6 index 4e6d1194526..26bb589e12f 100644 --- a/test/javascripts/helpers/review-pretender.js.es6 +++ b/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" });