From c62119b58f8bcc96d7d3296e70cc73c9bb52f1bc Mon Sep 17 00:00:00 2001 From: Emmanuel Hayford Date: Fri, 24 May 2019 23:59:10 +0200 Subject: [PATCH] Tiny refactor (#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 | 21 +++++-------------- app/models/image.rb | 34 +++++++++---------------------- test/jobs/digest_mail_job_test.rb | 7 ------- test/models/like_test.rb | 7 ------- test/performance/browsing_test.rb | 12 ----------- test/unit/answer_mailer_test.rb | 23 +++++++++++++-------- 6 files changed, 30 insertions(+), 74 deletions(-) delete mode 100644 test/jobs/digest_mail_job_test.rb delete mode 100644 test/models/like_test.rb delete mode 100644 test/performance/browsing_test.rb diff --git a/.rubocop_shopify_styleguide.yml b/.rubocop_shopify_styleguide.yml index 2550bcdf02..e11b5478c3 100644 --- a/.rubocop_shopify_styleguide.yml +++ b/.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: diff --git a/app/models/image.rb b/app/models/image.rb index a278d1f922..371aac1dc4 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -5,36 +5,27 @@ 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 @@ -42,12 +33,11 @@ def filetype 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 diff --git a/test/jobs/digest_mail_job_test.rb b/test/jobs/digest_mail_job_test.rb deleted file mode 100644 index d251f60445..0000000000 --- a/test/jobs/digest_mail_job_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class DigestMailJobTest < ActiveJob::TestCase - # test "the truth" do - # assert true - # end -end diff --git a/test/models/like_test.rb b/test/models/like_test.rb deleted file mode 100644 index 1eea9915b6..0000000000 --- a/test/models/like_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class LikeTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end -end diff --git a/test/performance/browsing_test.rb b/test/performance/browsing_test.rb deleted file mode 100644 index b249533246..0000000000 --- a/test/performance/browsing_test.rb +++ /dev/null @@ -1,12 +0,0 @@ -#require 'test_helper' -#require 'rails/performance_test_help' - -#class BrowsingTest < ActionDispatch::PerformanceTest - # Refer to the documentation for all available options - # self.profile_options = { :runs => 5, :metrics => [:wall_time, :memory] - # :output => 'tmp/performance', :formats => [:flat] } - -# def test_homepage -# get '/' -# end -#end diff --git a/test/unit/answer_mailer_test.rb b/test/unit/answer_mailer_test.rb index 7d40811de5..45e609cf67 100644 --- a/test/unit/answer_mailer_test.rb +++ b/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! #{ answer.author.name } responded : -

#{ answer.content }

") + 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! #{answer.author.name} responded : +

#{answer.content}

") 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 '#{answer.node.title}' 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 #{answer.node.title} 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 #{answer.node.title} 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