From 395ce20fe69b5c531946e96363da10a13da7d579 Mon Sep 17 00:00:00 2001 From: Shane Murnaghan Date: Mon, 2 Mar 2020 10:45:32 -0700 Subject: [PATCH 1/2] Rename methods to be more informative, fix bug with double rendering page image url --- app/helpers/page_layout_helper.rb | 6 ++--- app/models/community.rb | 2 +- app/models/concerns/draft_properties.rb | 2 +- app/models/jupiter_core/depositable.rb | 2 +- app/views/application/_feature_image.html.erb | 4 ++-- app/views/application/_thumbnail.html.erb | 4 ++-- app/views/layouts/application.html.erb | 4 ++-- test/helpers/page_layout_helper_test.rb | 24 +++++++++++++------ 8 files changed, 29 insertions(+), 19 deletions(-) diff --git a/app/helpers/page_layout_helper.rb b/app/helpers/page_layout_helper.rb index d1dcb853e..b7abbced7 100644 --- a/app/helpers/page_layout_helper.rb +++ b/app/helpers/page_layout_helper.rb @@ -45,12 +45,12 @@ def page_description(description = nil) # rubocop:enable Rails/HelperInstanceVariable # rubocop:disable Rails/HelperInstanceVariable - def page_image + def page_image_url default_url = image_url('era-logo.png') # We only have images on community and item/thesis show pages - image_url = @community&.thumbnail_url || @item&.thumbnail_url + image_path = @community&.thumbnail_path || @item&.thumbnail_path - image_url || default_url + image_path ? request.base_url + image_path : default_url end # rubocop:enable Rails/HelperInstanceVariable diff --git a/app/models/community.rb b/app/models/community.rb index c078b3087..be0c4b1ca 100644 --- a/app/models/community.rb +++ b/app/models/community.rb @@ -51,7 +51,7 @@ def remove_logo=(val) end # compatibility with item thumbnail API - def thumbnail_url(args = { resize: '100x100', auto_orient: true }) + def thumbnail_path(args = { resize: '100x100', auto_orient: true }) return nil if logo_attachment.blank? Rails.application.routes.url_helpers.rails_representation_path(logo_attachment.variant(args).processed) diff --git a/app/models/concerns/draft_properties.rb b/app/models/concerns/draft_properties.rb index 606e53dcb..f74c4d653 100644 --- a/app/models/concerns/draft_properties.rb +++ b/app/models/concerns/draft_properties.rb @@ -46,7 +46,7 @@ def thumbnail end # compatibility with the thumbnail API used in Items/Theses and Communities - def thumbnail_url(args = { resize: '100x100', auto_orient: true }) + def thumbnail_path(args = { resize: '100x100', auto_orient: true }) return nil unless thumbnail.present? && thumbnail.blob.present? Rails.application.routes.url_helpers.rails_representation_path(thumbnail.variant(args).processed) diff --git a/app/models/jupiter_core/depositable.rb b/app/models/jupiter_core/depositable.rb index 774a35603..5d03476bb 100644 --- a/app/models/jupiter_core/depositable.rb +++ b/app/models/jupiter_core/depositable.rb @@ -104,7 +104,7 @@ def set_thumbnail(attachment) end # rubocop:enable Naming/AccessorMethodName - def thumbnail_url(args = { resize: '100x100', auto_orient: true }) + def thumbnail_path(args = { resize: '100x100', auto_orient: true }) logo = files.find_by(id: logo_id) return nil if logo.blank? diff --git a/app/views/application/_feature_image.html.erb b/app/views/application/_feature_image.html.erb index c18bf8dca..492b5be40 100644 --- a/app/views/application/_feature_image.html.erb +++ b/app/views/application/_feature_image.html.erb @@ -1,6 +1,6 @@ <% if policy(object).thumbnail? %> - <% if object.thumbnail_url.present? %> - <%= safe_thumbnail_tag(object.thumbnail_url(resize:'350x350', auto_orient: true), alt: '', size: '350x350') %> + <% if object.thumbnail_path.present? %> + <%= safe_thumbnail_tag(object.thumbnail_path(resize:'350x350', auto_orient: true), alt: '', size: '350x350') %> <% else %>
<%= fa_icon file_icon(object.thumbnail_file&.content_type), class: 'fa-5x text-muted' %> diff --git a/app/views/application/_thumbnail.html.erb b/app/views/application/_thumbnail.html.erb index 07426b856..a26f3b95a 100644 --- a/app/views/application/_thumbnail.html.erb +++ b/app/views/application/_thumbnail.html.erb @@ -1,6 +1,6 @@ <% if policy(object).thumbnail? %> - <% if object.thumbnail_url.present? %> - <%= safe_thumbnail_tag(object.thumbnail_url, alt: '', title: object.title, size: '100x100') %> + <% if object.thumbnail_path.present? %> + <%= safe_thumbnail_tag(object.thumbnail_path, alt: '', title: object.title, size: '100x100') %> <% else %> <% if object.is_a? Community %> <%= image_tag('era-logo-without-text.png', class: 'j-thumbnail img-thumbnail', alt: '', title: object.title, size: '100x100') %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 2e8e2a6fa..16f10cb6d 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -26,7 +26,7 @@ - + <%# Twitter Card - https://dev.twitter.com/cards/types/summary %> @@ -34,7 +34,7 @@ - + <%= yield(:extra_social_meta) %> diff --git a/test/helpers/page_layout_helper_test.rb b/test/helpers/page_layout_helper_test.rb index 0e0b7ded2..23292de68 100644 --- a/test/helpers/page_layout_helper_test.rb +++ b/test/helpers/page_layout_helper_test.rb @@ -2,6 +2,16 @@ class PageLayoutHelperTest < ActionView::TestCase + attr_reader :request + + def setup + @request = Class.new do + def base_url + 'https://example.com' + end + end.new + end + # page_title test 'should return the page title when given one' do @@ -62,25 +72,25 @@ class PageLayoutHelperTest < ActionView::TestCase assert_equal 'Bold Text: & "Header"', page_description end - # page_image + # page_image_url - test 'page_image defaults to the jupiter logo' do - assert_equal image_url('era-logo.png'), page_image + test 'page_image_url defaults to the jupiter logo' do + assert_equal image_url('era-logo.png'), page_image_url end - test 'page_image should return default image on community/item with no logo' do + test 'page_image_url should return default image on community/item with no logo' do @community = Community.create!(title: 'Random community', owner_id: users(:admin).id) - assert_equal image_url('era-logo.png'), page_image + assert_equal image_url('era-logo.png'), page_image_url end - test 'page_image should return community/item logo' do + test 'page_image_url should return community/item logo' do @community = Community.create!(title: 'Random community', owner_id: users(:admin).id) @community.logo.attach io: File.open(file_fixture('image-sample.jpeg')), filename: 'image-sample.jpeg', content_type: 'image/jpeg' - assert_equal page_image, @community.thumbnail_url + assert_equal page_image_url, request.base_url + @community.thumbnail_path end test 'canonical_href is returning the correct canoncial url' do From bbd22ce38e469df2f15a3c37776f4f33cc8701be Mon Sep 17 00:00:00 2001 From: Shane Murnaghan Date: Mon, 2 Mar 2020 10:53:42 -0700 Subject: [PATCH 2/2] Add changelog entry for page_image_url fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7466501bc..8d20caa5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and releases in Jupiter project adheres to [Semantic Versioning](http://semver.o ### Fixed - failing tests [#1376](https://github.com/ualbertalib/jupiter/issues/1376) - Fix Sprockets v4.0.0 upgrade problem with how Sass Variables were being defined +- Fix bug for page_image_url helper which was double rendering urls for default image [PR#1512](https://github.com/ualbertalib/jupiter/pull/1512) ## [1.2.18] - 2019-10-22 - Removed Rack Attack