Tiny refactor (publiclab#5779)

* Tiny refactor

* Put mysql gem back to where it belongs

* Put msql gem back to where it belongs

* Fix old testing review PR

* Remove unused file [skip ci]

* Remove unused folder [skip ci]

* Update configs

.rubocop_shopify_styleguide.yml

@@ -157,7 +157,7 @@ Naming/FileName:
   Regex:
   IgnoreExecutableScripts: true
 
-Layout/FirstParameterIndentation:
+Layout/IndentFirstArgument:
   EnforcedStyle: consistent
   SupportedStyles:
   - consistent
@@ -178,17 +178,6 @@ Style/FormatString:
   - sprintf
   - percent
 
-Style/FrozenStringLiteralComment:
-  Details: >-
-    Add `# frozen_string_literal: true` to the top of the file. Frozen string
-    literals will become the default in a future Ruby version, and we want to
-    make sure we're ready.
-  EnforcedStyle: when_needed
-  SupportedStyles:
-    - when_needed
-    - always
-    - never
-
 Style/HashSyntax:
   EnforcedStyle: ruby19
   SupportedStyles:
@@ -208,7 +197,7 @@ Layout/IndentationConsistency:
 Layout/IndentationWidth:
   Width: 2
 
-Layout/IndentArray:
+Layout/IndentFirstArrayElement:
   EnforcedStyle: consistent
   SupportedStyles:
   - special_inside_parentheses
@@ -219,7 +208,7 @@ Layout/IndentArray:
 Layout/IndentAssignment:
   IndentationWidth:
 
-Layout/IndentHash:
+Layout/IndentFirstHashElement:
   EnforcedStyle: consistent
   SupportedStyles:
   - special_inside_parentheses
@@ -1048,13 +1037,13 @@ Performance/RedundantBlockCall:
 Performance/RedundantMatch:
   Enabled: true
 
-Performance/RedundantSortBy:
+Style/RedundantSortBy:
   Enabled: true
 
 Performance/ReverseEach:
   Enabled: true
 
-Performance/Sample:
+Style/Sample:
   Enabled: true
 
 Performance/Size:

app/models/image.rb

@@ -5,49 +5,39 @@ class Image < ApplicationRecord
   belongs_to :node, foreign_key: :nid
 
   has_attached_file :photo, styles: { thumb: '200x150>', medium: '500x375>', large: '800x600>' } # ,
-  #:url  => "/system/images/photos/:id/:style/:basename.:extension",
-  #:path => ":rails_root/public/system/images/photos/:id/:style/:basename.:extension"
 
   validates :uid, presence: true
   validates :photo, presence: true, unless: :remote_url_provided?
+  validates :remote_url, presence: true, if: :remote_url_provided?
+
   do_not_validate_attachment_file_type :photo_file_name
-  # disabling type validation as we support many more such as PDF, SVG, see /app/views/editor/rich.html.erb#L232
-  # validates_attachment_content_type :photo, :content_type => ['image/jpeg', 'image/png', 'image/jpg', 'image/gif']
-  # validates_attachment_content_type :photo_file_name, :content_type => %w(image/jpeg image/jpg image/png)
-  # validates :title, :presence => true, :format => {:with => /\A[a-zA-Z0-9\ -_]+\z/, :message => "Only letters, numbers, and spaces allowed"}, :length => { :maximum => 60 }
 
   before_validation :download_remote_image, if: :remote_url_provided?
-  validates :remote_url, presence: true, if: :remote_url_provided? # , :message => "is invalid or inaccessible" # this message thing is old-style rails 2.3.x
+
   before_post_process :is_image?, :skip_large_gifs
 
   def is_image?
-    (filetype == 'jpg' || filetype == 'jpeg' || filetype == 'gif' || filetype == 'png')
+    %w(jpg jpeg gif png).include?(filetype)
   end
 
   def skip_large_gifs
-    ! is_big_gif
-  end
-
-  def is_big_gif
-    (filetype == 'gif' && photo_file_size.to_i > 3.megabytes)
+    !(filetype == 'gif' && photo_file_size.to_i > 3.megabytes)
   end
 
   def filetype
     if remote_url_provided? && remote_url[0..9] == "data:image"
-      # data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==
       remote_url.split(';').first.split('/').last.downcase
     else
       filename.split('.').last.downcase
     end
   end
 
   def path(size = :medium)
-    size = :original if is_big_gif
-    if is_image?
-      size = :medium if size == :default
-    else
-      size = :original
-    end
+    size = if is_image? && size == :default
+             :medium
+           else
+             :original
+           end
     photo.url(size)
   end
 
@@ -65,8 +55,6 @@ def absolute_uri
     Rails.env.production? ? 'https://publiclab.org' : ''
   end
 
-  # all subsequent code from http://trevorturk.com/2008/12/11/easy-upload-via-url-with-paperclip/
-
   def remote_url_provided?
     !remote_url.blank?
   end
@@ -81,8 +69,6 @@ def do_download_remote_image
 
     io.blank? ? nil : io
   rescue StandardError
-    # Let's sign up with Rollbar's free service to get insights on
-    # on what errors we get so we can address them accordingly
     raise
   end
 end

test/unit/answer_mailer_test.rb

@@ -12,9 +12,9 @@ class AnswerMailerTest < ActionMailer::TestCase
     email = ActionMailer::Base.deliveries.last
     assert_equal ["notifications@#{request_host}"], email.from
     assert_equal [user.email], email.to
-    assert_equal '[PLab] Question: ' + answer.node.title.truncate(30,omission: '...?') + ' An answer has been posted on Public Lab' + + " (#a#{answer.id})", email.subject
-    assert email.body.include?("Hi! <a href='https://#{request_host}/profile/#{ answer.author.name }'>#{ answer.author.name }</a> responded :
-<hr /><p>#{ answer.content }</p><hr />")
+    assert_equal '[PLab] Question: ' + answer.node.title.truncate(30, omission: '...?') + ' An answer has been posted on Public Lab' + + " (#a#{answer.id})", email.subject
+    assert email.body.include?("Hi! <a href='https://#{request_host}/profile/#{answer.author.name}'>#{answer.author.name}</a> responded :
+<hr /><p>#{answer.content}</p><hr />")
   end
 
   test 'notify other answer authors' do
@@ -43,32 +43,39 @@ class AnswerMailerTest < ActionMailer::TestCase
     email = ActionMailer::Base.deliveries.last
     assert_equal ["notifications@#{request_host}"], email.from
     assert_equal [user.email], email.to
-    assert_equal "[PublicLab] New answer to Question: " + answer.node.title + " (#a#{answer.id})" , email.subject
+    assert_equal "[PublicLab] New answer to Question: " + answer.node.title + " (#a#{answer.id})", email.subject
     assert email.body.include?("Hi! There's been a new answer posted for the question '<a href='https://#{request_host}#{answer.node.path(:question)}'>#{answer.node.title}</a>' that you liked")
   end
 
   test 'notify answer author when answer is accepted' do
     user = users(:bob)
     answer = answers(:one)
+
     assert_difference 'ActionMailer::Base.deliveries.size', 1 do
       AnswerMailer.notify_answer_accept(user, answer).deliver_now
     end
-    assert !ActionMailer::Base.deliveries.empty?
+
+    assert_not ActionMailer::Base.deliveries.empty?
 
     email = ActionMailer::Base.deliveries.last
+    answer_accepted = '[PublicLab] Your answer has been accepted'
+    answer_for_question = "Your answer for the question <a href='https://#{request_host}#{answer.node.path(:question)}'>#{answer.node.title}</a> has been accepted"
+
     assert_equal ["notifications@#{request_host}"], email.from
     assert_equal [user.email], email.to
-    assert_equal '[PublicLab] Your answer has been accepted', email.subject
-    assert email.body.include?("Your answer for the question <a href='https://#{request_host}#{answer.node.path(:question)}'>#{answer.node.title}</a> has been accepted")
+    assert_equal answer_accepted, email.subject
+    assert email.body.include?(answer_for_question)
   end
 
   test 'notify answer author when answer is liked' do
     user = users(:bob)
     answer = answers(:one)
+
     assert_difference 'ActionMailer::Base.deliveries.size', 1 do
       AnswerMailer.notify_answer_like(user, answer).deliver_now
     end
-    assert !ActionMailer::Base.deliveries.empty?
+
+    assert_not ActionMailer::Base.deliveries.empty?
 
     email = ActionMailer::Base.deliveries.last
     assert_equal ["notifications@#{request_host}"], email.from