Skip to content

Commit

Permalink
FEATURE: Allow manual assignment of related post to badge
Browse files Browse the repository at this point in the history
PERF: clean up performance of user badges admin when large number of badges exist
  • Loading branch information
SamSaffron committed Feb 25, 2015
1 parent ff84275 commit fe578f9
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 24 deletions.
Expand Up @@ -13,6 +13,42 @@ export default Ember.ArrayController.extend({
sortProperties: ['granted_at'],
sortAscending: false,

groupedBadges: function(){
const badges = this.get('model');

var grouped = _.groupBy(badges, badge => badge.badge_id);

var expanded = [];

This comment has been minimized.

Copy link
@eviltrout

eviltrout Feb 25, 2015

Contributor

This could be const too - it only protects against reassignment, it doesn't freeze objects from mutating themselves.

const expandedBadges = badges.get('expandedBadges');

_(grouped).each(function(badges){
var lastGranted = badges[0].granted_at;

_.each(badges, function(badge) {
lastGranted = lastGranted < badge.granted_at ? badge.granted_at : lastGranted;
});

if(badges.length===1 || _.include(expandedBadges, badges[0].badge.id)){
_.each(badges, badge => expanded.push(badge));
return;
}

var result = {
badge: badges[0].badge,
granted_at: lastGranted,
badges: badges,

This comment has been minimized.

Copy link
@eviltrout

eviltrout Feb 25, 2015

Contributor

can just be badges now, wow! 🐕

count: badges.length,
grouped: true
};

expanded.push(result);
});

return _(expanded).sortBy(group => group.granted_at).reverse().value();


}.property('model', 'model.@each', 'model.expandedBadges.@each'),

/**
Array of badges that have not been granted to this user.
Expand Down Expand Up @@ -45,6 +81,12 @@ export default Ember.ArrayController.extend({

actions: {

expandGroup: function(userBadge){
const model = this.get('model');
model.set('expandedBadges', model.get('expandedBadges') || []);
model.get('expandedBadges').pushObject(userBadge.badge.id);
},

/**
Grant the selected badge to the user.
Expand All @@ -53,7 +95,8 @@ export default Ember.ArrayController.extend({
**/
grantBadge: function(badgeId) {
var self = this;
Discourse.UserBadge.grant(badgeId, this.get('user.username')).then(function(userBadge) {
Discourse.UserBadge.grant(badgeId, this.get('user.username'), this.get('badgeReason')).then(function(userBadge) {
self.set('badgeReason', '');
self.pushObject(userBadge);
Ember.run.next(function() {
// Update the selected badge ID after the combobox has re-rendered.
Expand Down
43 changes: 28 additions & 15 deletions app/assets/javascripts/admin/templates/user_badges.hbs
Expand Up @@ -9,40 +9,53 @@
{{#loading-spinner condition=loading}}
<div class='admin-container user-badges'>
<h2>{{i18n 'admin.badges.grant_badge'}}</h2>
<br>
{{#if noBadges}}
<p>{{i18n 'admin.badges.no_badges'}}</p>
{{else}}
<br>
<form class="form-horizontal">
<div>
<label>{{i18n 'admin.badges.badge'}}</label>
{{combo-box valueAttribute="id" value=controller.selectedBadgeId content=controller.grantableBadges}}
</div>
<label>
<label>{{i18n 'admin.badges.reason'}}</label>
{{input type="text" value=badgeReason}}<br><small>{{i18n 'admin.badges.reason_help'}}</small>
</label>
<button class='btn btn-primary' {{action "grantBadge" controller.selectedBadgeId}}>{{i18n 'admin.badges.grant'}}</button>
</form>
{{/if}}

<br>
<br>

<h2>{{i18n 'admin.badges.granted_badges'}}</h2>
<br>

<table>
<table id='user-badges'>
<tr>
<th>{{i18n 'admin.badges.badge'}}</th>
<th>{{i18n 'admin.badges.granted_by'}}</th>
<th class='reason'>{{i18n 'admin.badges.reason'}}</th>
<th>{{i18n 'admin.badges.granted_at'}}</th>
<th></th>
</tr>

{{#each}}
{{#each userBadge in groupedBadges}}
<tr>
<td>{{user-badge badge=badge}}</td>
<td>{{user-badge badge=userBadge.badge count=userBadge.count}}</td>
<td>
{{#link-to 'adminUser' badge.granted_by}}
{{avatar granted_by imageSize="tiny"}}
{{granted_by.username}}
{{#link-to 'adminUser' userBadge.badge.granted_by}}
{{avatar userBadge.granted_by imageSize="tiny"}}
{{userBadge.granted_by.username}}
{{/link-to}}
</td>
<td>{{age-with-tooltip granted_at}}</td>
<td class='reason'>
{{#if userBadge.postUrl}}
<a href="{{unbound userBadge.postUrl}}">{{userBadge.topic_title}}</a>
{{/if}}
</td>
<td>{{age-with-tooltip userBadge.granted_at}}</td>
<td>
<button class='btn' {{action "revokeBadge" this}}>{{i18n 'admin.badges.revoke'}}</button>
{{#if userBadge.grouped}}
<button class='btn' {{action "expandGroup" userBadge}}>{{{i18n 'admin.badges.expand'}}}</button>
{{else}}
<button class='btn btn-danger' {{action "revokeBadge" userBadge}}>{{i18n 'admin.badges.revoke'}}</button>
{{/if}}
</td>
</tr>
{{else}}
Expand Down
12 changes: 9 additions & 3 deletions app/assets/javascripts/discourse/models/user_badge.js
Expand Up @@ -7,6 +7,11 @@
@module Discourse
**/
Discourse.UserBadge = Discourse.Model.extend({
postUrl: function() {
if(this.get('topic_title')) {
return "/t/-/" + this.get('topic_id') + "/" + this.get('post_number');
}
}.property(), // avoid the extra bindings for now
/**
Revoke this badge.
Expand Down Expand Up @@ -93,7 +98,7 @@ Discourse.UserBadge.reopenClass({
@returns {Promise} a promise that resolves to an array of `Discourse.UserBadge`.
**/
findByUsername: function(username, options) {
var url = "/users/" + username + "/badges_json.json";
var url = "/user-badges/" + username + ".json";
if (options && options.grouped) {
url += "?grouped=true";
}
Expand Down Expand Up @@ -128,12 +133,13 @@ Discourse.UserBadge.reopenClass({
@param {String} username username of the user to be granted the badge.
@returns {Promise} a promise that resolves to an instance of `Discourse.UserBadge`.
**/
grant: function(badgeId, username) {
grant: function(badgeId, username, reason) {
return Discourse.ajax("/user_badges", {
type: "POST",
data: {
username: username,
badge_id: badgeId
badge_id: badgeId,
reason: reason
}
}).then(function(json) {
return Discourse.UserBadge.createFromJson(json);
Expand Down
6 changes: 6 additions & 0 deletions app/assets/stylesheets/common/admin/admin_base.scss
Expand Up @@ -1440,3 +1440,9 @@ tr.not-activated {
.preview {
margin-top: 5px;
}

table#user-badges {
.reason {
max-width: 200px;
}
}
21 changes: 18 additions & 3 deletions app/controllers/user_badges_controller.rb
Expand Up @@ -25,8 +25,10 @@ def username
end

user_badges = user_badges.includes(badge: [:badge_grouping, :badge_type])
.includes(post: :topic)
.includes(:granted_by)

render_serialized(user_badges, BasicUserBadgeSerializer, root: "user_badges")
render_serialized(user_badges, DetailedUserBadgeSerializer, root: "user_badges")
end

def create
Expand All @@ -39,9 +41,22 @@ def create
end

badge = fetch_badge_from_params
user_badge = BadgeGranter.grant(badge, user, granted_by: current_user)
post_id = nil

render_serialized(user_badge, UserBadgeSerializer, root: "user_badge")
if params[:reason].present?
path = URI.parse(params[:reason]).path rescue nil
route = Rails.application.routes.recognize_path(path) if path
if route
topic_id = route[:topic_id].to_i
post_number = route[:post_number] || 1

post_id = Post.find_by(topic_id: topic_id, post_number: post_number).try(:id) if topic_id > 0
end
end

user_badge = BadgeGranter.grant(badge, user, granted_by: current_user, post_id: post_id)

render_serialized(user_badge, DetailedUserBadgeSerializer, root: "user_badge")
end

def destroy
Expand Down
26 changes: 26 additions & 0 deletions app/serializers/detailed_user_badge_serializer.rb
@@ -0,0 +1,26 @@
class DetailedUserBadgeSerializer < BasicUserBadgeSerializer
has_one :granted_by

attributes :post_number, :topic_id, :topic_title

def include_post_number?
object.post
end

alias :include_topic_id? :include_post_number?
alias :include_topic_title? :include_post_number?


def post_number
object.post.post_number if object.post
end

def topic_id
object.post.topic_id if object.post
end

def topic_title
object.post.topic.title if object.post && object.post.topic
end

end
3 changes: 3 additions & 0 deletions config/locales/client.en.yml
Expand Up @@ -2194,10 +2194,13 @@ en:
modal_title: Badge Groupings
granted_by: Granted By
granted_at: Granted At
reason_help: (A link to a post or topic)
save: Save
delete: Delete
delete_confirm: Are you sure you want to delete this badge?
revoke: Revoke
reason: Reason
expand: Expand &hellip;
revoke_confirm: Are you sure you want to revoke this badge?
edit_badges: Edit Badges
grant_badge: Grant Badge
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -261,7 +261,7 @@
get "users/by-external/:external_id" => "users#show"
get "users/:username/flagged-posts" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/deleted-posts" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/badges_json" => "user_badges#username"
get "user-badges/:username" => "user_badges#username"

post "user_avatar/:username/refresh_gravatar" => "user_avatars#refresh_gravatar"
get "letter_avatar/:username/:size/:version.png" => "user_avatars#show_letter", format: false, constraints: { hostname: /[\w\.-]+/ }
Expand Down
12 changes: 11 additions & 1 deletion spec/controllers/user_badges_controller_spec.rb
Expand Up @@ -63,13 +63,23 @@

it 'grants badges from staff' do
admin = Fabricate(:admin)
post = create_post

log_in_user admin

StaffActionLogger.any_instance.expects(:log_badge_grant).once
xhr :post, :create, badge_id: badge.id, username: user.username

xhr :post, :create, badge_id: badge.id,
username: user.username,
reason: Discourse.base_url + post.url

expect(response.status).to eq(200)

user_badge = UserBadge.find_by(user: user, badge: badge)

expect(user_badge).to be_present
expect(user_badge.granted_by).to eq(admin)
expect(user_badge.post_id).to eq(post.id)
end

it 'does not grant badges from regular api calls' do
Expand Down

0 comments on commit fe578f9

Please sign in to comment.