From f2dfdc53f9442dd80fc7445ad1851c51bf6b9a98 Mon Sep 17 00:00:00 2001 From: Mateusz Grotek Date: Thu, 25 Jul 2024 16:30:54 +0100 Subject: [PATCH] Use content_id for matching list items --- app/controllers/lists_controller.rb | 5 +++-- app/models/list.rb | 10 +++++----- app/models/list_item.rb | 4 ---- app/views/lists/edit_list_items.html.erb | 2 +- spec/controllers/list_items_controller_spec.rb | 4 ++-- spec/controllers/lists_controller_spec.rb | 12 +++++++----- spec/factories.rb | 1 + spec/models/list_spec.rb | 10 +++++----- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/controllers/lists_controller.rb b/app/controllers/lists_controller.rb index c6060c16e..6d4d29ba8 100644 --- a/app/controllers/lists_controller.rb +++ b/app/controllers/lists_controller.rb @@ -95,10 +95,11 @@ def edit_list_items_params def save_list_items @list.errors.add(:list_items, "Select a link to add to the list") and return false if edit_list_items_params.blank? - edit_list_items_params.each do |base_path| - linked_item = @available_list_items.select { |item| item.base_path == base_path }.first + edit_list_items_params.each do |content_id| + linked_item = @available_list_items.select { |item| item.content_id == content_id }.first @list.list_items.create!( base_path: linked_item.base_path, + content_id: linked_item.content_id, title: linked_item.title, index: @list.list_items.length + 1, ) diff --git a/app/models/list.rb b/app/models/list.rb index 526e8e9b3..0ac9ff4ce 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -13,7 +13,7 @@ def tagged_list_items def list_items_with_tagging_status @list_items_with_tagging_status ||= list_items.order(:index).map do |list_item| - list_item.tagged = tagged_base_paths.include?(list_item.base_path) + list_item.tagged = tagged_content_ids.include?(list_item.content_id) list_item end end @@ -24,14 +24,14 @@ def available_list_items .documents .reject do |link| list_items - .map(&:base_path) - .include?(link["base_path"]) + .map(&:content_id) + .include?(link["content_id"]) end end private - def tagged_base_paths - @tagged_base_paths ||= tag.tagged_documents.map(&:base_path) + def tagged_content_ids + @tagged_content_ids ||= tag.tagged_documents.map(&:content_id) end end diff --git a/app/models/list_item.rb b/app/models/list_item.rb index 1f2e7b1d5..7bf392a32 100644 --- a/app/models/list_item.rb +++ b/app/models/list_item.rb @@ -11,8 +11,4 @@ class ListItem < ApplicationRecord def display_title list.tag.tagged_document_for_base_path(base_path).try(:title) || title end - - def content_id - list.tag.tagged_document_for_base_path(base_path).content_id - end end diff --git a/app/views/lists/edit_list_items.html.erb b/app/views/lists/edit_list_items.html.erb index febd9336d..12caa328f 100644 --- a/app/views/lists/edit_list_items.html.erb +++ b/app/views/lists/edit_list_items.html.erb @@ -21,7 +21,7 @@ items: @list.available_list_items.map do |list_item| { label: list_item.title, - value: list_item.base_path, + value: list_item.content_id, } end, } %> diff --git a/spec/controllers/list_items_controller_spec.rb b/spec/controllers/list_items_controller_spec.rb index e9b3c70c8..887c1657a 100644 --- a/spec/controllers/list_items_controller_spec.rb +++ b/spec/controllers/list_items_controller_spec.rb @@ -125,8 +125,8 @@ let!(:list_item2) { create(:list_item, list: list1, index: 2) } let(:tagged_documents) do [ - TaggedDocuments::Document.new(list_item1.title, list_item1.base_path, "123"), - TaggedDocuments::Document.new(list_item2.title, list_item2.base_path, "456"), + TaggedDocuments::Document.new(list_item1.title, "/some-path-1", list_item1.content_id), + TaggedDocuments::Document.new(list_item2.title, "/some-path-2", list_item2.content_id), TaggedDocuments::Document.new("New list", "/new-list", "789"), ] end diff --git a/spec/controllers/lists_controller_spec.rb b/spec/controllers/lists_controller_spec.rb index 3efd800b0..eb0e8d908 100644 --- a/spec/controllers/lists_controller_spec.rb +++ b/spec/controllers/lists_controller_spec.rb @@ -63,8 +63,8 @@ let!(:list_item2) { create(:list_item, list:, index: 2) } let(:tagged_documents) do [ - TaggedDocuments::Document.new(list_item1.title, list_item1.base_path, "123"), - TaggedDocuments::Document.new(list_item2.title, list_item2.base_path, "456"), + TaggedDocuments::Document.new(list_item1.title, "/some-path-1", list_item1.content_id), + TaggedDocuments::Document.new(list_item2.title, "/some-path-2", list_item2.content_id), TaggedDocuments::Document.new("New list", "/new-list", "789"), TaggedDocuments::Document.new("Newer list", "/newer-list", "012"), ] @@ -84,7 +84,7 @@ patch :update_list_items, params: { tag_id: tag.content_id, id: list.id, - list: { list_items: ["/new-list", "/newer-list"] }, + list: { list_items: %w[789 012] }, } new_list_item1, new_list_item2 = list.list_items.last(2) @@ -92,9 +92,11 @@ expect(list.list_items.count).to eq 4 expect(new_list_item1.title).to eq "New list" expect(new_list_item1.base_path).to eq "/new-list" + expect(new_list_item1.content_id).to eq "789" expect(new_list_item1.index).to eq 3 expect(new_list_item2.title).to eq "Newer list" expect(new_list_item2.base_path).to eq "/newer-list" + expect(new_list_item2.content_id).to eq "012" expect(new_list_item2.index).to eq 4 expect(response.status).to eq(302) expect(response).to redirect_to(tag_list_path(tag, list)) @@ -176,8 +178,8 @@ let!(:list_item2) { create(:list_item, list:, index: 2) } let(:tagged_documents) do [ - TaggedDocuments::Document.new(list_item1.title, list_item1.base_path, "123"), - TaggedDocuments::Document.new(list_item2.title, list_item2.base_path, "456"), + TaggedDocuments::Document.new(list_item1.title, "/some-path-1", list_item1.content_id), + TaggedDocuments::Document.new(list_item2.title, "/some-path-2", list_item2.content_id), TaggedDocuments::Document.new("New list", "/new-list", "789"), ] end diff --git a/spec/factories.rb b/spec/factories.rb index 99cd531bd..55c276b01 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -185,6 +185,7 @@ factory :list_item do sequence(:title) { |n| "List item #{n}" } sequence(:base_path) { |n| "/base-path-#{n}" } + content_id { SecureRandom.uuid } sequence(:index) end diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index fc368aae2..2e3514eb6 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -29,9 +29,9 @@ describe "#list_items_with_tagging_status" do it "returns the list items with tagged set to true if they're tagged" do list = create(:list, tag: create(:tag, slug: "subtag")) - create(:list_item, list:, base_path: "/tagged-item") - create(:list_item, list:, base_path: "/untagged-item") - publishing_api_has_linked_items(list.tag.content_id, items: [{ base_path: "/tagged-item" }]) + create(:list_item, list:, content_id: "123") + create(:list_item, list:, content_id: "456") + publishing_api_has_linked_items(list.tag.content_id, items: [{ content_id: "123" }]) list_item = list.list_items_with_tagging_status.first @@ -40,7 +40,7 @@ it "returns the list items with tagged set to false if they're not tagged" do list = create(:list, tag: create(:tag, slug: "subtag")) - create(:list_item, list:, base_path: "/untagged-item") + create(:list_item, list:, content_id: "456") publishing_api_has_no_linked_items list_item = list.list_items_with_tagging_status.first @@ -52,7 +52,7 @@ describe "#available_list_items" do it "returns tagged list items from the publishing api that are in the list" do list = create(:list) - create(:list_item, list:, base_path: "/item-in-list") + create(:list_item, list:, content_id: "123") publishing_api_has_linked_items( list.tag.content_id, items: [