mirror of
https://github.com/discourse/discourse.git
synced 2025-02-20 14:02:00 +08:00
Merge pull request #2717 from riking/badge-solid
Admin badge interface improvements
This commit is contained in:
Sam
commit
e0c8abc911
|
|
@ -0,0 +1,49 @@
|
|||
export default Ember.Controller.extend({
|
||||
needs: ['modal'],
|
||||
|
||||
sample: Em.computed.alias('model.sample'),
|
||||
errors: Em.computed.alias('model.errors'),
|
||||
count: Em.computed.alias('model.grant_count'),
|
||||
|
||||
count_warning: function() {
|
||||
if (this.get('count') <= 10) {
|
||||
return this.get('sample.length') !== this.get('count');
|
||||
} else {
|
||||
return this.get('sample.length') !== 10;
|
||||
}
|
||||
}.property('count', 'sample.length'),
|
||||
|
||||
has_query_plan: function() {
|
||||
return !!this.get('model.query_plan');
|
||||
}.property('model.query_plan'),
|
||||
|
||||
query_plan_html: function() {
|
||||
var raw = this.get('model.query_plan'),
|
||||
returned = "<pre>";
|
||||
|
||||
_.each(raw, function(linehash) {
|
||||
returned += Handlebars.Utils.escapeExpression(linehash["QUERY PLAN"]);
|
||||
returned += "<br>";
|
||||
});
|
||||
|
||||
returned += "</pre>";
|
||||
return returned;
|
||||
}.property('model.query_plan'),
|
||||
|
||||
processed_sample: Ember.computed.map('model.sample', function(grant) {
|
||||
var i18nKey = 'admin.badges.preview.grant.with',
|
||||
i18nParams = { username: Handlebars.Utils.escapeExpression(grant.username) };
|
||||
|
||||
if (grant.post_id) {
|
||||
i18nKey += "_post";
|
||||
i18nParams.link = "<a href='/p/" + grant.post_id + "' data-auto-route='true'>" + Handlebars.Utils.escapeExpression(grant.title) + "</a>";
|
||||
}
|
||||
|
||||
if (grant.granted_at) {
|
||||
i18nKey += "_time";
|
||||
i18nParams.time = Handlebars.Utils.escapeExpression(moment(grant.granted_at).format(I18n.t('dates.long_with_year')));
|
||||
}
|
||||
|
||||
return I18n.t(i18nKey, i18nParams);
|
||||
})
|
||||
});
|
||||
|
|
@ -78,21 +78,6 @@ export default Ember.ArrayController.extend({
|
|||
|
||||
actions: {
|
||||
|
||||
preview: function(badge) {
|
||||
// TODO wire modal and localize
|
||||
Discourse.ajax('/admin/badges/preview.json', {
|
||||
method: 'post',
|
||||
data: {sql: badge.query, target_posts: !!badge.target_posts}
|
||||
}).then(function(json){
|
||||
if(json.error){
|
||||
bootbox.alert(json.error);
|
||||
} else {
|
||||
bootbox.alert(json.grant_count + " badges to be assigned");
|
||||
}
|
||||
});
|
||||
|
||||
},
|
||||
|
||||
/**
|
||||
Create a new badge and select it.
|
||||
|
||||
|
|
@ -128,16 +113,21 @@ export default Ember.ArrayController.extend({
|
|||
'enabled', 'show_posts',
|
||||
'target_posts', 'name', 'description',
|
||||
'icon', 'query', 'badge_grouping_id',
|
||||
'trigger', 'badge_type_id'];
|
||||
'trigger', 'badge_type_id'],
|
||||
self = this;
|
||||
|
||||
if(this.get('selectedItem.system')){
|
||||
if (this.get('selectedItem.system')){
|
||||
var protectedFields = this.get('protectedSystemFields');
|
||||
fields = _.filter(fields, function(f){
|
||||
return !_.include(protectedFields,f);
|
||||
});
|
||||
}
|
||||
|
||||
this.get('selectedItem').save(fields);
|
||||
this.get('selectedItem').save(fields).catch(function(error) {
|
||||
// this shows the admin-badge-preview modal with the error
|
||||
// kinda weird, but it consolidates the display logic for badge errors
|
||||
self.send('saveError', error);
|
||||
});
|
||||
}
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -15,8 +15,39 @@ Discourse.AdminBadgesRoute = Discourse.Route.extend({
|
|||
},
|
||||
|
||||
actions: {
|
||||
editGroupings: function(model){
|
||||
editGroupings: function(model) {
|
||||
Discourse.Route.showModal(this, 'admin_edit_badge_groupings', model);
|
||||
},
|
||||
|
||||
saveError: function(jqXhr) {
|
||||
if (jqXhr.status === 422) {
|
||||
Discourse.Route.showModal(this, 'admin_badge_preview', jqXhr.responseJSON);
|
||||
} else {
|
||||
Em.Logger.error(jqXhr);
|
||||
bootbox.alert(I18n.t('errors.description.unknown'));
|
||||
}
|
||||
},
|
||||
|
||||
preview: function(badge, explain) {
|
||||
var self = this;
|
||||
|
||||
badge.set('preview_loading', true);
|
||||
Discourse.ajax('/admin/badges/preview.json', {
|
||||
method: 'post',
|
||||
data: {
|
||||
sql: badge.query,
|
||||
target_posts: !!badge.target_posts,
|
||||
trigger: badge.trigger,
|
||||
explain: explain
|
||||
}
|
||||
}).then(function(json) {
|
||||
badge.set('preview_loading', false);
|
||||
Discourse.Route.showModal(self, 'admin_badge_preview', json);
|
||||
}).catch(function(error) {
|
||||
badge.set('preview_loading', false);
|
||||
Em.Logger.error(error);
|
||||
bootbox.alert("Network error");
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -76,7 +76,12 @@
|
|||
|
||||
{{#if hasQuery}}
|
||||
|
||||
<a href="/admin/badges/preview" {{action preview this}}>{{i18n admin.badges.preview}}</a>
|
||||
<a href {{action preview this "false"}}>{{i18n admin.badges.preview.link_text}}</a>
|
||||
|
|
||||
<a href {{action preview this "true"}}>{{i18n admin.badges.preview.plan_text}}</a>
|
||||
{{#if preview_loading}}
|
||||
{{i18n loading}}...
|
||||
{{/if}}
|
||||
|
||||
<div>
|
||||
<span>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,44 @@
|
|||
<div class="badge-query-preview">
|
||||
{{#if errors}}
|
||||
<p class="error-header">{{i18n admin.badges.preview.sql_error_header}}</p>
|
||||
|
||||
<div class="badge-errors">
|
||||
{{errors}}
|
||||
</div>
|
||||
<!--
|
||||
TODO we want some help pages for this, link to those instead
|
||||
<p>
|
||||
{{i18n admin.badges.preview.error_help}}
|
||||
</p>
|
||||
<ul>
|
||||
<li><a href="https://meta.discourse.org/t/triggered-custom-badge-queries/19336">https://meta.discourse.org/t/triggered-custom-badge-queries/19336</a></li>
|
||||
</ul>
|
||||
-->
|
||||
{{else}}
|
||||
<p class="grant-count">{{{i18n admin.badges.preview.grant_count count=count}}}</p>
|
||||
|
||||
{{#if count_warning}}
|
||||
<div class="count-warning">
|
||||
<p class="heading"><i class="fa fa-warning"></i> {{i18n admin.badges.preview.bad_count_warning.header}}</p>
|
||||
<p class="body">{{i18n admin.badges.preview.bad_count_warning.text}}</p>
|
||||
</div>
|
||||
{{/if}}
|
||||
|
||||
{{#if sample}}
|
||||
<p class="sample">
|
||||
{{i18n admin.badges.preview.sample}}
|
||||
</p>
|
||||
<ul>
|
||||
{{#each html in processed_sample}}
|
||||
<li>{{{html}}}</li>
|
||||
{{/each}}
|
||||
</ul>
|
||||
{{/if}}
|
||||
|
||||
{{#if has_query_plan}}
|
||||
<div class="badge-query-plan">
|
||||
{{{query_plan_html}}}
|
||||
</div>
|
||||
{{/if}}
|
||||
{{/if}}
|
||||
</div>
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
|
||||
Discourse.AdminBadgePreviewView = Discourse.ModalBodyView.extend({
|
||||
templateName: 'admin/templates/modal/admin_badge_preview',
|
||||
title: I18n.t('admin.badges.preview.modal_title')
|
||||
});
|
||||
|
|
@ -146,10 +146,10 @@ Discourse.Badge = Discourse.Model.extend({
|
|||
self.set('savingStatus', I18n.t('saved'));
|
||||
self.set('saving', false);
|
||||
return self;
|
||||
}, function(error){
|
||||
self.set('savingStatus', '');
|
||||
}).catch(function(error) {
|
||||
self.set('savingStatus', I18n.t('failed'));
|
||||
self.set('saving', false);
|
||||
bootbox.alert(error.responseText);
|
||||
throw error;
|
||||
});
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -419,6 +419,38 @@ section.details {
|
|||
}
|
||||
}
|
||||
|
||||
.badge-query-preview {
|
||||
.grant-count, .sample, .error-header {
|
||||
margin-left: 10px;
|
||||
}
|
||||
|
||||
.badge-query-plan, .badge-errors {
|
||||
padding: 4px;
|
||||
background-color: scale-color-diff();
|
||||
}
|
||||
|
||||
.badge-query-plan {
|
||||
font-size: 80%;
|
||||
}
|
||||
.badge-errors {
|
||||
font-family: monospace;
|
||||
}
|
||||
|
||||
.count-warning {
|
||||
background-color: dark-light-diff(rgba($danger,.7), $secondary, 50%, -60%);
|
||||
margin: 0 0 7px 0;
|
||||
padding: 10px 20px;
|
||||
|
||||
p {
|
||||
margin: 0;
|
||||
}
|
||||
.heading {
|
||||
color: $danger;
|
||||
font-weight: bold;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Customise area
|
||||
.customize {
|
||||
.nav.nav-pills {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,10 @@ class Admin::BadgesController < Admin::AdminController
|
|||
end
|
||||
|
||||
def preview
|
||||
render json: BadgeGranter.preview(params[:sql], target_posts: params[:target_posts] == "true")
|
||||
render json: BadgeGranter.preview(params[:sql],
|
||||
target_posts: params[:target_posts] == "true",
|
||||
explain: params[:explain] == "true",
|
||||
trigger: params[:trigger].to_i)
|
||||
end
|
||||
|
||||
def badge_types
|
||||
|
|
@ -53,9 +56,28 @@ class Admin::BadgesController < Admin::AdminController
|
|||
|
||||
def update
|
||||
badge = find_badge
|
||||
update_badge_from_params(badge)
|
||||
badge.save!
|
||||
render_serialized(badge, BadgeSerializer, root: "badge")
|
||||
|
||||
error = nil
|
||||
Badge.transaction do
|
||||
update_badge_from_params(badge)
|
||||
|
||||
# Perform checks to prevent bad queries
|
||||
begin
|
||||
BadgeGranter.contract_checks!(badge.query, { target_posts: badge.target_posts, trigger: badge.trigger })
|
||||
rescue => e
|
||||
# noinspection RubyUnusedLocalVariable
|
||||
error = e.message
|
||||
raise ActiveRecord::Rollback
|
||||
end
|
||||
|
||||
badge.save!
|
||||
end
|
||||
|
||||
if error
|
||||
render_json_error error
|
||||
else
|
||||
render_serialized(badge, BadgeSerializer, root: "badge")
|
||||
end
|
||||
end
|
||||
|
||||
def destroy
|
||||
|
|
|
|||
|
|
@ -31,6 +31,18 @@ class Badge < ActiveRecord::Base
|
|||
PostRevision = 2
|
||||
TrustLevelChange = 4
|
||||
UserChange = 8
|
||||
|
||||
def self.is_none?(trigger)
|
||||
[None].include? trigger
|
||||
end
|
||||
|
||||
def self.uses_user_ids?(trigger)
|
||||
[TrustLevelChange, UserChange].include? trigger
|
||||
end
|
||||
|
||||
def self.uses_post_ids?(trigger)
|
||||
[PostAction, PostRevision].include? trigger
|
||||
end
|
||||
end
|
||||
|
||||
module Queries
|
||||
|
|
|
|||
|
|
@ -132,9 +132,40 @@ class BadgeGranter
|
|||
"badge_queue".freeze
|
||||
end
|
||||
|
||||
# Options:
|
||||
# :target_posts - whether the badge targets posts
|
||||
# :trigger - the Badge::Trigger id
|
||||
def self.contract_checks!(sql, opts = {})
|
||||
return unless sql.present?
|
||||
if Badge::Trigger.uses_post_ids?(opts[:trigger])
|
||||
raise "Contract violation:\nQuery triggers on posts, but does not reference the ':post_ids' array" unless sql.match /:post_ids/
|
||||
raise "Contract violation:\nQuery triggers on posts, but references the ':user_ids' array" if sql.match /:user_ids/
|
||||
end
|
||||
if Badge::Trigger.uses_user_ids?(opts[:trigger])
|
||||
raise "Contract violation:\nQuery triggers on users, but does not reference the ':user_ids' array" unless sql.match /:user_ids/
|
||||
raise "Contract violation:\nQuery triggers on users, but references the ':post_ids' array" if sql.match /:post_ids/
|
||||
end
|
||||
if opts[:trigger] && !Badge::Trigger.is_none?(opts[:trigger])
|
||||
raise "Contract violation:\nQuery is triggered, but does not reference the ':backfill' parameter.\n(Hint: if :backfill is TRUE, you should ignore the :post_ids/:user_ids)" unless sql.match /:backfill/
|
||||
end
|
||||
|
||||
# TODO these three conditions have a lot of false negatives
|
||||
if opts[:target_posts]
|
||||
raise "Contract violation:\nQuery targets posts, but does not return a 'post_id' column" unless sql.match /post_id/
|
||||
end
|
||||
raise "Contract violation:\nQuery does not return a 'user_id' column" unless sql.match /user_id/
|
||||
raise "Contract violation:\nQuery does not return a 'granted_at' column" unless sql.match /granted_at/
|
||||
end
|
||||
|
||||
# Options:
|
||||
# :target_posts - whether the badge targets posts
|
||||
# :trigger - the Badge::Trigger id
|
||||
# :explain - return the EXPLAIN query
|
||||
def self.preview(sql, opts = {})
|
||||
params = {user_ids: [], post_ids: [], backfill: true}
|
||||
|
||||
BadgeGranter.contract_checks!(sql, opts)
|
||||
|
||||
# hack to allow for params, otherwise sanitizer will trigger sprintf
|
||||
count_sql = "SELECT COUNT(*) count FROM (#{sql}) q WHERE :backfill = :backfill"
|
||||
grant_count = SqlBuilder.map_exec(OpenStruct, count_sql, params).first.count.to_i
|
||||
|
|
@ -156,23 +187,34 @@ class BadgeGranter
|
|||
LIMIT 10"
|
||||
end
|
||||
|
||||
query_plan = nil
|
||||
query_plan = ActiveRecord::Base.exec_sql("EXPLAIN #{sql}", params) if opts[:explain]
|
||||
|
||||
sample = SqlBuilder.map_exec(OpenStruct, grants_sql, params).map(&:to_h)
|
||||
|
||||
{grant_count: grant_count, sample: sample}
|
||||
sample.each do |result|
|
||||
raise "Query returned a non-existent user ID:\n#{result[:id]}" unless User.find(result[:id]).present?
|
||||
raise "Query did not return a badge grant time\n(Try using 'current_timestamp granted_at')" unless result[:granted_at]
|
||||
if opts[:target_posts]
|
||||
raise "Query did not return a post ID" unless result[:post_id]
|
||||
raise "Query returned a non-existent post ID:\n#{result[:post_id]}" unless Post.find(result[:post_id]).present?
|
||||
end
|
||||
end
|
||||
|
||||
{grant_count: grant_count, sample: sample, query_plan: query_plan}
|
||||
rescue => e
|
||||
{error: e.to_s}
|
||||
{errors: e.message}
|
||||
end
|
||||
|
||||
MAX_ITEMS_FOR_DELTA = 200
|
||||
def self.backfill(badge, opts=nil)
|
||||
return unless badge.query.present? && badge.enabled
|
||||
|
||||
post_ids = user_ids = nil
|
||||
|
||||
post_ids = opts[:post_ids] if opts
|
||||
user_ids = opts[:user_ids] if opts
|
||||
|
||||
post_ids = nil unless post_ids.present?
|
||||
user_ids = nil unless user_ids.present?
|
||||
|
||||
# safeguard fall back to full backfill if more than 200
|
||||
if (post_ids && post_ids.length > MAX_ITEMS_FOR_DELTA) ||
|
||||
(user_ids && user_ids.length > MAX_ITEMS_FOR_DELTA)
|
||||
|
|
@ -180,6 +222,9 @@ class BadgeGranter
|
|||
user_ids = nil
|
||||
end
|
||||
|
||||
post_ids = nil unless post_ids.present?
|
||||
user_ids = nil unless user_ids.present?
|
||||
|
||||
full_backfill = !user_ids && !post_ids
|
||||
|
||||
post_clause = badge.target_posts ? "AND (q.post_id = ub.post_id OR NOT :multiple_grant)" : ""
|
||||
|
|
|
|||
|
|
@ -204,6 +204,7 @@ en:
|
|||
disable: "Disable"
|
||||
undo: "Undo"
|
||||
revert: "Revert"
|
||||
failed: "Failed"
|
||||
|
||||
banner:
|
||||
close: "Dismiss this banner."
|
||||
|
|
@ -2024,7 +2025,6 @@ en:
|
|||
target_posts: Query targets posts
|
||||
auto_revoke: Run revocation query daily
|
||||
show_posts: Show post granting badge on badge page
|
||||
preview: Preview badge
|
||||
trigger: Trigger
|
||||
trigger_type:
|
||||
none: "Update daily"
|
||||
|
|
@ -2032,6 +2032,25 @@ en:
|
|||
post_revision: "When a user edits or creates a post"
|
||||
trust_level_change: "When a user changes trust level"
|
||||
user_change: "When a user is edited or created"
|
||||
preview:
|
||||
link_text: "Preview granted badges"
|
||||
plan_text: "Preview with query plan"
|
||||
modal_title: "Badge Query Preview"
|
||||
sql_error_header: "There was an error with the query."
|
||||
error_help: "See the following links for help with badge queries."
|
||||
bad_count_warning:
|
||||
header: "WARNING!"
|
||||
text: "There are missing grant samples. This happens when the badge query returns user IDs or post IDs that do not exist. This may cause unexpected results later on - please double-check your query."
|
||||
grant_count:
|
||||
zero: "No badges to be assigned."
|
||||
one: "<b>1</b> badge to be assigned."
|
||||
other: "<b>%{count}</b> badges to be assigned."
|
||||
sample: "Sample:"
|
||||
grant:
|
||||
with: <span class="username">%{username}</span>
|
||||
with_post: <span class="username">%{username}</span> for post in %{link}
|
||||
with_post_time: <span class="username">%{username}</span> for post in %{link} at <span class="time">%{time}</span>
|
||||
with_time: <span class="username">%{username}</span> at <span class="time">%{time}</span>
|
||||
|
||||
lightbox:
|
||||
download: "download"
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user