From bde0ef865fd04e4d97aa7fdef6a8dc36df6fe9f0 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 4 Sep 2019 09:56:25 -0600 Subject: [PATCH] FEATURE: Adds a pop up that shows a more detailed score for reviewables (#8035) If you click a (?) icon beside the reviewable status a pop up will appear with expanded informatio that explains how the reviewable got its score, and how it compares to system thresholds. --- .../adapters/reviewable-explanation.js.es6 | 9 ++++ .../components/reviewable-item.js.es6 | 8 ++++ .../controllers/explain-reviewable.js.es6 | 15 ++++++ .../discourse/helpers/float.js.es6 | 5 ++ .../templates/components/reviewable-item.hbs | 3 ++ .../templates/components/score-value.hbs | 11 +++++ .../templates/modal/explain-reviewable.hbs | 47 +++++++++++++++++++ .../common/base/explain-reviewable.scss | 37 +++++++++++++++ .../stylesheets/common/base/reviewables.scss | 3 ++ app/controllers/reviewables_controller.rb | 12 +++++ app/models/reviewable.rb | 19 ++++++++ app/models/reviewable_score.rb | 16 ++++++- .../reviewable_explanation_serializer.rb | 38 +++++++++++++++ ...reviewable_score_explanation_serializer.rb | 24 ++++++++++ config/locales/client.en.yml | 17 +++++++ config/routes.rb | 1 + spec/requests/reviewables_controller_spec.rb | 24 ++++++++++ 17 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/discourse/adapters/reviewable-explanation.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/explain-reviewable.js.es6 create mode 100644 app/assets/javascripts/discourse/helpers/float.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/score-value.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/explain-reviewable.hbs create mode 100644 app/assets/stylesheets/common/base/explain-reviewable.scss create mode 100644 app/serializers/reviewable_explanation_serializer.rb create mode 100644 app/serializers/reviewable_score_explanation_serializer.rb diff --git a/app/assets/javascripts/discourse/adapters/reviewable-explanation.js.es6 b/app/assets/javascripts/discourse/adapters/reviewable-explanation.js.es6 new file mode 100644 index 00000000000..2ae3837e947 --- /dev/null +++ b/app/assets/javascripts/discourse/adapters/reviewable-explanation.js.es6 @@ -0,0 +1,9 @@ +import RestAdapter from "discourse/adapters/rest"; + +export default RestAdapter.extend({ + jsonMode: true, + + pathFor(store, type, id) { + return `/review/${id}/explain.json`; + } +}); diff --git a/app/assets/javascripts/discourse/components/reviewable-item.js.es6 b/app/assets/javascripts/discourse/components/reviewable-item.js.es6 index d94128f3489..4507704d95b 100644 --- a/app/assets/javascripts/discourse/components/reviewable-item.js.es6 +++ b/app/assets/javascripts/discourse/components/reviewable-item.js.es6 @@ -3,6 +3,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import computed from "ember-addons/ember-computed-decorators"; import Category from "discourse/models/category"; import optionalService from "discourse/lib/optional-service"; +import showModal from "discourse/lib/show-modal"; let _components = {}; @@ -140,6 +141,13 @@ export default Ember.Component.extend({ }, actions: { + explainReviewable(reviewable) { + showModal("explain-reviewable", { + title: "review.explain.title", + model: reviewable + }); + }, + edit() { this.set("editing", true); this._updates = { payload: {} }; diff --git a/app/assets/javascripts/discourse/controllers/explain-reviewable.js.es6 b/app/assets/javascripts/discourse/controllers/explain-reviewable.js.es6 new file mode 100644 index 00000000000..49e57228a62 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/explain-reviewable.js.es6 @@ -0,0 +1,15 @@ +import ModalFunctionality from "discourse/mixins/modal-functionality"; + +export default Ember.Controller.extend(ModalFunctionality, { + loading: null, + reviewableExplanation: null, + + onShow() { + this.setProperties({ loading: true, reviewableExplanation: null }); + + this.store + .find("reviewable-explanation", this.model.id) + .then(result => this.set("reviewableExplanation", result)) + .finally(() => this.set("loading", false)); + } +}); diff --git a/app/assets/javascripts/discourse/helpers/float.js.es6 b/app/assets/javascripts/discourse/helpers/float.js.es6 new file mode 100644 index 00000000000..4d0fa564a24 --- /dev/null +++ b/app/assets/javascripts/discourse/helpers/float.js.es6 @@ -0,0 +1,5 @@ +import { registerUnbound } from "discourse-common/lib/helpers"; + +registerUnbound("float", function(n) { + return parseFloat(n).toFixed(1); +}); diff --git a/app/assets/javascripts/discourse/templates/components/reviewable-item.hbs b/app/assets/javascripts/discourse/templates/components/reviewable-item.hbs index 7df7ee102eb..abd106a474a 100644 --- a/app/assets/javascripts/discourse/templates/components/reviewable-item.hbs +++ b/app/assets/javascripts/discourse/templates/components/reviewable-item.hbs @@ -10,6 +10,9 @@ {{reviewable-status reviewable.status}} + + {{d-icon "question-circle"}} +
diff --git a/app/assets/javascripts/discourse/templates/components/score-value.hbs b/app/assets/javascripts/discourse/templates/components/score-value.hbs new file mode 100644 index 00000000000..e78eeb32b23 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/score-value.hbs @@ -0,0 +1,11 @@ +{{#if value}} + + {{float value}} + {{#if label}} + + {{i18n (concat "review.explain." label ".name")}} + + {{/if}} + + + +{{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/explain-reviewable.hbs b/app/assets/javascripts/discourse/templates/modal/explain-reviewable.hbs new file mode 100644 index 00000000000..76f955c45b0 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/explain-reviewable.hbs @@ -0,0 +1,47 @@ +{{#d-modal-body class="explain-reviewable"}} + {{#conditional-loading-spinner condition=loading}} + + + + + + {{#each reviewableExplanation.scores as |s|}} + + + + + {{/each}} + + + + +
{{i18n "review.explain.formula"}}{{i18n "review.explain.subtotal"}}
+ {{score-value value="1.0" tagName=""}} + {{score-value value=s.type_bonus label="type_bonus" tagName=""}} + {{score-value value=s.take_action_bonus label="take_action_bonus" tagName=""}} + {{score-value value=s.trust_level_bonus label="trust_level_bonus" tagName=""}} + {{score-value value=s.user_accuracy_bonus label="user_accuracy_bonus" tagName=""}} + {{float s.score}}
{{i18n "review.explain.total"}}{{float reviewableExplanation.total_score}}
+ + + + + + + + + + +
{{i18n "review.explain.min_score_visibility"}} + {{float reviewableExplanation.min_score_visibility}} +
{{i18n "review.explain.score_to_hide"}} + {{float reviewableExplanation.hide_post_score}} +
+ + {{/conditional-loading-spinner}} + +{{/d-modal-body}} + +
+ {{d-button action=(route-action "closeModal") label="close"}} +
diff --git a/app/assets/stylesheets/common/base/explain-reviewable.scss b/app/assets/stylesheets/common/base/explain-reviewable.scss new file mode 100644 index 00000000000..ec7026e8139 --- /dev/null +++ b/app/assets/stylesheets/common/base/explain-reviewable.scss @@ -0,0 +1,37 @@ +.explain-reviewable { + min-width: 500px; + + .thresholds { + margin-top: 1em; + } + table { + width: 100%; + } + table td { + padding: 0.5em; + } + td.sum { + text-align: right; + } + td.sum.total { + font-weight: bold; + } + tr.total { + td { + background-color: $primary-low; + font-weight: bold; + } + } + + .op { + font-weight: bold; + } + + .score-value-type { + color: $primary-medium; + } + + .op:last-of-type { + display: none; + } +} diff --git a/app/assets/stylesheets/common/base/reviewables.scss b/app/assets/stylesheets/common/base/reviewables.scss index 31071ac39d3..0228f0c1c70 100644 --- a/app/assets/stylesheets/common/base/reviewables.scss +++ b/app/assets/stylesheets/common/base/reviewables.scss @@ -20,6 +20,9 @@ } } } + .explain { + margin-left: 0.5em; + } .nav-pills { margin-bottom: 1em; diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index d2bed1f9e48..dffe6cd1724 100644 --- a/app/controllers/reviewables_controller.rb +++ b/app/controllers/reviewables_controller.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require_dependency 'reviewable_explanation_serializer' class ReviewablesController < ApplicationController requires_login @@ -102,6 +103,17 @@ class ReviewablesController < ApplicationController ) end + def explain + reviewable = find_reviewable + + render_serialized( + { reviewable: reviewable, scores: reviewable.explain_score }, + ReviewableExplanationSerializer, + rest_serializer: true, + root: 'reviewable_explanation' + ) + end + def show reviewable = find_reviewable diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index b0142d1eb3f..dd7be14eb37 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -481,6 +481,25 @@ class Reviewable < ActiveRecord::Base .count end + def explain_score + DB.query(<<~SQL, reviewable_id: id) + SELECT rs.reviewable_id, + rs.user_id, + CASE WHEN (u.admin OR u.moderator) THEN 5.0 ELSE u.trust_level END AS trust_level_bonus, + us.flags_agreed, + us.flags_disagreed, + us.flags_ignored, + rs.score, + rs.take_action_bonus, + COALESCE(pat.score_bonus, 0.0) AS type_bonus + FROM reviewable_scores AS rs + INNER JOIN users AS u ON u.id = rs.user_id + LEFT OUTER JOIN user_stats AS us ON us.user_id = rs.user_id + LEFT OUTER JOIN post_action_types AS pat ON pat.id = rs.reviewable_score_type + WHERE rs.reviewable_id = :reviewable_id + SQL + end + protected def recalculate_score diff --git a/app/models/reviewable_score.rb b/app/models/reviewable_score.rb index 0063e2458a2..14ba16ed48e 100644 --- a/app/models/reviewable_score.rb +++ b/app/models/reviewable_score.rb @@ -59,10 +59,22 @@ class ReviewableScore < ActiveRecord::Base user_stat = user&.user_stat return 0.0 if user_stat.blank? - total = (user_stat.flags_agreed + user_stat.flags_disagreed + user_stat.flags_ignored).to_f + calc_user_accuracy_bonus( + user_stat.flags_agreed, + user_stat.flags_disagreed, + user_stat.flags_ignored + ) + end + + def self.calc_user_accuracy_bonus(agreed, disagreed, ignored) + agreed ||= 0 + disagreed ||= 0 + ignored ||= 0 + + total = (agreed + disagreed + ignored).to_f return 0.0 if total <= 5 - (user_stat.flags_agreed / total) * 5.0 + (agreed / total) * 5.0 end def reviewable_conversation diff --git a/app/serializers/reviewable_explanation_serializer.rb b/app/serializers/reviewable_explanation_serializer.rb new file mode 100644 index 00000000000..057c4dc3ce0 --- /dev/null +++ b/app/serializers/reviewable_explanation_serializer.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +require_dependency 'reviewable_score_explanation_serializer' + +class ReviewableExplanationSerializer < ApplicationSerializer + attributes( + :id, + :total_score, + :scores, + :min_score_visibility, + :hide_post_score + ) + + has_many :scores, serializer: ReviewableScoreExplanationSerializer, embed: :objects + + def id + object[:reviewable].id + end + + def hide_post_score + Reviewable.score_required_to_hide_post + end + + def spam_silence_score + Reviewable.spam_score_to_silence_new_user + end + + def min_score_visibility + Reviewable.min_score_for_priority + end + + def total_score + object[:reviewable].score + end + + def scores + object[:scores] + end +end diff --git a/app/serializers/reviewable_score_explanation_serializer.rb b/app/serializers/reviewable_score_explanation_serializer.rb new file mode 100644 index 00000000000..caff0cee266 --- /dev/null +++ b/app/serializers/reviewable_score_explanation_serializer.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class ReviewableScoreExplanationSerializer < ApplicationSerializer + attributes( + :user_id, + :type_bonus, + :trust_level_bonus, + :take_action_bonus, + :flags_agreed, + :flags_disagreed, + :flags_ignored, + :user_accuracy_bonus, + :score + ) + + def user_accuracy_bonus + ReviewableScore.calc_user_accuracy_bonus( + object.flags_agreed, + object.flags_disagreed, + object.flags_ignored + ) + end + +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index cfcafba0474..2b0eab6e196 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -370,6 +370,23 @@ en: review: order_by: "Order by" in_reply_to: "in reply to" + explain: + why: "explain why this item ended up in the queue" + title: "Reviewable Scoring" + formula: "Formula" + subtotal: "Subtotal" + total: "Total" + min_score_visibility: "Minimum Score for Visibility" + score_to_hide: "Score to Hide Post" + user_accuracy_bonus: + name: "user accuracy" + title: "Users whose flags have been historically agreed with are given a bonus." + trust_level_bonus: + name: "trust level" + title: "Reviewable items created by higher trust level users have a higher score." + type_bonus: + name: "type bonus" + title: "Certain reviewable types can be assigned a bonus by staff to make them a higher priority." claim_help: optional: "You can claim this item to prevent others from reviewing it." required: "You must claim items before you can review them." diff --git a/config/routes.rb b/config/routes.rb index 7b638267d6a..328ad0e8b94 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -324,6 +324,7 @@ Discourse::Application.routes.draw do get "review" => "reviewables#index" # For ember app get "review/:reviewable_id" => "reviewables#show", constraints: { reviewable_id: /\d+/ } + get "review/:reviewable_id/explain" => "reviewables#explain", constraints: { reviewable_id: /\d+/ } get "review/topics" => "reviewables#topics" get "review/settings" => "reviewables#settings" put "review/settings" => "reviewables#settings" diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb index d83135e79ff..dba29199b01 100644 --- a/spec/requests/reviewables_controller_spec.rb +++ b/spec/requests/reviewables_controller_spec.rb @@ -236,6 +236,30 @@ describe ReviewablesController do end end + context "#explain" do + context "basics" do + fab!(:reviewable) { Fabricate(:reviewable) } + + before do + sign_in(Fabricate(:moderator)) + end + + it "returns the explanation as json" do + get "/review/#{reviewable.id}/explain.json" + expect(response.code).to eq("200") + + json = ::JSON.parse(response.body) + expect(json['reviewable_explanation']['id']).to eq(reviewable.id) + expect(json['reviewable_explanation']['total_score']).to eq(reviewable.score) + end + + it "returns 404 for a missing reviewable" do + get "/review/123456789/explain.json" + expect(response.code).to eq("404") + end + end + end + context "#perform" do fab!(:reviewable) { Fabricate(:reviewable) } before do