Merge pull request #8245 from alphagov/push-content-with-non-legacy-e…

…ndpoints-to-pub-api

Push content with non legacy endpoints to pub api

app/controllers/admin/attachments_controller.rb

@@ -30,9 +30,12 @@ def create
   def edit; end
 
   def update
+    use_non_legacy_endpoints = attachment.attachment_data&.use_non_legacy_endpoints
     attachment.attributes = attachment_params
     message = "Attachment '#{attachment.title}' updated"
     if attachment.is_a?(FileAttachment)
+      attachment.attachment_data.use_non_legacy_endpoints = use_non_legacy_endpoints
+
       attachment.attachment_data.attachable = attachable
       if attachment.filename_changed? && attachable.allows_inline_attachments?
         message += ". You must replace the attachment markdown with the new markdown below."
@@ -69,12 +72,6 @@ def attachable_attachments_path(attachable)
 
   def attachment
     @attachment ||= find_attachment || build_attachment
-
-    if @attachment.attachment_data
-      @attachment.attachment_data.use_non_legacy_endpoints = use_non_legacy_endpoints?
-    end
-
-    @attachment
   end
   helper_method :attachment
 
@@ -107,6 +104,7 @@ def build_file_attachment
     FileAttachment.new(attachment_params).tap do |file_attachment|
       file_attachment.build_attachment_data unless file_attachment.attachment_data
       file_attachment.attachment_data.attachable = attachable
+      file_attachment.attachment_data.use_non_legacy_endpoints = use_non_legacy_endpoints?
     end
   end
 
@@ -193,11 +191,8 @@ def save_attachment
     end
 
     if attachable_is_an_edition?
-      @draft_updater ||= Whitehall.edition_services.draft_updater(attachable)
-
-      if @draft_updater.can_perform?
-        @draft_updater.perform!
-      end
+      draft_updater = Whitehall.edition_services.draft_updater(attachable)
+      draft_updater.perform!
     end
 
     result

app/helpers/admin/editions_helper.rb

@@ -261,13 +261,6 @@ def warn_about_lack_of_contacts_in_body?(edition)
     end
   end
 
-  def attachment_uploading_status(attachment)
-    return unless attachment.attachment_data
-
-    tag.p("Uploading", class: "asset-manager-uploading") unless
-      attachment.attachment_data.uploaded_to_asset_manager?
-  end
-
   def attachment_metadata_tag(attachment)
     labels = {
       isbn: "ISBN",

app/helpers/application_helper.rb

@@ -39,7 +39,16 @@ def link_to_attachment(attachment, options = {})
 
     name = attachment.name_for_link
     html_class = options.delete(:class)
-    link_to name, attachment.url(options), class: html_class
+
+    return link_to name, attachment.url(options), class: html_class if !attachment.file? || attachment.attachment_data.all_asset_variants_uploaded?
+
+    tag.span(name)
+  end
+
+  def link_to_attachment_data(attachment_data)
+    return link_to attachment_data.filename, attachment_data.url, class: "govuk-link" if attachment_data.all_asset_variants_uploaded?
+
+    tag.span(attachment_data.filename)
   end
 
   def worldwide_office_type_options

app/helpers/attachments_helper.rb

@@ -60,7 +60,9 @@ def attachment_component_params(attachment, alternative_format_contact_email: ni
   end
 
   def block_attachments(attachments = [], alternative_format_contact_email = nil)
-    attachments.map do |attachment|
+    attachments
+      .select { |attachment| !attachment.file? || attachment.attachment_data.all_asset_variants_uploaded? }
+      .map do |attachment|
       render(
         partial: "govuk_publishing_components/components/attachment",
         locals: {

app/helpers/govspeak_helper.rb

@@ -63,7 +63,9 @@ def prepare_images(images)
   end
 
   def prepare_attachments(attachments, alternative_format_contact_email)
-    attachments.map do |attachment|
+    attachments
+      .select { |attachment| !attachment.file? || attachment.attachment_data.all_asset_variants_uploaded? }
+      .map do |attachment|
       attachment_component_params(attachment, alternative_format_contact_email:)
     end
   end

app/models/asset.rb

@@ -4,6 +4,7 @@ class Asset < ApplicationRecord
   validates :asset_manager_id, presence: true
   validates :assetable, presence: true
   validates :variant, presence: true
+  validates :filename, presence: true
 
   enum variant: {
     original: "original".freeze,

app/models/attachable.rb

@@ -41,6 +41,10 @@ def process_associations_after_save(edition)
         end
       end
     end
+
+    def attachments_ready_for_publishing
+      attachments.select { |attachment| !attachment.file? || attachment.attachment_data.all_asset_variants_uploaded? }
+    end
   end
 
   class Null
@@ -95,7 +99,7 @@ def uploaded_to_asset_manager?
     attachments
       .map(&:attachment_data)
       .compact
-      .all?(&:uploaded_to_asset_manager?)
+      .all?(&:all_asset_variants_uploaded?)
   end
 
   def allows_attachments?

app/models/attachment_data.rb

@@ -16,7 +16,7 @@ class AttachmentData < ApplicationRecord
   validates :file, presence: true
   validate :file_is_not_empty
 
-  attr_accessor :to_replace_id, :attachable, :use_non_legacy_endpoints
+  attr_accessor :to_replace_id, :attachable
 
   belongs_to :replaced_by, class_name: "AttachmentData"
   validate :cant_be_replaced_by_self
@@ -27,15 +27,15 @@ class AttachmentData < ApplicationRecord
   attribute :present_at_unpublish, :boolean, default: false
 
   def filename
-    url && File.basename(url)
+    file.present? && file.file.filename
   end
 
   def filename_without_extension
-    url && filename.sub(/.[^.]*$/, "")
+    filename && filename.sub(/.[^.]*$/, "")
   end
 
   def file_extension
-    File.extname(url).delete(".") if url.present?
+    File.extname(filename).delete(".") if filename.present?
   end
 
   def pdf?
@@ -47,7 +47,9 @@ def txt?
   end
 
   def csv?
-    file_extension.casecmp("csv").zero?
+    return file_extension.casecmp("csv").zero? if file_extension
+
+    false
   end
 
   # Is in OpenDocument format? (see https://en.wikipedia.org/wiki/OpenDocument)
@@ -59,6 +61,14 @@ def indexable?
     AttachmentUploader::INDEXABLE_TYPES.include?(file_extension)
   end
 
+  def all_asset_variants_uploaded?
+    if use_non_legacy_endpoints
+      return assets.size == (pdf? ? 2 : 1)
+    end
+
+    uploaded_to_asset_manager?
+  end
+
   def update_file_attributes
     if carrierwave_file.present? && carrierwave_file_changed?
       self.content_type = file.file.content_type

app/models/image_data.rb

@@ -1,12 +1,15 @@
 require "mini_magick"
 
 class ImageData < ApplicationRecord
-  attr_accessor :validate_on_image, :use_non_legacy_endpoints
+  attr_accessor :validate_on_image
 
   VALID_WIDTH = 960
   VALID_HEIGHT = 640
 
   has_many :images
+  has_many :assets,
+           as: :assetable,
+           inverse_of: :assetable
 
   mount_uploader :file, ImageUploader, mount_on: :carrierwave_image
 

app/presenters/publishing_api/call_for_evidence_presenter.rb

@@ -139,7 +139,7 @@ def documents
       end
 
       def featured_attachments
-        call_for_evidence.attachments.map { |a| a.publishing_api_details[:id] }
+        call_for_evidence.attachments_ready_for_publishing.map { |a| a.publishing_api_details[:id] }
       end
     end
 
@@ -203,7 +203,7 @@ def outcome_documents
       end
 
       def outcome_attachments
-        outcome.attachments.map { |a| a.publishing_api_details[:id] }
+        outcome.attachments_ready_for_publishing.map { |a| a.publishing_api_details[:id] }
       end
     end
 

app/presenters/publishing_api/consultation_presenter.rb

@@ -140,7 +140,7 @@ def documents
       end
 
       def featured_attachments
-        consultation.attachments.map { |a| a.publishing_api_details[:id] }
+        consultation.attachments_ready_for_publishing.map { |a| a.publishing_api_details[:id] }
       end
     end
 
@@ -204,7 +204,7 @@ def final_outcome_documents
       end
 
       def final_outcome_attachments
-        outcome.attachments.map { |a| a.publishing_api_details[:id] }
+        outcome.attachments_ready_for_publishing.map { |a| a.publishing_api_details[:id] }
       end
     end
 
@@ -272,7 +272,7 @@ def documents
       end
 
       def attachments
-        public_feedback.attachments.map { |a| a.publishing_api_details[:id] }
+        public_feedback.attachments_ready_for_publishing.map { |a| a.publishing_api_details[:id] }
       end
 
       def publication_date

app/presenters/publishing_api/payload_builder/attachments.rb

@@ -19,7 +19,7 @@ def call
       def attachments
         items.flat_map do |item|
           if item
-            item.attachments.map(&:publishing_api_details)
+            item.attachments_ready_for_publishing.map(&:publishing_api_details)
           else
             []
           end

app/presenters/publishing_api/publication_presenter.rb

@@ -101,7 +101,7 @@ def featured_attachments
     end
 
     def attachments_for_current_locale
-      attachments = item.attachments
+      attachments = item.attachments_ready_for_publishing
       # nil/"" locale should always be returned
       locales_that_match = [I18n.locale.to_s, ""]
       attachments.to_a.select do |attachment|

app/services/asset_manager/attachment_deleter.rb

@@ -2,13 +2,13 @@ class AssetManager::AttachmentDeleter
   def self.call(attachment_data)
     return unless attachment_data.deleted?
 
-    if attachment_data.assets.empty?
-      AssetManager::AssetDeleter.call(attachment_data.file.asset_manager_path, nil)
-      AssetManager::AssetDeleter.call(attachment_data.file.thumbnail.asset_manager_path, nil) if attachment_data.pdf?
-    else
+    if attachment_data.use_non_legacy_endpoints
       attachment_data.assets.each do |asset|
         AssetManager::AssetDeleter.call(nil, asset.asset_manager_id)
       end
+    else
+      AssetManager::AssetDeleter.call(attachment_data.file.asset_manager_path, nil)
+      AssetManager::AssetDeleter.call(attachment_data.file.thumbnail.asset_manager_path, nil) if attachment_data.pdf?
     end
   end
 end

app/services/asset_manager/attachment_updater.rb

@@ -10,7 +10,15 @@ def self.call(
     return if attachment_data.deleted?
 
     # This logic will be eventually removed as part of cleaning up the feature flag for removing legacy_url_path
-    if attachment_data.assets.empty?
+    if attachment_data.use_non_legacy_endpoints
+      attachment_data.assets.each do |asset|
+        asset_attributes = get_asset_attributes(attachment_data, asset, access_limited, draft_status, link_header, redirect_url, replacement_id)
+
+        next unless asset_attributes.any?
+
+        AssetManager::AssetUpdater.call(asset.asset_manager_id, attachment_data, nil, asset_attributes.deep_stringify_keys)
+      end
+    else
       updates = []
 
       updates += AccessLimitedUpdates.call(attachment_data).to_a if access_limited
@@ -20,14 +28,6 @@ def self.call(
       updates += ReplacementIdUpdates.call(attachment_data).to_a if replacement_id
 
       combined_updates(updates).each(&:call)
-    else
-      attachment_data.assets.each do |asset|
-        asset_attributes = get_asset_attributes(attachment_data, asset, access_limited, draft_status, link_header, redirect_url, replacement_id)
-
-        next unless asset_attributes.any?
-
-        AssetManager::AssetUpdater.call(asset.asset_manager_id, attachment_data, nil, asset_attributes.deep_stringify_keys)
-      end
     end
   end
 
@@ -50,7 +50,7 @@ def self.get_asset_attributes(attachment_data, asset, access_limited, draft_stat
       access_limited_organisation_ids: access_limited ? get_access_limited(attachment_data) : nil,
       draft: draft_status ? get_draft(attachment_data) : nil,
       parent_document_url: link_header ? get_link_header(attachment_data) : nil,
-      replacement_id: replacement_id ? get_replacement_id(attachment_data, asset) : nil,
+      replacement_id: replacement_id ? get_replacement_id(attachment_data, asset.variant) : nil,
     }.compact
     new_attributes.merge!(redirect_url: get_redirect_url(attachment_data)) if redirect_url
 
@@ -91,15 +91,18 @@ def self.get_redirect_url(attachment_data)
     attachment_data.unpublished_edition.unpublishing.document_url
   end
 
-  def self.get_replacement_id(attachment_data, asset)
-    return nil unless attachment_data.replaced?
+  def self.get_replacement_id(replaced_attachment_data, variant)
+    return nil unless replaced_attachment_data.replaced?
+
+    replacement = replaced_attachment_data.replaced_by
+    replacement_asset = replacement.assets.where(variant:).first
 
-    replacement = attachment_data.replaced_by
-    replacement_asset = replacement.assets.where(variant: asset.variant).first
     if replacement_asset
       replacement_asset.asset_manager_id
     else
-      replacement.assets.where(variant: Asset.variants[:original]).first.asset_manager_id
+      original_variant = replacement.assets.where(variant: Asset.variants[:original]).first
+
+      original_variant ? original_variant.asset_manager_id : nil
     end
   end
 end

app/views/admin/attachments/_attachment_data_fields.html.erb

@@ -1,7 +1,7 @@
 <%= form.fields_for(:attachment_data, include_id: false) do |attachment_data_fields| %>
   <% if attachment_data_fields.object.filename %>
     <%= attachment_data_fields.hidden_field(:to_replace_id, value: attachment_data_fields.object.to_replace_id || attachment_data_fields.object.id) %>
-    <p class="govuk-body">Current file: <%= link_to attachment_data_fields.object.filename, attachment_data_fields.object.url, class: "govuk-link" %></p>
+    <p class="govuk-body">Current file: <%= link_to_attachment_data(attachment_data_fields.object) %></p>
   <% end %>
 
   <%= render "govuk_publishing_components/components/file_upload", {

app/views/admin/attachments/_attachments.html.erb

@@ -7,7 +7,7 @@
     <li class="govuk-grid-row">
       <div class="govuk-grid-column-full">
         <p class="govuk-body">
-          <strong>Title:</strong> <%= attachment.title %> <% unless attachment.attachment_data.blank? || attachment.attachment_data.uploaded_to_asset_manager? %><span class="govuk-tag govuk-tag--green">Uploading</span><% end %>
+          <strong>Title:</strong> <%= attachment.title %> <% unless attachment.attachment_data.blank? || attachment.attachment_data.all_asset_variants_uploaded? %><span class="govuk-tag govuk-tag--green">Uploading</span><% end %>
         </p>
         <p class="govuk-body">
           <strong>Type: </strong><%= attachment.is_a?(HtmlAttachment) ? attachment.readable_type : attachment.readable_type.capitalize %>

app/workers/asset_manager_attachment_metadata_worker.rb

@@ -3,7 +3,7 @@ class AssetManagerAttachmentMetadataWorker < WorkerBase
 
   def perform(attachment_data_id)
     attachment_data = AttachmentData.find(attachment_data_id)
-    return unless attachment_data.present? && attachment_data.uploaded_to_asset_manager_at
+    return unless attachment_data.present? && attachment_data.all_asset_variants_uploaded?
 
     AssetManager::AttachmentUpdater.call(
       attachment_data,