From d14f6691dc4e66ad1845aa2b9085086c37becb70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 23 Jul 2021 19:32:04 +0200 Subject: [PATCH 1/6] Return document max file size in megabytes The same way it's done for images. We were converting the number of megabytes to bytes and then converting it to megabytes again. Instead, we can leave it as it is and only convert it to bytes when necessary (only one place). --- app/components/documents/nested_component.rb | 4 ++-- app/helpers/documentables_helper.rb | 4 ---- app/models/concerns/documentable.rb | 2 +- app/models/document.rb | 4 ++-- spec/shared/models/document_validations.rb | 2 +- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/components/documents/nested_component.rb b/app/components/documents/nested_component.rb index a37587e81..50acc9b5c 100644 --- a/app/components/documents/nested_component.rb +++ b/app/components/documents/nested_component.rb @@ -1,6 +1,6 @@ class Documents::NestedComponent < ApplicationComponent attr_reader :f - delegate :documentable_humanized_accepted_content_types, :max_file_size, to: :helpers + delegate :documentable_humanized_accepted_content_types, to: :helpers def initialize(f) @f = f @@ -19,7 +19,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), - max_file_size: max_file_size(documentable.class) + max_file_size: documentable.class.max_file_size end def max_documents_allowed? diff --git a/app/helpers/documentables_helper.rb b/app/helpers/documentables_helper.rb index 5dcffacb3..2f75221b6 100644 --- a/app/helpers/documentables_helper.rb +++ b/app/helpers/documentables_helper.rb @@ -1,8 +1,4 @@ module DocumentablesHelper - def max_file_size(documentable_class) - documentable_class.max_file_size / Numeric::MEGABYTE - end - def accepted_content_types(documentable_class) documentable_class.accepted_content_types end diff --git a/app/models/concerns/documentable.rb b/app/models/concerns/documentable.rb index dcff3461d..dd4aa531a 100644 --- a/app/models/concerns/documentable.rb +++ b/app/models/concerns/documentable.rb @@ -12,7 +12,7 @@ module Documentable end def max_file_size - Setting["uploads.documents.max_size"].to_i.megabytes + Setting["uploads.documents.max_size"].to_i end def accepted_content_types diff --git a/app/models/document.rb b/app/models/document.rb index da9bbfd08..886367437 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -78,10 +78,10 @@ class Document < ApplicationRecord def validate_attachment_size if documentable_class.present? && - attachment_file_size > documentable_class.max_file_size + attachment_file_size > documentable_class.max_file_size.megabytes errors.add(:attachment, I18n.t("documents.errors.messages.in_between", min: "0 Bytes", - max: "#{max_file_size(documentable_class)} MB")) + max: "#{documentable_class.max_file_size} MB")) end end diff --git a/spec/shared/models/document_validations.rb b/spec/shared/models/document_validations.rb index 57daf7cd1..0a3356111 100644 --- a/spec/shared/models/document_validations.rb +++ b/spec/shared/models/document_validations.rb @@ -2,7 +2,7 @@ shared_examples "document validations" do |documentable_factory| include DocumentablesHelper let!(:document) { build(:document, documentable_factory.to_sym) } - let!(:maxfilesize) { max_file_size(document.documentable.class) } + let!(:maxfilesize) { document.documentable.class.max_file_size } let!(:acceptedcontenttypes) { accepted_content_types(document.documentable.class) } it "is valid" do From 4d8842c0d47c9822905e5d18bd212e2c7dc088fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 23 Jul 2021 19:56:30 +0200 Subject: [PATCH 2/6] Remove unneeded helpers inclusion We aren't using any methods from these helpers in these models. --- app/models/document.rb | 1 - app/models/image.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/app/models/document.rb b/app/models/document.rb index 886367437..bcc189c3a 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -1,5 +1,4 @@ class Document < ApplicationRecord - include DocumentsHelper include DocumentablesHelper has_attached_file :attachment, url: "/system/:class/:prefix/:style/:hash.:extension", hash_data: ":class/:style/:custom_hash_data", diff --git a/app/models/image.rb b/app/models/image.rb index 961307b23..c944e292c 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -1,5 +1,4 @@ class Image < ApplicationRecord - include ImagesHelper include ImageablesHelper has_attached_file :attachment, styles: { 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 3/6] 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 From e01940c1665718567328ac8cd1340908d6239fd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 24 Jul 2021 02:39:26 +0200 Subject: [PATCH 4/6] Move image/document attachments code to a concern This way we reduce some of the duplication in these classes. --- app/models/concerns/attachable.rb | 75 +++++++++++++++++++++++++++++++ app/models/document.rb | 69 +++------------------------- app/models/image.rb | 68 +++------------------------- 3 files changed, 89 insertions(+), 123 deletions(-) create mode 100644 app/models/concerns/attachable.rb diff --git a/app/models/concerns/attachable.rb b/app/models/concerns/attachable.rb new file mode 100644 index 000000000..fbc29a055 --- /dev/null +++ b/app/models/concerns/attachable.rb @@ -0,0 +1,75 @@ +module Attachable + extend ActiveSupport::Concern + + included do + attr_accessor :cached_attachment + + # Disable paperclip security validation due to polymorphic configuration + # Paperclip do not allow to use Procs on valiations definition + do_not_validate_attachment_file_type :attachment + validate :attachment_presence + validate :validate_attachment_content_type, if: -> { attachment.present? } + validate :validate_attachment_size, if: -> { attachment.present? } + + before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } + + Paperclip.interpolates :prefix do |attachment, style| + attachment.instance.prefix(attachment, style) + end + end + + def association_class + type = send("#{association_name}_type") + + type.constantize if type.present? + end + + def set_cached_attachment_from_attachment + self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem + attachment.path + else + attachment.url + end + end + + def set_attachment_from_cached_attachment + self.attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem + File.open(cached_attachment) + else + URI.parse(cached_attachment) + end + end + + def prefix(attachment, _style) + if attachment.instance.persisted? + ":attachment/:id_partition" + else + "cached_attachments/user/#{attachment.instance.user_id}" + end + end + + private + + def validate_attachment_size + if association_class && attachment_file_size > max_file_size.megabytes + errors.add(:attachment, I18n.t("#{model_name.plural}.errors.messages.in_between", + min: "0 Bytes", + max: "#{max_file_size} MB")) + end + end + + def validate_attachment_content_type + if association_class && !accepted_content_types.include?(attachment_content_type) + message = I18n.t("#{model_name.plural}.errors.messages.wrong_content_type", + content_type: attachment_content_type, + accepted_content_types: self.class.humanized_accepted_content_types) + errors.add(:attachment, message) + end + end + + def attachment_presence + if attachment.blank? && cached_attachment.blank? + errors.add(:attachment, I18n.t("errors.messages.blank")) + end + end +end diff --git a/app/models/document.rb b/app/models/document.rb index f610ba61c..cc24ef871 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -1,48 +1,21 @@ class Document < ApplicationRecord + include Attachable + has_attached_file :attachment, url: "/system/:class/:prefix/:style/:hash.:extension", hash_data: ":class/:style/:custom_hash_data", use_timestamp: false, hash_secret: Rails.application.secrets.secret_key_base - attr_accessor :cached_attachment belongs_to :user belongs_to :documentable, polymorphic: true - # Disable paperclip security validation due to polymorphic configuration - # Paperclip do not allow to use Procs on valiations definition - do_not_validate_attachment_file_type :attachment - validate :attachment_presence - validate :validate_attachment_content_type, if: -> { attachment.present? } - validate :validate_attachment_size, if: -> { attachment.present? } validates :title, presence: true validates :user_id, presence: true validates :documentable_id, presence: true, if: -> { persisted? } validates :documentable_type, presence: true, if: -> { persisted? } - before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } - scope :admin, -> { where(admin: true) } - def set_cached_attachment_from_attachment - self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem - attachment.path - else - attachment.url - end - end - - def set_attachment_from_cached_attachment - self.attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem - File.open(cached_attachment) - else - URI.parse(cached_attachment) - end - end - - Paperclip.interpolates :prefix do |attachment, style| - attachment.instance.prefix(attachment, style) - end - Paperclip.interpolates :custom_hash_data do |attachment, _style| attachment.instance.custom_hash_data(attachment) end @@ -51,14 +24,6 @@ class Document < ApplicationRecord Setting.accepted_content_types_for("documents").join(", ") end - def prefix(attachment, _style) - if attachment.instance.persisted? - ":attachment/:id_partition" - else - "cached_attachments/user/#{attachment.instance.user_id}" - end - end - def custom_hash_data(attachment) original_filename = if attachment.instance.persisted? attachment.instance.title @@ -82,31 +47,11 @@ class Document < ApplicationRecord private + def association_name + :documentable + end + def documentable_class - documentable_type.constantize if documentable_type.present? - end - - def validate_attachment_size - if documentable_class.present? && - attachment_file_size > max_file_size.megabytes - errors.add(:attachment, I18n.t("documents.errors.messages.in_between", - min: "0 Bytes", - max: "#{max_file_size} MB")) - end - end - - def validate_attachment_content_type - 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: self.class.humanized_accepted_content_types) - errors.add(:attachment, message) - end - end - - def attachment_presence - if attachment.blank? && cached_attachment.blank? - errors.add(:attachment, I18n.t("errors.messages.blank")) - end + association_class end end diff --git a/app/models/image.rb b/app/models/image.rb index 781b51da3..19825d629 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -1,4 +1,6 @@ class Image < ApplicationRecord + include Attachable + has_attached_file :attachment, styles: { large: "x#{Setting["uploads.images.min_height"]}", medium: "300x300#", @@ -8,17 +10,10 @@ class Image < ApplicationRecord hash_data: ":class/:style", use_timestamp: false, hash_secret: Rails.application.secrets.secret_key_base - attr_accessor :cached_attachment belongs_to :user belongs_to :imageable, polymorphic: true - # Disable paperclip security validation due to polymorphic configuration - # Paperclip do not allow to use Procs on valiations definition - do_not_validate_attachment_file_type :attachment - validate :attachment_presence - validate :validate_attachment_content_type, if: -> { attachment.present? } - validate :validate_attachment_size, if: -> { attachment.present? } validates :title, presence: true validate :validate_title_length validates :user_id, presence: true @@ -26,8 +21,6 @@ class Image < ApplicationRecord validates :imageable_type, presence: true, if: -> { persisted? } validate :validate_image_dimensions, if: -> { attachment.present? && attachment.dirty? } - before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } - def self.max_file_size Setting["uploads.images.max_size"].to_i end @@ -48,38 +41,14 @@ class Image < ApplicationRecord self.class.accepted_content_types end - def set_cached_attachment_from_attachment - self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem - attachment.path - else - attachment.url - end - end - - def set_attachment_from_cached_attachment - self.attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem - File.open(cached_attachment) - else - URI.parse(cached_attachment) - end - end - - Paperclip.interpolates :prefix do |attachment, style| - attachment.instance.prefix(attachment, style) - end - - def prefix(attachment, _style) - if attachment.instance.persisted? - ":attachment/:id_partition" - else - "cached_attachments/user/#{attachment.instance.user_id}" - end - end - private + def association_name + :imageable + end + def imageable_class - imageable_type.constantize if imageable_type.present? + association_class end def validate_image_dimensions @@ -94,14 +63,6 @@ class Image < ApplicationRecord end end - def validate_attachment_size - if imageable_class && attachment_file_size > max_file_size.megabytes - errors.add(:attachment, I18n.t("images.errors.messages.in_between", - min: "0 Bytes", - max: "#{max_file_size} MB")) - end - end - def validate_title_length if title.present? title_min_length = Setting["uploads.images.title.min_length"].to_i @@ -117,21 +78,6 @@ class Image < ApplicationRecord end end - def validate_attachment_content_type - 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: self.class.humanized_accepted_content_types) - errors.add(:attachment, message) - end - end - - def attachment_presence - if attachment.blank? && cached_attachment.blank? - errors.add(:attachment, I18n.t("errors.messages.blank")) - end - end - def attachment_of_valid_content_type? attachment.present? && accepted_content_types.include?(attachment_content_type) end From 507fa7c5f5ceb1af5b4d347ccf71f5632bfcbd50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 25 Jul 2021 00:20:57 +0200 Subject: [PATCH 5/6] Remove duplication in attachable fields components These classes were almost identical. --- .../attachable/fields_component.html.erb | 31 ++++++++++ app/components/attachable/fields_component.rb | 62 +++++++++++++++++++ .../documents/fields_component.html.erb | 34 ++-------- app/components/documents/fields_component.rb | 32 ---------- .../images/fields_component.html.erb | 35 ++--------- app/components/images/fields_component.rb | 44 ------------- 6 files changed, 105 insertions(+), 133 deletions(-) create mode 100644 app/components/attachable/fields_component.html.erb create mode 100644 app/components/attachable/fields_component.rb diff --git a/app/components/attachable/fields_component.html.erb b/app/components/attachable/fields_component.html.erb new file mode 100644 index 000000000..ef329a31e --- /dev/null +++ b/app/components/attachable/fields_component.html.erb @@ -0,0 +1,31 @@ +
+ <%= f.hidden_field :id %> + <%= f.hidden_field :user_id, value: current_user.id %> + <%= f.hidden_field :cached_attachment %> + +
+ <%= f.text_field :title, placeholder: t("#{plural_name}.form.title_placeholder") %> +
+ + <% if attachable.attachment.exists? && attachable.attachment.styles.keys.include?(:thumb) %> + <%= render_image(attachable, :thumb, false) %> + <% end %> + +
+

<%= file_name %>

+ +
+ <%= file_field %> +
+ +
+ <%= destroy_link %> +
+
+ +
+
+
+ +
+
diff --git a/app/components/attachable/fields_component.rb b/app/components/attachable/fields_component.rb new file mode 100644 index 000000000..2dc87a9f0 --- /dev/null +++ b/app/components/attachable/fields_component.rb @@ -0,0 +1,62 @@ +class Attachable::FieldsComponent < ApplicationComponent + attr_reader :f, :resource_type, :resource_id, :relation_name + delegate :current_user, :render_image, to: :helpers + + def initialize(f, resource_type:, resource_id:, relation_name:) + @f = f + @resource_type = resource_type + @resource_id = resource_id + @relation_name = relation_name + end + + private + + def attachable + f.object + end + + def singular_name + attachable.model_name.singular + end + + def plural_name + attachable.model_name.plural + end + + def file_name + attachable.attachment_file_name + end + + def destroy_link + if !attachable.persisted? && attachable.cached_attachment.present? + link_to t("#{plural_name}.form.delete_button"), "#", class: "delete remove-cached-attachment" + else + link_to_remove_association attachable.new_record? ? t("documents.form.cancel_button") : t("#{plural_name}.form.delete_button"), f, class: "delete remove-#{singular_name}" + end + end + + def file_field + klass = attachable.persisted? || attachable.cached_attachment.present? ? " hide" : "" + f.file_field :attachment, + label_options: { class: "button hollow #{klass}" }, + accept: accepted_content_types_extensions, + class: "js-#{singular_name}-attachment", + data: { url: direct_upload_path } + end + + def direct_upload_path + direct_uploads_path("direct_upload[resource_type]": resource_type, + "direct_upload[resource_id]": resource_id, + "direct_upload[resource_relation]": relation_name) + end + + def accepted_content_types_extensions + Setting.accepted_content_types_for(plural_name).map do |content_type| + if content_type == "jpg" + ".jpg,.jpeg" + else + ".#{content_type}" + end + end.join(",") + end +end diff --git a/app/components/documents/fields_component.html.erb b/app/components/documents/fields_component.html.erb index 5822f89ba..4520d7527 100644 --- a/app/components/documents/fields_component.html.erb +++ b/app/components/documents/fields_component.html.erb @@ -1,28 +1,6 @@ -
- <%= f.hidden_field :id %> - <%= f.hidden_field :user_id, value: current_user.id %> - <%= f.hidden_field :cached_attachment %> - -
- <%= f.text_field :title, placeholder: t("documents.form.title_placeholder") %> -
- -
-

<%= file_name %>

- -
- <%= file_field %> -
- -
- <%= destroy_link %> -
-
- -
-
-
- -
- -
+<%= render Attachable::FieldsComponent.new( + f, + resource_type: document.documentable_type, + resource_id: document.documentable_id, + relation_name: "documents" +) %> diff --git a/app/components/documents/fields_component.rb b/app/components/documents/fields_component.rb index 1ef9febea..01da058c0 100644 --- a/app/components/documents/fields_component.rb +++ b/app/components/documents/fields_component.rb @@ -1,6 +1,5 @@ class Documents::FieldsComponent < ApplicationComponent attr_reader :f - delegate :current_user, to: :helpers def initialize(f) @f = f @@ -11,35 +10,4 @@ class Documents::FieldsComponent < ApplicationComponent def document f.object end - - def file_name - document.attachment_file_name - end - - def destroy_link - if !document.persisted? && document.cached_attachment.present? - link_to t("documents.form.delete_button"), "#", class: "delete remove-cached-attachment" - else - link_to_remove_association document.new_record? ? t("documents.form.cancel_button") : t("documents.form.delete_button"), f, class: "delete remove-document" - end - end - - def file_field - klass = document.persisted? || document.cached_attachment.present? ? " hide" : "" - f.file_field :attachment, - label_options: { class: "button hollow #{klass}" }, - accept: accepted_content_types_extensions, - class: "js-document-attachment", - data: { url: direct_upload_path } - end - - def direct_upload_path - direct_uploads_path("direct_upload[resource_type]": document.documentable_type, - "direct_upload[resource_id]": document.documentable_id, - "direct_upload[resource_relation]": "documents") - end - - def accepted_content_types_extensions - Setting.accepted_content_types_for("documents").map { |content_type| ".#{content_type}" }.join(",") - end end diff --git a/app/components/images/fields_component.html.erb b/app/components/images/fields_component.html.erb index 49d8f1fd8..4a2df33af 100644 --- a/app/components/images/fields_component.html.erb +++ b/app/components/images/fields_component.html.erb @@ -1,29 +1,6 @@ -
- <%= f.hidden_field :id %> - <%= f.hidden_field :user_id, value: current_user.id %> - <%= f.hidden_field :cached_attachment %> - -
- <%= f.text_field :title, placeholder: t("images.form.title_placeholder") %> -
- - <%= render_image(image, :thumb, false) if image.attachment.exists? %> - -
-

<%= file_name %>

- -
- <%= file_field %> -
- -
- <%= destroy_link %> -
-
- -
-
-
- -
-
+<%= render Attachable::FieldsComponent.new( + f, + resource_type: imageable.class.name, + resource_id: imageable.id, + relation_name: "image" +) %> diff --git a/app/components/images/fields_component.rb b/app/components/images/fields_component.rb index ddf60fa45..44c3ad0a1 100644 --- a/app/components/images/fields_component.rb +++ b/app/components/images/fields_component.rb @@ -1,52 +1,8 @@ class Images::FieldsComponent < ApplicationComponent attr_reader :f, :imageable - delegate :current_user, :render_image, to: :helpers def initialize(f, imageable:) @f = f @imageable = imageable end - - private - - def image - f.object - end - - def file_name - image.attachment_file_name - end - - def destroy_link - if !image.persisted? && image.cached_attachment.present? - link_to t("images.form.delete_button"), "#", class: "delete remove-cached-attachment" - else - link_to_remove_association t("images.form.delete_button"), f, class: "delete remove-image" - end - end - - def file_field - klass = image.persisted? || image.cached_attachment.present? ? " hide" : "" - f.file_field :attachment, - label_options: { class: "button hollow #{klass}" }, - accept: accepted_content_types_extensions, - class: "js-image-attachment", - data: { url: direct_upload_path } - end - - def direct_upload_path - direct_uploads_path("direct_upload[resource_type]": imageable.class.name, - "direct_upload[resource_id]": imageable.id, - "direct_upload[resource_relation]": "image") - end - - def accepted_content_types_extensions - Setting.accepted_content_types_for("images").map do |content_type| - if content_type == "jpg" - ".jpg,.jpeg" - else - ".#{content_type}" - end - end.join(",") - end end From 7bbaad9a19fe3ce7d82b5059b95275879ec4f00a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 11 Sep 2021 17:11:36 +0200 Subject: [PATCH 6/6] Extract method for link to remove association text We had an ultra-long line which was hard to read. --- app/components/attachable/fields_component.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/components/attachable/fields_component.rb b/app/components/attachable/fields_component.rb index 2dc87a9f0..ae217456a 100644 --- a/app/components/attachable/fields_component.rb +++ b/app/components/attachable/fields_component.rb @@ -31,7 +31,15 @@ class Attachable::FieldsComponent < ApplicationComponent if !attachable.persisted? && attachable.cached_attachment.present? link_to t("#{plural_name}.form.delete_button"), "#", class: "delete remove-cached-attachment" else - link_to_remove_association attachable.new_record? ? t("documents.form.cancel_button") : t("#{plural_name}.form.delete_button"), f, class: "delete remove-#{singular_name}" + link_to_remove_association remove_association_text, f, class: "delete remove-#{singular_name}" + end + end + + def remove_association_text + if attachable.new_record? + t("documents.form.cancel_button") + else + t("#{plural_name}.form.delete_button") end end