From b609c3dae4ca778e9b2ac49d7e937bb322aca212 Mon Sep 17 00:00:00 2001
From: Colin Darie <colin@darie.eu>
Date: Mon, 21 Nov 2022 18:32:17 +0100
Subject: [PATCH] refactor(attachment): bring batch user_can_destroy, update
 tests

---
 app/components/attachment/edit_component.rb   |  13 +-
 .../edit_component/edit_component.html.haml   |  15 +--
 .../multiple_component.html.haml              |   4 +-
 .../titre_identite_component.html.haml        |   2 +-
 app/views/root/patron.html.haml               |   4 +
 .../attachment/edit_component_spec.rb         | 112 ++++++++++++++++++
 .../attachment/show_component_spec.rb}        |  25 ++--
 .../attachment/_update.html.haml_spec.rb      |  62 ----------
 .../shared/dossiers/_edit.html.haml_spec.rb   |   4 +-
 9 files changed, 152 insertions(+), 89 deletions(-)
 create mode 100644 spec/components/attachment/edit_component_spec.rb
 rename spec/{views/shared/attachment/_show.html.haml_spec.rb => components/attachment/show_component_spec.rb} (76%)
 delete mode 100644 spec/views/shared/attachment/_update.html.haml_spec.rb

diff --git a/app/components/attachment/edit_component.rb b/app/components/attachment/edit_component.rb
index 7b19a4889..264cf6ecc 100644
--- a/app/components/attachment/edit_component.rb
+++ b/app/components/attachment/edit_component.rb
@@ -4,12 +4,14 @@ class Attachment::EditComponent < ApplicationComponent
   attr_reader :attachment
   attr_reader :user_can_download
   alias user_can_download? user_can_download
+  attr_reader :user_can_destroy
+  alias user_can_destroy? user_can_destroy
   attr_reader :as_multiple
   alias as_multiple? as_multiple
 
   EXTENSIONS_ORDER = ['jpeg', 'png', 'pdf', 'zip'].freeze
 
-  def initialize(champ: nil, auto_attach_url: nil, field_name: nil, attached_file:, direct_upload: true, index: 0, as_multiple: false, user_can_download: false, **kwargs)
+  def initialize(champ: nil, auto_attach_url: nil, field_name: nil, attached_file:, direct_upload: true, index: 0, as_multiple: false, user_can_download: false, user_can_destroy: true, **kwargs)
     @as_multiple = as_multiple
     @attached_file = attached_file
     @auto_attach_url = auto_attach_url
@@ -17,6 +19,7 @@ class Attachment::EditComponent < ApplicationComponent
     @direct_upload = direct_upload
     @index = index
     @user_can_download = user_can_download
+    @user_can_destroy = user_can_destroy
 
     # attachment passed by kwarg because we don't want a default (nil) value.
     @attachment = if kwargs.key?(:attachment)
@@ -128,6 +131,14 @@ class Attachment::EditComponent < ApplicationComponent
     !!attachment&.persisted?
   end
 
+  def downloadable?
+    return false unless user_can_download?
+    return false if attachment.virus_scanner_error?
+    return false if attachment.watermark_pending?
+
+    true
+  end
+
   def error?
     attachment.virus_scanner_error?
   end
diff --git a/app/components/attachment/edit_component/edit_component.html.haml b/app/components/attachment/edit_component/edit_component.html.haml
index db489e64a..eaf0c031e 100644
--- a/app/components/attachment/edit_component/edit_component.html.haml
+++ b/app/components/attachment/edit_component/edit_component.html.haml
@@ -2,14 +2,15 @@
   - if persisted?
     %div{ id: dom_id(attachment, :persisted_row) }
       .flex.flex-gap-2{ class: class_names("attachment-error": attachment.virus_scanner_error?) }
-        = link_to('Supprimer', destroy_attachment_path, **remove_button_options, class: "fr-btn fr-btn--tertiary fr-btn--sm fr-icon-delete-line", title: "Supprimer le fichier #{attachment.filename}")
+        - if user_can_destroy?
+          = link_to('Supprimer', destroy_attachment_path, **remove_button_options, class: "fr-btn fr-btn--tertiary fr-btn--sm fr-icon-delete-line", title: "Supprimer le fichier #{attachment.filename}")
 
-        .fr-py-1v
-          = link_to_if(user_can_download?, attachment.filename.to_s, attachment.url, class: "attachment-filename", download: "") do
-            %span.attachment-filename= attachment.filename.to_s
-          - if in_progress?
-            %p.fr-badge.fr-badge--info.fr-badge--sm.fr-badge--no-icon.fr-ml-1w
-              = progress_bar_label
+          .fr-py-1v
+            = link_to_if(downloadable?, attachment.filename.to_s, attachment.url, class: "attachment-filename", download: "") do
+              %span.attachment-filename= attachment.filename.to_s
+            - if in_progress?
+              %p.fr-badge.fr-badge--info.fr-badge--sm.fr-badge--no-icon.fr-ml-1w
+                = progress_bar_label
 
       - if error?
         %p.fr-error-text= error_message
diff --git a/app/components/attachment/multiple_component/multiple_component.html.haml b/app/components/attachment/multiple_component/multiple_component.html.haml
index 5304f8ec5..bc76eae52 100644
--- a/app/components/attachment/multiple_component/multiple_component.html.haml
+++ b/app/components/attachment/multiple_component/multiple_component.html.haml
@@ -3,10 +3,10 @@
 
   - each_attachment do |attachment, index|
     %div{ id: dom_id(attachment) }
-      = render Attachment::EditComponent.new(champ:, attached_file:, attachment:, index:, as_multiple: true)
+      = render Attachment::EditComponent.new(champ:, attached_file:, attachment:, index:, as_multiple: true, user_can_destroy:)
 
   %div{ id: empty_component_id, class: class_names("hidden": !can_attach_next?) }
-    = render Attachment::EditComponent.new(champ:, attached_file:, attachment: nil, index: attachments_count)
+    = render Attachment::EditComponent.new(champ:, attached_file:, attachment: nil, index: attachments_count, user_can_destroy:)
 
   // single poll and refresh message for all attachments
   - if in_progress?
diff --git a/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml b/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml
index 76b3c9410..3ec1ed42a 100644
--- a/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml
+++ b/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml
@@ -1,2 +1,2 @@
 - user_can_destroy = !@champ.mandatory? || @champ.dossier.brouillon?
-= render Attachment::EditComponent.new(champ: @form.object, attached_file: @champ.piece_justificative_file, attachment: @champ.piece_justificative_file[0])
+= render Attachment::EditComponent.new(champ: @form.object, attached_file: @champ.piece_justificative_file, attachment: @champ.piece_justificative_file[0], user_can_destroy:)
diff --git a/app/views/root/patron.html.haml b/app/views/root/patron.html.haml
index fab497799..d0c4fe1b7 100644
--- a/app/views/root/patron.html.haml
+++ b/app/views/root/patron.html.haml
@@ -365,6 +365,9 @@
       %h3.fr-mt-4w Existing attachment
       = render Attachment::EditComponent.new(champ:, attached_file: champ.piece_justificative_file, attachment:)
 
+      %h3.fr-mt-4w Existing attachment, user can not destroy
+      = render Attachment::EditComponent.new(champ:, attached_file: champ.piece_justificative_file, attachment:, user_can_destroy: false)
+
       %h3.fr-mt-4w Existing attachment, antivirus in progress
       - attachment.blob.metadata[:virus_scan_result] = ActiveStorage::VirusScanner::PENDING
       - attachment.created_at = Time.zone.now
@@ -379,6 +382,7 @@
       - attachment.blob.metadata[:virus_scan_result] = ActiveStorage::VirusScanner::INFECTED
       = render Attachment::EditComponent.new(champ:, attached_file: Champ.new.piece_justificative_file, attachment:)
 
+
       %h3.fr-mt-4w New attachment on TypeDeChamp
       = render Attachment::EditComponent.new(auto_attach_url: "/some-auto-attach-path", attached_file: tdc.piece_justificative_template, attachment: nil)
 
diff --git a/spec/components/attachment/edit_component_spec.rb b/spec/components/attachment/edit_component_spec.rb
new file mode 100644
index 000000000..4f0237b85
--- /dev/null
+++ b/spec/components/attachment/edit_component_spec.rb
@@ -0,0 +1,112 @@
+RSpec.describe Attachment::EditComponent, type: :component do
+  let(:champ) { create(:champ_titre_identite, dossier: create(:dossier)) }
+  let(:attached_file) { champ.piece_justificative_file }
+  let(:attachment) { attached_file.attachments.first }
+  let(:kwargs) { {} }
+
+  let(:component) do
+    described_class.new(
+      champ:,
+      attached_file:,
+      attachment:,
+      **kwargs
+    )
+  end
+
+  subject { render_inline(component).to_html }
+
+  context 'when there is no attachment yet' do
+    let(:attachment) { nil }
+
+    it 'renders a form field for uploading a file' do
+      expect(subject).to have_selector('input[type=file]:not(.hidden)')
+    end
+
+    it 'renders max size' do
+      expect(subject).to have_content(/Taille maximale :\s+20 Mo/)
+    end
+
+    it 'renders allowed formats' do
+      expect(subject).to have_content(/Formats supportés :\s+jpeg, png/)
+    end
+  end
+
+  context 'when there is an attachment' do
+    it 'renders the filename' do
+      expect(subject).to have_content(attachment.filename.to_s)
+    end
+
+    it 'hides the file field by default' do
+      expect(subject).to have_selector('input[type=file].hidden')
+    end
+
+    it 'shows the Delete button by default' do
+      expect(subject).to have_selector('[title^="Supprimer le fichier"]')
+    end
+  end
+
+  context 'when the user cannot destroy the attachment' do
+    let(:kwargs) { { user_can_destroy: false } }
+
+    it 'hides the Delete button' do
+      expect(subject).not_to have_selector("[title^='Supprimer le fichier']")
+    end
+  end
+
+  context 'within multiple attachments' do
+    let(:index) { 0 }
+    let(:component) do
+      described_class.new(
+        champ:,
+        attached_file:,
+        attachment: nil,
+        as_multiple: true,
+        index:
+      )
+    end
+
+    it 'does not render an empty file' do # (is is rendered by MultipleComponent)
+      expect(subject).not_to have_selector('input[type=file]')
+    end
+
+    it 'renders max size for first index' do
+      expect(subject).to have_content(/Taille maximale :\s+20 Mo/)
+    end
+
+    context 'when index is not 0' do
+      let(:index) { 1 }
+
+      it 'renders max size for first index' do
+        expect(subject).not_to have_content('Taille maximale')
+      end
+    end
+  end
+
+  context 'when user can download' do
+    let(:kwargs) { { user_can_download: true } }
+    let(:filename) { champ.piece_justificative_file[0].filename.to_s }
+
+    it 'renders a link to download the file' do
+      expect(subject).to have_link(filename)
+    end
+
+    context 'when watermark is pending' do
+      let(:champ) { create(:champ_titre_identite) }
+      let(:kwargs) { { user_can_download: true } }
+
+      it 'displays the filename, but doesn’t allow to download the file' do
+        expect(attachment.watermark_pending?).to be_truthy
+        expect(subject).to have_text(filename)
+        expect(subject).to have_link('Supprimer')
+        expect(subject).to have_no_link(text: filename) # don't match "Delete" link which also include filename in title attribute
+        expect(subject).to have_text('Traitement en cours')
+      end
+    end
+  end
+
+  context 'TODO: with a pending antivirus scan' do
+  end
+
+  context 'TODO: with an error' do
+  end
+end
diff --git a/spec/views/shared/attachment/_show.html.haml_spec.rb b/spec/components/attachment/show_component_spec.rb
similarity index 76%
rename from spec/views/shared/attachment/_show.html.haml_spec.rb
rename to spec/components/attachment/show_component_spec.rb
index 1a1f4d0c4..c6b7d88ad 100644
--- a/spec/views/shared/attachment/_show.html.haml_spec.rb
+++ b/spec/components/attachment/show_component_spec.rb
@@ -1,13 +1,21 @@
-describe 'shared/attachment/_show.html.haml', type: :view do
+RSpec.describe Attachment::ShowComponent, type: :component do
   let(:champ) { create(:champ_piece_justificative) }
   let(:virus_scan_result) { nil }
 
+  let(:attachment) {
+    champ.piece_justificative_file.attachments.first
+  }
+
+  let(:component) do
+    described_class.new(attachment:)
+  end
+
+  subject { render_inline(component).to_html }
+
   before do
     champ.piece_justificative_file[0].blob.update(metadata: champ.piece_justificative_file[0].blob.metadata.merge(virus_scan_result: virus_scan_result))
   end
 
-  subject { render Attachment::ShowComponent.new(attachment: champ.piece_justificative_file.attachments.first) }
-
   context 'when there is no anti-virus scan' do
     let(:virus_scan_result) { nil }
 
@@ -27,17 +35,6 @@ describe 'shared/attachment/_show.html.haml', type: :view do
     end
   end
 
-  context 'when watermark is pending' do
-    let(:champ) { create(:champ_titre_identite) }
-
-    it 'displays the filename, but doesn’t allow to download the file' do
-      expect(champ.piece_justificative_file.attachments[0].watermark_pending?).to be_truthy
-      expect(subject).to have_text(champ.piece_justificative_file[0].filename.to_s)
-      expect(subject).not_to have_link(champ.piece_justificative_file[0].filename.to_s)
-      expect(subject).to have_text('Traitement en cours')
-    end
-  end
-
   context 'when the file is scanned and safe' do
     let(:virus_scan_result) { ActiveStorage::VirusScanner::SAFE }
 
diff --git a/spec/views/shared/attachment/_update.html.haml_spec.rb b/spec/views/shared/attachment/_update.html.haml_spec.rb
deleted file mode 100644
index 791a7b2d4..000000000
--- a/spec/views/shared/attachment/_update.html.haml_spec.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-describe 'shared/attachment/_update.html.haml', type: :view do
-  let(:champ) { build(:champ_titre_identite, dossier: create(:dossier)) }
-  let(:attached_file) { champ.piece_justificative_file }
-  let(:user_can_destroy) { false }
-  let(:template) { nil }
-
-  subject do
-    view.render Attachment::EditComponent.new(champ: champ, attached_file: attached_file, attachment: attached_file[0])
-  end
-
-  context 'when there is no attached file' do
-    before do
-      champ.piece_justificative_file = nil
-    end
-
-    it 'renders a form field for uploading a file' do
-      expect(subject).to have_selector('input[type=file]:not(.hidden)')
-    end
-  end
-
-  context 'when there is an attached file' do
-    it 'renders a form field for uploading a file' do
-      expect(subject).to have_selector('input[type=file]:not(.hidden)')
-    end
-
-    it 'does not renders a link to the unsaved file' do
-      expect(subject).not_to have_content(attached_file.attachments[0].filename.to_s)
-    end
-
-    it 'does not render action buttons' do
-      expect(subject).not_to have_link('Supprimer')
-    end
-
-    context 'and the attachment has been saved' do
-      before { champ.save! }
-
-      it 'renders a link to the file' do
-        expect(subject).to have_content(attached_file.attachments[0].filename.to_s)
-      end
-
-      it 'hides the form field by default' do
-        expect(subject).to have_selector('input[type=file].hidden')
-      end
-
-      it 'shows the Delete button by default' do
-        is_expected.to have_link('Supprimer')
-      end
-    end
-  end
-
-  context 'when the user cannot destroy the attachment' do
-    subject do
-      render Attachment::EditComponent.new(champ: champ,
-        attached_file: attached_file,
-        attachment: attached_file[0])
-    end
-
-    it 'hides the Delete button' do
-      is_expected.not_to have_link('Supprimer')
-    end
-  end
-end
diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb
index f8bbe12c5..37ef06ec3 100644
--- a/spec/views/shared/dossiers/_edit.html.haml_spec.rb
+++ b/spec/views/shared/dossiers/_edit.html.haml_spec.rb
@@ -114,7 +114,7 @@ describe 'shared/dossiers/edit.html.haml', type: :view do
       before { dossier.champs_public << champ }
 
       it 'cannot delete a piece justificative' do
-        expect(subject).not_to have_text('Supprimer')
+        expect(subject).not_to have_selector("[title='Supprimer le fichier #{champ.piece_justificative_file.attachments[0].filename}']")
       end
     end
 
@@ -124,7 +124,7 @@ describe 'shared/dossiers/edit.html.haml', type: :view do
       end
 
       it 'can delete a piece justificative' do
-        expect(subject).to have_text('Supprimer')
+        expect(subject).to have_selector("[title='Supprimer le fichier #{champ.piece_justificative_file.attachments[0].filename}']")
       end
     end
   end