diff --git a/Gemfile b/Gemfile index f71cb950b..8dde556bb 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,7 @@ gem "dalli", "~> 2.7.11" gem "delayed_job_active_record", "~> 4.1.6" gem "devise", "~> 4.8.0" gem "devise-security", "~> 0.16.0" +gem "file_validators", "~> 3.0.0" gem "font-awesome-sass", "~> 5.15.1" # Remember to update vendor/assets/images/fontawesome when updating this gem gem "foundation-rails", "~> 6.6.2.0" gem "foundation_rails_helper", "~> 4.0.0" @@ -33,12 +34,12 @@ gem "jquery-fileupload-rails" gem "jquery-rails", "~> 4.4.0" gem "jquery-ui-rails", "~> 6.0.1" gem "kaminari", "~> 1.2.1" +gem "mini_magick", "~> 4.11.0" gem "omniauth", "~> 2.0.4" gem "omniauth-facebook", "~> 8.0.0" gem "omniauth-google-oauth2", "~> 1.0.0" gem "omniauth-rails_csrf_protection", "~> 1.0.0" gem "omniauth-twitter", "~> 1.4.0" -gem "paperclip", "~> 6.1.0" gem "paranoia", "~> 2.4.3" gem "pg", "~> 1.2.3" gem "pg_search", "~> 2.3.5" diff --git a/Gemfile.lock b/Gemfile.lock index dd0425134..efa05e5f6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -250,6 +250,9 @@ GEM faraday-patron (1.0.0) faraday-rack (1.0.0) ffi (1.15.4) + file_validators (3.0.0) + activemodel (>= 3.2) + mime-types (>= 1.0) font-awesome-sass (5.15.1) sassc (>= 1.11) foundation-rails (6.6.2.0) @@ -370,9 +373,7 @@ GEM mime-types (3.3.1) mime-types-data (~> 3.2015) mime-types-data (3.2021.0704) - mimemagic (0.3.10) - nokogiri (~> 1) - rake + mini_magick (4.11.0) mini_mime (1.1.0) mini_portile2 (2.6.1) minitest (5.14.4) @@ -426,12 +427,6 @@ GEM omniauth-oauth (~> 1.1) rack orm_adapter (0.5.0) - paperclip (6.1.0) - activemodel (>= 4.2.0) - activesupport (>= 4.2.0) - mime-types - mimemagic (~> 0.3.0) - terrapin (~> 0.6.0) parallel (1.20.1) paranoia (2.4.3) activerecord (>= 4.0, < 6.2) @@ -722,6 +717,7 @@ DEPENDENCIES erb_lint (~> 0.0.37) factory_bot_rails (~> 6.2.0) faker (~> 2.18.0) + file_validators (~> 3.0.0) font-awesome-sass (~> 5.15.1) foundation-rails (~> 6.6.2.0) foundation_rails_helper (~> 4.0.0) @@ -742,12 +738,12 @@ DEPENDENCIES launchy (~> 2.5.0) letter_opener_web (~> 1.4.0) mdl (~> 0.11.0) + mini_magick (~> 4.11.0) omniauth (~> 2.0.4) omniauth-facebook (~> 8.0.0) omniauth-google-oauth2 (~> 1.0.0) omniauth-rails_csrf_protection (~> 1.0.0) omniauth-twitter (~> 1.4.0) - paperclip (~> 6.1.0) paranoia (~> 2.4.3) pg (~> 1.2.3) pg_search (~> 2.3.5) diff --git a/app/components/admin/widget/cards/row_component.html.erb b/app/components/admin/widget/cards/row_component.html.erb index 974753dc8..c66b28708 100644 --- a/app/components/admin/widget/cards/row_component.html.erb +++ b/app/components/admin/widget/cards/row_component.html.erb @@ -12,7 +12,7 @@ <% if card.image.present? %> - <%= link_to t("admin.shared.show_image"), card.image_url(:large), + <%= link_to t("admin.shared.show_image"), card.image.variant(:large), title: card.image.title, target: "_blank" %> <% end %> diff --git a/app/components/attachable/fields_component.html.erb b/app/components/attachable/fields_component.html.erb index ef329a31e..e5e9854a6 100644 --- a/app/components/attachable/fields_component.html.erb +++ b/app/components/attachable/fields_component.html.erb @@ -7,7 +7,7 @@ <%= f.text_field :title, placeholder: t("#{plural_name}.form.title_placeholder") %> - <% if attachable.attachment.exists? && attachable.attachment.styles.keys.include?(:thumb) %> + <% if attachable.attachment.attached? && attachable.attachment.image? %> <%= render_image(attachable, :thumb, false) %> <% end %> diff --git a/app/components/budgets/budget_component.html.erb b/app/components/budgets/budget_component.html.erb index 7ba34034a..232d5da75 100644 --- a/app/components/budgets/budget_component.html.erb +++ b/app/components/budgets/budget_component.html.erb @@ -1,6 +1,6 @@ <% if budget.image.present? %>
+ style="background-image: url(<%= polymorphic_path(budget.image.variant(:large)) %>);"> <% else %>
<% end %> diff --git a/app/components/budgets/investment_component.html.erb b/app/components/budgets/investment_component.html.erb index 4ca171959..1b05e2c9b 100644 --- a/app/components/budgets/investment_component.html.erb +++ b/app/components/budgets/investment_component.html.erb @@ -1,6 +1,6 @@
<% if investment.image.present? %> - <%= image_tag investment.image_url(:large), alt: investment.image.title.unicode_normalize %> + <%= image_tag investment.image.variant(:large), alt: investment.image.title.unicode_normalize %> <% else %> <%= image_tag "budget_investment_no_image.jpg", alt: investment.title %> <% end %> diff --git a/app/components/budgets/phases_component.html.erb b/app/components/budgets/phases_component.html.erb index 8c881a4f1..b7921f712 100644 --- a/app/components/budgets/phases_component.html.erb +++ b/app/components/budgets/phases_component.html.erb @@ -55,7 +55,7 @@ <% if phase.image.present? %>
- <%= image_tag phase.image.attachment.url(:large), alt: "" %> + <%= image_tag phase.image.variant(:large), alt: "" %>
<% end %>
diff --git a/app/components/widget/feeds/proposal_component.html.erb b/app/components/widget/feeds/proposal_component.html.erb index 8f683776a..b9bbc83c8 100644 --- a/app/components/widget/feeds/proposal_component.html.erb +++ b/app/components/widget/feeds/proposal_component.html.erb @@ -9,7 +9,7 @@ <% if proposal.image.present? %>
- <%= image_tag proposal.image_url(:thumb), + <%= image_tag proposal.image.variant(:thumb), alt: proposal.image.title.unicode_normalize %>
diff --git a/app/controllers/direct_uploads_controller.rb b/app/controllers/direct_uploads_controller.rb index a1995c2e3..ca992cd8e 100644 --- a/app/controllers/direct_uploads_controller.rb +++ b/app/controllers/direct_uploads_controller.rb @@ -15,9 +15,9 @@ class DirectUploadsController < ApplicationController @direct_upload.relation.set_cached_attachment_from_attachment render json: { cached_attachment: @direct_upload.relation.cached_attachment, - filename: @direct_upload.relation.attachment.original_filename, + filename: @direct_upload.relation.attachment_file_name, destroy_link: render_destroy_upload_link(@direct_upload), - attachment_url: @direct_upload.relation.attachment.url } + attachment_url: polymorphic_path(@direct_upload.relation.attachment) } else render json: { errors: @direct_upload.errors[:attachment].join(", ") }, status: :unprocessable_entity diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e57d06576..0b9fb23f4 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -49,7 +49,13 @@ module ApplicationHelper end def image_path_for(filename) - SiteCustomization::Image.image_path_for(filename) || filename + image = SiteCustomization::Image.image_for(filename) + + if image + polymorphic_path(image) + else + filename + end end def content_block(name, locale = I18n.locale) diff --git a/app/helpers/documents_helper.rb b/app/helpers/documents_helper.rb index 3ff564a03..9110e45b8 100644 --- a/app/helpers/documents_helper.rb +++ b/app/helpers/documents_helper.rb @@ -3,7 +3,7 @@ module DocumentsHelper info_text = "#{document.humanized_content_type} | #{number_to_human_size(document.attachment_file_size)}" link_to safe_join([document.title, tag.small("(#{info_text})")], " "), - document.attachment.url, + document.attachment, target: "_blank", title: t("shared.target_blank") end diff --git a/app/helpers/images_helper.rb b/app/helpers/images_helper.rb index 0f64a9bc2..06d63e9f8 100644 --- a/app/helpers/images_helper.rb +++ b/app/helpers/images_helper.rb @@ -2,11 +2,7 @@ module ImagesHelper def image_absolute_url(image, version) return "" unless image - if Paperclip::Attachment.default_options[:storage] == :filesystem - URI(request.url) + image.attachment.url(version) - else - image.attachment.url(version) - end + polymorphic_url(image.variant(version)) end def image_class(image) @@ -14,7 +10,8 @@ module ImagesHelper end def render_image(image, version, show_caption = true) - version = image.persisted? ? version : :original - render "images/image", image: image, version: version, show_caption: show_caption + render "images/image", image: image, + version: (version if image.persisted?), + show_caption: show_caption end end diff --git a/app/helpers/welcome_helper.rb b/app/helpers/welcome_helper.rb index 5962ff682..cb77252d3 100644 --- a/app/helpers/welcome_helper.rb +++ b/app/helpers/welcome_helper.rb @@ -24,8 +24,9 @@ module WelcomeHelper end def calculate_image_path(recommended, image_default) - if recommended.respond_to?(:image) && recommended.image.present? && recommended.image.attachment.exists? - recommended.image.attachment.send("url", :medium) + if recommended.respond_to?(:image) && recommended.image.present? && + recommended.image.attachment.attached? + recommended.image.variant(:medium) elsif image_default.present? image_default end diff --git a/app/models/ckeditor/asset.rb b/app/models/ckeditor/asset.rb index 494d4fc76..ead24ea61 100644 --- a/app/models/ckeditor/asset.rb +++ b/app/models/ckeditor/asset.rb @@ -1,5 +1,4 @@ class Ckeditor::Asset < ApplicationRecord include Ckeditor::Orm::ActiveRecord::AssetBase include Ckeditor::Backend::ActiveStorage - include Ckeditor::Backend::Paperclip end diff --git a/app/models/ckeditor/picture.rb b/app/models/ckeditor/picture.rb index bcce7ee99..dc71277ea 100644 --- a/app/models/ckeditor/picture.rb +++ b/app/models/ckeditor/picture.rb @@ -1,16 +1,14 @@ class Ckeditor::Picture < Ckeditor::Asset - include HasAttachment + attr_accessor :data + has_one_attached :storage_data - has_attachment :data, - url: "/ckeditor_assets/pictures/:id/:style_:basename.:extension", - path: ":rails_root/public/ckeditor_assets/pictures/:id/:style_:basename.:extension", - styles: { content: "800>", thumb: "118x100#" } - - validates_attachment_presence :data - validates_attachment_size :data, less_than: 2.megabytes - validates_attachment_content_type :data, content_type: /\Aimage/ + validates :storage_data, file_content_type: { allow: /^image\/.*/ }, file_size: { less_than: 2.megabytes } def url_content - url(:content) + rails_representation_url(storage_data.variant(resize: "800>"), only_path: true) + end + + def url_thumb + rails_representation_url(storage_data.variant(resize: "118x100"), only_path: true) end end diff --git a/app/models/concerns/attachable.rb b/app/models/concerns/attachable.rb index 9fb8c7a80..ac6c1e33a 100644 --- a/app/models/concerns/attachable.rb +++ b/app/models/concerns/attachable.rb @@ -3,20 +3,32 @@ module Attachable extend ActiveSupport::Concern included do + has_attachment :attachment 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? } + validates :attachment, + file_content_type: { + allow: ->(record) { record.accepted_content_types }, + if: -> { association_class && attachment.attached? }, + message: ->(record, *) do + I18n.t("#{record.model_name.plural}.errors.messages.wrong_content_type", + content_type: record.attachment_content_type, + accepted_content_types: record.class.humanized_accepted_content_types) + end + }, + file_size: { + less_than_or_equal_to: ->(record) { record.max_file_size.megabytes }, + if: -> { association_class && attachment.attached? }, + message: ->(record, *) do + I18n.t("#{record.model_name.plural}.errors.messages.in_between", + min: "0 Bytes", + max: "#{record.max_file_size} MB") + end + } - Paperclip.interpolates :prefix do |attachment, style| - attachment.instance.prefix(attachment, style) - end + before_validation :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } end def association_class @@ -26,54 +38,37 @@ module Attachable end def set_cached_attachment_from_attachment - self.cached_attachment = if filesystem_storage? - attachment.path - else - attachment.url - end + self.cached_attachment = attachment.signed_id end def set_attachment_from_cached_attachment - if filesystem_storage? - File.open(cached_attachment) { |file| self.attachment = file } + self.attachment = cached_attachment + end + + def attachment_file_name + attachment.filename.to_s if attachment.attached? + end + + def attachment_content_type + attachment.blob.content_type if attachment.attached? + end + + def attachment_file_size + if attachment.attached? + attachment.blob.byte_size else - self.attachment = URI.parse(cached_attachment).open + 0 end end - def prefix(attachment, _style) - if attachment.instance.persisted? - ":attachment/:id_partition" - else - "cached_attachments/user/#{attachment.instance.user_id}" - end + def file_path + ActiveStorage::Blob.service.path_for(attachment.blob.key) end private - def filesystem_storage? - Paperclip::Attachment.default_options[:storage] == :filesystem - end - - 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? + unless attachment.attached? errors.add(:attachment, I18n.t("errors.messages.blank")) end end diff --git a/app/models/concerns/has_attachment.rb b/app/models/concerns/has_attachment.rb index 6f3f67844..2ffbd931b 100644 --- a/app/models/concerns/has_attachment.rb +++ b/app/models/concerns/has_attachment.rb @@ -2,22 +2,15 @@ module HasAttachment extend ActiveSupport::Concern class_methods do - def has_attachment(attribute, paperclip_options = {}) - has_one_attached :"storage_#{attribute}" - - has_attached_file attribute, paperclip_options - alias_method :"paperclip_#{attribute}=", :"#{attribute}=" + def has_attachment(attribute) + has_one_attached attribute define_method :"#{attribute}=" do |file| - if file.is_a?(IO) || file.is_a?(Tempfile) && !file.is_a?(Ckeditor::Http::QqFile) - send(:"storage_#{attribute}").attach(io: file, filename: File.basename(file.path)) - elsif file.nil? - send(:"storage_#{attribute}").detach + if file.nil? + send(attribute).detach else - send(:"storage_#{attribute}").attach(file) + send(attribute).attach(file) end - - send(:"paperclip_#{attribute}=", file) end end end diff --git a/app/models/concerns/imageable.rb b/app/models/concerns/imageable.rb index fffc871de..ad4630a6c 100644 --- a/app/models/concerns/imageable.rb +++ b/app/models/concerns/imageable.rb @@ -4,9 +4,5 @@ module Imageable included do has_one :image, as: :imageable, inverse_of: :imageable, dependent: :destroy accepts_nested_attributes_for :image, allow_destroy: true, update_only: true - - def image_url(style) - image&.attachment&.url(style) || "" - end end end diff --git a/app/models/direct_upload.rb b/app/models/direct_upload.rb index 7e66375e7..cd562ca07 100644 --- a/app/models/direct_upload.rb +++ b/app/models/direct_upload.rb @@ -34,7 +34,7 @@ class DirectUpload end def save_attachment - @relation.attachment.save + @relation.attachment.blob.save! end def persisted? @@ -53,7 +53,7 @@ class DirectUpload def relation_attributtes { - paperclip_attachment: @attachment, + attachment: @attachment, cached_attachment: @cached_attachment, user: @user } diff --git a/app/models/document.rb b/app/models/document.rb index a121e3bbd..9a48ed615 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -1,11 +1,6 @@ class Document < ApplicationRecord include Attachable - has_attachment :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 - belongs_to :user belongs_to :documentable, polymorphic: true, touch: true @@ -16,23 +11,10 @@ class Document < ApplicationRecord scope :admin, -> { where(admin: true) } - Paperclip.interpolates :custom_hash_data do |attachment, _style| - attachment.instance.custom_hash_data(attachment) - end - def self.humanized_accepted_content_types Setting.accepted_content_types_for("documents").join(", ") end - def custom_hash_data(attachment) - original_filename = if attachment.instance.persisted? - attachment.instance.title - else - attachment.instance.attachment_file_name - end - "#{attachment.instance.user_id}/#{original_filename}" - end - def humanized_content_type attachment_content_type.split("/").last.upcase end diff --git a/app/models/image.rb b/app/models/image.rb index de1405f2b..eda89ecc8 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -1,15 +1,13 @@ class Image < ApplicationRecord include Attachable - has_attachment :attachment, styles: { - large: "x#{Setting["uploads.images.min_height"]}", - medium: "300x300#", - thumb: "140x245#" - }, - url: "/system/:class/:prefix/:style/:hash.:extension", - hash_data: ":class/:style", - use_timestamp: false, - hash_secret: Rails.application.secrets.secret_key_base + def self.styles + { + large: { resize: "x#{Setting["uploads.images.min_height"]}" }, + medium: { combine_options: { gravity: "center", resize: "300x300^", crop: "300x300+0+0" }}, + thumb: { combine_options: { gravity: "center", resize: "140x245^", crop: "140x245+0+0" }} + } + end belongs_to :user belongs_to :imageable, polymorphic: true, touch: true @@ -19,7 +17,7 @@ class Image < ApplicationRecord validates :user_id, presence: true validates :imageable_id, presence: true, if: -> { persisted? } validates :imageable_type, presence: true, if: -> { persisted? } - validate :validate_image_dimensions, if: -> { attachment.present? && attachment.dirty? } + validate :validate_image_dimensions, if: -> { attachment.attached? && attachment.new_record? } def self.max_file_size Setting["uploads.images.max_size"].to_i @@ -41,6 +39,14 @@ class Image < ApplicationRecord self.class.accepted_content_types end + def variant(style) + if style + attachment.variant(self.class.styles[style]) + else + attachment + end + end + private def association_name @@ -52,14 +58,17 @@ class Image < ApplicationRecord end def validate_image_dimensions - if attachment_of_valid_content_type? + if accepted_content_types.include?(attachment_content_type) return true if imageable_class == Widget::Card - dimensions = Paperclip::Geometry.from_file(attachment.queued_for_write[:original].path) + attachment.analyze unless attachment.analyzed? + + width = attachment.metadata[:width] + height = attachment.metadata[:height] min_width = Setting["uploads.images.min_width"].to_i min_height = Setting["uploads.images.min_height"].to_i - errors.add(:attachment, :min_image_width, required_min_width: min_width) if dimensions.width < min_width - errors.add(:attachment, :min_image_height, required_min_height: min_height) if dimensions.height < min_height + errors.add(:attachment, :min_image_width, required_min_width: min_width) if width < min_width + errors.add(:attachment, :min_image_height, required_min_height: min_height) if height < min_height end end @@ -77,8 +86,4 @@ class Image < ApplicationRecord end end end - - def attachment_of_valid_content_type? - attachment.present? && accepted_content_types.include?(attachment_content_type) - end end diff --git a/app/models/site_customization/image.rb b/app/models/site_customization/image.rb index 592ea3b47..c9c41ed6a 100644 --- a/app/models/site_customization/image.rb +++ b/app/models/site_customization/image.rb @@ -14,7 +14,7 @@ class SiteCustomization::Image < ApplicationRecord has_attachment :image validates :name, presence: true, uniqueness: true, inclusion: { in: VALID_IMAGES.keys } - validates_attachment_content_type :image, content_type: ["image/png", "image/jpeg"] + validates :image, file_content_type: { allow: ["image/png", "image/jpeg"], if: -> { image.attached? }} validate :check_image def self.all_images @@ -23,11 +23,10 @@ class SiteCustomization::Image < ApplicationRecord end end - def self.image_path_for(filename) + def self.image_for(filename) image_name = filename.split(".").first - imageable = find_by(name: image_name) - imageable.present? && imageable.image.exists? ? imageable.image.url : nil + find_by(name: image_name)&.persisted_image end def required_width @@ -38,19 +37,29 @@ class SiteCustomization::Image < ApplicationRecord VALID_IMAGES[name]&.second end + def persisted_image + image if persisted_attachment? + end + + def persisted_attachment? + image.attachment&.persisted? + end + private def check_image - return unless image? + return unless image.attached? - dimensions = Paperclip::Geometry.from_file(image.queued_for_write[:original].path) + image.analyze unless image.analyzed? + width = image.metadata[:width] + height = image.metadata[:height] if name == "logo_header" - errors.add(:image, :image_width, required_width: required_width) unless dimensions.width <= required_width + errors.add(:image, :image_width, required_width: required_width) unless width <= required_width else - errors.add(:image, :image_width, required_width: required_width) unless dimensions.width == required_width + errors.add(:image, :image_width, required_width: required_width) unless width == required_width end - errors.add(:image, :image_height, required_height: required_height) unless dimensions.height == required_height + errors.add(:image, :image_height, required_height: required_height) unless height == required_height end end diff --git a/app/views/admin/milestones/_milestones.html.erb b/app/views/admin/milestones/_milestones.html.erb index 14e99f1b2..43479ce86 100644 --- a/app/views/admin/milestones/_milestones.html.erb +++ b/app/views/admin/milestones/_milestones.html.erb @@ -40,14 +40,14 @@ <%= link_to t("admin.milestones.index.show_image"), - milestone.image_url(:large), + milestone.image.variant(:large), target: :_blank if milestone.image.present? %> <% if milestone.documents.present? %> <% milestone.documents.each do |document| %> <%= link_to document.title, - document.attachment.url, + document.attachment, target: "_blank", rel: "nofollow" %>
<% end %> diff --git a/app/views/admin/poll/questions/answers/documents.html.erb b/app/views/admin/poll/questions/answers/documents.html.erb index 53d78996a..4c0396849 100644 --- a/app/views/admin/poll/questions/answers/documents.html.erb +++ b/app/views/admin/poll/questions/answers/documents.html.erb @@ -33,7 +33,7 @@ <% @answer.documents.each do |document| %> - <%= link_to document.title, document.attachment.url %> + <%= link_to document.title, document.attachment %> <%= render Admin::TableActionsComponent.new(document, @@ -42,7 +42,7 @@ ) do |actions| %> <%= actions.action(:download, text: t("documents.buttons.download_document"), - path: document.attachment.url, + path: document.attachment, target: "_blank", rel: "nofollow") %> diff --git a/app/views/admin/site_customization/documents/index.html.erb b/app/views/admin/site_customization/documents/index.html.erb index 491d9e069..4f3f93497 100644 --- a/app/views/admin/site_customization/documents/index.html.erb +++ b/app/views/admin/site_customization/documents/index.html.erb @@ -22,8 +22,8 @@ <%= document.title %> <%= document.attachment.content_type %> - <%= number_to_human_size(document.attachment.size) %> - <%= link_to document.title, document.attachment.url, target: :blank %> + <%= number_to_human_size(document.attachment.byte_size) %> + <%= link_to document.title, document.attachment, target: :blank %>
<%= render Admin::TableActionsComponent.new( diff --git a/app/views/admin/site_customization/images/index.html.erb b/app/views/admin/site_customization/images/index.html.erb index f6ce49975..354c3626c 100644 --- a/app/views/admin/site_customization/images/index.html.erb +++ b/app/views/admin/site_customization/images/index.html.erb @@ -16,12 +16,12 @@ <%= form_for([:admin, image], html: { id: "edit_#{dom_id(image)}" }) do |f| %>
- <%= image_tag image.image.url if image.image.exists? %> + <%= image_tag image.image if image.persisted_attachment? %> <%= f.file_field :image, label: false %>
<%= f.submit(t("admin.site_customization.images.index.update"), class: "button hollow") %> - <%= link_to t("admin.site_customization.images.index.delete"), admin_site_customization_image_path(image), method: :delete, class: "button hollow alert" if image.image.exists? %> + <%= link_to t("admin.site_customization.images.index.delete"), admin_site_customization_image_path(image), method: :delete, class: "button hollow alert" if image.persisted_attachment? %>
<% end %> diff --git a/app/views/budgets/executions/_image.html.erb b/app/views/budgets/executions/_image.html.erb index 639761875..ebe933504 100644 --- a/app/views/budgets/executions/_image.html.erb +++ b/app/views/budgets/executions/_image.html.erb @@ -1,9 +1,9 @@ <% milestone = first_milestone_with_image(investment) %> <% if milestone&.image.present? %> - <%= image_tag milestone.image_url(:large), alt: milestone.image.title %> + <%= image_tag milestone.image.variant(:large), alt: milestone.image.title %> <% elsif investment.image.present? %> - <%= image_tag investment.image_url(:large), alt: investment.image.title %> + <%= image_tag investment.image.variant(:large), alt: investment.image.title %> <% else %> <%= image_tag "budget_execution_no_image.jpg", alt: investment.title %> <% end %> diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index fea6b4241..1a4009952 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -5,7 +5,7 @@
- <%= image_tag investment.image_url(:thumb), alt: investment.image.title.unicode_normalize %> + <%= image_tag investment.image.variant(:thumb), alt: investment.image.title.unicode_normalize %>
diff --git a/app/views/budgets/investments/_investment_show.html.erb b/app/views/budgets/investments/_investment_show.html.erb index 2b31e476f..d7a46cf51 100644 --- a/app/views/budgets/investments/_investment_show.html.erb +++ b/app/views/budgets/investments/_investment_show.html.erb @@ -3,8 +3,8 @@ social_url: budget_investments_path(investment), social_title: investment.title, social_description: investment.description, - twitter_image_url: (investment.image.present? ? investment.image_url(:thumb) : nil), - og_image_url: (investment.image.present? ? investment.image_url(:thumb) : nil) %> + twitter_image_url: (investment.image.present? ? polymorphic_path(investment.image.variant(:thumb)) : nil), + og_image_url: (investment.image.present? ? polymorphic_path(investment.image.variant(:thumb)) : nil) %> <% end %> <% cache [locale_and_user_status(investment), diff --git a/app/views/dashboard/_document.html.erb b/app/views/dashboard/_document.html.erb index 0b61002a2..d59177a30 100644 --- a/app/views/dashboard/_document.html.erb +++ b/app/views/dashboard/_document.html.erb @@ -1,5 +1,5 @@

- <%= link_to document.attachment.url, target: "_blank" do %> + <%= link_to document.attachment, target: "_blank" do %> <%= document.title %> (<%= document.humanized_content_type %>  |  diff --git a/app/views/dashboard/mailer/forward.html.erb b/app/views/dashboard/mailer/forward.html.erb index f3f25fe12..95cea001c 100644 --- a/app/views/dashboard/mailer/forward.html.erb +++ b/app/views/dashboard/mailer/forward.html.erb @@ -15,7 +15,7 @@ <% if @proposal.image.present? %> - <%= image_tag @proposal.image.attachment.url(:large), style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> + <%= image_tag @proposal.image.variant(:large), style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> <% else %> <%= image_tag "default_mailing.jpg", style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> <% end %> diff --git a/app/views/dashboard/mailing/index.html.erb b/app/views/dashboard/mailing/index.html.erb index 1886d07c8..bf1f01fb9 100644 --- a/app/views/dashboard/mailing/index.html.erb +++ b/app/views/dashboard/mailing/index.html.erb @@ -9,7 +9,7 @@

- <%= image_tag(proposal.image_url(:large).presence || "default_mailing.jpg") %> + <%= image_tag(proposal.image&.variant(:large).presence || "default_mailing.jpg") %>
diff --git a/app/views/dashboard/poster/index.html.erb b/app/views/dashboard/poster/index.html.erb index 7cdfb2faa..e1cddb5d4 100644 --- a/app/views/dashboard/poster/index.html.erb +++ b/app/views/dashboard/poster/index.html.erb @@ -18,7 +18,7 @@

<% if proposal.image.present? %> -
+
<% else %>
);">
<% end %> diff --git a/app/views/dashboard/poster/index.pdf.erb b/app/views/dashboard/poster/index.pdf.erb index c1ba4ea78..c762156d2 100644 --- a/app/views/dashboard/poster/index.pdf.erb +++ b/app/views/dashboard/poster/index.pdf.erb @@ -23,7 +23,7 @@

<% if proposal.image.present? %> -
+
<% else %>
');">
<% end %> diff --git a/app/views/documents/_document.html.erb b/app/views/documents/_document.html.erb index 07d31e026..977e3acc4 100644 --- a/app/views/documents/_document.html.erb +++ b/app/views/documents/_document.html.erb @@ -1,6 +1,6 @@
  • <%= link_to t("documents.buttons.download_document"), - document.attachment.url, target: "_blank", + document.attachment, target: "_blank", rel: "nofollow", class: "button hollow medium float-right" %> <%= document.title %> diff --git a/app/views/images/_image.html.erb b/app/views/images/_image.html.erb index 65ef9324e..7cb5feb68 100644 --- a/app/views/images/_image.html.erb +++ b/app/views/images/_image.html.erb @@ -1,6 +1,6 @@
    - <%= image_tag image.attachment.url(version), + <%= image_tag image.variant(version), class: image_class(image), alt: image.title.unicode_normalize, title: image.title.unicode_normalize %> diff --git a/app/views/legislation/processes/_header.html.erb b/app/views/legislation/processes/_header.html.erb index 8917176e5..eb05108a5 100644 --- a/app/views/legislation/processes/_header.html.erb +++ b/app/views/legislation/processes/_header.html.erb @@ -45,7 +45,7 @@ <% if @process.image.present? %>
    - <%= image_tag(@process.image_url(:large), alt: @process.title, id: "image") %> + <%= image_tag(@process.image.variant(:large), alt: @process.title, id: "image") %> <% end %> <% if process.draft_publication.enabled? %> diff --git a/app/views/legislation/proposals/_proposal.html.erb b/app/views/legislation/proposals/_proposal.html.erb index ad3770683..0142be99c 100644 --- a/app/views/legislation/proposals/_proposal.html.erb +++ b/app/views/legislation/proposals/_proposal.html.erb @@ -7,7 +7,7 @@ <% if proposal.image.present? %>
    - <%= image_tag proposal.image_url(:thumb), + <%= image_tag proposal.image.variant(:thumb), alt: proposal.image.title.unicode_normalize %>
    diff --git a/app/views/milestones/_milestone.html.erb b/app/views/milestones/_milestone.html.erb index 6de305b85..52f8e6956 100644 --- a/app/views/milestones/_milestone.html.erb +++ b/app/views/milestones/_milestone.html.erb @@ -22,7 +22,7 @@

    <% end %> - <%= image_tag(milestone.image_url(:large), { id: "image_#{milestone.id}", alt: milestone.image.title, class: "margin" }) if milestone.image.present? %> + <%= image_tag(milestone.image.variant(:large), { id: "image_#{milestone.id}", alt: milestone.image.title, class: "margin" }) if milestone.image.present? %>

    <%= sanitize_and_auto_link milestone.description %> @@ -37,7 +37,7 @@ <% milestone.documents.each do |document| %> <%= link_to document.title, - document.attachment.url, + document.attachment, target: "_blank", rel: "nofollow" %>
    diff --git a/app/views/polls/_gallery.html.erb b/app/views/polls/_gallery.html.erb index f62f28831..43ac86cf6 100644 --- a/app/views/polls/_gallery.html.erb +++ b/app/views/polls/_gallery.html.erb @@ -18,8 +18,8 @@ <% answer.images.reverse.each_with_index do |image, index| %>

  • - <%= link_to image.attachment.url(:original), target: "_blank" do %> - <%= image_tag image.attachment.url(:original), + <%= link_to image.attachment, target: "_blank" do %> + <%= image_tag image.attachment, class: "orbit-image", alt: image.title.unicode_normalize %> <% end %> diff --git a/app/views/polls/_poll_group.html.erb b/app/views/polls/_poll_group.html.erb index 3a4d66296..58268e21b 100644 --- a/app/views/polls/_poll_group.html.erb +++ b/app/views/polls/_poll_group.html.erb @@ -23,7 +23,7 @@
    <% if poll.image.present? %> - <%= image_tag poll.image_url(:large), alt: poll.image.title.unicode_normalize %> + <%= image_tag poll.image.variant(:large), alt: poll.image.title.unicode_normalize %> <% end %>
    diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb index ebc55daa7..1aa0e89ec 100644 --- a/app/views/polls/show.html.erb +++ b/app/views/polls/show.html.erb @@ -86,7 +86,7 @@ <% answer.documents.each do |document| %> <%= link_to document.title, - document.attachment.url, + document.attachment, target: "_blank", rel: "nofollow" %>
    <% end %> diff --git a/app/views/proposals/_proposal.html.erb b/app/views/proposals/_proposal.html.erb index 733e3adf2..0b3c8ec33 100644 --- a/app/views/proposals/_proposal.html.erb +++ b/app/views/proposals/_proposal.html.erb @@ -8,7 +8,7 @@ <% if proposal.image.present? %>
    - <%= image_tag proposal.image_url(:thumb), + <%= image_tag proposal.image.variant(:thumb), alt: proposal.image.title.unicode_normalize %>
    diff --git a/app/views/proposals/show.html.erb b/app/views/proposals/show.html.erb index 15f52d8c7..02dc1c38c 100644 --- a/app/views/proposals/show.html.erb +++ b/app/views/proposals/show.html.erb @@ -6,8 +6,8 @@ social_url: proposal_url(@proposal), social_title: @proposal.title, social_description: @proposal.summary, - twitter_image_url: (@proposal.image.present? ? @proposal.image_url(:thumb) : nil), - og_image_url: (@proposal.image.present? ? @proposal.image_url(:thumb) : nil) %> + twitter_image_url: (@proposal.image.present? ? polymorphic_path(@proposal.image.variant(:thumb)) : nil), + og_image_url: (@proposal.image.present? ? polymorphic_path(@proposal.image.variant(:thumb)) : nil) %> <% end %> <% content_for :canonical do %> <%= render "shared/canonical", href: proposal_url(@proposal) %> diff --git a/app/views/shared/_card.html.erb b/app/views/shared/_card.html.erb index 4e6c1e459..8a591d4f4 100644 --- a/app/views/shared/_card.html.erb +++ b/app/views/shared/_card.html.erb @@ -4,7 +4,7 @@
    <% if card.image.present? %> - <%= image_tag(card.image_url(:large), alt: card.image.title) %> + <%= image_tag(card.image.variant(:large), alt: card.image.title) %> <% end %>
    <% if card.label.present? %> diff --git a/app/views/shared/_header.html.erb b/app/views/shared/_header.html.erb index 06e0fbc54..9eec4fc3f 100644 --- a/app/views/shared/_header.html.erb +++ b/app/views/shared/_header.html.erb @@ -16,7 +16,7 @@ <% if header.image.present? %>
    - <%= image_tag(header.image_url(:large), + <%= image_tag(header.image.variant(:large), class: "margin", alt: header.image.title) %>
    diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index b5932360a..8fe6467f7 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -169,6 +169,7 @@ ignore_unused: - "admin.dashboard.administrator_tasks.index.filter*" - "admin.dashboard.actions.index.default.*" - "admin.users.index.filter*" + - "errors.messages.allowed_file_content_types" - "moderation.comments.index.filter*" - "moderation.comments.index.orders.*" - "moderation.debates.index.filter*" diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb deleted file mode 100644 index 97a4079d9..000000000 --- a/config/initializers/paperclip.rb +++ /dev/null @@ -1,13 +0,0 @@ -class Paperclip::UrlGenerator - private - - # Code copied from the paperclip gem, only replacing - # `URI.escape` with `URI::DEFAULT_PARSER.escape` - def escape_url(url) - if url.respond_to?(:escape) - url.escape - else - URI::DEFAULT_PARSER.escape(url).gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" } - end - end -end diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index 01477295b..049f090f7 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -150,6 +150,7 @@ en: submit_button: Save changes errors: messages: + allowed_file_content_types: "content type must be one of %{types}" user_not_found: User not found invalid_date_range: "Invalid date range" form: diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index c76541d7e..bb1ecca96 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -150,6 +150,7 @@ es: submit_button: Guardar cambios errors: messages: + allowed_file_content_types: "el tipo de contenido debe ser uno de los siguientes: %{types}" user_not_found: Usuario no encontrado invalid_date_range: "El rango de fechas no es válido" form: diff --git a/db/dev_seeds/budgets.rb b/db/dev_seeds/budgets.rb index c686a5c9c..e3dc861c2 100644 --- a/db/dev_seeds/budgets.rb +++ b/db/dev_seeds/budgets.rb @@ -15,14 +15,12 @@ end def add_image_to(imageable) # imageable should respond to #title & #author - File.open(INVESTMENT_IMAGE_FILES.sample) do |file| - imageable.image = Image.create!({ - imageable: imageable, - title: imageable.title, - attachment: file, - user: imageable.author - }) - end + imageable.image = Image.create!({ + imageable: imageable, + title: imageable.title, + attachment: Rack::Test::UploadedFile.new(INVESTMENT_IMAGE_FILES.sample), + user: imageable.author + }) imageable.save! end diff --git a/db/dev_seeds/proposals.rb b/db/dev_seeds/proposals.rb index 756dd00b3..a2ccbfb08 100644 --- a/db/dev_seeds/proposals.rb +++ b/db/dev_seeds/proposals.rb @@ -12,14 +12,12 @@ end def add_image_to(imageable) # imageable should respond to #title & #author - File.open(IMAGE_FILES.sample) do |file| - imageable.image = Image.create!({ - imageable: imageable, - title: imageable.title, - attachment: file, - user: imageable.author - }) - end + imageable.image = Image.create!({ + imageable: imageable, + title: imageable.title, + attachment: Rack::Test::UploadedFile.new(IMAGE_FILES.sample), + user: imageable.author + }) imageable.save! end diff --git a/db/dev_seeds/widgets.rb b/db/dev_seeds/widgets.rb index b672d28d5..d82d655fe 100644 --- a/db/dev_seeds/widgets.rb +++ b/db/dev_seeds/widgets.rb @@ -1,7 +1,7 @@ section "Creating header and cards for the homepage" do def create_image_attachment(type) { - cached_attachment: Rails.root.join("db/dev_seeds/images/#{type}_background.jpg"), + attachment: Rack::Test::UploadedFile.new("db/dev_seeds/images/#{type}_background.jpg"), title: "#{type}_background.jpg", user: User.first } diff --git a/lib/ckeditor/backend/active_storage.rb b/lib/ckeditor/backend/active_storage.rb index 7ce8aa611..8ff442443 100644 --- a/lib/ckeditor/backend/active_storage.rb +++ b/lib/ckeditor/backend/active_storage.rb @@ -14,9 +14,9 @@ module Ckeditor module ClassMethods def self.extended(base) base.class_eval do - before_save :apply_data + before_validation :apply_data validate do - if data.nil? || storage_file.nil? + if data.nil? || file.nil? errors.add(:data, :not_data_present, message: "data must be present") end end @@ -46,25 +46,19 @@ module Ckeditor protected - def storage_file - @storage_file ||= storage_data + def file + @file ||= storage_data end def blob - @blob ||= ::ActiveStorage::Blob.find(storage_file.attachment.blob_id) + @blob ||= ::ActiveStorage::Blob.find(file.attachment.blob_id) end def apply_data - non_paperclip_data = if data.is_a?(::Paperclip::Attachment) - file.instance_variable_get("@target") - else - data - end - - if non_paperclip_data.is_a?(Ckeditor::Http::QqFile) - storage_data.attach(io: non_paperclip_data, filename: non_paperclip_data.original_filename) + if data.is_a?(Ckeditor::Http::QqFile) + storage_data.attach(io: data, filename: data.original_filename) else - storage_data.attach(non_paperclip_data) + storage_data.attach(data) end self.data_file_name = storage_data.blob.filename diff --git a/lib/tasks/active_storage.rake b/lib/tasks/active_storage.rake index 548d5cfc9..2f65e829b 100644 --- a/lib/tasks/active_storage.rake +++ b/lib/tasks/active_storage.rake @@ -1,98 +1,9 @@ -# This code is based on Thoughtbot's guide to migrating from Paperclip -# to Active Storage: -# https://github.com/thoughtbot/paperclip/blob/master/MIGRATING.md namespace :active_storage do - desc "Copy paperclip's attachment database columns to active storage" - task migrate_from_paperclip: :environment do - logger = ApplicationLogger.new - connection = ActiveRecord::Base.connection.raw_connection - statement_name = "active_storage_statement" - - unless connection.exec("SELECT COUNT(*) FROM pg_prepared_statements WHERE name='#{statement_name}'").each_row.to_a[0][0] > 0 - connection.prepare(statement_name, <<-SQL) - with rows as( - INSERT INTO active_storage_blobs ( - key, filename, content_type, metadata, byte_size, checksum, created_at - ) VALUES ($1, $2, $3, '{}', $4, $5, $6) RETURNING id - ) - INSERT INTO active_storage_attachments ( - name, record_type, record_id, blob_id, created_at - ) VALUES ($7, $8, $9, (SELECT id FROM rows), $10) - SQL - end - - Rails.application.eager_load! - models = ActiveRecord::Base.descendants.reject(&:abstract_class?) - paperclip_storage = Paperclip::Attachment.default_options[:storage] - - ActiveRecord::Base.transaction do - models.each do |model| - next if model.name == "OldPassword" && !model.table_exists? - - attachments = model.column_names.map do |c| - if c =~ /(.+)_file_name$/ - $1 - end - end.compact - - if attachments.blank? - next - end - - model.find_each.each do |instance| - attachments.each do |attachment| - next if instance.send(:"storage_#{attachment}").attached? - - source = if paperclip_storage == :filesystem - instance.send(attachment).path - else - instance.send(attachment).url - end - - next if source.blank? || source == "/images/original/missing.png" - - file = if paperclip_storage == :filesystem - File.read(source) if File.exist?(source) - else - Net::HTTP.get(URI(source)) - end - - connection.exec_prepared( - statement_name, [ - SecureRandom.uuid, # Alternatively instance.send("#{attachment}_file_name"), - instance.send("#{attachment}_file_name"), - instance.send("#{attachment}_content_type"), - instance.send("#{attachment}_file_size"), - file && Digest::MD5.base64digest(file) || SecureRandom.hex(32), - instance.updated_at.iso8601, - "storage_#{attachment}", - model.name, - instance.id, - instance.updated_at.iso8601 - ]) - end - end - end - end - - ActiveStorage::Attachment.find_each do |attachment| - blob = attachment.blob - - next if blob.service.exist?(blob.key) || !attachment.record - - name = attachment.name.delete_prefix("storage_") - paperclip_attachment = attachment.record.send(name) - - next unless paperclip_attachment.exists? - - source_file = if paperclip_storage == :filesystem - paperclip_attachment.path - else - URI.parse(paperclip_attachment.url).open.read - end - - logger.info "Copying #{paperclip_attachment.url} to active storage" - blob.service.upload(blob.key, source_file) - end + desc "Updates ActiveStorage::Attachment old records created when we were also using paperclip" + task remove_paperclip_compatibility_in_existing_attachments: :environment do + ApplicationLogger.new.info "Updating Active Storage attachment records" + ActiveStorage::Attachment.where("name ILIKE 'storage_%'") + .where.not(record_type: "Ckeditor::Asset") + .update_all("name = replace(name, 'storage_', '')") end end diff --git a/lib/tasks/consul.rake b/lib/tasks/consul.rake index a45012ab5..6bd19134b 100644 --- a/lib/tasks/consul.rake +++ b/lib/tasks/consul.rake @@ -2,10 +2,10 @@ namespace :consul do desc "Runs tasks needed to upgrade to the latest version" task execute_release_tasks: ["settings:rename_setting_keys", "settings:add_new_settings", - "execute_release_1.4.0_tasks"] + "execute_release_1.5.0_tasks"] - desc "Runs tasks needed to upgrade from 1.3.0 to 1.4.0" - task "execute_release_1.4.0_tasks": [ - "active_storage:migrate_from_paperclip" + desc "Runs tasks needed to upgrade from 1.4.0 to 1.5.0" + task "execute_release_1.5.0_tasks": [ + "active_storage:remove_paperclip_compatibility_in_existing_attachments" ] end diff --git a/lib/tasks/files.rake b/lib/tasks/files.rake index ef46097cc..978b5f4f1 100644 --- a/lib/tasks/files.rake +++ b/lib/tasks/files.rake @@ -1,18 +1,8 @@ namespace :files do desc "Removes cached attachments which weren't deleted for some reason" task remove_old_cached_attachments: :environment do - paths = Dir.glob(Rails.root.join("public/system/*/cached_attachments/**/*")) - logger = ApplicationLogger.new - - paths.each do |path| - if File.file?(path) - if File.mtime(path) < 1.day.ago - File.delete(path) - logger.info "The file #{path} has been removed" - end - else - Dir.delete(path) if Dir.empty?(path) - end - end + ActiveStorage::Blob.unattached + .where("active_storage_blobs.created_at <= ?", 1.day.ago) + .find_each(&:purge_later) end end diff --git a/spec/controllers/proposals_controller_spec.rb b/spec/controllers/proposals_controller_spec.rb index ae586123b..05e1cc279 100644 --- a/spec/controllers/proposals_controller_spec.rb +++ b/spec/controllers/proposals_controller_spec.rb @@ -8,38 +8,4 @@ describe ProposalsController do expect { get :index }.to raise_exception(FeatureFlags::FeatureDisabled) end end - - describe "PUT update" do - let(:file) { Rails.root.join("spec/hacked") } - - before do - File.delete(file) if File.exist?(file) - InvisibleCaptcha.timestamp_enabled = false - end - - after { InvisibleCaptcha.timestamp_enabled = true } - - it "ignores malicious cached attachments with remote storages" do - allow_any_instance_of(Image).to receive(:filesystem_storage?).and_return(false) - user = create(:user) - proposal = create(:proposal, author: user) - sign_in user - - begin - put :update, params: { - id: proposal, - proposal: { - image_attributes: { - title: "Hacked!", - user_id: user.id, - cached_attachment: "| touch #{file}" - } - } - } - rescue StandardError - ensure - expect(file).not_to exist - end - end - end end diff --git a/spec/factories/administration.rb b/spec/factories/administration.rb index a59f30776..07a110ca5 100644 --- a/spec/factories/administration.rb +++ b/spec/factories/administration.rb @@ -58,7 +58,7 @@ FactoryBot.define do end factory :site_customization_image, class: "SiteCustomization::Image" do - image { File.new("spec/fixtures/files/logo_header.png") } + image { Rack::Test::UploadedFile.new("spec/fixtures/files/logo_header.png") } name { "logo_header" } end diff --git a/spec/factories/files.rb b/spec/factories/files.rb index 0926ee576..5876c90c8 100644 --- a/spec/factories/files.rb +++ b/spec/factories/files.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :image do - attachment { File.new("spec/fixtures/files/clippy.jpg") } + attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.jpg") } title { "Lorem ipsum dolor sit amet" } association :user, factory: :user @@ -16,7 +16,7 @@ FactoryBot.define do factory :document do sequence(:title) { |n| "Document title #{n}" } association :user, factory: :user - attachment { File.new("spec/fixtures/files/empty.pdf") } + attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/empty.pdf") } trait :proposal_document do association :documentable, factory: :proposal @@ -47,11 +47,11 @@ FactoryBot.define do trait :documents do resource_relation { "documents" } - attachment { File.new("spec/fixtures/files/empty.pdf") } + attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/empty.pdf") } end trait :image do resource_relation { "image" } - attachment { File.new("spec/fixtures/files/clippy.jpg") } + attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.jpg") } end initialize_with { new(attributes) } end diff --git a/spec/fixtures/files/apple-touch-icon-custom-200.png b/spec/fixtures/files/apple-touch-icon-custom-200.png new file mode 100644 index 000000000..61216ad39 Binary files /dev/null and b/spec/fixtures/files/apple-touch-icon-custom-200.png differ diff --git a/spec/lib/remote_census_api_spec.rb b/spec/lib/remote_census_api_spec.rb index 2e25b6e1e..616dfdce5 100644 --- a/spec/lib/remote_census_api_spec.rb +++ b/spec/lib/remote_census_api_spec.rb @@ -26,7 +26,7 @@ describe RemoteCensusApi do end describe "request messages" do - let(:valid_response) { File.read("spec/fixtures/files/remote_census_api/valid.xml") } + let(:valid_response) { File.read(file_fixture("remote_census_api/valid.xml")) } def request_with(params) { "request" => params } diff --git a/spec/lib/tasks/active_storage_spec.rb b/spec/lib/tasks/active_storage_spec.rb index f554d05de..bf394d84d 100644 --- a/spec/lib/tasks/active_storage_spec.rb +++ b/spec/lib/tasks/active_storage_spec.rb @@ -1,88 +1,67 @@ require "rails_helper" describe "active storage tasks" do - describe "migrate_from_paperclip" do + describe "remove_paperclip_compatibility_in_existing_attachments" do let(:run_rake_task) do - Rake::Task["active_storage:migrate_from_paperclip"].reenable - Rake.application.invoke_task("active_storage:migrate_from_paperclip") + Rake::Task["active_storage:remove_paperclip_compatibility_in_existing_attachments"].reenable + Rake.application.invoke_task("active_storage:remove_paperclip_compatibility_in_existing_attachments") end - let(:storage_root) { ActiveStorage::Blob.service.root } - before { FileUtils.rm_rf storage_root } + it "updates old regular attachments" do + document = create(:document) + image = create(:image) + site_customization_image = create(:site_customization_image) - it "migrates records and attachments" do - document = create(:document, - attachment: nil, - paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf")) + document.attachment.attachment.update_column(:name, "storage_attachment") + image.attachment.attachment.update_column(:name, "storage_attachment") + site_customization_image.image.attachment.update_column(:name, "storage_image") - expect(ActiveStorage::Attachment.count).to eq 0 - expect(ActiveStorage::Blob.count).to eq 0 - expect(test_storage_file_paths.count).to eq 0 + document.reload + image.reload + site_customization_image.reload + + expect(document.attachment.attachment).to be nil + expect(image.attachment.attachment).to be nil + expect(site_customization_image.image.attachment).to be nil run_rake_task document.reload + image.reload + site_customization_image.reload - expect(ActiveStorage::Attachment.count).to eq 1 - expect(ActiveStorage::Blob.count).to eq 1 - expect(document.storage_attachment.filename).to eq "clippy.pdf" - expect(test_storage_file_paths.count).to eq 1 - expect(storage_file_path(document)).to eq test_storage_file_paths.first + expect(document.attachment.attachment.name).to eq "attachment" + expect(image.attachment.attachment.name).to eq "attachment" + expect(site_customization_image.reload.image.attachment.name).to eq "image" end - it "migrates records with deleted files ignoring the files" do - document = create(:document, - attachment: nil, - paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf")) - FileUtils.rm(document.attachment.path) + it "does not modify old ckeditor attachments" do + image = Ckeditor::Picture.create!(data: fixture_file_upload("clippy.png")) + + expect(image.storage_data.attachment.name).to eq "storage_data" + + run_rake_task + image.reload + + expect(image.storage_data.attachment.name).to eq "storage_data" + end + + it "does not modify new regular attachments" do + document = create(:document) + image = create(:image) + site_customization_image = create(:site_customization_image) + + expect(document.attachment.attachment.name).to eq "attachment" + expect(image.attachment.attachment.name).to eq "attachment" + expect(site_customization_image.image.attachment.name).to eq "image" run_rake_task document.reload + image.reload + site_customization_image.reload - expect(ActiveStorage::Attachment.count).to eq 1 - expect(ActiveStorage::Blob.count).to eq 1 - expect(document.storage_attachment.filename).to eq "clippy.pdf" - expect(test_storage_file_paths.count).to eq 0 - end - - it "does not migrate already migrated records" do - document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf")) - - migrated_file = test_storage_file_paths.first - attachment_id = document.storage_attachment.attachment.id - blob_id = document.storage_attachment.blob.id - - run_rake_task - document.reload - - expect(ActiveStorage::Attachment.count).to eq 1 - expect(ActiveStorage::Blob.count).to eq 1 - expect(document.storage_attachment.attachment.id).to eq attachment_id - expect(document.storage_attachment.blob.id).to eq blob_id - - expect(test_storage_file_paths.count).to eq 1 - expect(storage_file_path(document)).to eq migrated_file - expect(test_storage_file_paths.first).to eq migrated_file - end - - it "does not migrate files for deleted records" do - document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf")) - FileUtils.rm storage_file_path(document) - Document.delete_all - - run_rake_task - - expect(ActiveStorage::Attachment.count).to eq 1 - expect(ActiveStorage::Blob.count).to eq 1 - expect(document.storage_attachment.filename).to eq "clippy.pdf" - expect(test_storage_file_paths.count).to eq 0 - end - - def test_storage_file_paths - Dir.glob("#{storage_root}/**/*").select { |file_or_folder| File.file?(file_or_folder) } - end - - def storage_file_path(record) - ActiveStorage::Blob.service.path_for(record.storage_attachment.blob.key) + expect(document.attachment.attachment.name).to eq "attachment" + expect(image.attachment.attachment.name).to eq "attachment" + expect(site_customization_image.reload.image.attachment.name).to eq "image" end end end diff --git a/spec/lib/tasks/files_spec.rb b/spec/lib/tasks/files_spec.rb index 2c464bc82..4d55334bb 100644 --- a/spec/lib/tasks/files_spec.rb +++ b/spec/lib/tasks/files_spec.rb @@ -11,26 +11,26 @@ describe "files tasks" do image = build(:image) document = build(:document) - image.attachment.save - document.attachment.save + image.attachment.blob.save! + document.attachment.blob.save! travel_to(2.days.from_now) { run_rake_task } - expect(File.exists?(image.attachment.path)).to be false - expect(File.exists?(document.attachment.path)).to be false + expect(File.exists?(image.file_path)).to be false + expect(File.exists?(document.file_path)).to be false end it "does not delete recent cached attachments" do image = build(:image) document = build(:document) - image.attachment.save - document.attachment.save + image.attachment.blob.save! + document.attachment.blob.save! travel_to(2.minutes.from_now) { run_rake_task } - expect(File.exists?(image.attachment.path)).to be true - expect(File.exists?(document.attachment.path)).to be true + expect(File.exists?(image.file_path)).to be true + expect(File.exists?(document.file_path)).to be true end it "does not delete old regular attachments" do @@ -39,8 +39,8 @@ describe "files tasks" do travel_to(2.days.from_now) { run_rake_task } - expect(File.exists?(image.attachment.path)).to be true - expect(File.exists?(document.attachment.path)).to be true + expect(File.exists?(image.file_path)).to be true + expect(File.exists?(document.file_path)).to be true end end end diff --git a/spec/models/direct_upload_spec.rb b/spec/models/direct_upload_spec.rb index 9c5657d55..ec342f0de 100644 --- a/spec/models/direct_upload_spec.rb +++ b/spec/models/direct_upload_spec.rb @@ -10,10 +10,10 @@ describe DirectUpload do end it "is not valid for different kind of combinations with invalid atttachment content types" do - expect(build(:direct_upload, :proposal, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid - expect(build(:direct_upload, :proposal, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid - expect(build(:direct_upload, :budget_investment, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid - expect(build(:direct_upload, :budget_investment, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid + expect(build(:direct_upload, :proposal, :documents, attachment: fixture_file_upload("clippy.png"))).not_to be_valid + expect(build(:direct_upload, :proposal, :image, attachment: fixture_file_upload("empty.pdf"))).not_to be_valid + expect(build(:direct_upload, :budget_investment, :documents, attachment: fixture_file_upload("clippy.png"))).not_to be_valid + expect(build(:direct_upload, :budget_investment, :image, attachment: fixture_file_upload("empty.pdf"))).not_to be_valid end it "is not valid without resource_type" do @@ -34,13 +34,14 @@ describe DirectUpload do end context "save_attachment" do - it "saves uploaded file" do - proposal_document_direct_upload = build(:direct_upload, :proposal, :documents) + it "saves uploaded file without creating an attachment record" do + direct_upload = build(:direct_upload, :proposal, :documents) - proposal_document_direct_upload.save_attachment + direct_upload.save_attachment - expect(File.exist?(proposal_document_direct_upload.relation.attachment.path)).to eq(true) - expect(proposal_document_direct_upload.relation.attachment.path).to include("cached_attachments") + expect(File.exist?(direct_upload.relation.file_path)).to be true + expect(direct_upload.relation.attachment.blob).to be_persisted + expect(direct_upload.relation.attachment.attachment).not_to be_persisted end end end diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb index 2f8cf65fe..0180c4010 100644 --- a/spec/models/document_spec.rb +++ b/spec/models/document_spec.rb @@ -4,14 +4,11 @@ describe Document do it_behaves_like "document validations", "budget_investment_document" it_behaves_like "document validations", "proposal_document" - it "stores attachments with both Paperclip and Active Storage" do - document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf")) + it "stores attachments with Active Storage" do + document = create(:document, attachment: fixture_file_upload("clippy.pdf")) - expect(document.attachment).to exist - expect(document.attachment_file_name).to eq "clippy.pdf" - - expect(document.storage_attachment).to be_attached - expect(document.storage_attachment.filename).to eq "clippy.pdf" + expect(document.attachment).to be_attached + expect(document.attachment.filename).to eq "clippy.pdf" end context "scopes" do diff --git a/spec/models/image_spec.rb b/spec/models/image_spec.rb index d119c8138..6b4603923 100644 --- a/spec/models/image_spec.rb +++ b/spec/models/image_spec.rb @@ -4,13 +4,10 @@ describe Image do it_behaves_like "image validations", "budget_investment_image" it_behaves_like "image validations", "proposal_image" - it "stores attachments with both Paperclip and Active Storage" do - image = create(:image, attachment: File.new("spec/fixtures/files/clippy.jpg")) + it "stores attachments with Active Storage" do + image = create(:image, attachment: fixture_file_upload("clippy.jpg")) - expect(image.attachment).to exist - expect(image.attachment_file_name).to eq "clippy.jpg" - - expect(image.storage_attachment).to be_attached - expect(image.storage_attachment.filename).to eq "clippy.jpg" + expect(image.attachment).to be_attached + expect(image.attachment.filename).to eq "clippy.jpg" end end diff --git a/spec/models/local_census_records/import_spec.rb b/spec/models/local_census_records/import_spec.rb index 3aaca8a11..7a97f0df5 100644 --- a/spec/models/local_census_records/import_spec.rb +++ b/spec/models/local_census_records/import_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe LocalCensusRecords::Import do - let(:base_files_path) { %w[spec fixtures files local_census_records import] } + let(:base_files_path) { "local_census_records/import/" } let(:import) { build(:local_census_records_import) } describe "Validations" do @@ -17,7 +17,7 @@ describe LocalCensusRecords::Import do context "When file extension" do it "is wrong it should not be valid" do - file = Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.gif") + file = fixture_file_upload("clippy.gif") import = build(:local_census_records_import, file: file) expect(import).not_to be_valid @@ -25,7 +25,7 @@ describe LocalCensusRecords::Import do it "is csv it should be valid" do path = base_files_path << "valid.csv" - file = Rack::Test::UploadedFile.new(Rails.root.join(*path)) + file = fixture_file_upload(path) import = build(:local_census_records_import, file: file) expect(import).to be_valid @@ -35,7 +35,7 @@ describe LocalCensusRecords::Import do context "When file headers" do it "are all missing it should not be valid" do path = base_files_path << "valid-without-headers.csv" - file = Rack::Test::UploadedFile.new(Rails.root.join(*path)) + file = fixture_file_upload(path) import = build(:local_census_records_import, file: file) expect(import).not_to be_valid @@ -64,7 +64,7 @@ describe LocalCensusRecords::Import do it "Add invalid local census records to invalid_records array" do path = base_files_path << "invalid.csv" - file = Rack::Test::UploadedFile.new(Rails.root.join(*path)) + file = fixture_file_upload(path) import.file = file import.save! diff --git a/spec/models/site_customization/image_spec.rb b/spec/models/site_customization/image_spec.rb index ddd266d0e..076fc01d5 100644 --- a/spec/models/site_customization/image_spec.rb +++ b/spec/models/site_customization/image_spec.rb @@ -1,22 +1,19 @@ require "rails_helper" describe SiteCustomization::Image do - it "stores images with both Paperclip and Active Storage" do + it "stores images with Active Storage" do image = create(:site_customization_image, name: "map", - image: File.new("spec/fixtures/files/custom_map.jpg")) + image: fixture_file_upload("custom_map.jpg")) - expect(image.image).to exist - expect(image.image_file_name).to eq "custom_map.jpg" - - expect(image.storage_image).to be_attached - expect(image.storage_image.filename).to eq "custom_map.jpg" + expect(image.image).to be_attached + expect(image.image.filename).to eq "custom_map.jpg" end describe "logo" do it "is valid with a 260x80 image" do image = build(:site_customization_image, name: "logo_header", - image: File.new("spec/fixtures/files/logo_header-260x80.png")) + image: fixture_file_upload("logo_header-260x80.png")) expect(image).to be_valid end @@ -24,7 +21,7 @@ describe SiteCustomization::Image do it "is valid with a 223x80 image" do image = build(:site_customization_image, name: "logo_header", - image: File.new("spec/fixtures/files/logo_header.png")) + image: fixture_file_upload("logo_header.png")) expect(image).to be_valid end @@ -32,7 +29,7 @@ describe SiteCustomization::Image do it "is not valid with a 400x80 image" do image = build(:site_customization_image, name: "logo_header", - image: File.new("spec/fixtures/files/logo_email_custom.png")) + image: fixture_file_upload("logo_email_custom.png")) expect(image).not_to be_valid end diff --git a/spec/shared/models/acts_as_imageable.rb b/spec/shared/models/acts_as_imageable.rb index b4624372d..6e9810538 100644 --- a/spec/shared/models/acts_as_imageable.rb +++ b/spec/shared/models/acts_as_imageable.rb @@ -7,21 +7,21 @@ shared_examples "acts as imageable" do |imageable_factory| describe "file extension" do it "is not valid with '.png' extension" do - image.attachment = File.new("spec/fixtures/files/clippy.png") + image.attachment = fixture_file_upload("clippy.png") expect(image).not_to be_valid expect(image.errors[:attachment].size).to eq(1) end it "is not valid with '.gif' extension" do - image.attachment = File.new("spec/fixtures/files/clippy.gif") + image.attachment = fixture_file_upload("clippy.gif") expect(image).not_to be_valid expect(image.errors[:attachment].size).to eq(1) end it "is valid with '.jpg' extension" do - image.attachment = File.new("spec/fixtures/files/clippy.jpg") + image.attachment = fixture_file_upload("clippy.jpg") expect(image).to be_valid end @@ -33,7 +33,7 @@ shared_examples "acts as imageable" do |imageable_factory| end it "is not valid when image dimmensions are smaller than 475X475" do - image.attachment = File.new("spec/fixtures/files/logo_header.jpg") + image.attachment = fixture_file_upload("logo_header.jpg") expect(image).not_to be_valid end diff --git a/spec/shared/models/document_validations.rb b/spec/shared/models/document_validations.rb index 194c8fbba..11235f771 100644 --- a/spec/shared/models/document_validations.rb +++ b/spec/shared/models/document_validations.rb @@ -22,14 +22,14 @@ shared_examples "document validations" do |documentable_factory| it "is valid for all accepted content types" do acceptedcontenttypes.each do |content_type| extension = content_type.split("/").last - document.attachment = File.new("spec/fixtures/files/empty.#{extension}") + document.attachment = fixture_file_upload("empty.#{extension}") expect(document).to be_valid end end it "is not valid for attachments larger than documentable max_file_size definition" do - allow(document).to receive(:attachment_file_size).and_return(maxfilesize.megabytes + 1.byte) + allow(document.attachment).to receive(:byte_size).and_return(maxfilesize.megabytes + 1.byte) max_size_error_message = "must be in between 0 Bytes and #{maxfilesize} MB" expect(document).not_to be_valid diff --git a/spec/shared/models/image_validations.rb b/spec/shared/models/image_validations.rb index 092372550..5d1245904 100644 --- a/spec/shared/models/image_validations.rb +++ b/spec/shared/models/image_validations.rb @@ -21,7 +21,7 @@ shared_examples "image validations" do |imageable_factory| it "is valid for all accepted content types" do acceptedcontenttypes.each do |content_type| extension = content_type.split("/").last - image.attachment = File.new("spec/fixtures/files/clippy.#{extension}") + image.attachment = fixture_file_upload("clippy.#{extension}") expect(image).to be_valid end @@ -30,7 +30,7 @@ shared_examples "image validations" do |imageable_factory| it "is not valid for png and gif image content types" do ["gif", "png"].each do |content_type| extension = content_type.split("/").last - image.attachment = File.new("spec/fixtures/files/clippy.#{extension}") + image.attachment = fixture_file_upload("clippy.#{extension}") expect(image).not_to be_valid end @@ -38,7 +38,7 @@ shared_examples "image validations" do |imageable_factory| it "is not valid for attachments larger than imageable max_file_size definition" do larger_size = Setting["uploads.images.max_size"].to_i.megabytes + 1.byte - allow(image).to receive(:attachment_file_size).and_return(larger_size) + allow(image.attachment).to receive(:byte_size).and_return(larger_size) expect(image).not_to be_valid expect(image.errors[:attachment]).to include "must be in between 0 Bytes and 1 MB" diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index cb3d0d6bb..51cba390f 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/spec/shared/system/nested_documentable.rb @@ -51,7 +51,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) documentable.class.max_documents_allowed.times.each do - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) end expect(page).to have_css ".max-documents-notice" @@ -77,7 +77,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na click_link "Add new document" within "#nested-documents" do - attach_file "Choose document", Rails.root.join("spec/fixtures/files/empty.pdf") + attach_file "Choose document", file_fixture("empty.pdf") expect(page).to have_css ".loading-bar.complete" end @@ -90,7 +90,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) expect_document_has_title(0, "empty.pdf") end @@ -104,7 +104,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na within "#nested-documents" do input = find("input[name$='[title]']") fill_in input[:id], with: "My Title" - attach_file "Choose document", Rails.root.join("spec/fixtures/files/empty.pdf") + attach_file "Choose document", file_fixture("empty.pdf") expect(page).to have_css ".loading-bar.complete" end @@ -116,7 +116,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) expect(page).to have_css ".loading-bar.complete" end @@ -125,10 +125,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file( - Rails.root.join("spec/fixtures/files/logo_header.gif"), - false - ) + documentable_attach_new_file(file_fixture("logo_header.gif"), false) expect(page).to have_css ".loading-bar.errors" end @@ -137,21 +134,25 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + click_link "Add new document" - expect_document_has_cached_attachment(0, ".pdf") + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose document", file_fixture("empty.pdf") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty end scenario "Should not update document cached_attachment field after invalid file upload" do do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file( - Rails.root.join("spec/fixtures/files/logo_header.gif"), - false - ) + documentable_attach_new_file(file_fixture("logo_header.gif"), false) - expect_document_has_cached_attachment(0, "") + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty end scenario "Should show document errors after documentable submit with @@ -171,7 +172,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na do_login_for user_to_login visit send(path, arguments) - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) click_link "Remove document" expect(page).not_to have_css("#nested-documents .document") @@ -194,7 +195,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) click_on submit_button expect(page).to have_content documentable_success_notice @@ -208,7 +209,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) click_on submit_button documentable_redirected_to_resource_show_or_navigate_to @@ -232,7 +233,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na send(fill_resource_method_name) if fill_resource_method_name %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/#{filename}.pdf")) + documentable_attach_new_file(file_fixture("#{filename}.pdf")) end click_on submit_button @@ -279,6 +280,26 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).not_to have_css ".document" end + + scenario "Same attachment URL after editing the title" do + do_login_for user_to_login + + visit send(path, arguments) + documentable_attach_new_file(file_fixture("empty.pdf")) + within_fieldset("Documents") { fill_in "Title", with: "Original" } + click_button submit_button + + expect(page).to have_content documentable_success_notice + + original_url = find_link("Download file")[:href] + + visit send(path, arguments) + within_fieldset("Documents") { fill_in "Title", with: "Updated" } + click_button submit_button + + expect(page).to have_content documentable_success_notice + expect(find_link("Download file")[:href]).to eq original_url + end end describe "When allow attached documents setting is disabled" do @@ -330,14 +351,6 @@ def expect_document_has_title(index, title) end end -def expect_document_has_cached_attachment(index, extension) - document = all(".document")[index] - - within document do - expect(find("input[name$='[cached_attachment]']", visible: :hidden).value).to end_with(extension) - end -end - def documentable_fill_new_valid_proposal fill_in_new_proposal_title with: "Proposal title #{rand(9999)}" fill_in "Proposal summary", with: "Proposal summary" diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb index 57c2329ec..123a54425 100644 --- a/spec/shared/system/nested_imageable.rb +++ b/spec/shared/system/nested_imageable.rb @@ -38,7 +38,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p visit send(path, arguments) click_link "Add image" - attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg") + attach_file "Choose image", file_fixture("clippy.jpg") expect(page).to have_selector ".file-name", text: "clippy.jpg" end @@ -47,7 +47,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) expect_image_has_title("clippy.jpg") end @@ -59,7 +59,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p click_link "Add image" input_title = find(".image input[name$='[title]']") fill_in input_title[:id], with: "Title" - attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg") + attach_file "Choose image", file_fixture("clippy.jpg") if has_many_images expect(find("input[id$='_title']").value).to eq "Title" @@ -72,7 +72,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) expect(page).to have_selector ".loading-bar.complete" end @@ -81,7 +81,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/logo_header.png"), false) + imageable_attach_new_file(file_fixture("logo_header.png"), false) expect(page).to have_selector ".loading-bar.errors" end @@ -90,18 +90,26 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + click_link "Add image" - expect_image_has_cached_attachment(".jpg") + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty end scenario "Should not update image cached_attachment field after invalid file upload" do do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/logo_header.png"), false) + imageable_attach_new_file(file_fixture("logo_header.png"), false) - expect_image_has_cached_attachment("") + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + + expect(cached_attachment_field.value).to be_empty end scenario "Should show nested image errors after invalid form submit" do @@ -120,11 +128,24 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p end end + scenario "Render image preview after sending the form with validation errors" do + skip "Question answers behave differently" if imageable.is_a?(Poll::Question::Answer) + do_login_for user + visit send(path, arguments) + + imageable_attach_new_file(file_fixture("clippy.jpg")) + within_fieldset("Descriptive image") { fill_in "Title", with: "" } + click_on submit_button + + expect(page).to have_content "can't be blank" + expect(page).to have_css "img[src$='clippy.jpg']" + end + scenario "Should remove nested image after valid file upload and click on remove button" do do_login_for user visit send(path, arguments) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) within "#nested-image .image" do click_link "Remove image" @@ -151,7 +172,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) expect(page).to have_selector ".loading-bar.complete" @@ -165,7 +186,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) expect(page).to have_selector ".loading-bar.complete" @@ -180,6 +201,22 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p end end + scenario "Different URLs for different images" do + do_login_for user + visit send(path, arguments) + + imageable_attach_new_file(file_fixture("clippy.jpg")) + + original_src = find(:fieldset, "Descriptive image").find("img")[:src] + + click_link "Remove image" + imageable_attach_new_file(file_fixture("custom_map.jpg")) + + updated_src = find(:fieldset, "Descriptive image").find("img")[:src] + + expect(updated_src).not_to eq original_src + end + if path.include? "edit" scenario "show persisted image" do create(:image, imageable: imageable) @@ -268,16 +305,6 @@ def expect_image_has_title(title) end end -def expect_image_has_cached_attachment(extension) - within "#nested-image" do - image = find(".image") - - within image do - expect(find("input[name$='[cached_attachment]']", visible: :hidden).value).to end_with(extension) - end - end -end - def show_caption_for?(imageable_factory_name) imageable_factory_name != "budget" end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b89e55699..b8b155795 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,6 +9,7 @@ Dir["./spec/shared/**/*.rb"].sort.each { |f| require f } RSpec.configure do |config| config.use_transactional_fixtures = true + config.fixture_path = "spec/fixtures/files" config.filter_run_when_matching :focus config.include RequestSpecHelper, type: :request diff --git a/spec/support/common_actions/remote_census_mock.rb b/spec/support/common_actions/remote_census_mock.rb index b140c17e4..0ec9aac17 100644 --- a/spec/support/common_actions/remote_census_mock.rb +++ b/spec/support/common_actions/remote_census_mock.rb @@ -5,15 +5,15 @@ module RemoteCensusMock include DocumentParser def mock_valid_remote_census_response - mock_remote_census_response(File.read("spec/fixtures/files/remote_census_api/valid.xml")) + mock_remote_census_response(File.read(file_fixture("remote_census_api/valid.xml"))) end def mock_invalid_remote_census_response - mock_remote_census_response(File.read("spec/fixtures/files/remote_census_api/invalid.xml")) + mock_remote_census_response(File.read(file_fixture("remote_census_api/invalid.xml"))) end def mock_invalid_signature_sheet_remote_census_response - xml = File.read("spec/fixtures/files/remote_census_api/invalid.xml") + xml = File.read(file_fixture("remote_census_api/invalid.xml")) Signature.new.document_types.each do |document_type| get_document_number_variants(document_type, "12345678Z").each do diff --git a/spec/system/admin/budget_phases_spec.rb b/spec/system/admin/budget_phases_spec.rb index 0d07bcc2f..257b33141 100644 --- a/spec/system/admin/budget_phases_spec.rb +++ b/spec/system/admin/budget_phases_spec.rb @@ -46,7 +46,7 @@ describe "Admin budget phases" do scenario "shows successful notice when updating the phase with a valid image" do visit edit_admin_budget_budget_phase_path(budget, budget.current_phase) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) click_on "Save changes" diff --git a/spec/system/admin/legislation/processes_spec.rb b/spec/system/admin/legislation/processes_spec.rb index d19a3a16c..ca001bbf1 100644 --- a/spec/system/admin/legislation/processes_spec.rb +++ b/spec/system/admin/legislation/processes_spec.rb @@ -175,7 +175,7 @@ describe "Admin collaborative legislation", :admin do fill_in "End", with: base_date + 5.days end - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) click_button "Create process" diff --git a/spec/system/admin/local_census_records/imports_spec.rb b/spec/system/admin/local_census_records/imports_spec.rb index 29154ebbb..a5d39d4dd 100644 --- a/spec/system/admin/local_census_records/imports_spec.rb +++ b/spec/system/admin/local_census_records/imports_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "Imports", :admin do - let(:base_files_path) { %w[spec fixtures files local_census_records import] } + let(:base_files_path) { "local_census_records/import/" } describe "New" do scenario "Should show import form" do @@ -17,7 +17,7 @@ describe "Imports", :admin do scenario "Should show success notice after successful import" do within "form#new_local_census_records_import" do path = base_files_path << "valid.csv" - file = File.join(Rails.root, *path) + file = file_fixture(path) attach_file("local_census_records_import_file", file) click_button "Save" end @@ -35,8 +35,7 @@ describe "Imports", :admin do scenario "Should show alert when file is not supported" do within "form#new_local_census_records_import" do - path = ["spec", "fixtures", "files", "clippy.jpg"] - file = File.join(Rails.root, *path) + file = file_fixture("clippy.jpg") attach_file("local_census_records_import_file", file) click_button "Save" end @@ -47,7 +46,7 @@ describe "Imports", :admin do scenario "Should show successfully created local census records at created group" do within "form#new_local_census_records_import" do path = base_files_path << "valid.csv" - file = File.join(Rails.root, *path) + file = file_fixture(path) attach_file("local_census_records_import_file", file) click_button "Save" end @@ -59,7 +58,7 @@ describe "Imports", :admin do scenario "Should show invalid local census records at errored group" do within "form#new_local_census_records_import" do path = base_files_path << "invalid.csv" - file = File.join(Rails.root, *path) + file = file_fixture(path) attach_file("local_census_records_import_file", file) click_button "Save" end @@ -71,7 +70,7 @@ describe "Imports", :admin do scenario "Should show error messages inside cells at errored group" do within "form#new_local_census_records_import" do path = base_files_path << "invalid.csv" - file = File.join(Rails.root, *path) + file = file_fixture(path) attach_file("local_census_records_import_file", file) click_button "Save" end diff --git a/spec/system/admin/poll/questions/answers/images/images_spec.rb b/spec/system/admin/poll/questions/answers/images/images_spec.rb index 2d8049f1f..154e2e8fd 100644 --- a/spec/system/admin/poll/questions/answers/images/images_spec.rb +++ b/spec/system/admin/poll/questions/answers/images/images_spec.rb @@ -38,7 +38,7 @@ describe "Images", :admin do expect(page).not_to have_content("clippy.jpg") visit new_admin_answer_image_path(answer) - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) click_button "Save image" expect(page).to have_css("img[title='clippy.jpg']") diff --git a/spec/system/admin/site_customization/documents_spec.rb b/spec/system/admin/site_customization/documents_spec.rb index f0ff988d9..5e86779d6 100644 --- a/spec/system/admin/site_customization/documents_spec.rb +++ b/spec/system/admin/site_customization/documents_spec.rb @@ -18,12 +18,12 @@ describe "Documents", :admin do 1.times { create(:document) } document = Document.first - attachment = document.attachment + url = polymorphic_path(document.attachment) visit admin_site_customization_documents_path expect(page).to have_content "There are 3 documents" - expect(page).to have_link document.title, href: attachment.url + expect(page).to have_link document.title, href: url end scenario "Index (empty)" do @@ -54,7 +54,7 @@ describe "Documents", :admin do scenario "Create" do visit new_admin_site_customization_document_path - attach_file("document_attachment", "#{Rails.root}/spec/fixtures/files/logo.pdf") + attach_file("document_attachment", file_fixture("logo.pdf")) click_button "Upload" expect(page).to have_content "Document uploaded succesfully" diff --git a/spec/system/admin/site_customization/images_spec.rb b/spec/system/admin/site_customization/images_spec.rb index 26f1b13c4..85f3736ee 100644 --- a/spec/system/admin/site_customization/images_spec.rb +++ b/spec/system/admin/site_customization/images_spec.rb @@ -10,7 +10,7 @@ describe "Admin custom images", :admin do end within("tr#image_logo_header") do - attach_file "site_customization_image_image", "spec/fixtures/files/logo_header.png" + attach_file "site_customization_image_image", file_fixture("logo_header.png") click_button "Update" end @@ -22,7 +22,7 @@ describe "Admin custom images", :admin do visit admin_site_customization_images_path within("tr#image_map") do - attach_file "site_customization_image_image", "spec/fixtures/files/custom_map.jpg" + attach_file "site_customization_image_image", file_fixture("custom_map.jpg") click_button "Update" end @@ -37,7 +37,7 @@ describe "Admin custom images", :admin do visit admin_site_customization_images_path within("tr#image_map") do - attach_file "site_customization_image_image", "spec/fixtures/files/custom_map.jpg" + attach_file "site_customization_image_image", file_fixture("custom_map.jpg") click_button "Update" end @@ -60,13 +60,24 @@ describe "Admin custom images", :admin do end end + scenario "Custom apple touch icon is replaced on front views" do + create(:site_customization_image, + name: "apple-touch-icon-200", + image: fixture_file_upload("apple-touch-icon-custom-200.png")) + + visit root_path + + expect(page).not_to have_css("link[href*='apple-touch-icon-200']", visible: :all) + expect(page).to have_css("link[href*='apple-touch-icon-custom-200']", visible: :hidden) + end + scenario "Image is replaced on admin newsletters" do newsletter = create(:newsletter, segment_recipient: "all_users") visit admin_site_customization_images_path within("tr#image_logo_email") do - attach_file "site_customization_image_image", "spec/fixtures/files/logo_email_custom.png" + attach_file "site_customization_image_image", file_fixture("logo_email_custom.png") click_button "Update" end @@ -81,7 +92,7 @@ describe "Admin custom images", :admin do visit admin_site_customization_images_path within("tr#image_social_media_icon") do - attach_file "site_customization_image_image", "spec/fixtures/files/logo_header.png" + attach_file "site_customization_image_image", file_fixture("logo_header.png") click_button "Update" end @@ -93,7 +104,7 @@ describe "Admin custom images", :admin do visit admin_site_customization_images_path within("tr#image_social_media_icon") do - attach_file "site_customization_image_image", "spec/fixtures/files/social_media_icon.png" + attach_file "site_customization_image_image", file_fixture("social_media_icon.png") click_button "Update" end diff --git a/spec/system/admin/widgets/cards_spec.rb b/spec/system/admin/widgets/cards_spec.rb index 650e374b3..ed299dd03 100644 --- a/spec/system/admin/widgets/cards_spec.rb +++ b/spec/system/admin/widgets/cards_spec.rb @@ -251,7 +251,7 @@ describe "Cards", :admin do def attach_image_to_card click_link "Add image" - attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg") + attach_file "Choose image", file_fixture("clippy.jpg") expect(page).to have_field("widget_card_image_attributes_title", with: "clippy.jpg") end diff --git a/spec/system/ckeditor_spec.rb b/spec/system/ckeditor_spec.rb index 03fc745b0..44d2a1096 100644 --- a/spec/system/ckeditor_spec.rb +++ b/spec/system/ckeditor_spec.rb @@ -33,7 +33,7 @@ describe "CKEditor" do click_link "Upload" within_frame(1) do - attach_file "Send it to the Server", Rails.root.join("spec/fixtures/files/clippy.jpg") + attach_file "Send it to the Server", file_fixture("clippy.jpg") end click_link "Send it to the Server" diff --git a/spec/system/legislation/proposals_spec.rb b/spec/system/legislation/proposals_spec.rb index cfeac71fa..80eef133f 100644 --- a/spec/system/legislation/proposals_spec.rb +++ b/spec/system/legislation/proposals_spec.rb @@ -146,7 +146,7 @@ describe "Legislation Proposals" do fill_in "Proposal title", with: "Legislation proposal with image" fill_in "Proposal summary", with: "Including an image on a legislation proposal" - imageable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.jpg")) + imageable_attach_new_file(file_fixture("clippy.jpg")) check "legislation_proposal_terms_of_service" click_button "Create proposal"