Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add creator/author on creation of Content Block Edition #9272

Merged
merged 7 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions db/migrate/20240716141505_create_content_block_edition_authors.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class CreateContentBlockEditionAuthors < ActiveRecord::Migration[7.1]
def change
create_table :content_block_edition_authors do |t|
t.references :user, index: true, null: false
t.references :content_block_edition, index: true, foreign_key: true, null: false
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
end
end
end
12 changes: 11 additions & 1 deletion 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: 2024_07_01_080000) do
ActiveRecord::Schema[7.1].define(version: 2024_07_16_141505) 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 @@ -199,6 +199,15 @@
t.datetime "updated_at", precision: nil
end

create_table "content_block_edition_authors", charset: "utf8mb3", force: :cascade do |t|
t.bigint "user_id", null: false
t.bigint "content_block_edition_id", null: false
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
t.index ["content_block_edition_id"], name: "idx_on_content_block_edition_id_9e1066caab"
t.index ["user_id"], name: "index_content_block_edition_authors_on_user_id"
end

create_table "content_block_editions", charset: "utf8mb3", force: :cascade do |t|
t.json "details", null: false
t.bigint "content_block_document_id", null: false
Expand Down Expand Up @@ -1309,6 +1318,7 @@
t.datetime "updated_at", precision: nil
end

add_foreign_key "content_block_edition_authors", "content_block_editions"
add_foreign_key "content_block_editions", "content_block_documents"
add_foreign_key "documents", "editions", column: "latest_edition_id", on_update: :cascade, on_delete: :nullify
add_foreign_key "documents", "editions", column: "live_edition_id", on_update: :cascade, on_delete: :nullify
Expand Down
Binary file modified 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.
6 changes: 6 additions & 0 deletions docs/diagrams/object_store_models.puml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ together {
ContentBlockEdition .> content_block_editions
}

class ContentBlockEditionAuthor {
Harriethw marked this conversation as resolved.
Show resolved Hide resolved
user_id
content_block_edition_id
}

ContentBlockDocument *-r- ContentBlockEdition : "has_many"
ContentBlockEdition *-r- ContentBlockEditionAuthor : "has_many"

@enduml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def initialize(content_block_edition:)
attr_reader :content_block_edition

def items
[title_item].concat(details_items)
[title_item].concat(details_items).concat([creator_item])
end

def title_item
Expand All @@ -23,4 +23,11 @@ def details_items
{ field: key.humanize, value: }
end
end

def creator_item
{
field: "Creator",
value: content_block_edition.creator.name,
}
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ def root_params
end

def edition_params
root_params.permit(:title, :block_type, details: @schema.fields)
root_params.permit(:title, :block_type, details: @schema.fields).merge(creator: current_user)
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module ContentObjectStore
module HasAuthors
extend ActiveSupport::Concern
include ContentObjectStore::HasCreator

included do
has_many :content_block_edition_authors, dependent: :destroy
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module ContentObjectStore
module HasCreator
extend ActiveSupport::Concern

included do
validates :creator, presence: true
end

def creator
content_block_edition_authors.first&.user
end

def creator=(user)
if new_record?
content_block_edition_author = content_block_edition_authors.first || content_block_edition_authors.build
content_block_edition_author.user = user
else
raise "author can only be set on new records"
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class ContentObjectStore::ContentBlockEdition < ApplicationRecord
include ContentObjectStore::Identifiable
include ContentObjectStore::ValidatesDetails
include ContentObjectStore::HasAuthors
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class ContentObjectStore::ContentBlockEditionAuthor < ApplicationRecord
belongs_to :content_block_edition
belongs_to :user
end
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@
Given("an email address content block has been created") do
@content_blocks ||= []
@email_address = "foo@example.com"
@content_block = create(:content_block_edition, :email_address, details: { email_address: @email_address })
@content_block = create(
:content_block_edition,
:email_address,
details: { email_address: @email_address },
creator: @user,
)
@content_blocks.push(@content_block)
end

Expand Down Expand Up @@ -113,6 +118,8 @@ def should_show_summary_details_for_email_address_content_block(content_block, e
expect(page).to have_selector(".govuk-summary-list__value", text: content_block.document.title)
expect(page).to have_selector(".govuk-summary-list__key", text: "Email address")
expect(page).to have_selector(".govuk-summary-list__value", text: email_address)
expect(page).to have_selector(".govuk-summary-list__key", text: "Creator")
expect(page).to have_selector(".govuk-summary-list__value", text: @user.name)
end

Then("I should see errors for the required fields") do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,28 @@ class ContentObjectStore::ContentBlockEdition::Index::SummaryCardComponentTest <

test "it renders a content block as a summary card" do
content_block_document = build(:content_block_document, :email_address)
content_block_edition = build(:content_block_edition, :email_address, id: 123, details: { foo: "bar", something: "else" }, content_block_document:)
content_block_edition = build(
:content_block_edition,
:email_address,
id: 123,
details: { foo: "bar", something: "else" },
content_block_document:, creator: build(:user)
)

render_inline(ContentObjectStore::ContentBlockEdition::Index::SummaryCardComponent.new(content_block_edition:))

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_object_store_content_block_edition_path(content_block_edition)}']"

assert_selector ".govuk-summary-list__row", count: 3
assert_selector ".govuk-summary-list__row", count: 4
assert_selector ".govuk-summary-list__key", text: "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"
assert_selector ".govuk-summary-list__key", text: "Something"
assert_selector ".govuk-summary-list__value", text: "else"
assert_selector ".govuk-summary-list__key", text: "Creator"
assert_selector ".govuk-summary-list__value", text: content_block_edition.creator.name
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@

class ContentObjectStore::ContentBlockEdition::Show::SummaryListComponentTest < ViewComponent::TestCase
test "renders a content block correctly" do
content_block_edition = build(:content_block_edition, :email_address, details: { foo: "bar", something: "else" })
content_block_edition = build(
:content_block_edition,
:email_address,
details: { foo: "bar", something: "else" },
creator: build(:user),
)
render_inline(ContentObjectStore::ContentBlockEdition::Show::SummaryListComponent.new(content_block_edition:))

assert_selector ".govuk-summary-list__row", count: 3
assert_selector ".govuk-summary-list__row", count: 4
assert_selector ".govuk-summary-list__key", text: "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"
assert_selector ".govuk-summary-list__key", text: "Something"
assert_selector ".govuk-summary-list__value", text: "else"
assert_selector ".govuk-summary-list__key", text: "Creator"
assert_selector ".govuk-summary-list__value", text: content_block_edition.creator.name
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
updated_at { Time.zone.now.utc }
block_type { "block_type" }
schema { build(:content_block_schema) }
creator

ContentObjectStore::ContentBlockSchema.valid_schemas.each do |type|
trait type.to_sym do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,27 @@ class ContentBlockEditionsTest < ActionDispatch::IntegrationTest

assert_changes -> { ContentObjectStore::ContentBlockDocument.count }, from: 0, to: 1 do
assert_changes -> { ContentObjectStore::ContentBlockEdition.count }, from: 0, to: 1 do
post content_object_store.content_object_store_content_block_editions_path, params: {
something: "else",
content_object_store_content_block_edition: {
title:,
block_type:,
details:,
},
}
assert_changes -> { ContentObjectStore::ContentBlockEditionAuthor.count }, from: 0, to: 1 do
post content_object_store.content_object_store_content_block_editions_path, params: {
something: "else",
content_object_store_content_block_edition: {
title:,
block_type:,
details:,
},
}
end
end
end

new_document = ContentObjectStore::ContentBlockDocument.find_by!(content_id: @content_id)
new_edition = new_document.content_block_editions.first
new_author = ContentObjectStore::ContentBlockEditionAuthor.first
assert_equal title, new_document.title
assert_equal block_type, new_document.block_type
assert_equal details, new_edition.details
assert_equal new_edition.content_block_document_id, new_document.id
assert_equal new_edition.creator, new_author.user
end

test "#create posts the new edition to the Publishing API" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ContentObjectStore::ContentBlockEditionTest < ActiveSupport::TestCase
@updated_at = Time.zone.local(2000, 12, 31, 23, 59, 59).utc
@details = { "some_field" => "some_content" }
@title = "Document title"
@creator = create(:user)

@content_block_edition = build(
:content_block_edition,
Expand All @@ -18,6 +19,7 @@ class ContentObjectStore::ContentBlockEditionTest < ActiveSupport::TestCase
details: @details,
title: @title,
content_block_document: nil,
creator: @creator,
)
end

Expand Down Expand Up @@ -88,4 +90,17 @@ class ContentObjectStore::ContentBlockEditionTest < ActiveSupport::TestCase
assert_invalid content_block_edition
assert content_block_edition.errors.full_messages.include?("Title can't be blank")
end

test "it adds a creator and first edition author for new records" do
@content_block_edition.save!
@content_block_edition.reload
assert_equal @content_block_edition.creator, @content_block_edition.content_block_edition_authors.first.user
end

test "#creator= raises an exception if called for a persisted record" do
@content_block_edition.save!
assert_raise RuntimeError do
@content_block_edition.creator = create(:user)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ContentObjectStore::CreateEditionServiceTest < ActiveSupport::TestCase
"foo" => "Foo text",
"bar" => "Bar text",
},
creator: build(:user),
}
end

Expand Down