diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e29ac5320..148b9f2b2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -15,8 +15,6 @@ require: # Offense count: 8 Performance/OpenStruct: Exclude: - - 'app/models/article.rb' - - 'app/models/organization.rb' - 'spec/models/github_repo_spec.rb' - 'spec/requests/github_repos_spec.rb' diff --git a/app/models/article.rb b/app/models/article.rb index 5dee076b0..1bf7d6bd7 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -588,22 +588,12 @@ class Article < ApplicationRecord def update_cached_user if organization - self.cached_organization = OpenStruct.new(set_cached_object(organization)) + self.cached_organization = Articles::CachedEntity.from_object(organization) end return unless user - self.cached_user = OpenStruct.new(set_cached_object(user)) - end - - def set_cached_object(object) - { - name: object.name, - username: object.username, - slug: object == organization ? object.slug : object.username, - profile_image_90: object.profile_image_90, - profile_image_url: object.profile_image_url - } + self.cached_user = Articles::CachedEntity.from_object(user) end def set_all_dates diff --git a/app/models/articles/cached_entity.rb b/app/models/articles/cached_entity.rb new file mode 100644 index 000000000..612444df5 --- /dev/null +++ b/app/models/articles/cached_entity.rb @@ -0,0 +1,14 @@ +module Articles + # NOTE: articles cache either users or organizations, but they have the same attributes. + CachedEntity = Struct.new(:name, :username, :slug, :profile_image_90, :profile_image_url) do + def self.from_object(object) + new( + object.name, + object.username, + object.respond_to?(:slug) ? object.slug : object.username, + object.profile_image_90, + object.profile_image_url, + ) + end + end +end diff --git a/app/models/organization.rb b/app/models/organization.rb index 9e64401ac..0a3661a35 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -127,14 +127,7 @@ class Organization < ApplicationRecord def update_articles return unless saved_change_to_slug || saved_change_to_name || saved_change_to_profile_image - cached_org_object = { - name: name, - username: username, - slug: slug, - profile_image_90: profile_image_90, - profile_image_url: profile_image_url - } - articles.update(cached_organization: OpenStruct.new(cached_org_object)) + articles.update(cached_organization: Articles::CachedEntity.from_object(self)) end def bust_cache diff --git a/app/views/stories/feeds/show.json.jbuilder b/app/views/stories/feeds/show.json.jbuilder index e6bb038e9..075804826 100644 --- a/app/views/stories/feeds/show.json.jbuilder +++ b/app/views/stories/feeds/show.json.jbuilder @@ -11,10 +11,10 @@ article_methods_to_include = %i[ json.array!(@stories) do |article| json.extract! article, *article_attributes_to_include - json.user article.cached_user.as_json["table"] + json.user article.cached_user.as_json if article.cached_organization? - json.organization article.cached_organization.as_json["table"] + json.organization article.cached_organization.as_json end if article.main_image? diff --git a/lib/data_update_scripts/20200723070918_update_articles_cached_entities.rb b/lib/data_update_scripts/20200723070918_update_articles_cached_entities.rb new file mode 100644 index 000000000..63fb808c0 --- /dev/null +++ b/lib/data_update_scripts/20200723070918_update_articles_cached_entities.rb @@ -0,0 +1,17 @@ +module DataUpdateScripts + class UpdateArticlesCachedEntities + def run + Article.where.not(cached_user: nil).or(Article.where.not(cached_organization: nil)).find_each do |article| + if article.cached_organization.present? + old_cached_org = article.cached_organization + article.update(cached_organization: Articles::CachedEntity.from_object(old_cached_org)) + end + + if article.cached_user.present? + old_cached_user = article.cached_user + article.update(cached_user: Articles::CachedEntity.from_object(old_cached_user)) + end + end + end + end +end diff --git a/spec/lib/data_update_scripts/update_articles_cached_entities_spec.rb b/spec/lib/data_update_scripts/update_articles_cached_entities_spec.rb new file mode 100644 index 000000000..e3cab0653 --- /dev/null +++ b/spec/lib/data_update_scripts/update_articles_cached_entities_spec.rb @@ -0,0 +1,36 @@ +require "rails_helper" +require Rails.root.join("lib/data_update_scripts/20200723070918_update_articles_cached_entities.rb") + +describe DataUpdateScripts::UpdateArticlesCachedEntities do + def make_ostruct(object) + # rubocop:disable Performance/OpenStruct + OpenStruct.new( + name: object.name, + userame: object.username, + slug: object.respond_to?(:slug) ? object.slug : object.username, + profile_image_90: object.profile_image_90, + profile_image_url: object.profile_image_url, + ) + # rubocop:enable Performance/OpenStruct + end + + it "changes cached organizations from OpenStructs to Structs" do + cached_org = make_ostruct(create(:organization)) + article = create(:article, cached_organization: cached_org) + + expect do + described_class.new.run + end.to change { article.reload.cached_organization.class }.from(OpenStruct).to(Articles::CachedEntity) + end + + it "changes cached users from OpenStructs to Structs" do + article = create(:article) + # NOTE: this uses `update_column` in order to skip the `set_caches` callback + # which would always change the serialized object's class + article.update_column(:cached_user, make_ostruct(article.user)) + + expect do + described_class.new.run + end.to change { article.reload.cached_user.class }.from(OpenStruct).to(Articles::CachedEntity) + end +end