Skip to content

Commit

Permalink
Rename embedded endpoint to host_content
Browse files Browse the repository at this point in the history
This has always been a bit misleaseading/confusing, so let’s rename the
endpoint and all associated classes / methods.

I’ve also added a route to point the old endpoint to the renamed
controller method, which can be removed once we update
`gds-api-adapters`
  • Loading branch information
pezholio committed Nov 26, 2024
1 parent f5ff481 commit 3b16af0
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 48 deletions.
4 changes: 2 additions & 2 deletions app/controllers/v2/content_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def linkables
).call
end

def embedded
results = GetEmbeddedContentService.new(
def host_content
results = GetHostContentService.new(
path_params[:content_id],
query_params[:order],
query_params[:page],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Presenters
class EmbeddedContentPresenter
class HostContentPresenter
def self.present(target_edition_id, host_content, total, total_pages)
new(target_edition_id, host_content, total, total_pages).present
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Queries
class GetEmbeddedContent
class GetHostContent
Result = Data.define(
:id,
:title,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class GetEmbeddedContentService
class GetHostContentService
def initialize(target_content_id, order, page, per_page)
@target_content_id = target_content_id
@order = order
Expand All @@ -8,11 +8,11 @@ def initialize(target_content_id, order, page, per_page)

def call
if Document.find_by(content_id: target_content_id).nil?
message = "Could not find an edition to get embedded content for"
message = "Could not find an edition to get host content for"
raise CommandError.new(code: 404, message:)
end

Presenters::EmbeddedContentPresenter.present(
Presenters::HostContentPresenter.present(
target_content_id,
host_content,
query.count,
Expand All @@ -25,7 +25,7 @@ def call
attr_accessor :target_content_id, :order, :page, :per_page

def query
@query ||= Queries::GetEmbeddedContent.new(target_content_id, order_field:, order_direction:, page:, per_page:)
@query ||= Queries::GetHostContent.new(target_content_id, order_field:, order_direction:, page:, per_page:)
end

def host_content
Expand Down
4 changes: 3 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ def content_id_constraint(request)
scope constraints: method(:content_id_constraint) do
put "/content/:content_id", to: "content_items#put_content"
get "/content/:content_id", to: "content_items#show"
get "/content/:content_id/embedded", to: "content_items#embedded"
get "/content/:content_id/host-content", to: "content_items#host_content"
# Point legacy `embedded` endpoint to `host_content` endpoint
get "/content/:content_id/embedded", to: "content_items#host_content"
post "/content/:content_id/publish", to: "content_items#publish"
post "/content/:content_id/republish", to: "content_items#republish"
post "/content/:content_id/unpublish", to: "content_items#unpublish"
Expand Down
7 changes: 3 additions & 4 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ message queue for other apps (e.g. `email-alert-service`) to consume.
- [`POST /v2/content/:content_id/discard-draft`](#post-v2contentcontent_iddiscard-draft)
- [`GET /v2/content`](#get-v2content)
- [`GET /v2/content/:content_id`](#get-v2contentcontent_id)
- [`GET /v2/content/:content_id/embedded`](#get-v2contentcontent_idembedded)
- [`GET /v2/content/:content_id/host-content`](#get-v2contentcontent_idhost-content)
- [`PATCH /v2/links/:content_id`](#patch-v2linkscontent_id)
- [`GET /v2/links/:content_id`](#get-v2linkscontent_id)
- [`GET /v2/expanded-links/:content_id`](#get-v2expanded-linkscontent_id)
Expand Down Expand Up @@ -428,10 +428,9 @@ included within the response.
- Specify a particular edition of this document
- If omitted the most recent edition.

## `GET /v2/content/:content_id/embedded`
## `GET /v2/content/:content_id/host-content`

Retrieves a summary list of any draft or published content which has an embedded
reference to the target `:content_id`.
Retrieves a summary list of content which has an embedded reference to the target `:content_id`.

<!-- TODO: Include a link to how Resuable Content works here when we have it -->
Content can include an embedded reference in its body when
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
RSpec.describe "Embedded documents" do
RSpec.describe "Host content" do
include_context "PutContent call"

let!(:publishing_organisation) do
Expand All @@ -21,22 +21,22 @@

context "when the target edition doesn't exist" do
it "returns a 404" do
get "/v2/content/#{SecureRandom.uuid}/embedded"
get "/v2/content/#{SecureRandom.uuid}/host-content"

expect(response.status).to eq(404)
end
end

context "when no editions embed the content block" do
context "when the content block has no host editions" do
it "returns an empty results array" do
unembedded_edition = create(:live_edition)
edition = create(:live_edition)

get "/v2/content/#{unembedded_edition.content_id}/embedded"
get "/v2/content/#{edition.content_id}/host-content"

expect(response.status).to eq(200)
response_body = parsed_response

expect(response_body["content_id"]).to eq(unembedded_edition.content_id)
expect(response_body["content_id"]).to eq(edition.content_id)
expect(response_body["total"]).to eq(0)
expect(response_body["results"]).to eq([])
end
Expand All @@ -54,7 +54,7 @@

statistics_cache = create(:statistics_cache, document: host_edition.document, unique_pageviews: 333)

get "/v2/content/#{content_block.content_id}/embedded"
get "/v2/content/#{content_block.content_id}/host-content"

expect(response.status).to eq(200)

Expand Down Expand Up @@ -82,15 +82,15 @@
)
end

context "when embedded content appears more than once in a field" do
context "when host content appears more than once in a field" do
let!(:host_edition) do
create_live_edition(
body: "<p>{{embed:content_block_email_address:#{content_block.content_id}}} {{embed:content_block_email_address:#{content_block.content_id}}}</p>\n",
)
end

it "should return multiple instances" do
get "/v2/content/#{content_block.content_id}/embedded"
get "/v2/content/#{content_block.content_id}/host-content"
response_body = parsed_response

expect(response_body["content_id"]).to eq(content_block.content_id)
Expand All @@ -108,7 +108,7 @@
end

it "should return only one instance" do
get "/v2/content/#{content_block.content_id}/embedded"
get "/v2/content/#{content_block.content_id}/host-content"
response_body = parsed_response

expect(response_body["content_id"]).to eq(content_block.content_id)
Expand All @@ -129,7 +129,7 @@
end

it "returns the first page by default" do
get "/v2/content/#{content_block.content_id}/embedded"
get "/v2/content/#{content_block.content_id}/host-content"
response_body = parsed_response

expect(response_body["total"]).to eq(12)
Expand All @@ -138,7 +138,7 @@
end

it "allows the next page to be requested" do
get "/v2/content/#{content_block.content_id}/embedded?page=2"
get "/v2/content/#{content_block.content_id}/host-content?page=2"
response_body = parsed_response

expect(response_body["total"]).to eq(12)
Expand All @@ -147,7 +147,7 @@
end

it "allows a per_page argument to be passed" do
get "/v2/content/#{content_block.content_id}/embedded?per_page=1"
get "/v2/content/#{content_block.content_id}/host-content?per_page=1"
response_body = parsed_response

expect(response_body["total"]).to eq(12)
Expand Down Expand Up @@ -300,7 +300,7 @@
end

def expect_request_to_order_by(order_argument:, expected_results:)
get "/v2/content/#{content_block.content_id}/embedded?order=#{order_argument}"
get "/v2/content/#{content_block.content_id}/host-content?order=#{order_argument}"
response_body = parsed_response

expect(response.status).to eq(200)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
RSpec.describe Presenters::EmbeddedContentPresenter do
RSpec.describe Presenters::HostContentPresenter do
describe "#present" do
let(:organisation_edition_id) { SecureRandom.uuid }
let(:target_edition_id) { SecureRandom.uuid }
Expand All @@ -9,7 +9,7 @@
let(:total_pages) { 23 }

let(:host_editions) do
[double("Queries::GetEmbeddedContent::Result",
[double("Queries::GetHostContent::Result",
id: "1",
title: "foo",
base_path: "/foo",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
RSpec.describe Queries::GetEmbeddedContent do
RSpec.describe Queries::GetHostContent do
describe "#call" do
let(:organisation) do
edition_params = {
Expand Down Expand Up @@ -130,13 +130,13 @@
let(:target_content_id) { SecureRandom.uuid }

it "sorts by unique_pageviews by default" do
expect_sort_call_for(order_field: Queries::GetEmbeddedContent::ORDER_FIELDS[:unique_pageviews], order_direction: :asc)
expect_sort_call_for(order_field: Queries::GetHostContent::ORDER_FIELDS[:unique_pageviews], order_direction: :asc)

described_class.new(target_content_id).call
end

it "allows searching in descending order with the default field" do
expect_sort_call_for(order_field: Queries::GetEmbeddedContent::ORDER_FIELDS[:unique_pageviews], order_direction: :desc)
expect_sort_call_for(order_field: Queries::GetHostContent::ORDER_FIELDS[:unique_pageviews], order_direction: :desc)

described_class.new(target_content_id, order_direction: :desc).call
end
Expand All @@ -153,8 +153,8 @@
}.to raise_error(KeyError, "Unknown order direction: foo")
end

Queries::GetEmbeddedContent::ORDER_FIELDS.each do |key, order_field|
Queries::GetEmbeddedContent::ORDER_DIRECTIONS.each do |order_direction|
Queries::GetHostContent::ORDER_FIELDS.each do |key, order_field|
Queries::GetHostContent::ORDER_DIRECTIONS.each do |order_direction|
it "allows searching by #{key} #{order_direction}" do
expect_sort_call_for(order_field:, order_direction:)

Expand Down Expand Up @@ -190,7 +190,7 @@ def expect_sort_call_for(order_field:, order_direction:)
it "requests the first page by default" do
expect(ActiveRecord::Base.connection).to receive(:select_all) { |arel_query|
expect(arel_query.offset).to eq(0)
expect(arel_query.limit).to eq(Queries::GetEmbeddedContent::DEFAULT_PER_PAGE)
expect(arel_query.limit).to eq(Queries::GetHostContent::DEFAULT_PER_PAGE)
}.and_return([])

described_class.new(target_content_id).call
Expand All @@ -199,7 +199,7 @@ def expect_sort_call_for(order_field:, order_direction:)
it "accepts a page argument" do
expect(ActiveRecord::Base.connection).to receive(:select_all) { |arel_query|
expect(arel_query.offset).to eq(10)
expect(arel_query.limit).to eq(Queries::GetEmbeddedContent::DEFAULT_PER_PAGE)
expect(arel_query.limit).to eq(Queries::GetHostContent::DEFAULT_PER_PAGE)
}.and_return([])

described_class.new(target_content_id, page: 1).call
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
RSpec.describe GetEmbeddedContentService do
RSpec.describe GetHostContentService do
describe "#call" do
let(:organisation) do
edition_params = {
Expand Down Expand Up @@ -29,7 +29,7 @@
it "returns 404" do
expect { described_class.new(SecureRandom.uuid, nil, nil, nil).call }.to raise_error(CommandError) do |error|
expect(error.code).to eq(404)
expect(error.message).to eq("Could not find an edition to get embedded content for")
expect(error.message).to eq("Could not find an edition to get host content for")
end
end
end
Expand All @@ -39,21 +39,21 @@
let(:host_editions_stub) { double("ActiveRecord::Relation") }
let(:count) { 12 }
let(:total_pages) { 2 }
let(:embedded_content_stub) { double(Queries::GetEmbeddedContent, call: host_editions_stub, count:, total_pages:) }
let(:embedded_content_stub) { double(Queries::GetHostContent, call: host_editions_stub, count:, total_pages:) }
let(:result_stub) { double }

before do
allow(Document).to receive(:find_by).and_return(anything)
allow(Queries::GetEmbeddedContent).to receive(:new).and_return(embedded_content_stub)
allow(Presenters::EmbeddedContentPresenter).to receive(:present).and_return(result_stub)
allow(Queries::GetHostContent).to receive(:new).and_return(embedded_content_stub)
allow(Presenters::HostContentPresenter).to receive(:present).and_return(result_stub)
end

it "returns a presented form of the response from the query" do
result = described_class.new(target_content_id, nil, nil, nil).call

expect(result).to eq(result_stub)

expect(Presenters::EmbeddedContentPresenter).to have_received(:present).with(
expect(Presenters::HostContentPresenter).to have_received(:present).with(
target_content_id,
host_editions_stub,
count,
Expand All @@ -65,23 +65,23 @@
it "requests page zero by default" do
described_class.new(target_content_id, nil, "", "").call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: nil, order_direction: nil, page: 0, per_page: nil
)
end

it "requests a zero indexed page" do
described_class.new(target_content_id, nil, "2", "").call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: nil, order_direction: nil, page: 1, per_page: nil
)
end

it "accepts a per_page argument" do
described_class.new(target_content_id, nil, "2", "5").call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: nil, order_direction: nil, page: 1, per_page: 5
)
end
Expand All @@ -91,23 +91,23 @@
it "does not send any ordering fields by default" do
described_class.new(target_content_id, nil, nil, nil).call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: nil, order_direction: nil, page: 0, per_page: nil
)
end

it "sends a field in ascending order when not preceded with a minus" do
described_class.new(target_content_id, "something", nil, nil).call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: :something, order_direction: :asc, page: 0, per_page: nil
)
end

it "sends a field in descending order when preceded with a minus" do
described_class.new(target_content_id, "-something", nil, nil).call

expect(Queries::GetEmbeddedContent).to have_received(:new).with(
expect(Queries::GetHostContent).to have_received(:new).with(
target_content_id, order_field: :something, order_direction: :desc, page: 0, per_page: nil
)
end
Expand Down

0 comments on commit 3b16af0

Please sign in to comment.