From d77789d51738fcde129c8b098dc793ec2aa70893 Mon Sep 17 00:00:00 2001 From: pezholio Date: Tue, 18 Feb 2025 11:51:44 +0000 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20change=20the=20object=20key=20w?= =?UTF-8?q?hen=20changing=20the=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids the issue where changing the name of a rate breaks the embed code. It does mean that once a name is set, this means the key of the object is immutable, but this is a similar issue to how slugs work in other documents. We can revisit this if it becomes an issue. --- .../editions/embedded_objects_controller.rb | 2 +- .../content_block_manager/content_block/edition.rb | 8 +------- .../content_block/editions/embedded_objects_test.rb | 6 +++--- .../unit/app/models/content_block_edition_test.rb | 11 ++--------- 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb index 1968a421c96..fa6728580a7 100644 --- a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb @@ -37,7 +37,7 @@ def update redirect_to content_block_manager.review_embedded_object_content_block_manager_content_block_edition_path( @content_block_edition, object_type: @subschema.block_type, - object_name: @content_block_edition.key_for_object(@object), + object_name: params[:object_name], ) end rescue ActiveRecord::RecordInvalid diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb index 2c2b4dfdc51..253a23cd9c0 100644 --- a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb +++ b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb @@ -53,13 +53,7 @@ def add_object_to_details(object_type, body) end def update_object_with_details(object_type, object_name, body) - key = key_for_object(body) - - if key != object_name - details[object_type].delete(object_name) - end - - add_object_to_details(object_type, body) + details[object_type][object_name] = body.to_h end def key_for_object(object) diff --git a/lib/engines/content_block_manager/test/integration/content_block/editions/embedded_objects_test.rb b/lib/engines/content_block_manager/test/integration/content_block/editions/embedded_objects_test.rb index 141f913384a..035fb3d168e 100644 --- a/lib/engines/content_block_manager/test/integration/content_block/editions/embedded_objects_test.rb +++ b/lib/engines/content_block_manager/test/integration/content_block/editions/embedded_objects_test.rb @@ -161,7 +161,7 @@ class ContentBlockManager::ContentBlock::Editions::EmbeddedObjectsTest < ActionD assert_equal "Something edited. You can add another something or continue to create schema block", flash[:notice] end - it "should rename the object if a new name is given" do + it "should not rename the object if a new name is given" do put content_block_manager.embedded_object_content_block_manager_content_block_edition_path( edition, object_type:, @@ -180,12 +180,12 @@ class ContentBlockManager::ContentBlock::Editions::EmbeddedObjectsTest < ActionD assert_redirected_to content_block_manager.review_embedded_object_content_block_manager_content_block_edition_path( edition, object_type:, - object_name: "new-name", + object_name: "embedded", ) updated_edition = edition.reload - assert_equal updated_edition.details, { "something" => { "new-name" => { "name" => "New Name", "is" => "different" } } } + assert_equal updated_edition.details, { "something" => { "embedded" => { "name" => "New Name", "is" => "different" } } } end it "should render errors if a validation error is thrown" do diff --git a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb index 84581cd9e0b..1b5497eb256 100644 --- a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb @@ -229,17 +229,10 @@ class ContentBlockManager::ContentBlockEditionTest < ActiveSupport::TestCase assert_equal content_block_edition.details["something"], { "my-thing" => { "name" => "My thing", "something" => "changed" } } end - it "removes the original object if the name changes" do + it "keeps the original key name if the name changes" do content_block_edition.update_object_with_details("something", "my-thing", { "name" => "Other thing", "something" => "changed" }) - assert_equal content_block_edition.details["something"], { "other-thing" => { "name" => "Other thing", "something" => "changed" } } - end - - it "creates a random key if a name is not provided" do - SecureRandom.expects(:alphanumeric).at_least_once.returns("RANDOM-STRING") - content_block_edition.update_object_with_details("something", "my-thing", { "something" => "changed" }) - - assert_equal content_block_edition.details["something"], { "random-string" => { "something" => "changed" } } + assert_equal content_block_edition.details["something"], { "my-thing" => { "name" => "Other thing", "something" => "changed" } } end end