From a7c50c7abab98b3db0b89e3b2fe78abe0dc9789c Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 17 Feb 2018 22:27:51 +0900 Subject: [PATCH 01/10] Limit the languages used for notification mailer test (#6487) Some available languages lack translations for notification mails. Now it tests for two languages which is certain to have required translations: German and English. German is the language the current project owner, Eugen Rochko speaks, and providing English translations for new messages is de facto mandatory. --- spec/mailers/notification_mailer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 8114356060..1be01e8baa 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -8,7 +8,7 @@ RSpec.describe NotificationMailer, type: :mailer do shared_examples 'localized subject' do |*args, **kwrest| it 'renders subject localized for the locale of the receiver' do - locale = I18n.available_locales.sample + locale = %i(de en).sample receiver.update!(locale: locale) expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) end From a71af984011ff98df2fa1b7f6c983706a91d99bf Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 17 Feb 2018 14:28:48 +0100 Subject: [PATCH 02/10] Push discovered status through streaming API within a time window (#6484) Time window of 6 hours --- app/lib/activitypub/activity.rb | 2 +- app/lib/ostatus/activity/creation.rb | 2 +- app/models/status.rb | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 0f9e4f2630..4617905c63 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -74,7 +74,7 @@ class ActivityPub::Activity # Only continue if the status is supposed to have # arrived in real-time - return unless @options[:override_timestamps] + return unless @options[:override_timestamps] || status.within_realtime_window? distribute_to_followers(status) end diff --git a/app/lib/ostatus/activity/creation.rb b/app/lib/ostatus/activity/creation.rb index b38407cd3d..7cf2d90dca 100644 --- a/app/lib/ostatus/activity/creation.rb +++ b/app/lib/ostatus/activity/creation.rb @@ -61,7 +61,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base Rails.logger.debug "Queuing remote status #{status.id} (#{id}) for distribution" LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? - DistributionWorker.perform_async(status.id) if @options[:override_timestamps] + DistributionWorker.perform_async(status.id) if @options[:override_timestamps] || status.within_realtime_window? status end diff --git a/app/models/status.rb b/app/models/status.rb index 0de89ad4e1..8186f47849 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -80,6 +80,8 @@ class Status < ApplicationRecord delegate :domain, to: :account, prefix: true + REAL_TIME_WINDOW = 6.hours + def searchable_by(preloaded = nil) ids = [account_id] @@ -108,6 +110,10 @@ class Status < ApplicationRecord !reblog_of_id.nil? end + def within_realtime_window? + created_at >= REAL_TIME_WINDOW.ago + end + def verb if destroyed? :delete From 9b8a4484778fb55bcb2526992807a51270229b72 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2018 06:35:05 +0900 Subject: [PATCH 03/10] Isolate each specs for cache store (#6450) The cache store is explicitly used by some specs, but they were not isolated and therefore not reliable. This fixes the issue by clearing the cache after each specs. --- config/environments/test.rb | 4 ++++ spec/models/setting_spec.rb | 13 ++++++++----- spec/presenters/instance_presenter_spec.rb | 12 +++--------- spec/rails_helper.rb | 2 ++ 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index e68cb156dd..20fe5f8138 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -23,6 +23,10 @@ Rails.application.configure do config.consider_all_requests_local = true config.action_controller.perform_caching = false + # The default store, file_store is shared by processses parallely executed + # and should not be used. + config.cache_store = :memory_store + # Raise exceptions instead of rendering exception templates. config.action_dispatch.show_exceptions = false diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index bbba5f98d9..1cc5286748 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -42,11 +42,6 @@ RSpec.describe Setting, type: :model do described_class[key] end - it 'calls Rails.cache.fetch' do - expect(Rails).to receive_message_chain(:cache, :fetch).with(cache_key) - described_class[key] - end - context 'Rails.cache does not exists' do before do allow(RailsSettings::Settings).to receive(:object).with(key).and_return(object) @@ -103,6 +98,14 @@ RSpec.describe Setting, type: :model do Rails.cache.write(cache_key, cache_value) end + it 'does not query the database' do + expect do |callback| + ActiveSupport::Notifications.subscribed callback, 'sql.active_record' do + described_class[key] + end + end.not_to yield_control + end + it 'returns the cached value' do expect(described_class[key]).to eq cache_value end diff --git a/spec/presenters/instance_presenter_spec.rb b/spec/presenters/instance_presenter_spec.rb index 7c47aed616..006403925f 100644 --- a/spec/presenters/instance_presenter_spec.rb +++ b/spec/presenters/instance_presenter_spec.rb @@ -90,9 +90,7 @@ describe InstancePresenter do describe "user_count" do it "returns the number of site users" do - cache = double - allow(Rails).to receive(:cache).and_return(cache) - allow(cache).to receive(:fetch).with("user_count").and_return(123) + Rails.cache.write 'user_count', 123 expect(instance_presenter.user_count).to eq(123) end @@ -100,9 +98,7 @@ describe InstancePresenter do describe "status_count" do it "returns the number of local statuses" do - cache = double - allow(Rails).to receive(:cache).and_return(cache) - allow(cache).to receive(:fetch).with("local_status_count").and_return(234) + Rails.cache.write 'local_status_count', 234 expect(instance_presenter.status_count).to eq(234) end @@ -110,9 +106,7 @@ describe InstancePresenter do describe "domain_count" do it "returns the number of known domains" do - cache = double - allow(Rails).to receive(:cache).and_return(cache) - allow(cache).to receive(:fetch).with("distinct_domain_count").and_return(345) + Rails.cache.write 'distinct_domain_count', 345 expect(instance_presenter.domain_count).to eq(345) end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 4f7399505c..dc1f32e085 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -51,6 +51,8 @@ RSpec.configure do |config| end config.after :each do + Rails.cache.clear + keys = Redis.current.keys Redis.current.del(keys) if keys.any? end From cba2897108dffe69d5a16befe6c6220f6eaae59f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 18 Feb 2018 03:14:46 +0100 Subject: [PATCH 04/10] Cache relationships in API (#6482) * Cache relationships in API * Fetch relationships for search results in UI * Only save one account's maps in each cache item --- app/javascript/mastodon/actions/search.js | 2 + app/models/account_domain_block.rb | 8 ++- app/models/block.rb | 4 +- app/models/concerns/relationship_cacheable.rb | 16 +++++ app/models/follow.rb | 1 + app/models/follow_request.rb | 1 + app/models/mute.rb | 4 +- .../account_relationships_presenter.rb | 68 +++++++++++++++++-- 8 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 app/models/concerns/relationship_cacheable.rb diff --git a/app/javascript/mastodon/actions/search.js b/app/javascript/mastodon/actions/search.js index 78c6109f72..73cb106ec4 100644 --- a/app/javascript/mastodon/actions/search.js +++ b/app/javascript/mastodon/actions/search.js @@ -1,4 +1,5 @@ import api from '../api'; +import { fetchRelationships } from './accounts'; export const SEARCH_CHANGE = 'SEARCH_CHANGE'; export const SEARCH_CLEAR = 'SEARCH_CLEAR'; @@ -38,6 +39,7 @@ export function submitSearch() { }, }).then(response => { dispatch(fetchSearchSuccess(response.data)); + dispatch(fetchRelationships(response.data.accounts.map(item => item.id))); }).catch(error => { dispatch(fetchSearchFail(error)); }); diff --git a/app/models/account_domain_block.rb b/app/models/account_domain_block.rb index abcc923b31..bc00b4f32b 100644 --- a/app/models/account_domain_block.rb +++ b/app/models/account_domain_block.rb @@ -16,12 +16,16 @@ class AccountDomainBlock < ApplicationRecord belongs_to :account validates :domain, presence: true, uniqueness: { scope: :account_id } - after_create :remove_blocking_cache - after_destroy :remove_blocking_cache + after_commit :remove_blocking_cache + after_commit :remove_relationship_cache private def remove_blocking_cache Rails.cache.delete("exclude_domains_for:#{account_id}") end + + def remove_relationship_cache + Rails.cache.delete_matched("relationship:#{account_id}:*") + end end diff --git a/app/models/block.rb b/app/models/block.rb index 441e6bca30..d6ecabd3b8 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -12,14 +12,14 @@ class Block < ApplicationRecord include Paginable + include RelationshipCacheable belongs_to :account belongs_to :target_account, class_name: 'Account' validates :account_id, uniqueness: { scope: :target_account_id } - after_create :remove_blocking_cache - after_destroy :remove_blocking_cache + after_commit :remove_blocking_cache private diff --git a/app/models/concerns/relationship_cacheable.rb b/app/models/concerns/relationship_cacheable.rb new file mode 100644 index 0000000000..0d9359f7e7 --- /dev/null +++ b/app/models/concerns/relationship_cacheable.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module RelationshipCacheable + extend ActiveSupport::Concern + + included do + after_commit :remove_relationship_cache + end + + private + + def remove_relationship_cache + Rails.cache.delete("relationship:#{account_id}:#{target_account_id}") + Rails.cache.delete("relationship:#{target_account_id}:#{account_id}") + end +end diff --git a/app/models/follow.rb b/app/models/follow.rb index f953b8e3e1..8e6fe537a5 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -13,6 +13,7 @@ class Follow < ApplicationRecord include Paginable + include RelationshipCacheable belongs_to :account, counter_cache: :following_count diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index bd6c4a0b9f..cde26ceed7 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -13,6 +13,7 @@ class FollowRequest < ApplicationRecord include Paginable + include RelationshipCacheable belongs_to :account belongs_to :target_account, class_name: 'Account' diff --git a/app/models/mute.rb b/app/models/mute.rb index 948f22444e..8efa27ac0d 100644 --- a/app/models/mute.rb +++ b/app/models/mute.rb @@ -13,14 +13,14 @@ class Mute < ApplicationRecord include Paginable + include RelationshipCacheable belongs_to :account belongs_to :target_account, class_name: 'Account' validates :account_id, uniqueness: { scope: :target_account_id } - after_create :remove_blocking_cache - after_destroy :remove_blocking_cache + after_commit :remove_blocking_cache private diff --git a/app/presenters/account_relationships_presenter.rb b/app/presenters/account_relationships_presenter.rb index bf1ba37161..d27fb7b01d 100644 --- a/app/presenters/account_relationships_presenter.rb +++ b/app/presenters/account_relationships_presenter.rb @@ -5,11 +5,67 @@ class AccountRelationshipsPresenter :muting, :requested, :domain_blocking def initialize(account_ids, current_account_id, **options) - @following = Account.following_map(account_ids, current_account_id).merge(options[:following_map] || {}) - @followed_by = Account.followed_by_map(account_ids, current_account_id).merge(options[:followed_by_map] || {}) - @blocking = Account.blocking_map(account_ids, current_account_id).merge(options[:blocking_map] || {}) - @muting = Account.muting_map(account_ids, current_account_id).merge(options[:muting_map] || {}) - @requested = Account.requested_map(account_ids, current_account_id).merge(options[:requested_map] || {}) - @domain_blocking = Account.domain_blocking_map(account_ids, current_account_id).merge(options[:domain_blocking_map] || {}) + @account_ids = account_ids.map { |a| a.is_a?(Account) ? a.id : a } + @current_account_id = current_account_id + + @following = cached[:following].merge(Account.following_map(@uncached_account_ids, @current_account_id)) + @followed_by = cached[:followed_by].merge(Account.followed_by_map(@uncached_account_ids, @current_account_id)) + @blocking = cached[:blocking].merge(Account.blocking_map(@uncached_account_ids, @current_account_id)) + @muting = cached[:muting].merge(Account.muting_map(@uncached_account_ids, @current_account_id)) + @requested = cached[:requested].merge(Account.requested_map(@uncached_account_ids, @current_account_id)) + @domain_blocking = cached[:domain_blocking].merge(Account.domain_blocking_map(@uncached_account_ids, @current_account_id)) + + cache_uncached! + + @following.merge!(options[:following_map] || {}) + @followed_by.merge!(options[:followed_by_map] || {}) + @blocking.merge!(options[:blocking_map] || {}) + @muting.merge!(options[:muting_map] || {}) + @requested.merge!(options[:requested_map] || {}) + @domain_blocking.merge!(options[:domain_blocking_map] || {}) + end + + private + + def cached + return @cached if defined?(@cached) + + @cached = { + following: {}, + followed_by: {}, + blocking: {}, + muting: {}, + requested: {}, + domain_blocking: {}, + } + + @uncached_account_ids = [] + + @account_ids.each do |account_id| + maps_for_account = Rails.cache.read("relationship:#{@current_account_id}:#{account_id}") + + if maps_for_account.is_a?(Hash) + @cached.merge!(maps_for_account) + else + @uncached_account_ids << account_id + end + end + + @cached + end + + def cache_uncached! + @uncached_account_ids.each do |account_id| + maps_for_account = { + following: { account_id => following[account_id] }, + followed_by: { account_id => followed_by[account_id] }, + blocking: { account_id => blocking[account_id] }, + muting: { account_id => muting[account_id] }, + requested: { account_id => requested[account_id] }, + domain_blocking: { account_id => domain_blocking[account_id] }, + } + + Rails.cache.write("relationship:#{@current_account_id}:#{account_id}", maps_for_account, expires_in: 1.day) + end end end From 51869f2a8ccf94fa8f82161d07e8f9e80c866bd7 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 19 Feb 2018 00:32:17 +0900 Subject: [PATCH 05/10] Remove unnecessary g++ configuration (#6499) --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2fba133c19..61d51ca21c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,6 @@ env: - RAILS_ENV=test - NOKOGIRI_USE_SYSTEM_LIBRARIES=true - PARALLEL_TEST_PROCESSORS=2 - - "PATH=$HOME:$PATH" addons: postgresql: 9.4 @@ -53,7 +52,6 @@ install: before_script: - ./bin/rails parallel:create parallel:load_schema parallel:prepare assets:precompile - - ln -s /usr/bin/x86_64-linux-gnu-g++-6 "$HOME/g++" script: - travis_retry bundle exec parallel_test spec/ --group-by filesize --type rspec From bc6751ecce2ba81e074fa3b672a3fe5ee6004302 Mon Sep 17 00:00:00 2001 From: HellPie Date: Sun, 18 Feb 2018 16:32:58 +0100 Subject: [PATCH 06/10] Remove outline from body window (Fixes #6501) (#6502) --- app/javascript/styles/mastodon/basics.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/javascript/styles/mastodon/basics.scss b/app/javascript/styles/mastodon/basics.scss index 0e3b9ad6bf..bec0d4d91a 100644 --- a/app/javascript/styles/mastodon/basics.scss +++ b/app/javascript/styles/mastodon/basics.scss @@ -122,5 +122,6 @@ button { height: 100%; align-items: center; justify-content: center; + outline: 0 !important; } } From 78936461d77e3c5c60dc2430c309c8caf670ac2e Mon Sep 17 00:00:00 2001 From: Kazushige Tominaga Date: Mon, 19 Feb 2018 00:34:03 +0900 Subject: [PATCH 07/10] Added fetch_remote_status_service call spec case actibitypub (#6500) * Added #link_header spec * Added #call spec * Delete spec of private methods * Added call test case activitypub --- .../fetch_remote_status_service_spec.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/spec/services/fetch_remote_status_service_spec.rb b/spec/services/fetch_remote_status_service_spec.rb index cbdecbf25f..fa5782b94a 100644 --- a/spec/services/fetch_remote_status_service_spec.rb +++ b/spec/services/fetch_remote_status_service_spec.rb @@ -1,4 +1,35 @@ require 'rails_helper' RSpec.describe FetchRemoteStatusService do + let(:account) { Fabricate(:account) } + let(:prefetched_body) { nil } + let(:valid_domain) { Rails.configuration.x.local_domain } + + let(:note) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: "https://#{valid_domain}/@foo/1234", + type: 'Note', + content: 'Lorem ipsum', + attributedTo: ActivityPub::TagManager.instance.uri_for(account), + } + end + + context 'protocol is :activitypub' do + subject { described_class.new.call(note[:id], prefetched_body, protocol) } + let(:prefetched_body) { Oj.dump(note) } + let(:protocol) { :activitypub } + + before do + account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) + subject + end + + it 'creates status' do + status = account.statuses.first + + expect(status).to_not be_nil + expect(status.text).to eq 'Lorem ipsum' + end + end end From bb26cdda242afff59f7718b3211732fc068980f1 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Sun, 18 Feb 2018 22:57:53 +0100 Subject: [PATCH 08/10] add parameter locked to /api/v1/update_credentials (#6506) --- app/controllers/api/v1/accounts/credentials_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/v1/accounts/credentials_controller.rb b/app/controllers/api/v1/accounts/credentials_controller.rb index da534d960e..68af225295 100644 --- a/app/controllers/api/v1/accounts/credentials_controller.rb +++ b/app/controllers/api/v1/accounts/credentials_controller.rb @@ -20,6 +20,6 @@ class Api::V1::Accounts::CredentialsController < Api::BaseController private def account_params - params.permit(:display_name, :note, :avatar, :header) + params.permit(:display_name, :note, :avatar, :header, :locked) end end From cbb69d41f6eda4d997f7bcb89e9aef8fd8bec2e4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 19 Feb 2018 02:39:18 +0100 Subject: [PATCH 09/10] Fix media spoiler design (#6507) - 4px rounded corners on media attachments - Better colors/contrast for CW/media spoiler on public pages - Fix vertical alignment of "Show more" button - Fix layout jump when unhiding standalone media --- .../mastodon/components/media_gallery.js | 6 +----- .../styles/mastodon/components.scss | 20 ++++++++++++++++--- .../styles/mastodon/stream_entries.scss | 20 ++++++------------- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/app/javascript/mastodon/components/media_gallery.js b/app/javascript/mastodon/components/media_gallery.js index 00943e205b..a3ffc45eae 100644 --- a/app/javascript/mastodon/components/media_gallery.js +++ b/app/javascript/mastodon/components/media_gallery.js @@ -227,12 +227,8 @@ export default class MediaGallery extends React.PureComponent { const style = {}; if (this.isStandaloneEligible()) { - if (!visible && width) { - // only need to forcibly set the height in "sensitive" mode + if (width) { style.height = width / this.props.media.getIn([0, 'meta', 'small', 'aspect']); - } else { - // layout automatically, using image's natural aspect ratio - style.height = ''; } } else { // crop the image diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index d1fbabfc54..6a256c466e 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -686,12 +686,13 @@ background: transparent; border: 0; color: lighten($ui-base-color, 8%); - font-weight: 500; + font-weight: 700; font-size: 11px; padding: 0 6px; text-transform: uppercase; - line-height: inherit; + line-height: 20px; cursor: pointer; + vertical-align: middle; } .status__prepend-icon-wrapper { @@ -899,6 +900,11 @@ height: 24px; margin: -1px 0 0; } + + .status__content__spoiler-link { + line-height: 24px; + margin: -1px 0 0; + } } .video-player { @@ -2667,12 +2673,16 @@ a.status-card { background: $base-overlay-background; color: $ui-primary-color; border: 0; + padding: 0; width: 100%; height: 100%; + border-radius: 4px; + appearance: none; &:hover, &:active, &:focus { + padding: 0; color: lighten($ui-primary-color, 8%); } } @@ -2685,7 +2695,7 @@ a.status-card { .media-spoiler__trigger { display: block; font-size: 11px; - font-weight: 500; + font-weight: 700; } .spoiler-button { @@ -4091,6 +4101,7 @@ a.status-card { box-sizing: border-box; margin-top: 8px; overflow: hidden; + border-radius: 4px; position: relative; width: 100%; } @@ -4101,6 +4112,8 @@ a.status-card { display: block; float: left; position: relative; + border-radius: 4px; + overflow: hidden; &.standalone { .media-gallery__item-gifv-thumbnail { @@ -4113,6 +4126,7 @@ a.status-card { cursor: zoom-in; display: block; text-decoration: none; + color: $ui-secondary-color; height: 100%; line-height: 0; diff --git a/app/javascript/styles/mastodon/stream_entries.scss b/app/javascript/styles/mastodon/stream_entries.scss index e11ca29d9e..442b143a04 100644 --- a/app/javascript/styles/mastodon/stream_entries.scss +++ b/app/javascript/styles/mastodon/stream_entries.scss @@ -146,10 +146,10 @@ a.status__content__spoiler-link { color: $primary-text-color; - background: $ui-primary-color; + background: $ui-base-color; &:hover { - background: lighten($ui-primary-color, 8%); + background: lighten($ui-base-color, 8%); } } } @@ -214,10 +214,10 @@ a.status__content__spoiler-link { color: $primary-text-color; - background: $ui-primary-color; + background: $ui-base-color; &:hover { - background: lighten($ui-primary-color, 8%); + background: lighten($ui-base-color, 8%); } } } @@ -260,16 +260,8 @@ } .media-spoiler { - background: $ui-primary-color; - color: $white; - transition: all 40ms linear; - - &:hover, - &:active, - &:focus { - background: darken($ui-primary-color, 2%); - color: unset; - } + background: $ui-base-color; + color: $ui-primary-color; } .pre-header { From 66105929e07fc7ddbdb8b66696b9ce1ed5d25957 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 19 Feb 2018 16:06:12 +0100 Subject: [PATCH 10/10] Don't resize avatars/headers if their dimensions are already right (#6508) Also don't apply "-quality 80" option which is probably the reason for slight color differences between original and remote image (because it would apply it twice, once on original instance, and again on the receiving instance) --- app/models/concerns/account_avatar.rb | 12 +++++++++--- app/models/concerns/account_header.rb | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/models/concerns/account_avatar.rb b/app/models/concerns/account_avatar.rb index 8a5c9a22c3..53d0d876f5 100644 --- a/app/models/concerns/account_avatar.rb +++ b/app/models/concerns/account_avatar.rb @@ -7,9 +7,15 @@ module AccountAvatar class_methods do def avatar_styles(file) - styles = { original: '120x120#' } - styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' + styles = {} + geometry = Paperclip::Geometry.from_file(file) + + styles[:original] = '120x120#' if geometry.width != geometry.height || geometry.width > 120 || geometry.height > 120 + styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' + styles + rescue Paperclip::Errors::NotIdentifiedByImageMagickError + {} end private :avatar_styles @@ -17,7 +23,7 @@ module AccountAvatar included do # Avatar upload - has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-quality 80 -strip' } + has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-strip' } validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES validates_attachment_size :avatar, less_than: 2.megabytes end diff --git a/app/models/concerns/account_header.rb b/app/models/concerns/account_header.rb index aff2aa3f90..991473d8c0 100644 --- a/app/models/concerns/account_header.rb +++ b/app/models/concerns/account_header.rb @@ -7,9 +7,15 @@ module AccountHeader class_methods do def header_styles(file) - styles = { original: '700x335#' } - styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' + styles = {} + geometry = Paperclip::Geometry.from_file(file) + + styles[:original] = '700x335#' unless geometry.width == 700 && geometry.height == 335 + styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' + styles + rescue Paperclip::Errors::NotIdentifiedByImageMagickError + {} end private :header_styles @@ -17,7 +23,7 @@ module AccountHeader included do # Header upload - has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-quality 80 -strip' } + has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-strip' } validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES validates_attachment_size :header, less_than: 2.megabytes end