DEV: Make attachment markdown reusable

DEV: Make attachment markdown reusable

diff --git a/app/jobs/regular/export_csv_file.rb b/app/jobs/regular/export_csv_file.rb
index d8d3a72..4db9fd2 100644
--- a/app/jobs/regular/export_csv_file.rb
+++ b/app/jobs/regular/export_csv_file.rb
@@ -4,12 +4,11 @@ require 'csv'
 require 'zip'
 require_dependency 'system_message'
 require_dependency 'upload_creator'
+require_dependency 'discourse_markdown'
 
 module Jobs
 
   class ExportCsvFile < Jobs::Base
-    include ActionView::Helpers::NumberHelper
-
     sidekiq_options retry: false
 
     HEADER_ATTRS_FOR ||= HashWithIndifferentAccess.new(
@@ -406,7 +405,7 @@ module Jobs
           SystemMessage.create_from_system_user(
             @current_user,
             :csv_export_succeeded,
-            download_link: "[#{upload.original_filename}|attachment](#{upload.short_url}) (#{number_to_human_size(upload.filesize)})",
+            download_link: DiscourseMarkdown.attachment_markdown(upload),
             export_title: export_title
           )
         else
diff --git a/lib/discourse_markdown.rb b/lib/discourse_markdown.rb
new file mode 100644
index 0000000..8200372
--- /dev/null
+++ b/lib/discourse_markdown.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require_dependency "file_helper"
+
+class DiscourseMarkdown
+  def self.upload_markdown(upload, display_name: nil)
+    if FileHelper.is_supported_image?(upload.original_filename)
+      image_markdown(upload)
+    else
+      attachment_markdown(upload, display_name: display_name)
+    end
+  end
+
+  def self.image_markdown(upload)
+    "![#{upload.original_filename}|#{upload.width}x#{upload.height}](#{upload.short_url})"
+  end
+
+  def self.attachment_markdown(upload, display_name: nil, with_filesize: true)
+    human_filesize = with_filesize ? " (#{upload.human_filesize})" : ""
+    display_name ||= upload.original_filename
+
+    "[#{display_name}|attachment](#{upload.short_url})#{human_filesize}"
+  end
+end
diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb
index 18452eb..04882bb 100644
--- a/lib/email/receiver.rb
+++ b/lib/email/receiver.rb
@@ -5,12 +5,11 @@ require_dependency "new_post_manager"
 require_dependency "html_to_markdown"
 require_dependency "plain_text_to_markdown"
 require_dependency "upload_creator"
+require_dependency "discourse_markdown"
 
 module Email
 
   class Receiver
-    include ActionView::Helpers::NumberHelper
-
     # If you add a new error, you need to
     #   * add it to Email::Processor#handle_failure()
     #   * add text to server.en.yml (parent key: "emails.incoming.errors")
@@ -1035,19 +1034,16 @@ module Email
 
                 InlineUploads.match_img(raw) do |match, src, replacement, _|
                   if src == upload.url
-                    raw = raw.sub(
-                      match,
-                      "![#{upload.original_filename}|#{upload.width}x#{upload.height}](#{upload.short_url})"
-                    )
+                    raw = raw.sub(match, DiscourseMarkdown.image_markdown(upload))
                   end
                 end
               elsif raw[/\[image:.*?\d+[^\]]*\]/i]
-                raw.sub!(/\[image:.*?\d+[^\]]*\]/i, attachment_markdown(upload))
+                raw.sub!(/\[image:.*?\d+[^\]]*\]/i, DiscourseMarkdown.upload_markdown(upload))
               else
-                raw << "\n\n#{attachment_markdown(upload)}\n\n"
+                raw << "\n\n#{DiscourseMarkdown.upload_markdown(upload)}\n\n"
               end
             else
-              raw << "\n\n#{attachment_markdown(upload)}\n\n"
+              raw << "\n\n#{DiscourseMarkdown.upload_markdown(upload)}\n\n"
             end
           else
             rejected_attachments << upload
@@ -1082,14 +1078,6 @@ module Email
       Email::Sender.new(client_message, :email_reject_attachment).send
     end
 
-    def attachment_markdown(upload)
-      if FileHelper.is_supported_image?(upload.original_filename)
-        "![#{upload.original_filename}|#{upload.width}x#{upload.height}](#{upload.short_url})"
-      else
-        "[#{upload.original_filename}|attachment](#{upload.short_url}) (#{number_to_human_size(upload.filesize)})"
-      end
-    end
-
     def create_post(options = {})
       options[:via_email] = true
       options[:raw_email] = @raw_email
diff --git a/script/import_scripts/base.rb b/script/import_scripts/base.rb
index 944f26c..b149ac7 100644
--- a/script/import_scripts/base.rb
+++ b/script/import_scripts/base.rb
@@ -21,8 +21,6 @@ module ImportScripts; end
 
 class ImportScripts::Base
 
-  include ActionView::Helpers::NumberHelper
-
   def initialize
     preload_i18n
 
diff --git a/script/import_scripts/base/uploader.rb b/script/import_scripts/base/uploader.rb
index 0d8acac..fc5df45 100644
--- a/script/import_scripts/base/uploader.rb
+++ b/script/import_scripts/base/uploader.rb
@@ -1,12 +1,10 @@
 # frozen_string_literal: true
 
 require_dependency 'url_helper'
-require_dependency 'file_helper'
+require_dependency 'discourse_markdown'
 
 module ImportScripts
   class Uploader
-    include ActionView::Helpers::NumberHelper
-
     # Creates an upload.
     # Expects path to be the full path and filename of the source file.
     # @return [Upload]
@@ -42,22 +40,15 @@ module ImportScripts
     end
 
     def html_for_upload(upload, display_filename)
-      if FileHelper.is_supported_image?(upload.url)
-        embedded_image_html(upload)
-      else
-        attachment_html(upload, display_filename)
-      end
+      DiscourseMarkdown.upload_markdown(upload, display_name: display_filename)
     end
 
     def embedded_image_html(upload)
-      image_width = [upload.width, SiteSetting.max_image_width].compact.min
-      image_height = [upload.height, SiteSetting.max_image_height].compact.min
-      upload_name = upload.short_url || upload.url
-      %Q~![#{upload.original_filename}|#{image_width}x#{image_height}](#{upload_name})~
+      DiscourseMarkdown.image_markdown(upload)
     end
 
     def attachment_html(upload, display_filename)
-      "[#{display_filename}|attachment](#{upload.short}) (#{number_to_human_size(upload.filesize)})"
+      DiscourseMarkdown.attachment_markdown(upload, display_name: display_filename)
     end
 
     private

GitHub sha: a61ff167

One tip here, if you are passing the same upload object and keyword args display_name from method to method, that’s a clue that it might work better as an instance variables.

Also maybe the idea in the future is to use it for other kinds of objects, but if everything is an upload I would suggest making it UploadMarkdown.new(upload).to_markdown

That was the idea. I often want to put small pieces of reusable code somewhere and because of all the specific helper names I seldomly find the right place to put things. And renaming classes after a while is really hard, so I try to name helper classes as broadly as possible.

Do you still want me to refactor it to UploadMarkdown?

I think in this case it makes sense because you end up passing the upload object everywhere. If you need a more broad one in the future you could have it use the UploadMarkdown class.

DEV: Refactor helper methods for upload markdown