Commit 0a807954 authored by Douwe Maan's avatar Douwe Maan

Merge branch '1128-geo-custom-avatars' into 'master'

Geo: Display Custom Avatars in secondary nodes

This MR introduces some small patches for `User`, `Project` and `Group` models and related classes to build the avatar URL pointing to the primary node.
We should be able to remove this in the future when we start replicating local assets in Geo (#76).

For the changes to take effect on all pages, it's required to run:

```
# for Omnibus GitLab installs:
gitlab-rake cache:clear

# for Source installs:
RAILS_ENV=production bundle exec rake cache:clear
```

Closes #1128

See merge request !904
parents 62f1c3c2 e6242106
...@@ -8,11 +8,7 @@ module GroupsHelper ...@@ -8,11 +8,7 @@ module GroupsHelper
group = Group.find_by(path: group) group = Group.find_by(path: group)
end end
if group && group.avatar.present? group.try(:avatar_url) || image_path('no_group_avatar.png')
group.avatar.url
else
image_path('no_group_avatar.png')
end
end end
def group_title(group, name = nil, url = nil) def group_title(group, name = nil, url = nil)
......
module EE
# Avatars in Geo mixin
#
# This module is intended to encapsulate Geo-specific logic
# and be **prepended** in the `Group`, `User`, `Project` models
module GeoAwareAvatar
def avatar_url(size = nil, scale = 2)
if self[:avatar].present? && ::Gitlab::Geo.secondary?
File.join(::Gitlab::Geo.primary_node.url, avatar.url)
else
super
end
end
end
end
# Group EE mixin
#
# This module is intended to encapsulate EE-specific model logic
# and be included in the `Group` model
module EE module EE
# Group EE mixin
#
# This module is intended to encapsulate EE-specific model logic
# and be included in the `Group` model
module Group module Group
extend ActiveSupport::Concern extend ActiveSupport::Concern
......
...@@ -9,6 +9,7 @@ class Group < Namespace ...@@ -9,6 +9,7 @@ class Group < Namespace
include AccessRequestable include AccessRequestable
include Referable include Referable
include SelectForProjectAuthorization include SelectForProjectAuthorization
prepend EE::GeoAwareAvatar
has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source
alias_method :members, :group_members alias_method :members, :group_members
...@@ -116,7 +117,7 @@ class Group < Namespace ...@@ -116,7 +117,7 @@ class Group < Namespace
allowed_by_projects allowed_by_projects
end end
def avatar_url(size = nil) def avatar_url(size = nil, scale = nil)
if self[:avatar].present? if self[:avatar].present?
[gitlab_config.url, avatar.url].join [gitlab_config.url, avatar.url].join
end end
......
...@@ -15,6 +15,7 @@ class Project < ActiveRecord::Base ...@@ -15,6 +15,7 @@ class Project < ActiveRecord::Base
include Elastic::ProjectsSearch include Elastic::ProjectsSearch
include ProjectFeaturesCompatibility include ProjectFeaturesCompatibility
include SelectForProjectAuthorization include SelectForProjectAuthorization
prepend EE::GeoAwareAvatar
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
...@@ -915,7 +916,7 @@ class Project < ActiveRecord::Base ...@@ -915,7 +916,7 @@ class Project < ActiveRecord::Base
repository.avatar repository.avatar
end end
def avatar_url def avatar_url(size = nil, scale = nil)
if self[:avatar].present? if self[:avatar].present?
[gitlab_config.url, avatar.url].join [gitlab_config.url, avatar.url].join
elsif avatar_in_git elsif avatar_in_git
......
...@@ -9,6 +9,7 @@ class User < ActiveRecord::Base ...@@ -9,6 +9,7 @@ class User < ActiveRecord::Base
include Sortable include Sortable
include CaseSensitivity include CaseSensitivity
include TokenAuthenticatable include TokenAuthenticatable
prepend EE::GeoAwareAvatar
DEFAULT_NOTIFICATION_LEVEL = :participating DEFAULT_NOTIFICATION_LEVEL = :participating
......
---
title: Geo: Display Custom Avatars in secondary nodes
merge_request: 904
author:
...@@ -18,7 +18,7 @@ module EE ...@@ -18,7 +18,7 @@ module EE
# Geo should only update Redis based cache, as data store in the database # Geo should only update Redis based cache, as data store in the database
# will be updated on primary and replicated to the secondaries. # will be updated on primary and replicated to the secondaries.
def perform_geo_secondary(project_id, refresh = []) def perform_geo_secondary(project_id, refresh = [])
project = Project.find_by(id: project_id) project = ::Project.find_by(id: project_id)
return unless project && project.repository.exists? return unless project && project.repository.exists?
......
require 'spec_helper' require 'spec_helper'
describe GroupsHelper do describe GroupsHelper do
let(:group) { create(:group) }
describe 'group_icon' do describe 'group_icon' do
avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
it 'returns an url for the avatar' do it 'returns an url for the avatar' do
group = create(:group)
group.avatar = File.open(avatar_file_path) group.avatar = File.open(avatar_file_path)
group.save! group.save!
expect(group_icon(group.path).to_s). expect(group_icon(group.path).to_s).to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
end end
it 'gives default avatar_icon when no avatar is present' do it 'gives default avatar_icon when no avatar is present' do
group = create(:group)
group.save! group.save!
expect(group_icon(group.path)).to match('group_avatar.png') expect(group_icon(group.path)).to match('group_avatar.png')
end end
context 'in a geo secondary node' do
let(:geo_url) { 'http://geo.example.com' }
before do
allow(Gitlab::Geo).to receive(:secondary?) { true }
allow(Gitlab::Geo).to receive_message_chain(:primary_node, :url) { geo_url }
end
it 'returns an url for the avatar pointing to the primary node base url' do
group.avatar = File.open(avatar_file_path)
group.save!
expect(group_icon(group.path).to_s).to match("#{geo_url}/uploads/group/avatar/#{group.id}/banana_sample.gif")
end
it 'gives default avatar_icon when no avatar is present' do
group.save!
expect(group_icon(group.path)).to match('group_avatar.png')
end
end
end end
describe 'group_lfs_status' do describe 'group_lfs_status' do
let(:group) { create(:group) }
let!(:project) { create(:empty_project, namespace_id: group.id) } let!(:project) { create(:empty_project, namespace_id: group.id) }
before do before do
......
...@@ -140,6 +140,36 @@ describe Group, models: true do ...@@ -140,6 +140,36 @@ describe Group, models: true do
end end
end end
describe '#avatar_url' do
let(:user) { create(:user) }
subject { group.avatar_url }
context 'when avatar file is uploaded' do
before do
group.add_user(user, GroupMember::MASTER)
group.update_columns(avatar: 'avatar.png')
allow(group.avatar).to receive(:present?) { true }
end
let(:avatar_path) do
"/uploads/group/avatar/#{group.id}/avatar.png"
end
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
context 'when in a geo secondary node' do
let(:geo_url) { 'http://geo.example.com' }
before do
allow(Gitlab::Geo).to receive(:secondary?) { true }
allow(Gitlab::Geo).to receive_message_chain(:primary_node, :url) { geo_url }
end
it { should eq "#{geo_url}#{avatar_path}" }
end
end
end
describe '.search' do describe '.search' do
it 'returns groups with a matching name' do it 'returns groups with a matching name' do
expect(described_class.search(group.name)).to eq([group]) expect(described_class.search(group.name)).to eq([group])
......
...@@ -818,6 +818,17 @@ describe Project, models: true do ...@@ -818,6 +818,17 @@ describe Project, models: true do
end end
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
context 'When in a geo secondary node' do
let(:geo_url) { 'http://geo.example.com' }
before do
allow(Gitlab::Geo).to receive(:secondary?) { true }
allow(Gitlab::Geo).to receive_message_chain(:primary_node, :url) { geo_url }
end
it { should eq "#{geo_url}#{avatar_path}" }
end
end end
context 'When avatar file in git' do context 'When avatar file in git' do
......
...@@ -814,6 +814,35 @@ describe User, models: true do ...@@ -814,6 +814,35 @@ describe User, models: true do
end end
end end
describe '#avatar_url' do
let(:user) { create(:user) }
subject { user.avatar_url }
context 'when avatar file is uploaded' do
before do
user.update_columns(avatar: 'uploads/avatar.png')
allow(user.avatar).to receive(:present?) { true }
end
let(:avatar_path) do
"/uploads/user/avatar/#{user.id}/uploads/avatar.png"
end
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
context 'when in a geo secondary node' do
let(:geo_url) { 'http://geo.example.com' }
before do
allow(Gitlab::Geo).to receive(:secondary?) { true }
allow(Gitlab::Geo).to receive_message_chain(:primary_node, :url) { geo_url }
end
it { should eq "#{geo_url}#{avatar_path}" }
end
end
end
describe '#requires_ldap_check?' do describe '#requires_ldap_check?' do
let(:user) { User.new } let(:user) { User.new }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment