FEATURE: Hide Reviewable scores, change score filter to Priority

We found score hard to understand. It is still there behind the scenes
for sorting purposes, but it is no longer shown.

You can now filter by minimum priority (low, med, high) instead of
score.
This commit is contained in:
Robin Ward 2019-05-07 13:25:11 -04:00
parent 5b5b5a5931
commit 5af7c90bab
25 changed files with 138 additions and 58 deletions

View File

@ -2,7 +2,7 @@ import computed from "ember-addons/ember-computed-decorators";
export default Ember.Controller.extend({
queryParams: [
"min_score",
"priority",
"type",
"status",
"category_id",
@ -11,7 +11,7 @@ export default Ember.Controller.extend({
],
type: null,
status: "pending",
min_score: null,
priority: "low",
category_id: null,
reviewables: null,
topic_id: null,
@ -20,7 +20,7 @@ export default Ember.Controller.extend({
init(...args) {
this._super(...args);
this.set("min_score", this.siteSettings.min_score_default_visibility);
this.set("priority", this.siteSettings.reviewable_default_visibility);
this.set("filtersExpanded", !this.site.mobileView);
},
@ -34,6 +34,16 @@ export default Ember.Controller.extend({
});
},
@computed
priorities() {
return ["low", "medium", "high"].map(priority => {
return {
id: priority,
name: I18n.t(`review.filters.priority.${priority}`)
};
});
},
@computed
statuses() {
return [
@ -71,15 +81,9 @@ export default Ember.Controller.extend({
},
refresh() {
// If filterScore is blank use the default
let filterScore = this.get("filterScore");
if (!filterScore || filterScore.length === 0) {
filterScore = this.siteSettings.min_score_default_visibility;
}
this.setProperties({
type: this.get("filterType"),
min_score: filterScore,
priority: this.get("filterPriority"),
status: this.get("filterStatus"),
category_id: this.get("filterCategoryId"),
username: this.get("filterUsername")

View File

@ -1,5 +0,0 @@
import { registerUnbound } from "discourse-common/lib/helpers";
registerUnbound("format-score", function(score) {
return I18n.toNumber(score || 0, { precision: 1 });
});

View File

@ -1,9 +1,5 @@
export default Discourse.Route.extend({
model(params) {
// `0` is a valid query param
if (params.min_score != null) {
params.min_score = params.min_score.toString();
}
return this.store.findAll("reviewable", params);
},
@ -22,8 +18,7 @@ export default Discourse.Route.extend({
filterStatus: meta.status,
filterTopic: meta.topic_id,
filterCategoryId: meta.category_id,
min_score: meta.min_score,
filterScore: meta.min_score,
filterPriority: meta.priority,
reviewableTypes: meta.reviewable_types,
filterUsername: meta.username
});

View File

@ -1,7 +1,6 @@
<div class='reviewable-item {{customClass}}' data-reviewable-id={{reviewable.id}}>
<div class='reviewable-meta-data'>
<span class='reviewable-type'>{{reviewable.humanType}}</span>
<span class='badge-notification new-posts score' title={{i18n "review.scores.about"}}>{{format-score reviewable.score}}</span>
{{#if reviewable.reply_count}}
<span class='reply-count'>{{i18n "review.replies" count=reviewable.reply_count}}</span>
{{/if}}

View File

@ -12,7 +12,6 @@
<td>
{{d-icon rs.score_type.icon}}
{{title}}
<span class="badge-notification new-posts score" title={{i18n "review.scores.about"}}>{{format-score rs.score}}</span>
</td>
<td>
{{format-date rs.created_at format="tiny"}}

View File

@ -29,8 +29,8 @@
</div>
<div class='reviewable-filter'>
<label class='filter-label'>{{i18n "review.filters.minimum_score"}}</label>
{{input value=filterScore class="score-filter"}}
<label class='filter-label'>{{i18n "review.filters.priority.title"}}</label>
{{combo-box value=filterPriority content=priorities}}
</div>
<div class='reviewable-filter'>

View File

@ -6,7 +6,6 @@ class ReviewablesController < ApplicationController
before_action :version_required, only: [:update, :perform]
def index
min_score = params[:min_score].nil? ? SiteSetting.min_score_default_visibility : params[:min_score].to_f
offset = params[:offset].to_i
if params[:type].present?
@ -23,7 +22,7 @@ class ReviewablesController < ApplicationController
status: status,
category_id: category_id,
topic_id: topic_id,
min_score: min_score,
priority: params[:priority],
username: params[:username],
type: params[:type]
}
@ -63,7 +62,7 @@ class ReviewablesController < ApplicationController
# topics isn't indexed on `reviewable_score` and doesn't know what the current user can see,
# so let's query from the inside out.
pending = Reviewable.viewable_by(current_user).pending
pending = pending.where("score >= ?", SiteSetting.min_score_default_visibility)
pending = pending.where("score >= ?", Reviewable.min_score_for_priority)
pending.each do |r|
topic_ids << r.topic_id

View File

@ -0,0 +1,15 @@
class Jobs::ReviewablePriorities < Jobs::Scheduled
every 1.day
def execute(args)
# We calculate the percentiles here for medium and high. Low is always 0 (all)
res = DB.query_single(<<~SQL)
SELECT COALESCE(PERCENTILE_DISC(0.4) WITHIN GROUP (ORDER BY score), 0.0) AS medium,
COALESCE(PERCENTILE_DISC(0.8) WITHIN GROUP (ORDER BY score), 0.0) AS high
FROM reviewables
SQL
Reviewable.set_priorities(medium: res[0], high: res[1])
end
end

View File

@ -56,7 +56,7 @@ class Reviewable < ActiveRecord::Base
end
def self.default_visible
where("score >= ?", SiteSetting.min_score_default_visibility)
where("score >= ?", min_score_for_priority)
end
def self.valid_type?(type)
@ -133,8 +133,8 @@ class Reviewable < ActiveRecord::Base
sub_total = (ReviewableScore.user_flag_score(user) + type_bonus + take_action_bonus)
# We can force a reviewable to hit the threshold, for example with queued posts
if force_review && sub_total < SiteSetting.min_score_default_visibility
sub_total = SiteSetting.min_score_default_visibility
if force_review && sub_total < Reviewable.min_score_for_priority
sub_total = Reviewable.min_score_for_priority
end
rs = reviewable_scores.new(
@ -155,6 +155,17 @@ 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
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
end
def history
reviewable_histories.order(:created_at)
end
@ -316,11 +327,10 @@ class Reviewable < ActiveRecord::Base
type: nil,
limit: nil,
offset: nil,
min_score: nil,
priority: nil,
username: nil
)
min_score ||= SiteSetting.min_score_default_visibility
min_score = Reviewable.min_score_for_priority(priority)
order = (status == :pending) ? 'score DESC, created_at DESC' : 'created_at DESC'
if username.present?
@ -335,7 +345,7 @@ class Reviewable < ActiveRecord::Base
result = result.where(type: type) if type
result = result.where(category_id: category_id) if category_id
result = result.where(topic_id: topic_id) if topic_id
result = result.where("score >= ?", min_score) if min_score
result = result.where("score >= ?", min_score) if min_score > 0
# If a reviewable doesn't have a target, allow us to filter on who created that reviewable.
if user_id

View File

@ -429,6 +429,12 @@ en:
refresh: "Refresh"
status: "Status:"
category: "Category:"
priority:
title: "Minimum Priority:"
low: "Low"
medium: "Medium"
high: "High"
conversation:
view_full: "view full conversation"
scores:

View File

@ -1547,7 +1547,6 @@ de:
auto_silence_fast_typers_max_trust_level: "Maximale Vertrauensstufe, um „Schnelltipper“ stummzuschalten."
auto_silence_first_post_regex: "Regulärer Ausdruck (ohne Groß- und Kleinschreibung), der dafür sorgt dass entsprechende erste Beiträge von Benutzern stummgeschaltet und in die Genehmigungswarteschlange verschoben werden. Beispiel: raging|a[bc]a sorgt dafür, dass Beiträge, die raging, aba oder aca enthalten, zunächst stummgeschaltet werden. Dies betrifft jedoch nur den ersten Beitrag."
flags_default_topics: "Zeige gemeldete Themen standardmäßig im Administrationsbereich"
min_score_default_visibility: "Zeige keine zu überprüfenden Einträge, außer sie erreichen diese Bewertung."
reply_by_email_enabled: "Aktviere das Antworten auf Themen via E-Mail."
reply_by_email_address: "Vorlage für die Antwort einer per E-Mail eingehender E-Mail-Adresse, zum Beispiel: %%{reply_key}@reply.example.com oder replies+%%{reply_key}@example.com"
alternative_reply_by_email_addresses: "Liste alternativer Vorlagen für eingehende E-Mail-Adressen für das Antworten per E-Mail. Beispiel: %%{reply_key}@antwort.example.com|antworten+%%{reply_key}@example.com"

View File

@ -1751,7 +1751,7 @@ en:
auto_silence_fast_typers_max_trust_level: "Maximum trust level to auto silence fast typers"
auto_silence_first_post_regex: "Case insensitive regex that if passed will cause first post by user to be silenced and sent to approval queue. Example: raging|a[bc]a , will cause all posts containing raging or aba or aca to be silenced on first. Only applies to first post."
flags_default_topics: "Show flagged topics by default in the admin section"
min_score_default_visibility: "Don't show reviewable items unless they meet this score"
reviewable_default_visibility: "Don't show reviewable items unless they meet this priority"
reply_by_email_enabled: "Enable replying to topics via email."
reply_by_email_address: "Template for reply by email incoming email address, for example: %%{reply_key}@reply.example.com or replies+%%{reply_key}@example.com"

View File

@ -1575,7 +1575,6 @@ es:
auto_silence_fast_typers_max_trust_level: "Máximo nivel de confianza por debajo del cual se podrán silenciar usuarios que escriban demasiado rápido en su primer post"
auto_silence_first_post_regex: "Expresión regular que no distingue mayúsculas y minúsculas que si es detectada hará que el primer post de un usuario sea silenciado y enviado a la cola de aprobación. Ejemplo: raging|a[bc]a hará que todos los posts que contengan gaging, aba o aca sean silenciados. Sólo tiene efecto en el primer mensaje de un usuario."
flags_default_topics: "Mostrar temas reportados por defecto en la sección de administración"
min_score_default_visibility: "No mostrar elementos revisables salvo que tengan al menos esta puntuación"
reply_by_email_enabled: "Habilitar la respuesta a temas por email."
reply_by_email_address: "Plantilla para la dirección de email que aparecerá al recibir correos con la función de respuesta por email: %%{reply_key}@respuesta.ejemplo.com o respuestas+%%{reply_key}@ejemplo.com"
alternative_reply_by_email_addresses: "Lista de plantillas alternativa para las direcciones de respuesta por email. Ejemplo: %%{reply_key}@reply.example.com|replies+%%{reply_key}@example.com"

View File

@ -1571,7 +1571,6 @@ fr:
auto_silence_fast_typers_max_trust_level: "Niveau de confiance maximum pour automatiquement mettre sous silence les utilisateurs rédigeant des messages trop rapidement."
auto_silence_first_post_regex: "Regex non sensible à la casse qui, si elle est déclenchée, mettre sous silence le premier message de l'utilisateur et l'enverra dans la file d'attente d'approbation.\nExemple: rageux|a[bc]a mettre sous slience les premiers messages contenant rageux ou aba ou aca."
flags_default_topics: "Afficher les sujets signalés par défaut dans la section administrateur"
min_score_default_visibility: "N'afficher les éléments à revoir que s'ils atteignent ce score"
reply_by_email_enabled: "Activer les réponses aux sujets via courriel."
reply_by_email_address: "Modèle pour la réponse par courriel entrant; exemple : %%{reply_key}@reply.example.com ou replies+%%{reply_key}@example.com"
alternative_reply_by_email_addresses: "Liste des templates alternatifs pour les adresses des courriels entrants de la réponse par courriel. Exemple : %%{reply_key}@reply.example.com|replies+%%{reply_key}@example.com"

View File

@ -1577,7 +1577,6 @@ nl:
auto_silence_fast_typers_max_trust_level: "Maximale vertrouwensniveau om snelle typers automatisch te dempen"
auto_silence_first_post_regex: "Hoofdlettergevoelige regex die bij overeenkomst het eerste bericht van een gebruiker dempt en in de wachtrij voor goedkeuring zet. Voorbeeld: raging|a[bc]a zorgt ervoor dat alle eerste berichten die raging, aba of aca bevatten worden gedempt. Alleen van toepassing op het eerste bericht."
flags_default_topics: "Gemarkeerde topics standaard in de beheersectie tonen"
min_score_default_visibility: "Geen beoordeelbare items tonen, tenzij ze aan deze score voldoen"
reply_by_email_enabled: "Antwoorden op topics via e-mail inschakelen."
reply_by_email_address: "Sjabloon voor adres voor inkomende e-mail bij antwoorden per e-mail, bijvoorbeeld: %%{reply_key}@reply.example.com of replies+%%{reply_key}@example.com"
alternative_reply_by_email_addresses: "Lijst van alternatieve sjablonen voor adressen voor inkomende e-mail bij antwoorden per e-mail. Voorbeeld: %%{reply_key}@reply.example.com|replies+%%{reply_key}@example.com"

View File

@ -1494,7 +1494,6 @@ zh_CN:
auto_silence_fast_typers_max_trust_level: "自动禁言输入快速的用户的适用信任等级"
auto_silence_first_post_regex: "检查用户首贴的大小写不相关的正则表达式。如匹配用户将被禁言并送至审核队列。例子raging|a[bc]a 将匹配 raging、aba 或者 aca因此禁言该用户。只对首贴有效。"
flags_default_topics: "在管理栏目中默认显示被标记的主题"
min_score_default_visibility: "除非达到分数否则不显示审核项目"
reply_by_email_enabled: "启用通过邮件回复。"
reply_by_email_address: "通过邮件回复的回复地址模板,例如:%%{reply_key}@reply.example.com 或 replies+%%{reply_key}@example.com"
alternative_reply_by_email_addresses: "通过邮件回复的回复地址模板,例如:%%{reply_key}@reply.example.com|replies+%%{reply_key}@example.com"

View File

@ -1388,9 +1388,14 @@ spam:
flags_default_topics:
default: false
client: true
min_score_default_visibility:
default: 0.0
reviewable_default_visibility:
client: true
type: enum
default: low
choices:
- low
- medium
- high
rate_limits:
unique_posts_mins: 5

View File

@ -173,7 +173,7 @@ module FlagQuery
params = {
pending: Reviewable.statuses[:pending],
min_score: SiteSetting.min_score_default_visibility
min_score: Reviewable.min_score_for_priority
}
results = DB.query(<<~SQL, params)

View File

@ -8,13 +8,15 @@ describe FlagQuery do
fab!(:codinghorror) { Fabricate(:coding_horror) }
describe "flagged_topics" do
it "respects `min_score_default_visibility`" do
it "respects `reviewable_default_visibility`" do
Reviewable.set_priorities(medium: 10.0)
admin = Fabricate(:admin)
moderator = Fabricate(:moderator)
post = create_post
SiteSetting.min_score_default_visibility = 2.0
SiteSetting.reviewable_default_visibility = 'low'
PostActionCreator.spam(moderator, post)
result = FlagQuery.flagged_topics
@ -23,7 +25,7 @@ describe FlagQuery do
expect(ft.topic).to eq(post.topic)
expect(ft.flag_counts).to eq(PostActionType.types[:spam] => 1)
SiteSetting.min_score_default_visibility = 10.0
SiteSetting.reviewable_default_visibility = 'medium'
result = FlagQuery.flagged_topics
expect(result[:flagged_topics]).to be_blank
@ -130,14 +132,16 @@ describe FlagQuery do
expect(posts.count).to eq(1)
end
it "respects `min_score_default_visibility`" do
it "respects `reviewable_default_visibility`" do
Reviewable.set_priorities(medium: 3.0)
SiteSetting.reviewable_default_visibility = 'medium'
admin = Fabricate(:admin)
flagger = Fabricate(:user)
post = create_post
PostActionCreator.create(flagger, post, :spam)
SiteSetting.min_score_default_visibility = 3.0
posts, topics, users = FlagQuery.flagged_posts_report(admin)
expect(posts).to be_blank
expect(topics).to be_blank

View File

@ -270,7 +270,8 @@ describe NewPostManager do
end
it "calls custom enqueuing handlers" do
SiteSetting.min_score_default_visibility = 20.5
Reviewable.set_priorities(high: 20.5)
SiteSetting.reviewable_default_visibility = 'high'
manager = NewPostManager.new(
topic.user,

View File

@ -65,7 +65,8 @@ describe Jobs::NotifyReviewable do
it "respects visibility" do
SiteSetting.enable_category_group_review = true
SiteSetting.min_score_default_visibility = 2.0
Reviewable.set_priorities(medium: 2.0)
SiteSetting.reviewable_default_visibility = 'medium'
GroupUser.create!(group_id: group.id, user_id: moderator.id)

View File

@ -56,19 +56,21 @@ describe Jobs::PendingReviewablesReminder do
expect(execute.sent_reminder).to eq(true)
end
context "min_score_default_visibility" do
context "reviewable_default_visibility" do
before do
create_flag(49.hours.ago)
create_flag(51.hours.ago)
end
it "doesn't send a message when min_score_default_visibility is not met" do
SiteSetting.min_score_default_visibility = 3.0
it "doesn't send a message when `reviewable_default_visibility` is not met" do
Reviewable.set_priorities(medium: 3.0)
SiteSetting.reviewable_default_visibility = 'medium'
expect(execute.sent_reminder).to eq(false)
end
it "sends a message when min_score_default_visibility is met" do
SiteSetting.min_score_default_visibility = 2.0
it "sends a message when `reviewable_default_visibility` is met" do
Reviewable.set_priorities(medium: 2.0)
SiteSetting.reviewable_default_visibility = 'medium'
expect(execute.sent_reminder).to eq(true)
end
end

View File

@ -0,0 +1,21 @@
require 'rails_helper'
describe Jobs::ReviewablePriorities do
it "will set priorities based on the maximum score" 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('medium')).to eq(3.0)
expect(Reviewable.min_score_for_priority('high')).to eq(5.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)
end
end

View File

@ -205,8 +205,9 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
expect(pending_count).to eq(0)
end
it "respects min_score_default_visibility" do
SiteSetting.min_score_default_visibility = 7.5
it "respects `reviewable_default_visibility`" do
Reviewable.set_priorities(high: 7.5)
SiteSetting.reviewable_default_visibility = 'high'
expect(pending_count).to eq(0)
PostActionCreator.off_topic(user, post)

View File

@ -281,4 +281,32 @@ RSpec.describe Reviewable, type: :model do
expect(user.user_stat.reload.flags_agreed).to eq(0)
end
end
context "priorities" do
it "returns 0 for unknown priorities" do
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)
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)
end
it "will return the default priority if none supplied" do
Reviewable.set_priorities(medium: 12.3, high: 45.6)
expect(Reviewable.min_score_for_priority).to eq(0.0)
SiteSetting.reviewable_default_visibility = 'medium'
expect(Reviewable.min_score_for_priority).to eq(12.3)
SiteSetting.reviewable_default_visibility = 'high'
expect(Reviewable.min_score_for_priority).to eq(45.6)
end
end
end