diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2018-02-14 12:59:08 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-02-14 12:59:08 +0000 |
commit | 2e8d366da713ea6c8c3d1626c3138fbf2298d28e (patch) | |
tree | 88ae8b7ab3813e052bd404272fb94dc0a031d000 | |
parent | 48c505ebba37f5b673215160040ac66770ce037e (diff) | |
parent | dd1d13b859c4c4cd7b6a64eb93f761c10c9262d4 (diff) | |
download | gitlab-ce-2e8d366da713ea6c8c3d1626c3138fbf2298d28e.tar.gz |
Merge branch '42800-change-usage-of-avatar_icon' into 'master'
Change all occurrences of ApplicationHelper#avatar_icon to use a User object where possible
Closes #42800
See merge request gitlab-org/gitlab-ce!16976
35 files changed, 127 insertions, 59 deletions
diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 98831f5be4a..83245aadf6e 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -21,7 +21,7 @@ class IssuesFinder < IssuableFinder CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER def klass - Issue + Issue.includes(:author) end def with_confidentiality_access_check diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 12157818bcd..33ee1e975b9 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -57,7 +57,7 @@ class NotesFinder types = %w(commit issue merge_request snippet) note_relations = types.map { |t| notes_for_type(t) } note_relations.map! { |notes| search(notes) } - UnionFinder.new.find_union(note_relations, Note) + UnionFinder.new.find_union(note_relations, Note.includes(:author)) end def noteables_for_type(noteable_type) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6530327698b..a6011eb9f30 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -68,18 +68,32 @@ module ApplicationHelper end end - def avatar_icon(user_or_email = nil, size = nil, scale = 2, only_path: true) - user = - if user_or_email.is_a?(User) - user_or_email - else - User.find_by_any_email(user_or_email.try(:downcase)) - end + # Takes both user and email and returns the avatar_icon by + # user (preferred) or email. + def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true) + if user + avatar_icon_for_user(user, size, scale, only_path: only_path) + elsif email + avatar_icon_for_email(email, size, scale, only_path: only_path) + else + default_avatar + end + end + + def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true) + user = User.find_by_any_email(email.try(:downcase)) + if user + avatar_icon_for_user(user, size, scale, only_path: only_path) + else + gravatar_icon(email, size, scale) + end + end + def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true) if user user.avatar_url(size: size, only_path: only_path) || default_avatar else - gravatar_icon(user_or_email, size, scale) + gravatar_icon(nil, size, scale) end end diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index be11d453898..21b6c0a8ad5 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -8,10 +8,22 @@ module AvatarsHelper })) end + def user_avatar_url_for(options = {}) + if options[:url] + options[:url] + elsif options[:user] + avatar_icon_for_user(options[:user], options[:size]) + else + avatar_icon_for_email(options[:user_email], options[:size]) + end + end + def user_avatar_without_link(options = {}) avatar_size = options[:size] || 16 user_name = options[:user].try(:name) || options[:user_name] - avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size) + + avatar_url = user_avatar_url_for(options.merge(size: avatar_size)) + has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip] data_attributes = options[:data] || {} css_class = %W[avatar s#{avatar_size}].push(*options[:css_class]) diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index b78d3072186..40ca666f1bf 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -33,7 +33,7 @@ module NamespacesHelper if namespace.is_a?(Group) group_icon(namespace) else - avatar_icon(namespace.owner.email, size) + avatar_icon_for_user(namespace.owner, size) end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 43c8b1b612b..b97b72d62c3 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -19,7 +19,7 @@ module ProjectsHelper classes = %W[avatar avatar-inline s#{opts[:size]}] classes << opts[:avatar_class] if opts[:avatar_class] - avatar = avatar_icon(author, opts[:size]) + avatar = avatar_icon_for_user(author, opts[:size]) src = opts[:lazy_load] ? nil : avatar image_tag(src, width: opts[:size], class: classes, alt: '', "data-src" => avatar) diff --git a/app/views/admin/users/_user.html.haml b/app/views/admin/users/_user.html.haml index 3b85586e45b..bbfeceff5b9 100644 --- a/app/views/admin/users/_user.html.haml +++ b/app/views/admin/users/_user.html.haml @@ -1,6 +1,6 @@ %li.flex-row .user-avatar - = image_tag avatar_icon(user), class: "avatar", alt: '' + = image_tag avatar_icon_for_user(user), class: "avatar", alt: '' .row-main-content .user-name.row-title.str-truncated-100 = link_to user.name, [:admin, user] diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 101667508a9..ec3be869797 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -10,7 +10,7 @@ = @user.name %ul.well-list %li - = image_tag avatar_icon(@user, 60), class: "avatar s60" + = image_tag avatar_icon_for_user(@user, 60), class: "avatar s60" %li %span.light Profile page: %strong diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 205320ed87c..8b9fa3d6b05 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -3,7 +3,7 @@ .timeline-entry-inner .timeline-icon = link_to user_path(discussion.author) do - = image_tag avatar_icon(discussion.author), class: "avatar s40" + = image_tag avatar_icon_for_user(discussion.author), class: "avatar s40" .timeline-content .discussion.js-toggle-container{ data: { discussion_id: discussion.id } } .discussion-header diff --git a/app/views/events/_event.atom.builder b/app/views/events/_event.atom.builder index 38741fe6662..d56234e6c1a 100644 --- a/app/views/events/_event.atom.builder +++ b/app/views/events/_event.atom.builder @@ -10,7 +10,7 @@ xml.entry do # eager-loaded. This allows us to re-use the user object's Email address, # instead of having to run additional queries to figure out what Email to use # for the avatar. - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(event.author)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(event.author)) xml.author do xml.username event.author_username diff --git a/app/views/help/ui.html.haml b/app/views/help/ui.html.haml index 445f0dffbcc..1c4d67a8d2c 100644 --- a/app/views/help/ui.html.haml +++ b/app/views/help/ui.html.haml @@ -68,7 +68,7 @@ .example .cover-block .avatar-holder - = image_tag avatar_icon('admin@example.com', 90), class: "avatar s90", alt: '' + = image_tag avatar_icon_for_email('admin@example.com', 90), class: "avatar s90", alt: '' .cover-title John Smith diff --git a/app/views/issues/_issue.atom.builder b/app/views/issues/_issue.atom.builder index 0c113c08526..21cf6d0dd65 100644 --- a/app/views/issues/_issue.atom.builder +++ b/app/views/issues/_issue.atom.builder @@ -3,7 +3,7 @@ xml.entry do xml.link href: project_issue_url(issue.project, issue) xml.title truncate(issue.title, length: 80) xml.updated issue.updated_at.xmlschema - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(issue.author_email)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(issue.author)) xml.author do xml.name issue.author_name diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index e7fc83a8d04..1d00ae928f6 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -45,7 +45,7 @@ = todos_count_format(todos_pending_count) %li.header-user.dropdown = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do - = image_tag avatar_icon(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" + = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" = sprite_icon('angle-down', css_class: 'caret-down') .dropdown-menu-nav.dropdown-menu-align-right %ul diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml index 8eb3f2d5192..38dab104eb5 100644 --- a/app/views/notify/pipeline_failed_email.html.haml +++ b/app/views/notify/pipeline_failed_email.html.haml @@ -60,7 +60,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.author %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" } @@ -76,7 +76,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.committer %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" } @@ -100,7 +100,7 @@ triggered by - if @pipeline.user %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" } - %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" } %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" } = @pipeline.user.name diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml index bae37292d62..7b06e8afa0b 100644 --- a/app/views/notify/pipeline_success_email.html.haml +++ b/app/views/notify/pipeline_success_email.html.haml @@ -60,7 +60,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.author %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" } @@ -76,7 +76,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.committer %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" } @@ -100,7 +100,7 @@ triggered by - if @pipeline.user %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" } - %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" } %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" } = @pipeline.user.name diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 5c76d2d8f51..110736dc557 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -20,8 +20,8 @@ or change it at #{link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host} .col-lg-8 .clearfix.avatar-image.append-bottom-default - = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do - = image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160' + = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do + = image_tag avatar_icon_for_user(@user, 160), alt: '', class: 'avatar s160' %h5.prepend-top-0= _("Upload new avatar") .prepend-top-5.append-bottom-10 %button.btn.js-choose-user-avatar-button{ type: 'button' }= _("Choose file...") diff --git a/app/views/projects/commits/_commit.atom.builder b/app/views/projects/commits/_commit.atom.builder index 04914888763..50f7e7a3a33 100644 --- a/app/views/projects/commits/_commit.atom.builder +++ b/app/views/projects/commits/_commit.atom.builder @@ -3,7 +3,7 @@ xml.entry do xml.link href: project_commit_url(@project, id: commit.id) xml.title truncate(commit.title, length: 80, escape: false) xml.updated commit.committed_date.xmlschema - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(commit.author_email)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_email(commit.author_email)) xml.author do |author| xml.name commit.author_name diff --git a/app/views/projects/jobs/_user.html.haml b/app/views/projects/jobs/_user.html.haml index 83f299da651..461d503f95d 100644 --- a/app/views/projects/jobs/_user.html.haml +++ b/app/views/projects/jobs/_user.html.haml @@ -1,7 +1,7 @@ by %a{ href: user_path(@build.user) } %span.hidden-xs - = image_tag avatar_icon(@build.user, 24), class: "avatar s24" + = image_tag avatar_icon_for_user(@build.user, 24), class: "avatar s24" %strong{ data: { toggle: 'tooltip', placement: 'top', title: @build.user.to_reference } } = @build.user.name %strong.visible-xs-inline= @build.user.to_reference diff --git a/app/views/projects/network/show.json.erb b/app/views/projects/network/show.json.erb index 7491b37310d..a0e82e891ff 100644 --- a/app/views/projects/network/show.json.erb +++ b/app/views/projects/network/show.json.erb @@ -9,7 +9,7 @@ author: { name: c.author_name, email: c.author_email, - icon: image_path(avatar_icon(c.author_email, 20)) + icon: image_path(avatar_icon_for_email(c.author_email, 20)) }, time: c.time, space: c.spaces.first, diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml index a8692b83b07..55d0e8bb7f9 100644 --- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml +++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml @@ -21,7 +21,7 @@ = s_("PipelineSchedules|Inactive") %td - if pipeline_schedule.owner - = image_tag avatar_icon(pipeline_schedule.owner, 20), class: "avatar s20" + = image_tag avatar_icon_for_user(pipeline_schedule.owner, 20), class: "avatar s20" = link_to user_path(pipeline_schedule.owner) do = pipeline_schedule.owner&.name %td diff --git a/app/views/projects/repositories/_feed.html.haml b/app/views/projects/repositories/_feed.html.haml index 170f9e259df..87895a15239 100644 --- a/app/views/projects/repositories/_feed.html.haml +++ b/app/views/projects/repositories/_feed.html.haml @@ -11,7 +11,7 @@ %div = link_to project_commits_path(@project, commit.id) do %code= commit.short_id - = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: '' + = image_tag avatar_icon_for_email(commit.author_email), class: "", width: 16, alt: '' = markdown(truncate(commit.title, length: 40), pipeline: :single_line, author: commit.author) %td %span.pull-right.cgray diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index c4a5131c1a7..57a0b64bfd5 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -7,7 +7,7 @@ = snippet.title by = link_to user_snippets_path(snippet.author) do - = image_tag avatar_icon(snippet.author), class: "avatar avatar-inline s16", alt: '' + = image_tag avatar_icon_for_user(snippet.author), class: "avatar avatar-inline s16", alt: '' = snippet.author_name %span.light= time_ago_with_tooltip(snippet.created_at) %h4.snippet-title diff --git a/app/views/search/results/_snippet_title.html.haml b/app/views/search/results/_snippet_title.html.haml index aef825691e0..65710c09a89 100644 --- a/app/views/search/results/_snippet_title.html.haml +++ b/app/views/search/results/_snippet_title.html.haml @@ -18,6 +18,6 @@ %span by = link_to user_snippets_path(snippet_title.author) do - = image_tag avatar_icon(snippet_title.author), class: "avatar avatar-inline s16", alt: '' + = image_tag avatar_icon_for_user(snippet_title.author), class: "avatar avatar-inline s16", alt: '' = snippet_title.author_name %span.light= time_ago_with_tooltip(snippet_title.created_at) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 71878e93255..ba57d922c6d 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -8,7 +8,7 @@ %li.member{ class: dom_class(member), id: dom_id(member) } %span.list-item-name - if user - = image_tag avatar_icon(user, 40), class: "avatar s40", alt: '' + = image_tag avatar_icon_for_user(user, 40), class: "avatar s40", alt: '' .user-info = link_to user.name, user_path(user), class: 'member' %span.cgray= user.to_reference @@ -36,7 +36,7 @@ Expires in #{distance_of_time_in_words_to_now(member.expires_at)} - else - = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: '' + = image_tag avatar_icon_for_email(member.invite_email, 40), class: "avatar s40", alt: '' .user-info .member= member.invite_email .cgray diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index 14395bcc661..479b7270b28 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -28,4 +28,4 @@ - assignees.each do |assignee| = link_to polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, assignee_id: assignee.id, state: 'all' }), class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do - - image_tag(avatar_icon(assignee, 16), class: "avatar s16", alt: '') + - image_tag(avatar_icon_for_user(assignee, 16), class: "avatar s16", alt: '') diff --git a/app/views/shared/milestones/_participants_tab.html.haml b/app/views/shared/milestones/_participants_tab.html.haml index 1615871385e..fe83040c168 100644 --- a/app/views/shared/milestones/_participants_tab.html.haml +++ b/app/views/shared/milestones/_participants_tab.html.haml @@ -2,7 +2,7 @@ - users.each do |user| %li = link_to user, title: user.name, class: "darken" do - = image_tag avatar_icon(user, 32), class: "avatar s32" + = image_tag avatar_icon_for_user(user, 32), class: "avatar s32" %strong= truncate(user.name, length: 40) %div %small.cgray= user.username diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 98e0161f7d1..bf359774ead 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -16,7 +16,7 @@ = icon_for_system_note(note) - else %a.image-diff-avatar-link{ href: user_path(note.author) } - = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40' + = image_tag avatar_icon_for_user(note.author), alt: '', class: 'avatar s40' - if note.is_a?(DiffNote) && note.on_image? - if show_image_comment_badge && note_counter == 0 -# Only show this for the first comment in the discussion diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml index e11f778adf5..b3f865c5b47 100644 --- a/app/views/shared/notes/_notes_with_form.html.haml +++ b/app/views/shared/notes/_notes_with_form.html.haml @@ -14,7 +14,7 @@ .timeline-icon.hidden-xs.hidden-sm %a.author_link{ href: user_path(current_user) } - = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' + = image_tag avatar_icon_for_user(current_user), alt: current_user.to_reference, class: 'avatar s40' .timeline-content.timeline-content-form = render "shared/notes/form", view: diff_view, supports_autocomplete: autocomplete - elsif !current_user diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 2a75b46d376..33435216c14 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -17,7 +17,7 @@ .avatar-container.s40 = link_to project_path(project), class: dom_class(project) do - if project.creator && use_creator_avatar - = image_tag avatar_icon(project.creator.email, 40), class: "avatar s40", alt:'' + = image_tag avatar_icon_for_user(project.creator, 40), class: "avatar s40", alt:'' - else = project_icon(project, alt: '', class: 'avatar project-avatar s40') .project-details diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml index 57b445321e2..491a8a41090 100644 --- a/app/views/shared/snippets/_snippet.html.haml +++ b/app/views/shared/snippets/_snippet.html.haml @@ -1,7 +1,7 @@ - link_project = local_assigns.fetch(:link_project, false) %li.snippet-row - = image_tag avatar_icon(snippet.author), class: "avatar s40 hidden-xs", alt: '' + = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 hidden-xs", alt: '' .title = link_to reliable_snippet_path(snippet) do diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 90aa1be30ac..c9a77d668a2 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -35,8 +35,8 @@ .profile-header .avatar-holder - = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do - = image_tag avatar_icon(@user, 90), class: "avatar s90", alt: '' + = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do + = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: '' .user-info .cover-title diff --git a/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml new file mode 100644 index 00000000000..00f4b7436a7 --- /dev/null +++ b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml @@ -0,0 +1,6 @@ +--- +title: Use a user object in ApplicationHelper#avatar_icon where possible to avoid + N+1 queries. +merge_request: 42800 +author: +type: performance diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index f7a4a7afced..43cb0dfe163 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -63,13 +63,29 @@ describe ApplicationHelper do end end - describe 'avatar_icon' do + describe 'avatar_icon_for' do + let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') } + let(:email) { 'foo@example.com' } + let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) } + + it 'prefers the user to retrieve the avatar_url' do + expect(helper.avatar_icon_for(user, email).to_s) + .to eq(user.avatar.url) + end + + it 'falls back to email lookup if no user given' do + expect(helper.avatar_icon_for(nil, email).to_s) + .to eq(another_user.avatar.url) + end + end + + describe 'avatar_icon_for_email' do let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } context 'using an email' do context 'when there is a matching user' do it 'returns a relative URL for the avatar' do - expect(helper.avatar_icon(user.email).to_s) + expect(helper.avatar_icon_for_email(user.email).to_s) .to eq(user.avatar.url) end end @@ -78,17 +94,37 @@ describe ApplicationHelper do it 'calls gravatar_icon' do expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2) - helper.avatar_icon('foo@example.com', 20, 2) + helper.avatar_icon_for_email('foo@example.com', 20, 2) + end + end + + context 'without an email passed' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(nil, 20, 2) + + helper.avatar_icon_for_email(nil, 20, 2) end end end + end + + describe 'avatar_icon_for_user' do + let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } - describe 'using a user' do + context 'with a user object passed' do it 'returns a relative URL for the avatar' do - expect(helper.avatar_icon(user).to_s) + expect(helper.avatar_icon_for_user(user).to_s) .to eq(user.avatar.url) end end + + context 'without a user object passed' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(nil, 20, 2) + + helper.avatar_icon_for_user(nil, 20, 2) + end + end end describe 'gravatar_icon' do diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index f44e7ef6843..04c6d259135 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -29,7 +29,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: 'avatar s16 has-tooltip', title: user.name @@ -43,7 +43,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 #{options[:css_class]} has-tooltip", title: user.name @@ -58,7 +58,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, options[:size]), + src: avatar_icon_for_user(user, options[:size]), data: { container: 'body' }, class: "avatar s#{options[:size]} has-tooltip", title: user.name @@ -89,7 +89,7 @@ describe AvatarsHelper do :img, alt: "#{user.name}'s avatar", src: LazyImageTagHelper.placeholder_image, - data: { container: 'body', src: avatar_icon(user, 16) }, + data: { container: 'body', src: avatar_icon_for_user(user, 16) }, class: "avatar s16 has-tooltip lazy", title: user.name ) @@ -104,7 +104,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: user.name @@ -119,7 +119,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), class: "avatar s16", title: user.name ) @@ -137,7 +137,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: user.name @@ -149,7 +149,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{options[:user_name]}'s avatar", - src: avatar_icon(options[:user_email], 16), + src: avatar_icon_for_email(options[:user_email], 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: options[:user_name] diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index c0251bf7dc0..b67fee2fcc0 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -215,7 +215,7 @@ describe ProjectsHelper do let(:expected) { double } before do - expect(helper).to receive(:avatar_icon).with(user, 16).and_return(expected) + expect(helper).to receive(:avatar_icon_for_user).with(user, 16).and_return(expected) end it 'returns image tag for member avatar' do |