Refactor message attachements to use active_storage

This commit is contained in:
Paul Chavard 2019-06-25 17:12:44 +02:00
parent 85f7412fa6
commit 3cb39c2840
20 changed files with 59 additions and 66 deletions

View file

@ -138,7 +138,7 @@ module Gestionnaires
end
def commentaire_params
params.require(:commentaire).permit(:body, :file)
params.require(:commentaire).permit(:body, :piece_jointe)
end
end
end

View file

@ -166,7 +166,7 @@ module Gestionnaires
end
def commentaire_params
params.require(:commentaire).permit(:body, :file)
params.require(:commentaire).permit(:body, :piece_jointe)
end
def champs_private_params

View file

@ -52,7 +52,7 @@ class SupportController < ApplicationController
email: email,
phone: params[:phone],
text: params[:text],
file: params[:file],
file: params[:piece_jointe],
dossier_id: dossier&.id,
browser: browser_name,
tags: tags
@ -61,7 +61,7 @@ class SupportController < ApplicationController
def create_commentaire
attributes = {
file: params[:file],
piece_jointe: params[:piece_jointe],
body: "[#{params[:subject]}]<br><br>#{params[:text]}"
}
commentaire = CommentaireService.build(current_user, dossier, attributes)

View file

@ -354,7 +354,7 @@ module Users
end
def commentaire_params
params.require(:commentaire).permit(:body, :file)
params.require(:commentaire).permit(:body, :piece_jointe)
end
def passage_en_construction?

View file

@ -5,9 +5,10 @@ class Commentaire < ApplicationRecord
belongs_to :gestionnaire
mount_uploader :file, CommentaireFileUploader
validates :file, file_size: { maximum: 20.megabytes, message: "La taille du fichier doit être inférieure à 20 Mo" }
validate :is_virus_free?
validate :messagerie_available?, on: :create
has_one_attached :piece_jointe
validates :body, presence: { message: "Votre message ne peut être vide" }
default_scope { order(created_at: :asc) }
@ -47,10 +48,15 @@ class Commentaire < ApplicationRecord
end
def file_url
if Flipflop.remote_storage?
if piece_jointe.attached?
if piece_jointe.virus_scanner.safe?
Rails.application.routes.url_helpers.url_for(piece_jointe)
end
elsif Flipflop.remote_storage?
RemoteDownloader.new(file.path).url
else
file.url
elsif file&.url
# FIXME: this is horrible but used only in dev and will be removed after migration
File.join(LOCAL_DOWNLOAD_URL, file.url)
end
end
@ -74,12 +80,6 @@ class Commentaire < ApplicationRecord
DossierMailer.notify_new_answer(dossier).deliver_later
end
def is_virus_free?
if file.present? && file_changed? && !ClamavService.safe_file?(file.path)
errors.add(:file, "Virus détecté dans le fichier joint, merci de changer de fichier")
end
end
def messagerie_available?
return if sent_by_system?
if dossier.present? && !dossier.messagerie_available?

View file

@ -117,7 +117,7 @@ class Dossier < ApplicationRecord
scope :nearing_end_of_retention, -> (duration = '1 month') { joins(:procedure).where("en_instruction_at + (duree_conservation_dossiers_dans_ds * interval '1 month') - now() < interval ?", duration) }
scope :since, -> (since) { where('dossiers.en_construction_at >= ?', since) }
scope :for_api, -> {
includes(commentaires: [],
includes(commentaires: { piece_jointe_attachment: :blob },
champs: [
:geo_areas,
:etablissement,

View file

@ -13,6 +13,11 @@ class CommentaireService
def build_with_email(email, dossier, params)
attributes = params.merge(email: email, dossier: dossier)
# For some reason ActiveStorage trows an error in tests if we passe an empty string here.
# I suspect it could be resolved in rails 6 by using explicit `attach()`
if attributes[:piece_jointe].blank?
attributes.delete(:piece_jointe)
end
Commentaire.new(attributes)
end
end

View file

@ -14,7 +14,7 @@ class LocalDownloader
end
def url
@url ||= File.join(TPS::Application::URL, 'downloads', random_folder_name, "#{@filename_suffix}.#{@extension}")
@url ||= File.join(LOCAL_DOWNLOAD_URL, 'downloads', random_folder_name, "#{@filename_suffix}.#{@extension}")
end
protected

View file

@ -61,7 +61,7 @@
.messagerie
%ul.messages-list
- @dossier.commentaires.each do |commentaire|
- @dossier.commentaires.with_attached_piece_jointe.each do |commentaire|
%li
= render partial: "shared/dossiers/messages/message", locals: { commentaire: commentaire, connected_user: current_gestionnaire, messagerie_seen_at: nil }

View file

@ -1,6 +1,6 @@
.messagerie.container
%ul.messages-list
- dossier.commentaires.each do |commentaire|
- dossier.commentaires.with_attached_piece_jointe.each do |commentaire|
%li.message{ class: commentaire_is_from_me_class(commentaire, connected_user) }
= render partial: "shared/dossiers/messages/message", locals: { commentaire: commentaire, connected_user: connected_user, messagerie_seen_at: messagerie_seen_at }

View file

@ -2,8 +2,8 @@
= f.text_area :body, rows: 5, placeholder: 'Répondre ici', required: true, class: 'message-textarea'
.flex.justify-between.wrap
%div
= f.file_field :file, id: :file, accept: commentaire.file.accept_extension_list
%label{ for: :file }
= f.file_field :piece_jointe, id: 'piece_jointe', direct_upload: true
%label{ for: :piece_jointe }
.notice
(taille max : 20 Mo)

View file

@ -10,7 +10,10 @@
= commentaire_date(commentaire)
.rich-text= sanitize(simple_format(commentaire.body))
- if commentaire.file.present?
- if commentaire.piece_jointe.attached?
.attachment-link
= render partial: "shared/attachment/show", locals: { attachment: commentaire.piece_jointe.attachment }
- elsif commentaire.file.present?
.attachment-link
= link_to commentaire.file_url, class: "button", target: "_blank", rel: "noopener", title: "Télécharger" do
%span.icon.attachment

View file

@ -76,7 +76,7 @@
Une capture décran peut nous aider à identifier plus facilement lendroit à améliorer.
.notice.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_AUTRE } }
Une capture décran peut nous aider à identifier plus facilement le problème.
= file_field_tag :file
= file_field_tag :piece_jointe
= hidden_field_tag :tags, @tags&.join(',')

View file

@ -31,8 +31,6 @@ module TPS
config.assets.paths << Rails.root.join('app', 'assets', 'fonts')
config.assets.precompile += ['.woff']
URL = ENV['APP_HOST'] || "http://localhost:3000/"
config.active_job.queue_adapter = :delayed_job
config.action_view.sanitized_allowed_tags = ActionView::Base.sanitized_allowed_tags + ['u']

View file

@ -28,3 +28,6 @@ FAQ_ADMIN_URL = "https://faq.demarches-simplifiees.fr/collection/1-administrateu
COMMENT_TROUVER_MA_DEMARCHE_URL = [FAQ_URL, "article", "59-comment-trouver-ma-demarche"].join("/")
STATUS_PAGE_URL = "https://status.demarches-simplifiees.fr"
MATOMO_IFRAME_URL = "https://stats.data.gouv.fr/index.php?module=CoreAdminHome&action=optOut&language=fr&&fontColor=333333&fontSize=16px&fontFamily=Muli"
# FIXME: This is only used in dev in couple of places and should be removed after PJ migration
LOCAL_DOWNLOAD_URL = "http://#{ENV.fetch('APP_HOST', 'localhost:3000')}"

View file

@ -92,7 +92,7 @@ describe Gestionnaires::AvisController, type: :controller do
let(:file) { nil }
let(:scan_result) { true }
subject { post :create_commentaire, params: { id: avis_without_answer.id, commentaire: { body: 'commentaire body', file: file } } }
subject { post :create_commentaire, params: { id: avis_without_answer.id, commentaire: { body: 'commentaire body', piece_jointe: file } } }
before do
allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
@ -110,16 +110,10 @@ describe Gestionnaires::AvisController, type: :controller do
it do
subject
expect(Commentaire.last.file.path).to include("piece_justificative_0.pdf")
expect(Commentaire.last.piece_jointe.filename).to eq("piece_justificative_0.pdf")
end
it { expect { subject }.to change(Commentaire, :count).by(1) }
context "and a virus" do
let(:scan_result) { false }
it { expect { subject }.not_to change(Commentaire, :count) }
end
end
end

View file

@ -350,15 +350,15 @@ describe Gestionnaires::DossiersController, type: :controller do
expect(flash.notice).to be_present
end
context "when the commentaire creation fails" do
context "when the commentaire created with virus file" do
let(:scan_result) { false }
it "renders the messagerie page with the invalid commentaire" do
expect { subject }.not_to change(Commentaire, :count)
it "creates a commentaire (shows message that file have a virus)" do
expect { subject }.to change(Commentaire, :count).by(1)
expect(gestionnaire.followed_dossiers).to include(dossier)
expect(response).to render_template :messagerie
expect(flash.alert).to be_present
expect(assigns(:commentaire).body).to eq("avant\napres")
expect(response).to redirect_to(messagerie_gestionnaire_dossier_path(dossier.procedure, dossier))
expect(flash.notice).to be_present
end
end
end

View file

@ -806,7 +806,7 @@ describe Users::DossiersController, type: :controller do
id: dossier.id,
commentaire: {
body: body,
file: file
piece_jointe: file
}
}
}
@ -822,18 +822,6 @@ describe Users::DossiersController, type: :controller do
expect(response).to redirect_to(messagerie_dossier_path(dossier))
expect(flash.notice).to be_present
end
context "when the commentaire creation fails" do
let(:scan_result) { false }
it "renders the messagerie page with the invalid commentaire" do
expect { subject }.not_to change(Commentaire, :count)
expect(response).to render_template :messagerie
expect(flash.alert).to be_present
expect(assigns(:commentaire).body).to eq("avant\napres")
end
end
end
describe '#ask_deletion' do

View file

@ -7,5 +7,9 @@ FactoryBot.define do
commentaire.dossier = create :dossier, :en_construction
end
end
trait :with_file do
file { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'application/pdf') }
end
end
end

View file

@ -1,24 +1,21 @@
require 'spec_helper'
describe CommentaireService do
include ActiveJob::TestHelper
describe '.create' do
let(:dossier) { create :dossier, :en_construction }
let(:sender) { dossier.user }
let(:body) { 'Contenu du message.' }
let(:file) { nil }
let(:scan_result) { true }
subject(:commentaire) { CommentaireService.build(sender, dossier, { body: body, file: file }) }
before do
allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
end
subject(:commentaire) { CommentaireService.build(sender, dossier, { body: body, piece_jointe: file }) }
it 'creates a new valid commentaire' do
expect(commentaire.email).to eq sender.email
expect(commentaire.dossier).to eq dossier
expect(commentaire.body).to eq 'Contenu du message.'
expect(commentaire.file).to be_blank
expect(commentaire.piece_jointe.attached?).to be_falsey
expect(commentaire).to be_valid
end
@ -34,14 +31,15 @@ describe CommentaireService do
context 'when it has a file' do
let(:file) { Rack::Test::UploadedFile.new("./spec/fixtures/files/piece_justificative_0.pdf", 'application/pdf') }
it 'saves the attached file' do
expect(commentaire.file).to be_present
expect(commentaire).to be_valid
before do
expect(ClamavService).to receive(:safe_file?).and_return(true)
end
context 'and a virus' do
let(:scan_result) { false }
it { expect(commentaire).not_to be_valid }
it 'saves the attached file' do
perform_enqueued_jobs do
commentaire.save
expect(commentaire.piece_jointe.attached?).to be_truthy
end
end
end
end