Ensuring article's cache updates on org image update (#17052)

There are three major things occurring in this pull request:

1. Renaming `Article#update_cached_user` to `Article#set_cached_entities`.
2. Reducing an organization's direct knowledge of which of the org's
   attributes an article caches.
3. Removing duplicate calls to update the article associated with the
   organization.

For renaming to `Article#set_cached_entities`, the prior method implied
we were updating the persistence layer.  However, we were not making any
save nor update calls.  This rename should clarify intention.

For reducing knowledge, the comments for
`Article::ATTRIBUTES_CACHED_FOR_RELATED_ENTITY` should explain the details.

And last, removing the duplicate calls; we had three methods that were
attempting to build and update the `Article#cached_organization`'s
value.

Closes forem/forem#17041

app/models/article.rb

@@ -11,6 +11,20 @@ class Article < ApplicationRecord
   include StringAttributeCleaner.nullify_blanks_for(:canonical_url, on: :before_save)
   DEFAULT_FEED_PAGINATION_WINDOW_SIZE = 50
 
+  # When we cache an entity, either {User} or {Organization}, these are the names of the attributes
+  # we cache.
+  #
+  # @note I would prefer that this constant were in the {Article::CachedEntity} namespace, but it
+  #       didn't work out well.  Further, since Organization doesn't really know about
+  #       Articles::CachedEntity, I'd rather it not "peek" into a class for which it has no
+  #       knowledge.
+  #
+  # @note [@jeremyf] I have added the profile_image attribute, even though that's not one of the
+  #       Articles::CachedEntity attributes.  This is necessary to detect the change.
+  #
+  # @see Articles::CachedEntity caching strategy for entity attributes
+  ATTRIBUTES_CACHED_FOR_RELATED_ENTITY = %i[name profile_image profile_image_url slug username].freeze
+
   attr_accessor :publish_under_org
   attr_writer :series
 
@@ -149,7 +163,7 @@ class Article < ApplicationRecord
   before_validation :evaluate_markdown, :create_slug
   before_validation :remove_prohibited_unicode_characters
   before_validation :normalize_title
-  before_save :update_cached_user
+  before_save :set_cached_entities
   before_save :set_all_dates
 
   before_save :calculate_base_scores
@@ -825,7 +839,7 @@ class Article < ApplicationRecord
     self.password = SecureRandom.hex(60)
   end
 
-  def update_cached_user
+  def set_cached_entities
     self.cached_organization = organization ? Articles::CachedEntity.from_object(organization) : nil
     self.cached_user = user ? Articles::CachedEntity.from_object(user) : nil
   end

app/models/organization.rb

@@ -13,17 +13,12 @@ class Organization < ApplicationRecord
   before_validation :check_for_slug_change
   before_validation :evaluate_markdown
 
-  # TODO: [@rhymes] revisit this callback and `update_articles_cached_organization`
-  # when we remove Elasticsearch
-  before_save :update_articles
   before_save :remove_at_from_usernames
   before_save :generate_secret
 
   after_save :bust_cache
 
-  # This callback will eventually invoke Article.update_cached_user to update the organization.name
-  # only when it has been changed, thus invoking the trigger on Article.reading_list_document
-  after_update_commit :update_articles_cached_organization
+  after_update_commit :conditionally_update_articles
   after_destroy_commit :bust_cache
 
   has_many :articles, dependent: :nullify
@@ -141,16 +136,10 @@ class Organization < ApplicationRecord
     self.slug = slug&.downcase
   end
 
-  def update_articles
-    return unless saved_change_to_slug || saved_change_to_name || saved_change_to_profile_image
+  def conditionally_update_articles
+    return unless Article::ATTRIBUTES_CACHED_FOR_RELATED_ENTITY.detect { |attr| saved_change_to_attribute?(attr) }
 
-    articles.update(cached_organization: Articles::CachedEntity.from_object(self))
-  end
-
-  def update_articles_cached_organization
-    return unless saved_change_to_attribute?(:name)
-
-    articles.update(cached_organization: Articles::CachedEntity.from_object(self))
+    articles.each(&:save)
   end
 
   def bust_cache

spec/models/organization_spec.rb

@@ -249,6 +249,41 @@ RSpec.describe Organization, type: :model do
         expect(article.path).to include(new_slug)
       end
 
+      it "updates article's cached_organization profile image", :aggregate_failures do
+        # What is going on with this spec? Follow the comments.
+        #
+        # tl;dr - verifying fix for https://github.com/forem/forem/issues/17041
+
+        # Create an organization and article, verify the article has cached the initial profile
+        # image of the organization.
+        original_org = create(:organization, profile_image: File.open(Rails.root.join("app/assets/images/1.png")))
+        article = create(:article, organization: original_org)
+        expect(article.cached_organization.profile_image_url).to eq(original_org.profile_image_url)
+
+        # For good measure let's have two of these articles
+        create(:article, organization: original_org)
+
+        # A foible of CarrierWave is that I can't re-upload for the same model in memory.  I need to
+        # refind the record.  #reload does not work either.  (I lost sleep on this portion of the
+        # test)
+        organization = described_class.find(original_org.id)
+
+        # Verify that the organization's image changes.  See the above CarrierWave foible
+        expect do
+          organization.profile_image = File.open(Rails.root.join("app/assets/images/2.png"))
+          organization.save!
+        end.to change { organization.reload.profile_image_url }
+
+        # I want to collect reloaded versions of the organization's articles so I can see their
+        # cached organization profile image
+        articles_profile_image_urls = organization.articles
+          .map { |art| art.reload.cached_organization.profile_image_url }.uniq
+
+        # Because of the change `{ organization.reload... }` the organization's profile_image_url is
+        # the most recent change.
+        expect(articles_profile_image_urls).to eq([organization.profile_image_url])
+      end
+
       it "updates article cached_organizations" do
         new_slug = "slug_#{rand(10_000)}"