Merge pull request #8542 from alphagov/use-asset-id-for-callforevidence

Use asset-id for for call_for_evidence_response_form_data

app/controllers/admin/editions_controller.rb

@@ -462,7 +462,7 @@ def clear_scheduled_publication_if_not_activated
   end
 
   def clear_response_form_file_cache
-    response_form_params = edition_params.dig(:consultation_participation_attributes, :consultation_response_form_attributes, :consultation_response_form_data_attributes)
+    response_form_params = edition_params.dig(:consultation_participation_attributes, :consultation_response_form_attributes, :consultation_response_form_data_attributes) || edition_params.dig(:call_for_evidence_participation_attributes, :call_for_evidence_response_form_attributes, :call_for_evidence_response_form_data_attributes)
     if response_form_params&.dig(:file).present? && response_form_params&.dig(:file_cache).present?
       response_form_params.delete(:file_cache)
     end

app/models/call_for_evidence.rb

@@ -12,6 +12,7 @@ class CallForEvidence < Publicationesque
   validates :closing_at, comparison: { greater_than: :opening_at, message: "must be after the opening on date" }, if: proc { |record| record.opening_at && record.closing_at }
   validates :external_url, presence: true, if: :external?
   validates :external_url, uri: true, allow_blank: true
+  validate :call_for_evidence_response_file_uploaded_to_asset_manager!, if: :call_for_evidence_response_file_in_asset_manager_check_required?
 
   has_one :call_for_evidence_participation, foreign_key: :edition_id, dependent: :destroy
   has_one :outcome, class_name: "CallForEvidenceOutcome", foreign_key: :edition_id, dependent: :destroy
@@ -173,4 +174,12 @@ def base_path
   def publishing_api_presenter
     PublishingApi::CallForEvidencePresenter
   end
+
+  def call_for_evidence_response_file_in_asset_manager_check_required?
+    has_call_for_evidence_participation? && call_for_evidence_participation.has_response_form? && published?
+  end
+
+  def call_for_evidence_response_file_uploaded_to_asset_manager!
+    errors.add(:call_for_evidence_response_form, "must have finished uploading") unless call_for_evidence_participation.call_for_evidence_response_form_uploaded_to_asset_manager?
+  end
 end

app/models/call_for_evidence_participation.rb

@@ -26,6 +26,10 @@ def has_postal_address?
 
   after_destroy :destroy_form_if_required
 
+  def call_for_evidence_response_form_uploaded_to_asset_manager?
+    has_response_form? && call_for_evidence_response_form&.call_for_evidence_response_form_data&.all_asset_variants_uploaded?
+  end
+
 private
 
   def destroy_form_if_required

app/models/call_for_evidence_response_form_data.rb

@@ -12,4 +12,15 @@ class CallForEvidenceResponseFormData < ApplicationRecord
   def auth_bypass_ids
     [call_for_evidence_response_form.call_for_evidence_participation.call_for_evidence.auth_bypass_id]
   end
+
+  def all_asset_variants_uploaded?
+    asset_variants = assets.map(&:variant).map(&:to_sym)
+    required_variants = [Asset.variants[:original].to_sym]
+
+    (required_variants - asset_variants).empty?
+  end
+
+  def filename
+    file.present? && file.file.filename
+  end
 end

app/models/consultation_response_form_data.rb

@@ -22,4 +22,8 @@ def all_asset_variants_uploaded?
 
     (required_variants - asset_variants).empty?
   end
+
+  def filename
+    file.present? && file.file.filename
+  end
 end

app/presenters/publishing_api/call_for_evidence_presenter.rb

@@ -261,7 +261,7 @@ def call
       alias_method :participation_response_form, :call_for_evidence_response_form
 
       def attachment_url
-        return unless participation.has_response_form?
+        return unless participation.has_response_form? && participation.call_for_evidence_response_form_uploaded_to_asset_manager?
 
         participation_response_form.file.url
       end

app/services/edition_auth_bypass_updater.rb

@@ -65,24 +65,4 @@ def update_consultation_attachments(edition)
       end
     end
   end
-
-  def update_call_for_evidence_attachments(edition)
-    return unless edition.is_a?(CallForEvidence)
-
-    if edition.call_for_evidence_participation&.call_for_evidence_response_form.present?
-      response_form = edition.call_for_evidence_participation.call_for_evidence_response_form
-      response_form_data_id = response_form.call_for_evidence_response_form_data.id
-      new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
-
-      AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "CallForEvidenceResponseFormData", response_form_data_id, new_attributes)
-    end
-
-    if edition.outcome.present?
-      edition.outcome.attachments.files.each do |file_attachment|
-        new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
-        attachment_data_id = file_attachment.attachment_data.id
-        AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
-      end
-    end
-  end
 end

app/views/admin/calls_for_evidence/_form.html.erb

@@ -104,8 +104,13 @@
 
             <% if call_for_evidence_response_form.call_for_evidence_response_form_data.try(:persisted?) %>
               <div class="attachment">
-                <p class="govuk-body">Current data: <%= link_to File.basename(call_for_evidence_response_form_data.file.path), call_for_evidence_response_form_data.file.url, class: "govuk-link" %></p>
-
+                <p class="govuk-body">Current data:
+                  <% if call_for_evidence_response_form_data.all_asset_variants_uploaded? %>
+                    <%= link_to call_for_evidence_response_form_data.filename, call_for_evidence_response_form_data.file.url, class: "govuk-link" %>
+                  <% else %>
+                    <%= call_for_evidence_response_form_data.filename %> <span class="govuk-tag govuk-tag--green">Processing</span>
+                  <% end %>
+                </p>
                 <%= render "govuk_publishing_components/components/radio", {
                   heading: "Actions:",
                   name: "edition[call_for_evidence_participation_attributes][call_for_evidence_response_form_attributes][attachment_action]",

app/views/admin/consultations/_form.html.erb

@@ -104,9 +104,9 @@
               <div class="attachment">
                   <p class="govuk-body">Current data:
                     <% if consultation_response_form_data.all_asset_variants_uploaded? %>
-                      <%= link_to File.basename(consultation_response_form_data.file.path), consultation_response_form_data.file.url, class: "govuk-link" %>
+                      <%= link_to consultation_response_form_data.filename, consultation_response_form_data.file.url, class: "govuk-link" %>
                     <% else %>
-                      <%= File.basename(consultation_response_form_data.file.path) %> <span class="govuk-tag govuk-tag--green">Processing</span>
+                      <%= consultation_response_form_data.filename %> <span class="govuk-tag govuk-tag--green">Processing</span>
                     <% end %>
                   </p>
                 <%= render "govuk_publishing_components/components/radio", {

lib/whitehall/asset_manager_storage.rb

@@ -102,7 +102,7 @@ def get_asset
   def self.use_non_legacy_behaviour?(model)
     return unless model
 
-    return true if model.instance_of?(AttachmentData) || model.instance_of?(ImageData) || model.instance_of?(Organisation) || model.instance_of?(FeaturedImageData) || model.instance_of?(TopicalEventFeaturingImageData) || model.instance_of?(PromotionalFeatureItem) || model.instance_of?(ConsultationResponseFormData)
+    return true if model.instance_of?(AttachmentData) || model.instance_of?(ImageData) || model.instance_of?(Organisation) || model.instance_of?(FeaturedImageData) || model.instance_of?(TopicalEventFeaturingImageData) || model.instance_of?(PromotionalFeatureItem) || model.instance_of?(ConsultationResponseFormData) || model.instance_of?(CallForEvidenceResponseFormData)
 
     model.respond_to?("use_non_legacy_endpoints") && model.use_non_legacy_endpoints
   end

test/factories/call_for_evidence_response_form_data.rb

@@ -1,5 +1,9 @@
 FactoryBot.define do
   factory :call_for_evidence_response_form_data do
     file { File.open(Rails.root.join("test/fixtures/two-pages.pdf")) }
+
+    after(:build) do |call_for_evidence_response_form_data|
+      call_for_evidence_response_form_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "two-pages.pdf")
+    end
   end
 end

test/functional/admin/calls_for_evidence_controller_test.rb

@@ -285,9 +285,6 @@ class Admin::CallsForEvidenceControllerTest < ActionController::TestCase
   end
 
   view_test "updating should respect the attachment_action for response forms to replace it" do
-    Services.asset_manager.stubs(:whitehall_asset).returns("id" => "http://asset-manager/assets/asset-id")
-    Services.asset_manager.stubs(:delete_asset)
-
     two_pages_pdf = upload_fixture("two-pages.pdf")
     greenpaper_pdf = upload_fixture("greenpaper.pdf")
 
@@ -318,6 +315,73 @@ class Admin::CallsForEvidenceControllerTest < ActionController::TestCase
     assert_equal "greenpaper.pdf", call_for_evidence.call_for_evidence_participation.call_for_evidence_response_form.call_for_evidence_response_form_data.carrierwave_file
   end
 
+  view_test "create should show 'Processing' tag if variant is missing" do
+    response_form = create(:call_for_evidence_response_form)
+    participation = create(:call_for_evidence_participation, call_for_evidence_response_form: response_form)
+    call_for_evidence = create(:call_for_evidence, call_for_evidence_participation: participation)
+    response_form.call_for_evidence_response_form_data.assets = []
+
+    get :edit, params: { id: call_for_evidence.id }
+
+    assert_select "span[class='govuk-tag govuk-tag--green']", text: "Processing"
+  end
+
+  test "PUT :update discards file_cache when a file is provided" do
+    two_pages_pdf = upload_fixture("two-pages.pdf")
+    greenpaper_pdf = upload_fixture("greenpaper.pdf")
+
+    response_form = create(:call_for_evidence_response_form)
+    participation = create(:call_for_evidence_participation, call_for_evidence_response_form: response_form)
+    call_for_evidence = create(:call_for_evidence, call_for_evidence_participation: participation)
+    response_form_data = build(:call_for_evidence_response_form_data, file: two_pages_pdf)
+
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/two-pages/), anything, anything, anything, anything, anything).never
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/greenpaper/), anything, anything, anything, anything, anything).times(1)
+
+    put :update,
+        params: { id: call_for_evidence,
+                  edition: {
+                    call_for_evidence_participation_attributes: {
+                      id: participation.id,
+                      call_for_evidence_response_form_attributes: {
+                        id: response_form.id,
+                        attachment_action: "replace",
+                        _destroy: "1",
+                        call_for_evidence_response_form_data_attributes: {
+                          id: response_form.call_for_evidence_response_form_data.id,
+                          file: greenpaper_pdf,
+                          file_cache: response_form_data.file_cache,
+                        },
+                      },
+                    },
+                  } }
+  end
+
+  test "POST :create discards file_cache when a file is provided" do
+    two_pages_pdf = upload_fixture("two-pages.pdf")
+    greenpaper_pdf = upload_fixture("greenpaper.pdf")
+
+    response_form_data = build(:call_for_evidence_response_form_data, file: two_pages_pdf)
+
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/two-pages/), anything, anything, anything, anything, anything).never
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/greenpaper/), anything, anything, anything, anything, anything).times(1)
+
+    attributes = controller_attributes_for(
+      :call_for_evidence,
+      call_for_evidence_participation_attributes: {
+        call_for_evidence_response_form_attributes: {
+          title: "the title of the response form",
+          call_for_evidence_response_form_data_attributes: {
+            file: greenpaper_pdf,
+            file_cache: response_form_data.file_cache,
+          },
+        },
+      },
+    )
+
+    post :create, params: { edition: attributes }
+  end
+
 private
 
   def controller_attributes_for(edition_type, attributes = {})

test/unit/app/models/call_for_evidence_response_form_data_test.rb

@@ -15,4 +15,17 @@ class CallForEvidenceResponseFormDataTest < ActiveSupport::TestCase
 
     assert_equal call_for_evidence_response_form_data.auth_bypass_ids, [auth_bypass_id]
   end
+
+  test "#all_asset_variants_uploaded? should return true when there is an original asset" do
+    call_for_evidence_response_form_data = build(:call_for_evidence_response_form_data)
+
+    assert call_for_evidence_response_form_data.all_asset_variants_uploaded?
+  end
+
+  test "#all_asset_variants_uploaded? should return false when there is no asset" do
+    call_for_evidence_response_form_data = build(:call_for_evidence_response_form_data)
+    call_for_evidence_response_form_data.assets = []
+
+    assert_equal false, call_for_evidence_response_form_data.all_asset_variants_uploaded?
+  end
 end

test/unit/app/models/call_for_evidence_test.rb

@@ -457,4 +457,14 @@ class CallForEvidenceTest < ActiveSupport::TestCase
       assert_equal call_for_evidence.document.slug, call_for_evidence.title
     end
   end
+
+  test "is invalid if consultation response form asset is missing" do
+    response_form = create(:call_for_evidence_response_form)
+    participation = create(:call_for_evidence_participation, call_for_evidence_response_form: response_form)
+    call_for_evidence = create(:open_call_for_evidence, call_for_evidence_participation: participation)
+    response_form.call_for_evidence_response_form_data.assets = []
+
+    assert_not call_for_evidence.valid?
+    assert_includes call_for_evidence.errors[:call_for_evidence_response_form], "must have finished uploading"
+  end
 end

test/unit/app/presenters/publishing_api/call_for_evidence_presenter_test.rb

@@ -281,10 +281,28 @@ class OpenCallForEvidenceWithParticipationTest < TestCase
 
     test "ways to respond" do
       Plek.any_instance.stubs(:asset_root).returns("https://asset-host.com")
-      expected_id = @participation.call_for_evidence_response_form.call_for_evidence_response_form_data.id
-      filename = @participation.call_for_evidence_response_form.call_for_evidence_response_form_data.carrierwave_file
+
+      expected_ways_to_respond = {
+        attachment_url: "https://asset-host.com/media/asset_manager_id_original/two-pages.pdf",
+        email: "[email protected]",
+        link_url: "http://www.example.com",
+        postal_address: <<-ADDRESS.strip_heredoc.chop,
+                        2 Home Farm Ln
+                        Kirklington
+                        Newark
+                        NG22 8PE
+                        UK
+        ADDRESS
+      }
+
+      assert_details_attribute :ways_to_respond, expected_ways_to_respond
+    end
+
+    test "ways to respond: filters out 'attachment_url' when call for evidence response form asset is missing" do
+      Plek.any_instance.stubs(:asset_root).returns("https://asset-host.com")
+      @participation.call_for_evidence_response_form.call_for_evidence_response_form_data.assets = []
+
       expected_ways_to_respond = {
-        attachment_url: "https://asset-host.com/government/uploads/system/uploads/call_for_evidence_response_form_data/file/#{expected_id}/#{filename}",
         email: "[email protected]",
         link_url: "http://www.example.com",
         postal_address: <<-ADDRESS.strip_heredoc.chop,

test/unit/lib/whitehall/asset_manager_storage_test.rb

@@ -172,6 +172,18 @@ def assets_protected?
       @uploader.store!(@file)
     end
   end
+
+  context "uploader model is CallForEvidenceResponseFormData" do
+    test "creates a sidekiq job and passes through the auth_bypass_id and no attachable class and id" do
+      model = CallForEvidenceResponseFormData.new
+      model.stubs(:auth_bypass_ids).returns([@auth_bypass_id])
+      @uploader.stubs(:model).returns(model)
+
+      AssetManagerCreateAssetWorker.expects(:perform_async).with(anything, anything, anything, nil, nil, [@auth_bypass_id])
+
+      @uploader.store!(@file)
+    end
+  end
 end
 
 class Whitehall::AssetManagerStorage::FileTest < ActiveSupport::TestCase