diff --git a/app/controllers/administrateurs/attestation_template_v2s_controller.rb b/app/controllers/administrateurs/attestation_template_v2s_controller.rb index ce926e84b..e44198380 100644 --- a/app/controllers/administrateurs/attestation_template_v2s_controller.rb +++ b/app/controllers/administrateurs/attestation_template_v2s_controller.rb @@ -2,8 +2,6 @@ module Administrateurs class AttestationTemplateV2sController < AdministrateurController - include UninterlacePngConcern - before_action :retrieve_procedure before_action :ensure_feature_active before_action :retrieve_attestation_template @@ -74,17 +72,6 @@ module Administrateurs @attestation_template.procedure = @procedure end - logo_file = attestation_params.delete(:logo) - signature_file = attestation_params.delete(:signature) - - if logo_file - attestation_params[:logo] = uninterlace_png(logo_file) - end - - if signature_file - attestation_params[:signature] = uninterlace_png(signature_file) - end - @attestation_template.assign_attributes(attestation_params) if @attestation_template.invalid? diff --git a/app/controllers/administrateurs/attestation_templates_controller.rb b/app/controllers/administrateurs/attestation_templates_controller.rb index d703b99b2..96f611f54 100644 --- a/app/controllers/administrateurs/attestation_templates_controller.rb +++ b/app/controllers/administrateurs/attestation_templates_controller.rb @@ -2,8 +2,6 @@ module Administrateurs class AttestationTemplatesController < AdministrateurController - include UninterlacePngConcern - before_action :retrieve_procedure before_action :preload_revisions @@ -63,16 +61,6 @@ module Administrateurs if @activated_attestation_params.nil? @activated_attestation_params = params.require(:attestation_template) .permit(:title, :body, :footer, :activated, :logo, :signature) - - logo_file = params['attestation_template'].delete('logo') - signature_file = params['attestation_template'].delete('signature') - - if logo_file.present? - @activated_attestation_params[:logo] = uninterlace_png(logo_file) - end - if signature_file.present? - @activated_attestation_params[:signature] = uninterlace_png(signature_file) - end end @activated_attestation_params diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 722c27b32..759bfe30c 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -5,7 +5,6 @@ module Administrateurs include ActiveSupport::NumberHelper include EmailSanitizableConcern include Logic - include UninterlacePngConcern include GroupeInstructeursSignatureConcern before_action :ensure_not_super_admin!, only: [:add_instructeur] diff --git a/app/controllers/concerns/groupe_instructeurs_signature_concern.rb b/app/controllers/concerns/groupe_instructeurs_signature_concern.rb index e7c843a4a..4e2fee3c0 100644 --- a/app/controllers/concerns/groupe_instructeurs_signature_concern.rb +++ b/app/controllers/concerns/groupe_instructeurs_signature_concern.rb @@ -19,9 +19,7 @@ module GroupeInstructeursSignatureConcern flash[:alert] = "Aucun fichier joint pour le tampon de l'attestation" render :show else - signature = uninterlace_png(signature_file) - - if @groupe_instructeur.signature.attach(signature) + if @groupe_instructeur.signature.attach(signature_file) handle_redirect :success else handle_redirect :alert diff --git a/app/controllers/concerns/uninterlace_png_concern.rb b/app/controllers/concerns/uninterlace_png_concern.rb deleted file mode 100644 index ff82fc7ba..000000000 --- a/app/controllers/concerns/uninterlace_png_concern.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -module UninterlacePngConcern - extend ActiveSupport::Concern - - private - - def uninterlace_png(uploaded_file) - if uploaded_file&.content_type == 'image/png' && interlaced?(uploaded_file.tempfile.to_path) - chunky_img = ChunkyPNG::Image.from_io(uploaded_file.to_io) - chunky_img.save(uploaded_file.tempfile.to_path, interlace: false) - uploaded_file.tempfile.reopen(uploaded_file.tempfile.to_path, 'rb') - end - uploaded_file - end - - def interlaced?(png_path) - png = MiniMagick::Image.open(png_path) - png.data["interlace"] != "None" - end -end diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index 2d3771e18..f7caec975 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -3,7 +3,6 @@ module Instructeurs class GroupeInstructeursController < InstructeurController include EmailSanitizableConcern - include UninterlacePngConcern include GroupeInstructeursSignatureConcern before_action :ensure_allowed! diff --git a/app/controllers/procedures_controller.rb b/app/controllers/procedures_controller.rb index 60fd044a0..b4037a4d8 100644 --- a/app/controllers/procedures_controller.rb +++ b/app/controllers/procedures_controller.rb @@ -5,7 +5,12 @@ class ProceduresController < ApplicationController def logo if @procedure.logo.attached? - redirect_to url_for(@procedure.logo.variant(:email)) + logo_variant = @procedure.logo.variant(resize_to_limit: [400, 400]) + if logo_variant.key.present? + redirect_to url_for(logo_variant.processed) + else + redirect_to url_for(@procedure.logo) + end else redirect_to ActionController::Base.helpers.image_url(PROCEDURE_DEFAULT_LOGO_SRC) end diff --git a/app/jobs/image_processor_job.rb b/app/jobs/image_processor_job.rb index 9efc16ec1..16b76f32d 100644 --- a/app/jobs/image_processor_job.rb +++ b/app/jobs/image_processor_job.rb @@ -22,6 +22,7 @@ class ImageProcessorJob < ApplicationJob return if ActiveStorage::Attachment.find_by(blob_id: blob.id)&.record_type == "ActiveStorage::VariantRecord" auto_rotate(blob) if ["image/jpeg", "image/jpg"].include?(blob.content_type) + uninterlace(blob) if blob.content_type == "image/png" create_representations(blob) if blob.representation_required? add_watermark(blob) if blob.watermark_pending? end @@ -40,6 +41,16 @@ class ImageProcessorJob < ApplicationJob end end + def uninterlace(blob) + blob.open do |file| + processed = UninterlaceService.new.process(file) + return if processed.blank? + + blob.upload(processed) + blob.save! + end + end + def create_representations(blob) blob.attachments.each do |attachment| next unless attachment&.representable? @@ -47,6 +58,9 @@ class ImageProcessorJob < ApplicationJob if attachment.blob.content_type.in?(RARE_IMAGE_TYPES) attachment.variant(resize_to_limit: [2000, 2000]).processed end + if attachment.record.class == ActionText::RichText + attachment.variant(resize_to_limit: [1024, 768]).processed + end end end diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index 8d9fdfca4..8aba5d352 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -106,7 +106,8 @@ class AttestationTemplate < ApplicationRecord def logo_url if logo.attached? - Rails.application.routes.url_helpers.url_for(logo) + logo_variant = logo.variant(resize_to_limit: [400, 400]) + logo_variant.key.present? ? logo_variant.processed.url : Rails.application.routes.url_helpers.url_for(logo) end end diff --git a/app/models/concerns/blob_image_processor_concern.rb b/app/models/concerns/blob_image_processor_concern.rb index 8a7870371..08d6f1067 100644 --- a/app/models/concerns/blob_image_processor_concern.rb +++ b/app/models/concerns/blob_image_processor_concern.rb @@ -10,7 +10,7 @@ module BlobImageProcessorConcern end def representation_required? - from_champ? || from_messagerie? + from_champ? || from_messagerie? || logo? || from_action_text? end private @@ -23,6 +23,14 @@ module BlobImageProcessorConcern attachments.any? { _1.record.class == Commentaire } end + def logo? + attachments.any? { _1.name == 'logo' } + end + + def from_action_text? + attachments.any? { _1.record.class == ActionText::RichText } + end + def watermark_required? attachments.any? { _1.record.class == Champs::TitreIdentiteChamp } end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 403d64f0e..1265171b0 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -146,9 +146,7 @@ class Procedure < ApplicationRecord belongs_to :defaut_groupe_instructeur, class_name: 'GroupeInstructeur', inverse_of: false, optional: true - has_one_attached :logo do |attachable| - attachable.variant :email, resize_to_limit: [450, 450] - end + has_one_attached :logo has_one_attached :notice has_one_attached :deliberation @@ -679,7 +677,8 @@ class Procedure < ApplicationRecord def logo_url if logo.attached? - Rails.application.routes.url_helpers.url_for(logo) + logo_variant = logo.variant(resize_to_limit: [400, 400]) + logo_variant.key.present? ? logo_variant.processed.url : Rails.application.routes.url_helpers.url_for(logo) else ActionController::Base.helpers.image_url(PROCEDURE_DEFAULT_LOGO_SRC) end diff --git a/app/services/uninterlace_service.rb b/app/services/uninterlace_service.rb new file mode 100644 index 000000000..d4cef3aaf --- /dev/null +++ b/app/services/uninterlace_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class UninterlaceService + def process(file) + uninterlace_png(file) + end + + private + + def uninterlace_png(uploaded_file) + if interlaced?(uploaded_file.to_path) + chunky_img = ChunkyPNG::Image.from_io(uploaded_file.to_io) + chunky_img.save(uploaded_file.to_path, interlace: false) + uploaded_file.reopen(uploaded_file.to_path, 'rb') + end + uploaded_file + end + + def interlaced?(png_path) + png = MiniMagick::Image.open(png_path) + png.data["interlace"] != "None" + end +end diff --git a/app/views/active_storage/blobs/_blob.html.erb b/app/views/active_storage/blobs/_blob.html.erb index 49ba357dd..8e95058a4 100644 --- a/app/views/active_storage/blobs/_blob.html.erb +++ b/app/views/active_storage/blobs/_blob.html.erb @@ -1,14 +1,19 @@ -
attachment--<%= blob.filename.extension %>"> - <% if blob.representable? %> - <%= image_tag blob.representation(resize_to_limit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %> - <% end %> - -
- <% if caption = blob.try(:caption) %> - <%= caption %> +<% if blob.representable? %> + <% representation = blob.representation(resize_to_limit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %> + <% if representation.image&.attached? || representation.key.present? %> +
+ <%= image_tag representation %> <% else %> - <%= blob.filename %> - <%= number_to_human_size blob.byte_size %> +
+ <%= image_tag blob %> <% end %> -
-
+ +
+ <% if caption = blob.try(:caption) %> + <%= caption %> + <% else %> + <%= blob.filename %> + <%= number_to_human_size blob.byte_size %> + <% end %> +
+<% end %> diff --git a/spec/controllers/administrateurs/attestation_templates_controller_spec.rb b/spec/controllers/administrateurs/attestation_templates_controller_spec.rb index f38e2fd06..1f6ef9243 100644 --- a/spec/controllers/administrateurs/attestation_templates_controller_spec.rb +++ b/spec/controllers/administrateurs/attestation_templates_controller_spec.rb @@ -8,8 +8,6 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do let(:logo2) { fixture_file_upload('spec/fixtures/files/white.png', 'image/png') } let(:signature) { fixture_file_upload('spec/fixtures/files/black.png', 'image/png') } let(:signature2) { fixture_file_upload('spec/fixtures/files/black.png', 'image/png') } - let(:interlaced_logo) { fixture_file_upload('spec/fixtures/files/interlaced-black.png', 'image/png') } - let(:uninterlaced_logo) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') } let(:invalid_logo) { fixture_file_upload('spec/fixtures/files/invalid_file_format.json', 'application/json') } before do diff --git a/spec/jobs/image_processor_job_spec.rb b/spec/jobs/image_processor_job_spec.rb index cc6d4fd9a..78c5702a0 100644 --- a/spec/jobs/image_processor_job_spec.rb +++ b/spec/jobs/image_processor_job_spec.rb @@ -13,6 +13,7 @@ describe ImageProcessorJob, type: :job do let(:antivirus_pending) { false } let(:watermark_service) { instance_double("WatermarkService") } let(:auto_rotate_service) { instance_double("AutoRotateService") } + let(:uninterlace_service) { instance_double("UninterlaceService") } before do virus_scanner_mock = instance_double("ActiveStorage::VirusScanner", pending?: antivirus_pending) @@ -23,9 +24,6 @@ describe ImageProcessorJob, type: :job do allow(WatermarkService).to receive(:new).and_return(watermark_service) allow(watermark_service).to receive(:process).and_return(true) - - allow(AutoRotateService).to receive(:new).and_return(auto_rotate_service) - allow(auto_rotate_service).to receive(:process).and_return(true) end context "when the blob is not scanned yet" do @@ -37,11 +35,33 @@ describe ImageProcessorJob, type: :job do end describe 'autorotate' do + let(:blob_info) do + { + filename: file.original_filename, + byte_size: file.size, + checksum: Digest::SHA256.file(file.path), + content_type: file.content_type, + # we don't want to run virus scanner on this file + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + } + end + + let(:blob) do + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_info) + blob.upload(file) + blob + end + + before do + allow(AutoRotateService).to receive(:new).and_return(auto_rotate_service) + allow(auto_rotate_service).to receive(:process).and_return(true) + end + context "when image is not a jpg" do - let(:rotated_file) { Tempfile.new("rotated.png") } + let(:file) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') } before do - allow(rotated_file).to receive(:size).and_return(100) + allow(file).to receive(:size).and_return(100) end it "it does not process autorotate" do @@ -52,6 +72,7 @@ describe ImageProcessorJob, type: :job do context "when image is a jpg " do let(:rotated_file) { Tempfile.new("rotated.jpg") } + let(:file) { fixture_file_upload('spec/fixtures/files/image-rotated.jpg', 'image/jpeg') } before do allow(rotated_file).to receive(:size).and_return(100) @@ -118,34 +139,67 @@ describe ImageProcessorJob, type: :job do describe 'watermark' do context "when watermark is already done" do before do - allow(blob).to receive(:watermark_done?).and_return(true) + allow(blob_jpg).to receive(:watermark_done?).and_return(true) end it "does not process the watermark" do expect(watermark_service).not_to receive(:process) - described_class.perform_now(blob) + described_class.perform_now(blob_jpg) end end context "when the blob is ready to be watermarked" do - let(:watermarked_file) { Tempfile.new("watermarked.png") } + let(:watermarked_file) { Tempfile.new("watermarked.jpg") } before do allow(watermarked_file).to receive(:size).and_return(100) - allow(blob).to receive(:watermark_pending?).and_return(true) + allow(blob_jpg).to receive(:watermark_pending?).and_return(true) end it "processes the blob with watermark" do expect(watermark_service).to receive(:process).and_return(watermarked_file) expect { - described_class.perform_now(blob) + described_class.perform_now(blob_jpg) }.to change { - blob.reload.checksum + blob_jpg.reload.checksum } - expect(blob.byte_size).to eq(100) - expect(blob.watermarked_at).to be_present + expect(blob_jpg.byte_size).to eq(100) + expect(blob_jpg.watermarked_at).to be_present + end + end + end + + describe 'uninterlace' do + let(:blob_info) do + { + filename: file.original_filename, + byte_size: file.size, + checksum: Digest::SHA256.file(file.path), + content_type: file.content_type, + # we don't want to run virus scanner on this file + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + } + end + + let(:blob) do + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_info) + blob.upload(file) + blob + end + + before do + allow(UninterlaceService).to receive(:new).and_return(uninterlace_service) + end + + context "when file is interlaced" do + let(:file) { fixture_file_upload('spec/fixtures/files/interlaced-black.png', 'image/png') } + let(:uninterlaced_file) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') } + + it "it process uninterlace" do + expect(uninterlace_service).to receive(:process).and_return(uninterlaced_file) + described_class.perform_now(blob) end end end