Merge pull request #8520 from alphagov/use-asset-id-for-consultationr…

…esponseformdata

Use asset-id for for consultation_response_form_data

app/models/consultation.rb

@@ -13,6 +13,7 @@ class Consultation < Publicationesque
   validates :external_url, presence: true, if: :external?
   validates :external_url, uri: true, allow_blank: true
   validate :validate_consultation_principles, unless: ->(consultation) { Edition::PRE_PUBLICATION_STATES.include? consultation.state }
+  validate :consultation_response_file_uploaded_to_asset_manager!, if: :consultation_response_file_in_asset_manager_check_required?
 
   has_one :outcome, class_name: "ConsultationOutcome", foreign_key: :edition_id, dependent: :destroy
   has_one :public_feedback, class_name: "ConsultationPublicFeedback", foreign_key: :edition_id, dependent: :destroy
@@ -191,4 +192,12 @@ def validate_consultation_principles
       errors.add :read_consultation_principles, "must be ticked"
     end
   end
+
+  def consultation_response_file_in_asset_manager_check_required?
+    has_consultation_participation? && consultation_participation.has_response_form? && published?
+  end
+
+  def consultation_response_file_uploaded_to_asset_manager!
+    errors.add(:consultation_response_form, "must have finished uploading") unless consultation_participation.consultation_response_form_uploaded_to_asset_manager?
+  end
 end

app/models/consultation_participation.rb

@@ -26,6 +26,10 @@ def has_postal_address?
 
   after_destroy :destroy_form_if_required
 
+  def consultation_response_form_uploaded_to_asset_manager?
+    has_response_form? && consultation_response_form&.consultation_response_form_data&.all_asset_variants_uploaded?
+  end
+
 private
 
   def destroy_form_if_required

app/models/consultation_response_form_data.rb

@@ -6,9 +6,20 @@ class ConsultationResponseFormData < ApplicationRecord
            as: :assetable,
            inverse_of: :assetable
 
+  has_many :assets,
+           as: :assetable,
+           inverse_of: :assetable
+
   validates :file, presence: true
 
   def auth_bypass_ids
     [consultation_response_form.consultation_participation.consultation.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
 end

app/presenters/publishing_api/consultation_presenter.rb

@@ -316,7 +316,7 @@ def call
       alias_method :participation_response_form, :consultation_response_form
 
       def attachment_url
-        return unless participation.has_response_form?
+        return unless participation.consultation_response_form_uploaded_to_asset_manager?
 
         participation_response_form.file.url
       end

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

@@ -100,11 +100,15 @@
 
           <%= response_form.fields_for :consultation_response_form_data, consultation_response_form_data do |consultation_response_form_data_form| %>
             <%= consultation_response_form_data_form.hidden_field  :file_cache, value: consultation_response_form_data.file_cache %>
-
             <% if consultation_response_form.consultation_response_form_data.try(:persisted?) %>
               <div class="attachment">
-                <p class="govuk-body">Current data: <%= link_to File.basename(consultation_response_form_data.file.path), consultation_response_form_data.file.url, class: "govuk-link" %></p>
-
+                  <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" %>
+                    <% else %>
+                      <%= File.basename(consultation_response_form_data.file.path) %> <span class="govuk-tag govuk-tag--green">Processing</span>
+                    <% end %>
+                  </p>
                 <%= render "govuk_publishing_components/components/radio", {
                   heading: "Actions:",
                   name: "edition[consultation_participation_attributes][consultation_response_form_attributes][attachment_action]",

app/workers/asset_manager_update_whitehall_asset_worker.rb

@@ -28,6 +28,6 @@ def perform(klass, id, attributes)
 private
 
   def should_use_non_legacy_endpoints?(asset_data)
-    asset_data.instance_of?(AttachmentData) || asset_data.instance_of?(ImageData)
+    asset_data.instance_of?(AttachmentData) || asset_data.instance_of?(ImageData) || asset_data.instance_of?(ConsultationResponseFormData)
   end
 end

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)
+    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.respond_to?("use_non_legacy_endpoints") && model.use_non_legacy_endpoints
   end

test/factories/consultation_response_form_data.rb

@@ -1,5 +1,9 @@
 FactoryBot.define do
   factory :consultation_response_form_data do
     file { File.open(Rails.root.join("test/fixtures/two-pages.pdf")) }
+
+    after(:build) do |consultation_response_form_data|
+      consultation_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/consultations_controller_test.rb

@@ -131,6 +131,17 @@ class Admin::ConsultationsControllerTest < ActionController::TestCase
     end
   end
 
+  view_test "create should show 'Processing' tag if variant is missing" do
+    response_form = create(:consultation_response_form)
+    participation = create(:consultation_participation, consultation_response_form: response_form)
+    consultation = create(:consultation, consultation_participation: participation)
+    response_form.consultation_response_form_data.assets = []
+
+    get :edit, params: { id: consultation }
+
+    assert_select "span[class='govuk-tag govuk-tag--green']", text: "Processing"
+  end
+
   view_test "show renders the summary" do
     draft_consultation = create(:draft_consultation, summary: "a-simple-summary")
     stub_publishing_api_expanded_links_with_taxons(draft_consultation.content_id, [])

test/integration/asset_access_options_integration_test.rb

@@ -238,14 +238,12 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
         click_button "Save"
 
         # Note that there is no access limiting applied to non attachments. This is existing behaviour that probably needs changing.
-        Services.asset_manager.expects(:create_whitehall_asset).with(
-          has_entries(
-            legacy_url_path: regexp_matches(/simple\.pdf/),
-            auth_bypass_ids: [edition.auth_bypass_id],
-          ),
-        )
-
-        AssetManagerCreateWhitehallAssetWorker.drain
+        Services.asset_manager.expects(:create_asset).with { |args|
+          args[:file].path =~ /simple\.pdf/
+          args[:auth_bypass_ids] == [edition.auth_bypass_id]
+        }.returns(asset_manager_response)
+
+        AssetManagerCreateAssetWorker.drain
       end
     end
 

test/integration/asset_manager_test.rb

@@ -161,6 +161,7 @@ class ReplacingAPersonImage < ActiveSupport::TestCase
   class CreatingAConsultationResponseFormData < ActiveSupport::TestCase
     setup do
       @filename = "greenpaper.pdf"
+      @asset_manager_response = { "id" => "http://asset-manager/assets/asset_manager_id", "name" => @filename }
       ConsultationResponseFormData.any_instance.stubs(:auth_bypass_ids).returns([])
       @consultation_response_form_data = FactoryBot.build(
         :consultation_response_form_data,
@@ -169,17 +170,17 @@ class CreatingAConsultationResponseFormData < ActiveSupport::TestCase
     end
 
     test "sends the consultation response form data file to Asset Manager" do
-      Services.asset_manager.expects(:create_whitehall_asset).with(
-        file_and_legacy_url_path_matching(/#{@filename}/),
-      )
+      Services.asset_manager.expects(:create_asset).with { |args|
+        args[:file].path =~ /#{@filename}/
+      }.returns(@asset_manager_response)
 
       Sidekiq::Testing.inline! do
         @consultation_response_form_data.save!
       end
     end
 
-    test "does not mark the consultation response form data as draft in Asset Manager" do
-      Services.asset_manager.expects(:create_whitehall_asset).with(Not(has_key(:draft)))
+    test "sends draft as false for consultation response form data to Asset Manager" do
+      Services.asset_manager.expects(:create_asset).with(has_entry(draft: false)).returns(@asset_manager_response)
 
       Sidekiq::Testing.inline! do
         @consultation_response_form_data.save!
@@ -190,24 +191,19 @@ class CreatingAConsultationResponseFormData < ActiveSupport::TestCase
   class RemovingAConsultationResponseFormData < ActiveSupport::TestCase
     setup do
       filename = "greenpaper.pdf"
-      @consultation_response_form_asset_id = "asset-id"
       ConsultationResponseFormData.any_instance.stubs(:auth_bypass_ids).returns([])
       @consultation_response_form_data = FactoryBot.create(
         :consultation_response_form_data,
         file: File.open(fixture_path.join(filename)),
       )
-      @consultation_response_form_data.reload
-      @file_path = @consultation_response_form_data.file.path
 
-      Services.asset_manager.stubs(:whitehall_asset)
-              .with(regexp_matches(/#{filename}/))
-              .returns("id" => "http://asset-manager/assets/#{@consultation_response_form_asset_id}")
-      Services.asset_manager.stubs(:delete_asset)
+      @asset_manager_id = @consultation_response_form_data.assets.first.asset_manager_id
+      Services.asset_manager.stubs(:asset).with(@asset_manager_id).returns("id" => "http://asset-manager/assets/#{@asset_manager_id}", "name" => filename)
     end
 
     test "removing a consultation response form data file removes it from asset manager" do
       Services.asset_manager.expects(:delete_asset)
-              .with(@consultation_response_form_asset_id)
+              .with(@asset_manager_id)
 
       Sidekiq::Testing.inline! do
         @consultation_response_form_data.file.remove!
@@ -218,26 +214,23 @@ class RemovingAConsultationResponseFormData < ActiveSupport::TestCase
   class ReplacingAConsultationResponseFormData < ActiveSupport::TestCase
     setup do
       filename = "greenpaper.pdf"
-      @consultation_response_form_asset_id = "asset-id"
       ConsultationResponseFormData.any_instance.stubs(:auth_bypass_ids).returns([])
       @consultation_response_form_data = FactoryBot.create(
         :consultation_response_form_data,
         file: File.open(fixture_path.join(filename)),
       )
-      @consultation_response_form_data.reload
-      @file_path = @consultation_response_form_data.file.path
 
-      Services.asset_manager.stubs(:whitehall_asset)
-              .with(regexp_matches(/#{filename}/))
-              .returns("id" => "http://asset-manager/assets/#{@consultation_response_form_asset_id}")
-      Services.asset_manager.stubs(:delete_asset)
+      @asset_manager_id = @consultation_response_form_data.assets.first.asset_manager_id
+      Services.asset_manager.stubs(:asset).with(@asset_manager_id).returns({ "id" => "http://asset-manager/assets/#{@asset_manager_id}", "name" => filename })
     end
 
     test "replacing a consultation response form data file removes the old file from asset manager" do
-      Services.asset_manager.expects(:delete_asset)
-              .with(@consultation_response_form_asset_id)
-
-      @consultation_response_form_data.file = File.open(fixture_path.join("whitepaper.pdf"))
+      replacement_filename = "whitepaper.pdf"
+      Services.asset_manager.expects(:create_asset).with { |args|
+        args[:file].path =~ /#{replacement_filename}/
+      }.returns({ "id" => "http://asset-manager/assets/asset_manager_id_new", "name" => replacement_filename })
+      Services.asset_manager.expects(:delete_asset).with(@asset_manager_id)
+      @consultation_response_form_data.file = File.open(fixture_path.join(replacement_filename))
 
       Sidekiq::Testing.inline! do
         @consultation_response_form_data.save!

test/unit/app/models/consultation_response_form_data_test.rb

@@ -15,4 +15,17 @@ class ConsultationResponseFormDataTest < ActiveSupport::TestCase
 
     assert_equal consultation_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
+    consultation_response_form_data = build(:consultation_response_form_data)
+
+    assert consultation_response_form_data.all_asset_variants_uploaded?
+  end
+
+  test "#all_asset_variants_uploaded? should return false when there is no asset" do
+    consultation_response_form_data = build(:consultation_response_form_data)
+    consultation_response_form_data.assets = []
+
+    assert_equal false, consultation_response_form_data.all_asset_variants_uploaded?
+  end
 end

test/unit/app/models/consultation_test.rb

@@ -515,4 +515,15 @@ class ConsultationTest < ActiveSupport::TestCase
       assert_equal consultation.document.slug, consultation.title
     end
   end
+
+  test "is invalid if consultation response form asset is missing" do
+    response_form_data = build(:consultation_response_form_data)
+    response_form_data.assets = []
+    response_form = build(:consultation_response_form, consultation_response_form_data: response_form_data)
+    participation = build(:consultation_participation, consultation_response_form: response_form)
+    consultation = build(:open_consultation, consultation_participation: participation)
+
+    assert_not consultation.valid?
+    assert_includes consultation.errors[:consultation_response_form], "must have finished uploading"
+  end
 end

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

@@ -282,10 +282,26 @@ class OpenConsultationWithParticipationTest < TestCase
 
     test "ways to respond" do
       Plek.any_instance.stubs(:asset_root).returns("https://asset-host.com")
-      expected_id = @participation.consultation_response_form.consultation_response_form_data.id
-      expected_filename = @participation.consultation_response_form.consultation_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 consultation response form asset is missing" do
+      @participation.consultation_response_form.consultation_response_form_data.assets = []
       expected_ways_to_respond = {
-        attachment_url: "https://asset-host.com/government/uploads/system/uploads/consultation_response_form_data/file/#{expected_id}/#{expected_filename}",
         email: "[email protected]",
         link_url: "http://www.example.com",
         postal_address: <<-ADDRESS.strip_heredoc.chop,

test/unit/app/services/editon_auth_bypass_updater_test.rb

@@ -98,6 +98,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
       participation = create(:consultation_participation, consultation: edition)
       consultation_response_form = create(:consultation_response_form, consultation_participation: participation)
 
+      edition.reload
       SecureRandom.stubs(uuid: uid)
       expected_attributes = { "auth_bypass_ids" => [uid] }