Extracting duplicate logic (#16035)

Prior to this commit, we had two places that need to know the nuances
ofquerying for tag flares and what we should include in our queries for
serialization.

With this commit, we're factoring towards a common source of knowledge
and providing a much needed test for the expected output of this
serialization.

Loosely related to #15916, #15983, #15994, and #16032.
This commit is contained in:
Jeremy Friesen 2022-01-10 15:19:04 -05:00 committed by GitHub
parent 03d4c50dfc
commit c5bf4bf880
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 32 deletions

View file

@ -1,5 +1,22 @@
module Homepage
class ArticleSerializer < ApplicationSerializer
# @param relation [ActiveRecord::Relation<Article>]
#
# @return [Hash]
def self.serialized_collection_from(relation:)
# Unfortunately the FlareTag class sends one SQL query per each article,
# as we want to optimize by loading them in one query, we're using a different class
tag_flares = Homepage::FetchTagFlares.call(relation)
# including user and organization as the last step as they are not needed
# by the query that fetches tag flares, they are only needed by the serializer
relation = relation.includes(:user, :organization)
new(relation, params: { tag_flares: tag_flares }, is_collection: true)
.serializable_hash[:data]
.pluck(:attributes)
end
attributes(
:class_name,
:cloudinary_video_url,

View file

@ -31,19 +31,7 @@ module Homepage
per_page: per_page,
)
# Unfortunately the FlareTag class sends one SQL query per each article,
# as we want to optimize by loading them in one query, we're using a different class
tag_flares = Homepage::FetchTagFlares.call(articles)
# including user and organization as the last step as they are not needed
# by the query that fetches tag flares, they are only needed by the serializer
# NOTE: wish there was a way to specify which columns to fetch for `User` and `Organization`...
articles = articles.includes(:user, :organization)
Homepage::ArticleSerializer
.new(articles, params: { tag_flares: tag_flares }, is_collection: true)
.serializable_hash[:data]
.pluck(:attributes)
Homepage::ArticleSerializer.serialized_collection_from(relation: articles)
end
end
end

View file

@ -16,13 +16,7 @@ module Search
relation = sort(relation, term, sort_by, sort_direction)
tag_flares = Homepage::FetchTagFlares.call(relation)
# including user and organization as the last step as they are not needed
# by the query that fetches tag flares, they are only needed by the serializer
relation = relation.includes(:user, :organization)
serialize(relation, tag_flares)
Homepage::ArticleSerializer.serialized_collection_from(relation: relation)
end
def self.sort(relation, term, sort_by, sort_direction)
@ -34,13 +28,5 @@ module Search
relation.reorder(DEFAULT_SORT_BY)
end
private_class_method :sort
def self.serialize(articles, tag_flares)
Homepage::ArticleSerializer
.new(articles, params: { tag_flares: tag_flares }, is_collection: true)
.serializable_hash[:data]
.pluck(:attributes)
end
private_class_method :serialize
end
end

View file

@ -0,0 +1,20 @@
require "rails_helper"
RSpec.describe Homepage::ArticleSerializer, type: :serializer do
describe "#serialized_collection_from" do
let(:user) { create(:user, name: "\"Rowdy\" Roddy Piper \\:/") }
let(:organization) { create(:organization) }
let(:tag) { create(:tag, name: "ama", bg_color_hex: "#f3f3f3", text_color_hex: "#cccccc") }
let(:article) { create(:article, user: user, organization: organization, tags: tag.name) }
before do
article
stub_const("FlareTag::FLARE_TAG_IDS_HASH", { "ama" => tag.id })
end
it "is parseable as JSON (once converted to_json)" do
response = described_class.serialized_collection_from(relation: Article.all)
expect(JSON.parse(response.to_json)[0].dig("user", "name")).to eq(user.name)
end
end
end

View file

@ -4,10 +4,10 @@ RSpec.describe Homepage::FetchArticles, type: :service do
describe ".call" do
# rubocop:disable RSpec/ExampleLength
it "returns results in the correct format", :aggregate_failures do
article = create(
:article, video_thumbnail_url: "https://example.com", tags: Constants::Tags::FLARE_TAG_NAMES.first
)
create(:comment, commentable: article)
tag = create(:tag, name: "ama", bg_color_hex: "#f3f3f3", text_color_hex: "#cccccc")
article = create(:article, video_thumbnail_url: "https://example.com", tags: tag.name)
stub_const("FlareTag::FLARE_TAG_IDS_HASH", { "ama" => tag.id })
result = described_class.call.first