Mange / roadie-rails

Making HTML emails comfortable for the Rails rockstars
MIT License
363 stars 65 forks source link

Question: How to work with remote asset? #10

Open PikachuEXE opened 10 years ago

PikachuEXE commented 10 years ago

I got something like //assets.my.domain/assets/mailer-4d19915455ba1fec1f18ca31aaccf9ce.css I got this generated URL with this code: = stylesheet_link_tag('mailer.css')

I have no idea how to get local assets

Mange commented 10 years ago

Have you set an asset_host? That is most likely the "problem".

You should be able to create an asset provider that recognizes your asset host and strips it from the path and then finds it on the file system. This is most likely the way I'll solve this bug officially. Den 3 jul 2014 04:38 skrev "PikachuEXE" notifications@github.com:

I got something like //assets.my.domain/assets/mailer-4d19915455ba1fec1f18ca31aaccf9ce.css I got this generated URL with this code: = stylesheet_link_tag('mailer.css')

I have no idea how to get local assets

— Reply to this email directly or view it on GitHub https://github.com/Mange/roadie-rails/issues/10.

PikachuEXE commented 10 years ago

production.rb:

asset_host = ENV["ASSET_HOST"]
  if asset_host.present?
    config.action_controller.asset_host = if asset_host =~ /%d/
      proc do |source|
        # has to call ENV["ASSET_HOST"] since `asset_host` here is not accessible on execution
        "//#{ENV["ASSET_HOST"] % (Zlib.crc32(source) % 4)}"
      end
    else
      "//#{asset_host}"
    end
    config.action_mailer.asset_host = config.action_controller.asset_host
  end
bcackerman commented 10 years ago

+1

Mange commented 10 years ago

I'll look into it.

Mange commented 10 years ago

You should be able to turn off asset_host in your specific mailer as a workaround:

class MyMailer < ActionMailer::Base
  self.asset_host = nil
end

This will not use the asset host for your images, so it's not a full "solution" to the problem.

Mange commented 10 years ago

Planning for the fix happens in #11. It's not trivial to support this, so be prepared to use a workaround for some time.

adamwaite commented 10 years ago

I have managed to maintain use of the asset_host setting for images (can still use image_tag) and made Roadie use a relative path for the styles by explicitly defining the stylesheet path as relative rather than using the stylesheet_link_tag helper. I had to use a non-digest version of the stylesheet.

Mange commented 10 years ago

Here's another workaround:

class MyMailer < ActionMailer::Base
  include Roadie::Rails::Automatic

  protected
  def roadie_options
    super.combine({
      before_transformation: method(:reverse_asset_host_on_stylesheets)
    })
  end

  def reverse_asset_host_on_stylesheets(dom)
    dom.all_css("link[rel=stylesheet]").each do |link_tag|
      if link_tag["href"]
        link_tag["href"] = link_tag["href"].sub(%r{^https?://assets\d\.mycoolapp\.com}, "")
      end
    end
  end
end

Does that make sense? At least you get asset_host applied to all your images while still having styles inlined. If this works out (or at least a variation of it), I might make this a first-class thing within AssetProviders; a callable you set to reverse the asset_host where possible.

milgner commented 9 years ago

Just came across this and found that while it works in principle, one also has to filter out the digest from the asset path. My code now looks like this:

  def reverse_asset_host_on_stylesheets(dom)
    unless Rails.application.config.action_mailer.asset_host.nil?
      dom.search('link[rel=stylesheet]').each do |link_tag|
        if link_tag['href']
          pipeline_path = link_tag['href'][Rails.application.config.action_mailer.asset_host.length..-1]
          pipeline_path.sub!(/-[0-9a-f]{32,}(\.\w{1,4})(?:\?.*)\Z?/, '\1')
          link_tag['href'] = '/' + pipeline_path
        end
      end
    end
  end

Edited to fix regex.

Mange commented 9 years ago

Thank you for the heads-up, Marcus! :+1:

For other people: Note that his code does not work if the asset host is configured with a block, or an array. Only a single string is supported by the above code. It is also important to remember what happens if you use a %s placeholder in the configured asset host: it will be replaced with a single character (1..4, I think) in the rendered markup, thereby reducing the length by one. In practice, this just means that an extra / might get stripped, but it's still useful to know about it. Make sure to write tests! :-)

Mange commented 8 years ago

A new version of roadie is now out: 3.1.0.rc1. This version allows Roadie to actually handle external stylesheets, either by literally downloading them over the network, or by rewriting the paths into local paths (or any combination thereof).

Maybe you can use this capability to work around this problem? See the roadie README for examples.

You should also upgrade roadie-rails to 1.1.0.rc2 to get full support.

PikachuEXE commented 8 years ago

Let me test it on Friday (if I have time then)

PikachuEXE commented 8 years ago

Successful case (asset provider) for development:

class UserAssetsProvider
  ABSOLUTE_ASSET_PATH_REGEXP = /\A#{Regexp.escape("//")}.+#{Regexp.escape("/assets/")}/i

  def find_stylesheet(name)
    puts "name: #{name}"
    return nil unless file_exists?(name)

    Roadie::Stylesheet.new("whatever", stylesheet_content(name))
  end

  def find_stylesheet!(name)
    stylesheet = find_stylesheet(name)

    if stylesheet.nil?
      raise Roadie::CssNotFound.new(
        name,
        "does not exists",
        self,
      )
    end

    stylesheet
  end

  private

  def file_exists?(name)
    if assets_precompiled?
      File.exists?(local_file_path(name))
    else
      sprockets_asset(name)
    end
  end

  # If on-the-fly asset compilation is disabled, we must be precompiling assets.
  def assets_precompiled?
    !Rails.configuration.assets.compile rescue false
  end

  def local_file_path(name)
    asset_path = asset_path(name)

    if asset_path.match(ABSOLUTE_ASSET_PATH_REGEXP)
      asset_path.gsub!(ABSOLUTE_ASSET_PATH_REGEXP, "assets/")
    end

    File.join(Rails.public_path, asset_path)
  end

  def sprockets_asset(name)
    asset_path = asset_path(name)

    if asset_path.match(ABSOLUTE_ASSET_PATH_REGEXP)
      asset_path.gsub!(ABSOLUTE_ASSET_PATH_REGEXP, "")
    end

    Rails.application.assets.find_asset(asset_path)
  end

  def asset_path(name)
    name.gsub(%r{^[/]?assets/}, "")
  end

  Contract String => String
  def stylesheet_content(name)
    if assets_precompiled?
      File.read(local_file_path(name))
    else
      # This will compile and return the asset
      sprockets_asset(name).to_s
    end.strip
  end
end

I put this class within app to make it easier to test

And I put the following code in ApplicationMailer to make the image URLs absolute:

def roadie_options
  ::Rails.application.config.roadie.tap do |options|
    options.asset_providers = UserAssetsProvider.new
    options.external_asset_providers = UserAssetsProvider.new
    options.url_options = url_options.slice(*[
      :host,
      :port,
      :path,
      :protocol,
      :scheme,
    ])
  end
end

I have the following code to ensure roadie works on mail preview:

def mail(*args, &block)
  super.tap do |m|
    options = roadie_options
    next unless options

    Roadie::Rails::MailInliner.new(m, options).execute
  end
end
PikachuEXE commented 8 years ago

Tested on a remote server (staging, similar env to production) Corrected code in previous comment for after testing

PikachuEXE commented 8 years ago

With my code above, it's working on production :)

harmdewit commented 8 years ago

I'm using the http provider in combination with the default Rails cache store. Seems to work fine :smile:

# config/initializers/roadie.rb
class RoadieRailsCacheStore

  def [](path)
    Rails.cache.read(path)
  end

  def []=(path, stylesheet)
    Rails.cache.write(path, stylesheet)
  end

end

Rails.configuration.roadie.external_asset_providers = 
  Roadie::CachedProvider.new(Roadie::NetHttpProvider.new, RoadieRailsCacheStore.new)
jufemaiz commented 8 years ago

@harmdewit you are a life saver!

QQism commented 7 years ago

@harmdewit You saved my day!

dinhhuydh commented 7 years ago

@harmdewit You're are awesome, man!

barjo commented 7 years ago

@harmdewit You, legend!

harmdewit commented 7 years ago

Haha :)

jufemaiz commented 7 years ago

Ok @harmdewit what happened in the last 8 hours?!!