Merge pull request #1512 from ualbertalib/fix-page-image-helper

Rename methods to be more informative, fix bug with double rendering …

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

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
 

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)

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)

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?
 

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 %>
     <div class="d-flex justify-content-center align-items-center img-thumbnail p-5">
       <%= fa_icon file_icon(object.thumbnail_file&.content_type), class: 'fa-5x text-muted' %>

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') %>

app/views/layouts/application.html.erb

@@ -26,15 +26,15 @@
     <meta property="og:locale"       content="en_US">
     <meta property="og:title"        content="<%= page_title %>">
     <meta property="og:description"  content="<%= page_description %>">
-    <meta property="og:image"        content="<%= request.base_url + page_image %>">
+    <meta property="og:image"        content="<%= page_image_url %>">
 
     <%#  Twitter Card - https://dev.twitter.com/cards/types/summary %>
     <meta name="twitter:card"        content="summary">
     <meta name="twitter:site"        content="@uofalibraries">
     <meta name="twitter:url"         content="<%= canonical_href %>">
     <meta name="twitter:title"       content="<%= page_title %>">
     <meta name="twitter:description" content="<%= page_description %>">
-    <meta name="twitter:image"       content="<%= request.base_url + page_image %>">
+    <meta name="twitter:image"       content="<%= page_image_url %>">
 
     <%= yield(:extra_social_meta) %>
 

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: &amp; "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