Skip to content

Commit

Permalink
Merge pull request #9791 from alphagov/content-modelling/177-edition-…
Browse files Browse the repository at this point in the history
…title

content-modelling/777 - remove title from Content Block Document and use the title on Content Block Editions
  • Loading branch information
Harriethw authored Jan 8, 2025
2 parents 4e9eb4d + 4d79134 commit 664d2c7
Show file tree
Hide file tree
Showing 29 changed files with 125 additions and 202 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class ChangeTitleToSluggableStringOnContentBlockDocuments < ActiveRecord::Migration[7.1]
def up
rename_column :content_block_documents, :title, :sluggable_string
end

def down
rename_column :content_block_documents, :sluggable_string, :title
end
end
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2025_01_06_111536) do
ActiveRecord::Schema[7.1].define(version: 2025_01_06_160308) do
create_table "assets", charset: "utf8mb3", force: :cascade do |t|
t.string "asset_manager_id", null: false
t.string "variant", null: false
Expand Down Expand Up @@ -193,7 +193,7 @@

create_table "content_block_documents", charset: "utf8mb3", force: :cascade do |t|
t.string "content_id"
t.string "title"
t.string "sluggable_string"
t.string "block_type"
t.datetime "created_at", precision: nil
t.datetime "updated_at", precision: nil
Expand Down
2 changes: 1 addition & 1 deletion docs/diagrams/content_block_manager_models.puml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ together {
class ContentBlockDocument {
content_id
block_type
title
}
database content_block_documents
ContentBlockDocument .> content_block_documents
Expand All @@ -17,6 +16,7 @@ together {
details
document_id
state
title
}
database content_block_editions
ContentBlockEdition .> content_block_editions
Expand Down
Binary file added docs/diagrams/object_store_models.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def edit_item
def title_item
{
field: "Title",
value: content_block_edition.document_title,
value: content_block_edition.title,
}
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def edition_params
"scheduled_publication(3i)",
"scheduled_publication(4i)",
"scheduled_publication(5i)",
document_attributes: %w[title block_type],
:title,
document_attributes: %w[block_type],
details: @schema.fields,
)
.merge!(creator: current_user)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class Document < ApplicationRecord
include Scopes::SearchableByUpdatedDate

extend FriendlyId
friendly_id :title, use: :slugged, slug_column: :content_id_alias, routes: :default
friendly_id :sluggable_string, use: :slugged, slug_column: :content_id_alias, routes: :default

has_many :editions,
-> { order(created_at: :asc, id: :asc) },
Expand All @@ -15,7 +15,7 @@ class Document < ApplicationRecord
enum :block_type, ContentBlockManager::ContentBlock::Schema.valid_schemas.index_with(&:to_s)
attr_readonly :block_type

validates :block_type, :title, presence: true
validates :block_type, :sluggable_string, presence: true

has_one :latest_edition,
-> { joins(:document).where("content_block_documents.latest_edition_id = content_block_editions.id") },
Expand All @@ -29,6 +29,10 @@ class Document < ApplicationRecord
def embed_code
"{{embed:content_block_#{block_type}:#{content_id}}}"
end

def title
@title ||= latest_edition&.title
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module ContentBlock::Document::Scopes::SearchableByKeyword
extend ActiveSupport::Concern

SQL = <<-SQL.freeze
content_block_documents.title REGEXP :pattern OR#{' '}
content_block_editions.title REGEXP :pattern OR#{' '}
content_block_editions.details REGEXP :pattern OR#{' '}
content_block_editions.instructions_to_publishers REGEXP :pattern
SQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class Edition < ApplicationRecord
include ValidatesDetails
include Workflow

validates :title, presence: true

scope :current_versions, lambda {
joins(
"LEFT JOIN content_block_documents document ON document.latest_edition_id = content_block_editions.id",
Expand All @@ -22,7 +24,7 @@ def render
ContentBlockTools::ContentBlock.new(
document_type: "content_block_#{block_type}",
content_id: document.content_id,
title: document_title,
title:,
details:,
).render
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,14 @@ module ContentBlock::Edition::Documentable
accepts_nested_attributes_for :document
end

def document_title
document&.title || @document_title
end

def block_type
document&.block_type || @block_type
@block_type ||= document&.block_type
end

def ensure_presence_of_document
if document.blank?
self.document = ContentBlock::Document.new(
content_id: create_random_id,
block_type: @block_type,
title: @document_title,
)
elsif document.new_record?
if document.new_record?
document.content_id = create_random_id if document.content_id.blank?
document.block_type = @block_type if document.block_type.blank?
document.title = @title if document.title.blank?
document.sluggable_string = title if document.sluggable_string.blank?
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ def initialize(schema)
end

def call(edition_params, document_id: nil)
edition_params[:title] = edition_params[:document_attributes][:title]
@new_edition = build_edition(edition_params, document_id)
@new_edition.assign_attributes(edition_params)
@new_edition.save!
Expand All @@ -18,10 +17,12 @@ def build_edition(edition_params, document_id)
if document_id.nil?
ContentBlockManager::ContentBlock::Edition.new(edition_params)
else
# TODO: is this covered when adding title?
ContentBlockManager::ContentBlock::Edition.new(
document_id:,
document_attributes: edition_params.delete(:document_attributes).except(:block_type).merge({ id: document_id }),
title: edition_params[:title],
document_attributes: edition_params.delete(:document_attributes)
.except(:block_type)
.merge({ id: document_id }),
)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def publish_with_rollback(content_block_edition)
content_id:,
content_id_alias:,
schema_id: schema.id,
title: content_block_edition.document_title,
title: content_block_edition.title,
details: content_block_edition.details,
instructions_to_publishers: content_block_edition.instructions_to_publishers,
links: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
label: {
text: "Title",
},
name: "content_block/edition[document_attributes][title]",
id: "content_block_manager/content_block/edition_document_title",
value: @form.content_block_edition.document&.title,
error_items: errors_for(@form.content_block_edition.errors, "document.title".to_sym),
name: "content_block/edition[title]",
id: "content_block_manager/content_block/edition_title",
value: @form.content_block_edition&.title,
error_items: errors_for(@form.content_block_edition.errors, "edition.title".to_sym),
} %>

<% @form.attributes.each do |field, _value| %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
assert_not_nil edition
assert_not_nil edition.document

assert_equal @title, edition.document_title if @title.present?
assert_equal @title, edition.title if @title.present?
assert_equal @instructions_to_publishers, edition.instructions_to_publishers if @instructions_to_publishers.present?

Expand Down Expand Up @@ -233,6 +232,7 @@ def has_support_button
details: { email_address: @email_address },
creator: @user,
organisation:,
title: "previously created title",
)
ContentBlockManager::ContentBlock::Edition::HasAuditTrail.acting_as(@user) do
@content_block.publish!
Expand All @@ -248,7 +248,7 @@ def has_support_button
instructions_to_publishers = fields.delete("instructions_to_publishers")

(1..count.to_i).each do |_i|
document = create(:content_block_document, block_type.to_sym, title:)
document = create(:content_block_document, block_type.to_sym, sluggable_string: title.parameterize(separator: "_"))

editions = create_list(
:content_block_edition,
Expand All @@ -259,6 +259,7 @@ def has_support_button
details: fields,
creator: @user,
instructions_to_publishers:,
title:,
)

document.latest_edition = editions.last
Expand All @@ -271,14 +272,16 @@ def has_support_button
@content_blocks ||= []
@email_address = "[email protected]"
organisation = create(:organisation)
document = create(:content_block_document, :email_address, title: fields[:title])
title = fields.delete("title") || "title"
document = create(:content_block_document, :email_address, sluggable_string: title.parameterize(separator: "_"))
@content_block = create(
:content_block_edition,
:email_address,
document:,
details: { email_address: fields[:email_address] },
creator: @user,
organisation:,
title:,
)
ContentBlockManager::ContentBlock::Edition::HasAuditTrail.acting_as(@user) do
@content_block.publish!
Expand Down Expand Up @@ -811,8 +814,8 @@ def click_save_and_continue
within(".govuk-summary-card", text: content_block_name) do
find("a", text: "Copy code").click
has_text?("Code copied")
document = ContentBlockManager::ContentBlock::Document.find_by(title: content_block_name)
@embed_code = document.embed_code
edition = ContentBlockManager::ContentBlock::Edition.find_by(title: content_block_name)
@embed_code = edition.document.embed_code
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class ContentBlockManager::ContentBlock::Document::Index::SummaryCardComponentTe
it "renders a published content block as a summary card" do
render_inline(ContentBlockManager::ContentBlock::Document::Index::SummaryCardComponent.new(content_block_document:))

assert_selector ".govuk-summary-card__title", text: content_block_edition.document_title
assert_selector ".govuk-summary-card__title", text: content_block_edition.title
assert_selector ".govuk-summary-card__action", count: 1
assert_selector ".govuk-summary-card__action .govuk-link[href='#{content_block_manager_content_block_document_path(content_block_document)}']"

Expand All @@ -33,7 +33,7 @@ class ContentBlockManager::ContentBlock::Document::Index::SummaryCardComponentTe
assert_selector ".govuk-summary-list__row", count: 6

assert_selector ".govuk-summary-list__key", text: "Title"
assert_selector ".govuk-summary-list__value", text: content_block_edition.document_title
assert_selector ".govuk-summary-list__value", text: content_block_edition.title

assert_selector ".govuk-summary-list__key", text: "Foo"
assert_selector ".govuk-summary-list__value", text: "bar"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ class ContentBlockManager::ContentBlockEdition::Show::ConfirmSummaryListComponen
it "renders a summary list component with the edition details to confirm" do
organisation = create(:organisation, name: "Department for Example")

content_block_document = create(:content_block_document, :email_address, title: "Some title")
content_block_document = create(:content_block_document, :email_address)

content_block_edition = create(
:content_block_edition,
:email_address,
title: "Some edition title",
details: { "interesting_fact" => "value of fact" },
organisation:,
document: content_block_document,
Expand All @@ -37,7 +38,7 @@ class ContentBlockManager::ContentBlockEdition::Show::ConfirmSummaryListComponen
assert_selector ".govuk-summary-list__key", text: "Email address details"
assert_selector ".govuk-summary-list__actions", text: "Edit"
assert_selector ".govuk-summary-list__key", text: "Title"
assert_selector ".govuk-summary-list__value", text: "Some title"
assert_selector ".govuk-summary-list__value", text: "Some edition title"
assert_selector ".govuk-summary-list__key", text: "New interesting fact"
assert_selector ".govuk-summary-list__value", text: "value of fact"
assert_selector ".govuk-summary-list__key", text: "Lead organisation"
Expand All @@ -50,7 +51,7 @@ class ContentBlockManager::ContentBlockEdition::Show::ConfirmSummaryListComponen
it "shows the scheduled date time" do
organisation = create(:organisation, name: "Department for Example")

content_block_document = create(:content_block_document, :email_address, title: "Some title")
content_block_document = create(:content_block_document, :email_address)

content_block_edition = create(
:content_block_edition,
Expand All @@ -76,7 +77,7 @@ class ContentBlockManager::ContentBlockEdition::Show::ConfirmSummaryListComponen
it "shows a publish now row" do
organisation = create(:organisation, name: "Department for Example")

content_block_document = create(:content_block_document, :email_address, title: "Some title")
content_block_document = create(:content_block_document, :email_address)

_previous_edition = create(
:content_block_edition,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FactoryBot.define do
factory :content_block_document, class: "ContentBlockManager::ContentBlock::Document" do
sequence(:content_id) { SecureRandom.uuid }
title { "Title" }
sluggable_string { "factory-example-title" }
created_at { Time.zone.now.utc }
updated_at { Time.zone.now.utc }
latest_edition_id { nil }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

instructions_to_publishers { nil }

title { "Factory Title for Edition" }

ContentBlockManager::ContentBlock::Schema.valid_schemas.each do |type|
trait type.to_sym do
document { build(:content_block_document, block_type: type) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ class ContentBlockManager::ContentBlock::DocumentsTest < ActionDispatch::Integra
details: { "email_address" => "[email protected]" },
document_id: document_with_latest_edition.id,
)
document_without_latest_edition = create(:content_block_document, :email_address, title: "no latest edition")
_document_without_latest_edition = create(:content_block_document, :email_address, sluggable_string: "no latest edition")

visit content_block_manager.content_block_manager_content_block_documents_path({ lead_organisation: "" })

assert_text document_with_latest_edition.latest_edition.details["email_address"]
assert_no_text document_without_latest_edition.title
assert_text "1 result"
end

describe "when no filter params are specified" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ class ContentBlockManager::ContentBlock::EditionsTest < ActionDispatch::Integrat
let(:content_block_document) { create(:content_block_document, :email_address) }
let!(:original_edition) { create(:content_block_edition, :email_address, document: content_block_document) }

let(:title) { "Some Title" }
let(:document_attributes) do
{
title: "Some Title",
block_type: "email_address",
}.with_indifferent_access
end
Expand All @@ -81,6 +81,7 @@ class ContentBlockManager::ContentBlock::EditionsTest < ActionDispatch::Integrat
document_attributes:,
details:,
organisation_id: organisation.id,
title:,
},
}
end
Expand All @@ -92,8 +93,8 @@ class ContentBlockManager::ContentBlock::EditionsTest < ActionDispatch::Integrat
new_version = ContentBlockManager::ContentBlock::Version.last
new_edition_organisation = ContentBlockManager::ContentBlock::EditionOrganisation.last

assert_equal document_attributes[:title], content_block_document.title
assert_equal document_attributes[:block_type], content_block_document.block_type
assert_equal title, new_edition.title
assert_equal details, new_edition.details

assert_equal new_edition.document_id, content_block_document.id
Expand Down Expand Up @@ -137,6 +138,7 @@ class ContentBlockManager::ContentBlock::EditionsTest < ActionDispatch::Integrat
"content_block/edition": {
document_attributes:,
details:,
title:,
organisation_id: organisation.id,
},
}
Expand All @@ -147,7 +149,7 @@ class ContentBlockManager::ContentBlock::EditionsTest < ActionDispatch::Integrat
version = ContentBlockManager::ContentBlock::Version.last
edition_organisation = ContentBlockManager::ContentBlock::EditionOrganisation.last

assert_equal document_attributes[:title], document.title
assert_equal title, edition.title
assert_equal document_attributes[:block_type], document.block_type
assert_equal details, edition.details

Expand Down
Loading

0 comments on commit 664d2c7

Please sign in to comment.