From b52ceb2c786dc54cfcda1c62596ba17783a6ef5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 24 Jul 2021 02:20:59 +0200 Subject: [PATCH] Move attachable methods from helpers to models We were using helper methods inside the model; we might as well include them in the model and use them from anywhere else. Note we're using a different logic for images and documents methods. That's because for images the logic was defined in the helper methods, but for documents the logic is defined in the Documentable concern. In the past, different documentable classes allowed different content types, while imageable classes have always allowed the same content types. I'm not sure which method is better; for now, I'm leaving it the way it was (except for the fact that we're removing the helper methods). --- app/components/documents/nested_component.rb | 3 +- app/components/images/nested_component.rb | 5 ++-- app/helpers/documentables_helper.rb | 9 ------ app/helpers/imageables_helper.rb | 12 -------- app/models/document.rb | 23 ++++++++++----- app/models/image.rb | 31 +++++++++++++++----- spec/shared/models/document_validations.rb | 6 ++-- spec/shared/models/image_validations.rb | 4 +-- 8 files changed, 46 insertions(+), 47 deletions(-) delete mode 100644 app/helpers/documentables_helper.rb diff --git a/app/components/documents/nested_component.rb b/app/components/documents/nested_component.rb index 50acc9b5c..2b4f6f909 100644 --- a/app/components/documents/nested_component.rb +++ b/app/components/documents/nested_component.rb @@ -1,6 +1,5 @@ class Documents::NestedComponent < ApplicationComponent attr_reader :f - delegate :documentable_humanized_accepted_content_types, to: :helpers def initialize(f) @f = f @@ -18,7 +17,7 @@ class Documents::NestedComponent < ApplicationComponent def note t "documents.form.note", max_documents_allowed: max_documents_allowed, - accepted_content_types: documentable_humanized_accepted_content_types(documentable.class), + accepted_content_types: Document.humanized_accepted_content_types, max_file_size: documentable.class.max_file_size end diff --git a/app/components/images/nested_component.rb b/app/components/images/nested_component.rb index 61087bcff..768f4d880 100644 --- a/app/components/images/nested_component.rb +++ b/app/components/images/nested_component.rb @@ -1,6 +1,5 @@ class Images::NestedComponent < ApplicationComponent attr_reader :f, :image_fields - delegate :imageable_humanized_accepted_content_types, :imageable_max_file_size, to: :helpers def initialize(f, image_fields: :image) @f = f @@ -14,7 +13,7 @@ class Images::NestedComponent < ApplicationComponent end def note - t "images.form.note", accepted_content_types: imageable_humanized_accepted_content_types, - max_file_size: imageable_max_file_size + t "images.form.note", accepted_content_types: Image.humanized_accepted_content_types, + max_file_size: Image.max_file_size end end diff --git a/app/helpers/documentables_helper.rb b/app/helpers/documentables_helper.rb deleted file mode 100644 index 2f75221b6..000000000 --- a/app/helpers/documentables_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -module DocumentablesHelper - def accepted_content_types(documentable_class) - documentable_class.accepted_content_types - end - - def documentable_humanized_accepted_content_types(documentable_class) - Setting.accepted_content_types_for("documents").join(", ") - end -end diff --git a/app/helpers/imageables_helper.rb b/app/helpers/imageables_helper.rb index c0e4c4893..930342025 100644 --- a/app/helpers/imageables_helper.rb +++ b/app/helpers/imageables_helper.rb @@ -2,16 +2,4 @@ module ImageablesHelper def can_destroy_image?(imageable) imageable.image.present? && can?(:destroy, imageable.image) end - - def imageable_max_file_size - Setting["uploads.images.max_size"].to_i - end - - def imageable_accepted_content_types - Setting["uploads.images.content_types"]&.split(" ") || ["image/jpeg"] - end - - def imageable_humanized_accepted_content_types - Setting.accepted_content_types_for("images").join(", ") - end end diff --git a/app/models/document.rb b/app/models/document.rb index bcc189c3a..f610ba61c 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -1,5 +1,4 @@ class Document < ApplicationRecord - include DocumentablesHelper has_attached_file :attachment, url: "/system/:class/:prefix/:style/:hash.:extension", hash_data: ":class/:style/:custom_hash_data", use_timestamp: false, @@ -48,6 +47,10 @@ class Document < ApplicationRecord attachment.instance.custom_hash_data(attachment) end + def self.humanized_accepted_content_types + Setting.accepted_content_types_for("documents").join(", ") + end + def prefix(attachment, _style) if attachment.instance.persisted? ":attachment/:id_partition" @@ -69,6 +72,14 @@ class Document < ApplicationRecord attachment_content_type.split("/").last.upcase end + def max_file_size + documentable_class.max_file_size + end + + def accepted_content_types + documentable_class.accepted_content_types + end + private def documentable_class @@ -77,20 +88,18 @@ class Document < ApplicationRecord def validate_attachment_size if documentable_class.present? && - attachment_file_size > documentable_class.max_file_size.megabytes + attachment_file_size > max_file_size.megabytes errors.add(:attachment, I18n.t("documents.errors.messages.in_between", min: "0 Bytes", - max: "#{documentable_class.max_file_size} MB")) + max: "#{max_file_size} MB")) end end def validate_attachment_content_type - if documentable_class && - !accepted_content_types(documentable_class).include?(attachment_content_type) - accepted_content_types = documentable_humanized_accepted_content_types(documentable_class) + if documentable_class && !accepted_content_types.include?(attachment_content_type) message = I18n.t("documents.errors.messages.wrong_content_type", content_type: attachment_content_type, - accepted_content_types: accepted_content_types) + accepted_content_types: self.class.humanized_accepted_content_types) errors.add(:attachment, message) end end diff --git a/app/models/image.rb b/app/models/image.rb index c944e292c..781b51da3 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -1,6 +1,4 @@ class Image < ApplicationRecord - include ImageablesHelper - has_attached_file :attachment, styles: { large: "x#{Setting["uploads.images.min_height"]}", medium: "300x300#", @@ -30,6 +28,26 @@ class Image < ApplicationRecord before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } + def self.max_file_size + Setting["uploads.images.max_size"].to_i + end + + def self.accepted_content_types + Setting["uploads.images.content_types"]&.split(" ") || ["image/jpeg"] + end + + def self.humanized_accepted_content_types + Setting.accepted_content_types_for("images").join(", ") + end + + def max_file_size + self.class.max_file_size + end + + def accepted_content_types + self.class.accepted_content_types + end + def set_cached_attachment_from_attachment self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem attachment.path @@ -77,11 +95,10 @@ class Image < ApplicationRecord end def validate_attachment_size - if imageable_class && - attachment_file_size > Setting["uploads.images.max_size"].to_i.megabytes + if imageable_class && attachment_file_size > max_file_size.megabytes errors.add(:attachment, I18n.t("images.errors.messages.in_between", min: "0 Bytes", - max: "#{imageable_max_file_size} MB")) + max: "#{max_file_size} MB")) end end @@ -104,7 +121,7 @@ class Image < ApplicationRecord if imageable_class && !attachment_of_valid_content_type? message = I18n.t("images.errors.messages.wrong_content_type", content_type: attachment_content_type, - accepted_content_types: imageable_humanized_accepted_content_types) + accepted_content_types: self.class.humanized_accepted_content_types) errors.add(:attachment, message) end end @@ -116,6 +133,6 @@ class Image < ApplicationRecord end def attachment_of_valid_content_type? - attachment.present? && imageable_accepted_content_types.include?(attachment_content_type) + attachment.present? && accepted_content_types.include?(attachment_content_type) end end diff --git a/spec/shared/models/document_validations.rb b/spec/shared/models/document_validations.rb index 0a3356111..194c8fbba 100644 --- a/spec/shared/models/document_validations.rb +++ b/spec/shared/models/document_validations.rb @@ -1,9 +1,7 @@ shared_examples "document validations" do |documentable_factory| - include DocumentablesHelper - let!(:document) { build(:document, documentable_factory.to_sym) } - let!(:maxfilesize) { document.documentable.class.max_file_size } - let!(:acceptedcontenttypes) { accepted_content_types(document.documentable.class) } + let!(:maxfilesize) { document.max_file_size } + let!(:acceptedcontenttypes) { document.accepted_content_types } it "is valid" do expect(document).to be_valid diff --git a/spec/shared/models/image_validations.rb b/spec/shared/models/image_validations.rb index 27cc26da8..092372550 100644 --- a/spec/shared/models/image_validations.rb +++ b/spec/shared/models/image_validations.rb @@ -1,8 +1,6 @@ shared_examples "image validations" do |imageable_factory| - include ImageablesHelper - let!(:image) { build(:image, imageable_factory.to_sym) } - let!(:acceptedcontenttypes) { imageable_accepted_content_types } + let!(:acceptedcontenttypes) { Image.accepted_content_types } it "is valid" do expect(image).to be_valid