Cache related content

app/controllers/concerns/action_page_display.rb

@@ -42,9 +42,6 @@ def set_action_display_variables
                               zipcode: current_zipcode,
                               country_code: current_country_code,
                               email: current_email }
-
-    @related_content = RelatedContent.new(@actionPage.related_content_url)
-    @related_content.load
   end
 
   def set_signatures

app/controllers/concerns/logged_invisible_captcha.rb

@@ -3,12 +3,12 @@ module LoggedInvisibleCaptcha
 
   def on_spam(options = {})
     log_failure
-    super(options)
+    super
   end
 
   def on_timestamp_spam(options = {})
     log_failure
-    super(options)
+    super
   end
 
   def log_failure

app/lib/related_content.rb

@@ -1,21 +1,30 @@
 class RelatedContent
+  def self.as_hash(url)
+    new(url).as_hash
+  end
+
   def initialize(url)
     @url = url
   end
 
+  def as_hash
+    load unless found?
+    return unless found?
+    { title: title, image_url: image }
+  end
+
   def load
     return if url.blank?
 
     begin
       open_page
-      @loaded_successfully = true
-    rescue OpenURI::HTTPError
+    rescue OpenURI::HttpError
       nil
     end
   end
 
   def found?
-    @loaded_successfully
+    !page.nil?
   end
 
   def title
@@ -35,7 +44,7 @@ def image
 
   private
 
-  attr_reader :url, :page, :loaded_successfully
+  attr_reader :url, :page
 
   def open_page
     @page ||= Nokogiri::HTML(URI::open(url))

app/models/action_page.rb

@@ -194,4 +194,11 @@ def set_congress_tag
 
     congress_message_campaign.update(campaign_tag: slug)
   end
+
+  def related_content
+    key = "#{cache_key_with_version}/related_content"
+    Rails.cache.fetch(key, skip_nil: true) do
+      RelatedContent.as_hash(related_content_url)
+    end
+  end
 end

app/views/action_page/_related_content.html.erb

@@ -0,0 +1,21 @@
+<div class="related-content">
+  <h4>Related Content</h4>
+  <%= link_to action_page.related_content_url do %>
+    <div class="image">
+      <%= image_tag(action_page.related_content[:image_url]) %>
+    </div>
+    <h5><%= action_page.related_content[:title] %></h5>
+  <% end %>
+</div>
+<div class="desc-text">
+  <p><%= markdown action_page.description -%></p>
+</div>
+<div class="related-content mobile">
+  <h4>Related Content</h4>
+  <%= link_to action_page.related_content_url do %>
+    <div class="image">
+      <%= image_tag(action_page.related_content[:image_url]) %>
+    </div>
+    <h5><%= action_page.related_content[:title] %></h5>
+  <% end %>
+</div>

app/views/action_page/show.html.erb

@@ -21,28 +21,8 @@
           <input type='checkbox' style='display: none' id="learn-more">
           <label for="learn-more">Learn More</label>
           <div class="description" id="description">
-            <% if @related_content.found? %>
-            <div class="related-content">
-              <h4>Related Content</h4>
-              <%= link_to @actionPage.related_content_url do %>
-                <div class="image">
-                  <%= image_tag(@related_content.image) %>
-                </div>
-                <h5><%= @related_content.title %></h5>
-              <% end %>
-            </div>
-              <div class="desc-text">
-                <p><%= markdown @actionPage.description -%></p>
-              </div>
-              <div class="related-content mobile">
-                <h4>Related Content</h4>
-                <%= link_to @actionPage.related_content_url do %>
-                  <div class="image">
-                    <%= image_tag(@related_content.image) %>
-                  </div>
-                  <h5><%= @related_content.title %></h5>
-                <% end %>
-              </div>
+            <% if @actionPage.related_content.present? %>
+                <%= render(partial: "related_content", locals: { action_page: @actionPage }) %>
             <% else %>
               <div class="desc-text full">
                 <p><%= markdown @actionPage.description -%></p>

spec/features/ahoy_spec.rb

@@ -34,7 +34,7 @@
       end
     end
   end
-  
+
   def take_email_action(action_page)
     visit action_page_path(action_page)
     click_on "Use default mail client"

spec/lib/related_content_spec.rb

@@ -0,0 +1,48 @@
+require "rails_helper"
+
+RSpec.describe RelatedContent do
+  let(:stub_url) { "https://eff.org" }
+  let(:action_page) do
+    FactoryBot.create(:action_page, related_content_url: stub_url)
+  end
+  # these values are from the fixture
+  let(:og_image_url) { "https://www.eff.org/files/banner_library/robotai.png" }
+  let(:title) do
+    "California’s A.B. 412: A Bill That Could Crush Startups and Cement A "\
+      "Big Tech AI Monopoly"
+  end
+  subject { described_class.new(stub_url) }
+
+  before do
+    deeplink = file_fixture("deeplink.html").read
+    stub_request(:any, stub_url).to_return(body: deeplink)
+  end
+
+  describe "#load" do
+    it "fetches the html from the given URL" do
+      described_class.new(stub_url).load
+      expect(WebMock).to have_requested(:get, stub_url)
+    end
+  end
+
+  describe "#as_hash" do
+    it "returns the title and image url of the blogpost" do
+      expect(subject.as_hash).to eq({ title: title, image_url: og_image_url })
+    end
+  end
+
+  context "data methods" do
+    before { subject.load }
+    describe "#title" do
+      it "returns the blogpost title" do
+        expect(subject.title).to eq(title)
+      end
+    end
+
+    describe "#image" do
+      it "returns the blogpost og image url" do
+        expect(subject.image).to eq(og_image_url)
+      end
+    end
+  end
+end

spec/models/action_page_spec.rb

@@ -14,16 +14,6 @@
     expect(action_page.redirect_from_archived_to_active_action?).to be_truthy
   end
 
-  # The test was a no-go because of the ajaxy html requiring nils... and Then
-  # changing them from nils to ""???  Needs effeciency review before crashyness review
-  # it "should not allow the creation of a model with so few attrs that it would crash the views" do
-  #   expect {
-  #     ActionPage.create!({ })
-  #   }.to raise_exception(ActiveRecord::RecordInvalid)
-  #
-  #   expect(ActionPage.count).to eq 0
-  # end
-
   describe "slug" do
     let(:page) { FactoryBot.create(:action_page) }
     let(:new_slug) { "a-better-slug" }
@@ -192,4 +182,43 @@
       end
     end
   end
+
+  describe "#related_content" do
+    include_context "with cache"
+    let(:url) { "related-content-url" }
+    let(:action_page) do
+      FactoryBot.create(:action_page, related_content_url: url)
+    end
+    let(:rc_hash) { { title: "Blog Title", image_url: "image-url" } }
+    let(:rc) do
+      class_double("RelatedContent")
+        .as_stubbed_const(transfer_nested_constants: true)
+    end
+    before { allow(rc).to receive(:as_hash).with(url).and_return(rc_hash) }
+
+    it "returns required data from the related content url" do
+      expect(action_page.related_content).to eq(rc_hash)
+    end
+
+    it "caches the hash" do
+      action_page.related_content
+      cache_key = "#{action_page.cache_key_with_version}/related_content"
+      expect(Rails.cache.fetch(cache_key)).to eq(rc_hash)
+    end
+
+    it "doesn't hit RelatedContent when cached" do
+      action_page.related_content
+      expect(rc).not_to receive(:as_hash)
+      action_page.related_content
+    end
+
+    it "cache becomes stale when the model is updated" do
+      action_page.related_content
+      new_url = "new-url"
+      new_hash = { title: "New", image_url: "new-image-url" }
+      action_page.update(related_content_url: "new-url")
+      allow(rc).to receive(:as_hash).with(new_url).and_return(new_hash)
+      expect(action_page.reload.related_content).to eq(new_hash)
+    end
+  end
 end

spec/support/cache_helpers.rb

@@ -0,0 +1,8 @@
+RSpec.shared_context "with cache", :with_cache do
+  let(:memory_store) { ActiveSupport::Cache.lookup_store(:memory_store) }
+
+  before do
+    allow(Rails).to receive(:cache).and_return(memory_store)
+    Rails.cache.clear
+  end
+end