From 6437cd03413a346976efef3e0a11a5eba0e2cf9c Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 11 Sep 2015 18:14:34 +1000 Subject: [PATCH] FEATURE: add support for generic external avatar services This changes it so we only ship an avatar template down to the client it has no magic, all it knows is how to plug in size --- .../discourse/components/who-liked.js.es6 | 2 +- .../discourse/helpers/application.js.es6 | 11 ++----- .../discourse/helpers/user-avatar.js.es6 | 17 ++++------ .../discourse/lib/avatar-template.js.es6 | 31 ------------------- .../discourse/models/composer.js.es6 | 4 +-- .../discourse/models/user-action.js.es6 | 2 -- .../javascripts/discourse/models/user.js.es6 | 6 ---- .../templates/components/stream-item.hbs | 2 +- .../templates/list/posters-column.raw.hbs | 2 +- .../discourse/views/composer.js.es6 | 7 +---- app/assets/javascripts/main_include.js | 2 -- app/controllers/users_controller.rb | 2 +- app/models/user.rb | 15 ++++++--- app/serializers/basic_post_serializer.rb | 10 ------ app/serializers/basic_user_serializer.rb | 10 +----- app/serializers/post_serializer.rb | 4 +-- app/serializers/user_action_serializer.rb | 14 +-------- config/locales/server.en.yml | 4 +-- config/site_settings.yml | 8 ++--- 19 files changed, 35 insertions(+), 118 deletions(-) delete mode 100644 app/assets/javascripts/discourse/lib/avatar-template.js.es6 diff --git a/app/assets/javascripts/discourse/components/who-liked.js.es6 b/app/assets/javascripts/discourse/components/who-liked.js.es6 index 34ba67223..5c12a91d9 100644 --- a/app/assets/javascripts/discourse/components/who-liked.js.es6 +++ b/app/assets/javascripts/discourse/components/who-liked.js.es6 @@ -13,7 +13,7 @@ export default Ember.Component.extend(StringBuffer, { iconsHtml += ""; iconsHtml += Discourse.Utilities.avatarImg({ size: 'small', - avatarTemplate: u.get('avatarTemplate'), + avatarTemplate: u.get('avatar_template'), title: u.get('username') }); iconsHtml += ""; diff --git a/app/assets/javascripts/discourse/helpers/application.js.es6 b/app/assets/javascripts/discourse/helpers/application.js.es6 index 02fa08fa8..5c72c6fc8 100644 --- a/app/assets/javascripts/discourse/helpers/application.js.es6 +++ b/app/assets/javascripts/discourse/helpers/application.js.es6 @@ -1,22 +1,17 @@ import registerUnbound from 'discourse/helpers/register-unbound'; -import avatarTemplate from 'discourse/lib/avatar-template'; import { longDate, autoUpdatingRelativeAge, number } from 'discourse/lib/formatter'; const safe = Handlebars.SafeString; -Em.Handlebars.helper('bound-avatar', function(user, size, uploadId) { +Em.Handlebars.helper('bound-avatar', function(user, size) { if (Em.isEmpty(user)) { return new safe("
"); } - const username = Em.get(user, 'username'), - letterAvatarColor = Em.get(user, 'letter_avatar_color'); - - if (arguments.length < 4) { uploadId = Em.get(user, 'uploaded_avatar_id'); } - const avatar = Em.get(user, 'avatar_template') || avatarTemplate(username, uploadId, letterAvatarColor); + const avatar = Em.get(user, 'avatar_template'); return new safe(Discourse.Utilities.avatarImg({ size: size, avatarTemplate: avatar })); -}, 'username', 'uploaded_avatar_id', 'letter_avatar_color', 'avatar_template'); +}, 'username', 'avatar_template'); /* * Used when we only have a template diff --git a/app/assets/javascripts/discourse/helpers/user-avatar.js.es6 b/app/assets/javascripts/discourse/helpers/user-avatar.js.es6 index aea2e9baa..c5eac31ad 100644 --- a/app/assets/javascripts/discourse/helpers/user-avatar.js.es6 +++ b/app/assets/javascripts/discourse/helpers/user-avatar.js.es6 @@ -1,15 +1,14 @@ import registerUnbound from 'discourse/helpers/register-unbound'; -import avatarTemplate from 'discourse/lib/avatar-template'; function renderAvatar(user, options) { options = options || {}; if (user) { - let username = Em.get(user, 'username'); - if (!username) { - if (!options.usernamePath) { return ''; } - username = Em.get(user, options.usernamePath); - } + + const username = Em.get(user, options.usernamePath || 'username'); + const avatarTemplate = Em.get(user, options.avatarTemplatePath || 'avatar_template'); + + if (!username || !avatarTemplate) { return ''; } let title; if (!options.ignoreTitle) { @@ -27,15 +26,11 @@ function renderAvatar(user, options) { } } - // this is simply done to ensure we cache images correctly - const uploadedAvatarId = Em.get(user, 'uploaded_avatar_id') || Em.get(user, 'user.uploaded_avatar_id'), - letterAvatarColor = Em.get(user, 'letter_avatar_color') || Em.get(user, 'user.letter_avatar_color'); - return Discourse.Utilities.avatarImg({ size: options.imageSize, extraClasses: Em.get(user, 'extras') || options.extraClasses, title: title || username, - avatarTemplate: Em.get("avatar_template") || avatarTemplate(username, uploadedAvatarId, letterAvatarColor) + avatarTemplate: avatarTemplate }); } else { return ''; diff --git a/app/assets/javascripts/discourse/lib/avatar-template.js.es6 b/app/assets/javascripts/discourse/lib/avatar-template.js.es6 deleted file mode 100644 index 4d2fdb2bf..000000000 --- a/app/assets/javascripts/discourse/lib/avatar-template.js.es6 +++ /dev/null @@ -1,31 +0,0 @@ -import { hashString } from 'discourse/lib/hash'; - -let _splitAvatars; - -function defaultAvatar(username, letterAvatarColor) { - const defaultAvatars = Discourse.SiteSettings.default_avatars, - version = Discourse.LetterAvatarVersion; - - if (defaultAvatars && defaultAvatars.length) { - _splitAvatars = _splitAvatars || defaultAvatars.split("\n"); - - if (_splitAvatars.length) { - const hash = hashString(username); - return _splitAvatars[Math.abs(hash) % _splitAvatars.length]; - } - } - - if (Discourse.SiteSettings.external_letter_avatars_enabled) { - const url = Discourse.SiteSettings.external_letter_avatars_url; - return `${url}/letter/${username[0]}/${letterAvatarColor}/{size}.png`; - } else { - return Discourse.getURLWithCDN(`/letter_avatar/${username.toLowerCase()}/{size}/${version}.png`); - } -} - -export default function(username, uploadedAvatarId, letterAvatarColor) { - if (uploadedAvatarId) { - return Discourse.getURLWithCDN(`/user_avatar/${Discourse.BaseUrl}/${username.toLowerCase()}/{size}/${uploadedAvatarId}.png`); - } - return defaultAvatar(username, letterAvatarColor); -} diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index 12cba4936..9cb5db069 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -567,7 +567,7 @@ const Composer = RestModel.extend({ username: user.get('username'), user_id: user.get('id'), user_title: user.get('title'), - uploaded_avatar_id: user.get('uploaded_avatar_id'), + avatar_template: user.get('avatar_template'), user_custom_fields: user.get('custom_fields'), post_type: this.site.get('post_types.regular'), actions_summary: [], @@ -587,7 +587,7 @@ const Composer = RestModel.extend({ reply_to_post_number: post.get('post_number'), reply_to_user: { username: post.get('username'), - uploaded_avatar_id: post.get('uploaded_avatar_id') + avatar_template: post.get('avatar_template') } }); } diff --git a/app/assets/javascripts/discourse/models/user-action.js.es6 b/app/assets/javascripts/discourse/models/user-action.js.es6 index 05e1e4929..f03d81908 100644 --- a/app/assets/javascripts/discourse/models/user-action.js.es6 +++ b/app/assets/javascripts/discourse/models/user-action.js.es6 @@ -154,8 +154,6 @@ const UserAction = RestModel.extend({ switchToActing() { this.setProperties({ username: this.get('acting_username'), - uploaded_avatar_id: this.get('acting_uploaded_avatar_id'), - letter_avatar_color: this.get('action_letter_avatar_color'), name: this.get('actingDisplayName') }); } diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 8fac2812e..4fab93f2d 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -1,6 +1,5 @@ import { url } from 'discourse/lib/computed'; import RestModel from 'discourse/models/rest'; -import avatarTemplate from 'discourse/lib/avatar-template'; import UserStream from 'discourse/models/user-stream'; import UserPostsStream from 'discourse/models/user-posts-stream'; import Singleton from 'discourse/mixins/singleton'; @@ -257,11 +256,6 @@ const User = RestModel.extend({ }); }, - @computed("username", "uploaded_avatar_id", "letter_avatar_color") - avatarTemplate(username, uploadedAvatarId, letterAvatarColor) { - return avatarTemplate(username, uploadedAvatarId, letterAvatarColor); - }, - /* Change avatar selection */ diff --git a/app/assets/javascripts/discourse/templates/components/stream-item.hbs b/app/assets/javascripts/discourse/templates/components/stream-item.hbs index c84082519..22900bbb2 100644 --- a/app/assets/javascripts/discourse/templates/components/stream-item.hbs +++ b/app/assets/javascripts/discourse/templates/components/stream-item.hbs @@ -23,7 +23,7 @@ {{fa-icon 'times'}} {{i18n "bookmarks.remove"}} {{else}} -
{{avatar grandChild imageSize="tiny" extraClasses="actor" ignoreTitle="true"}}
 +
{{avatar grandChild imageSize="tiny" extraClasses="actor" ignoreTitle="true" avatarTemplatePath="acting_avatar_template"}}
 {{#if grandChild.edit_reason}} — {{grandChild.edit_reason}}{{/if}} {{/if}} {{/each}} diff --git a/app/assets/javascripts/discourse/templates/list/posters-column.raw.hbs b/app/assets/javascripts/discourse/templates/list/posters-column.raw.hbs index 1b837fb5a..5adbfd3d7 100644 --- a/app/assets/javascripts/discourse/templates/list/posters-column.raw.hbs +++ b/app/assets/javascripts/discourse/templates/list/posters-column.raw.hbs @@ -1,5 +1,5 @@ {{#each poster in posters}} -{{avatar poster usernamePath="user.username" imageSize="small"}} +{{avatar poster avatarTemplatePath="user.avatar_template" usernamePath="user.username" imageSize="small"}} {{/each}} diff --git a/app/assets/javascripts/discourse/views/composer.js.es6 b/app/assets/javascripts/discourse/views/composer.js.es6 index 63a8675ec..5d62db416 100644 --- a/app/assets/javascripts/discourse/views/composer.js.es6 +++ b/app/assets/javascripts/discourse/views/composer.js.es6 @@ -1,7 +1,6 @@ import userSearch from 'discourse/lib/user-search'; import afterTransition from 'discourse/lib/after-transition'; import loadScript from 'discourse/lib/load-script'; -import avatarTemplate from 'discourse/lib/avatar-template'; import positioningWorkaround from 'discourse/lib/safari-hacks'; import debounce from 'discourse/lib/debounce'; import { linkSeenMentions, fetchUnseenMentions } from 'discourse/lib/link-mentions'; @@ -251,11 +250,7 @@ const ComposerView = Ember.View.extend(Ember.Evented, { if (posts && topicId === self.get('controller.controllers.topic.model.id')) { const quotedPost = posts.findProperty("post_number", postNumber); if (quotedPost) { - const username = quotedPost.get('username'), - uploadId = quotedPost.get('uploaded_avatar_id'), - letterAvatarColor = quotedPost.get("letter_avatar_color"); - - return Discourse.Utilities.tinyAvatar(avatarTemplate(username, uploadId, letterAvatarColor)); + return Discourse.Utilities.tinyAvatar(quotedPost.get('avatar_template')); } } } diff --git a/app/assets/javascripts/main_include.js b/app/assets/javascripts/main_include.js index 797cab865..757f7308e 100644 --- a/app/assets/javascripts/main_include.js +++ b/app/assets/javascripts/main_include.js @@ -14,7 +14,6 @@ //= require ./discourse/lib/load-script //= require ./discourse/lib/notification-levels //= require ./discourse/lib/app-events -//= require ./discourse/lib/avatar-template //= require ./discourse/lib/url //= require ./discourse/lib/debounce //= require ./discourse/lib/quote @@ -41,7 +40,6 @@ //= require ./discourse/lib/autocomplete //= require ./discourse/lib/after-transition //= require ./discourse/lib/debounce -//= require ./discourse/lib/avatar-template //= require ./discourse/lib/safari-hacks //= require_tree ./discourse/adapters //= require ./discourse/models/rest diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cf72afcf6..22334cc9b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -518,7 +518,7 @@ class UsersController < ApplicationController user_fields = [:username, :upload_avatar_template, :uploaded_avatar_id] user_fields << :name if SiteSetting.enable_names? - to_render = { users: results.as_json(only: user_fields, methods: [:avatar_template, :letter_avatar_color]) } + to_render = { users: results.as_json(only: user_fields, methods: [:avatar_template]) } if params[:include_groups] == "true" to_render[:groups] = Group.search_group(term, current_user).map {|m| {:name=>m.name, :usernames=> m.usernames.split(",")} } diff --git a/app/models/user.rb b/app/models/user.rb index 75d732ea6..4c8e1d9aa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -457,7 +457,7 @@ class User < ActiveRecord::Base split_avatars[hash.abs % split_avatars.size] end else - letter_avatar_template(username) + system_avatar_template(username) end end @@ -468,10 +468,15 @@ class User < ActiveRecord::Base UserAvatar.local_avatar_template(hostname, username.downcase, uploaded_avatar_id) end - def self.letter_avatar_template(username) - if SiteSetting.external_letter_avatars_enabled + def self.system_avatar_template(username) + # TODO it may be worth caching this in a distributed cache, should be benched + if SiteSetting.external_system_avatars_enabled color = letter_avatar_color(username) - "#{SiteSetting.external_letter_avatars_url}/letter/#{username[0]}/#{color}/{size}.png" + url = SiteSetting.external_system_avatars_url.dup + url.gsub! "{color}", color + url.gsub! "{username}", username + url.gsub! "{first_letter}", username[0].downcase + url else "#{Discourse.base_uri}/letter_avatar/#{username.downcase}/{size}/#{LetterAvatar.version}.png" end @@ -484,7 +489,7 @@ class User < ActiveRecord::Base def self.letter_avatar_color(username) username = username || "" color = LetterAvatar::COLORS[Digest::MD5.hexdigest(username)[0...15].to_i(16) % LetterAvatar::COLORS.length] - color.map { |c| c.to_s(16) }.join + color.map { |c| c.to_s(16).rjust(2, '0') }.join end def avatar_template diff --git a/app/serializers/basic_post_serializer.rb b/app/serializers/basic_post_serializer.rb index 4edb2b5cc..8969d19a0 100644 --- a/app/serializers/basic_post_serializer.rb +++ b/app/serializers/basic_post_serializer.rb @@ -4,8 +4,6 @@ class BasicPostSerializer < ApplicationSerializer :name, :username, :avatar_template, - :uploaded_avatar_id, - :letter_avatar_color, :created_at, :cooked, :cooked_hidden @@ -22,14 +20,6 @@ class BasicPostSerializer < ApplicationSerializer object.user.try(:avatar_template) end - def uploaded_avatar_id - object.user.try(:uploaded_avatar_id) - end - - def letter_avatar_color - object.user.try(:letter_avatar_color) - end - def cooked_hidden object.hidden && !scope.is_staff? end diff --git a/app/serializers/basic_user_serializer.rb b/app/serializers/basic_user_serializer.rb index 2c72eb342..8880c8dbd 100644 --- a/app/serializers/basic_user_serializer.rb +++ b/app/serializers/basic_user_serializer.rb @@ -1,5 +1,5 @@ class BasicUserSerializer < ApplicationSerializer - attributes :id, :username, :uploaded_avatar_id, :avatar_template, :letter_avatar_color + attributes :id, :username, :avatar_template def include_name? SiteSetting.enable_names? @@ -17,12 +17,4 @@ class BasicUserSerializer < ApplicationSerializer object[:user] || object end - def letter_avatar_color - if Hash === object - User.letter_avatar_color(user[:username]) - else - user.try(:letter_avatar_color) - end - end - end diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 9bc469f04..a10f9dbf6 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -177,9 +177,7 @@ class PostSerializer < BasicPostSerializer def reply_to_user { username: object.reply_to_user.username, - avatar_template: object.reply_to_user.avatar_template, - uploaded_avatar_id: object.reply_to_user.uploaded_avatar_id, - letter_avatar_color: object.reply_to_user.letter_avatar_color, + avatar_template: object.reply_to_user.avatar_template } end diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb index 4bf665004..d9609ff9b 100644 --- a/app/serializers/user_action_serializer.rb +++ b/app/serializers/user_action_serializer.rb @@ -26,12 +26,8 @@ class UserActionSerializer < ApplicationSerializer :action_code, :edit_reason, :category_id, - :uploaded_avatar_id, - :letter_avatar_color, :closed, - :archived, - :acting_uploaded_avatar_id, - :acting_letter_avatar_color + :archived def excerpt cooked = object.cooked || PrettyText.cook(object.raw) @@ -86,12 +82,4 @@ class UserActionSerializer < ApplicationSerializer object.topic_archived end - def letter_avatar_color - User.letter_avatar_color(username) - end - - def acting_letter_avatar_color - User.letter_avatar_color(acting_username) - end - end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 7dd33c874..1309a124e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -979,8 +979,8 @@ en: avatar_sizes: "List of automatically generated avatar sizes." - external_letter_avatars_enabled: "Use external letter avatars service." - external_letter_avatars_url: "URL of the external letter avatars service." + external_system_avatars_enabled: "Use external system avatars service." + external_system_avatars_url: "URL of the external system avatars service. Allowed substitions are {username} {first_letter} {color} {size}" enable_flash_video_onebox: "Enable embedding of swf and flv (Adobe Flash) links in oneboxes. WARNING: may introduce security risks." diff --git a/config/site_settings.yml b/config/site_settings.yml index add19d2db..5c96a0385 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -572,13 +572,13 @@ files: avatar_sizes: default: '20|25|32|45|60|120' type: list - external_letter_avatars_enabled: + external_system_avatars_enabled: default: false client: true - external_letter_avatars_url: - default: "https://avatars.discourse.org" + external_system_avatars_url: + default: "https://avatars.discourse.org/letter/{first_letter}/{color}/{size}.png" client: true - regex: '^https?:\/\/.+[^\/]$' + regex: '^https?:\/\/.+[^\/]' trust: default_trust_level: