diff --git a/app/models/popular_links_edition.rb b/app/models/popular_links_edition.rb index e08bfb097..080f7136f 100644 --- a/app/models/popular_links_edition.rb +++ b/app/models/popular_links_edition.rb @@ -11,12 +11,12 @@ def all_valid_urls_and_titles_are_present link_items.each_with_index do |item, index| errors.add("url#{index + 1}", "URL is required for Link #{index + 1}") unless url_present?(item) errors.add("title#{index + 1}", "Title is required for Link #{index + 1}") unless title_present?(item) - errors.add("url#{index + 1}", "URL is invalid for Link #{index + 1}, all URLs should have at least one '.' and no spaces.") if url_present?(item) && url_has_spaces_or_has_no_dot?(item[:url]) + errors.add("url#{index + 1}", "URL is invalid for Link #{index + 1}, all URLs should start with '/'") if url_present?(item) && url_is_not_valid_relative_url?(item[:url]) end end - def url_has_spaces_or_has_no_dot?(url) - url.include?(" ") || url.exclude?(".") + def url_is_not_valid_relative_url?(url) + !url.start_with?("/") end def title_present?(item) diff --git a/test/functional/homepage_controller_test.rb b/test/functional/homepage_controller_test.rb index e6d32a439..21676b5b7 100644 --- a/test/functional/homepage_controller_test.rb +++ b/test/functional/homepage_controller_test.rb @@ -59,18 +59,18 @@ class HomepageControllerTest < ActionController::TestCase should "update latest PopularLinksEdition with changed title and url" do assert_equal "title1", @popular_links.link_items[0][:title] - assert_equal "https://www.url1.com", @popular_links.link_items[0][:url] + assert_equal "/url1", @popular_links.link_items[0][:url] new_title = "title has changed" - new_url = "https://www.changedurl.com" + new_url = "/changedurl" patch :update, params: { id: @popular_links.id, "popular_links" => { "1" => { "title" => new_title, "url" => new_url }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_equal new_title, PopularLinksEdition.last.link_items[0][:title] assert_equal new_url, PopularLinksEdition.last.link_items[0][:url] @@ -81,12 +81,12 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => "title", "url" => "url.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => "title", "url" => "/url" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } end should "redirect to show path on success" do @@ -94,18 +94,18 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => new_title, "url" => "https://www.url1.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => new_title, "url" => "/url1" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_redirected_to show_popular_links_path end should "render edit template on validation error" do - links_with_missing_items = { "1" => { "title" => "title has changed", "url" => "https://www.url1.com" } } + links_with_missing_items = { "1" => { "title" => "title has changed", "url" => "/url1" } } patch :update, params: { id: @popular_links.id, "popular_links" => links_with_missing_items } @@ -123,12 +123,12 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => new_title, "url" => "https://www.url1.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => new_title, "url" => "/url1" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_equal "Due to an application error, the edition couldn't be saved.", flash[:danger] end @@ -138,12 +138,12 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => new_title, "url" => "https://www.url1.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => new_title, "url" => "/url1" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_template "homepage/popular_links/edit" end @@ -159,12 +159,12 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => new_title, "url" => "https://www.url1.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => new_title, "url" => "/url1" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_template "homepage/popular_links/edit" end @@ -174,12 +174,12 @@ class HomepageControllerTest < ActionController::TestCase patch :update, params: { id: @popular_links.id, "popular_links" => - { "1" => { "title" => new_title, "url" => "https://www.url1.com" }, - "2" => { "title" => "title2", "url" => "https://www.url2.com" }, - "3" => { "title" => "title3", "url" => "https://www.url3.com" }, - "4" => { "title" => "title4", "url" => "https://www.url4.com" }, - "5" => { "title" => "title5", "url" => "https://www.url5.com" }, - "6" => { "title" => "title6", "url" => "https://www.url6.com" } } } + { "1" => { "title" => new_title, "url" => "/url1" }, + "2" => { "title" => "title2", "url" => "/url2" }, + "3" => { "title" => "title3", "url" => "/url3" }, + "4" => { "title" => "title4", "url" => "/url4" }, + "5" => { "title" => "title5", "url" => "/url5" }, + "6" => { "title" => "title6", "url" => "/url6" } } } assert_equal "Popular links save was unsuccessful due to a service problem. Please wait for a few minutes and try again.", flash[:danger] end diff --git a/test/integration/homepage_popular_links_test.rb b/test/integration/homepage_popular_links_test.rb index bb25687e9..fb012d4ac 100644 --- a/test/integration/homepage_popular_links_test.rb +++ b/test/integration/homepage_popular_links_test.rb @@ -130,11 +130,11 @@ class HomepagePopularLinksTest < JavascriptIntegrationTest end should "trim spaces from start and end of urls" do - fill_in "popular_links[1][url]", with: " www.abc.com " + fill_in "popular_links[1][url]", with: " /abc " click_button("Save") - assert page.has_text?("www.abc.com") - assert_not page.has_text?(" www.abc.com ") + assert page.has_text?("/abc") + assert_not page.has_text?(" /abc ") end should "render create page when 'Cancel' is clicked" do @@ -144,10 +144,10 @@ class HomepagePopularLinksTest < JavascriptIntegrationTest end should "not save any changes when 'Cancel' is clicked" do - fill_in "popular_links[1][url]", with: "www.abc.com" + fill_in "popular_links[1][url]", with: "/abc" click_link("Cancel") - assert_not page.has_text?("www.abc.com") + assert_not page.has_text?("/abc") end end diff --git a/test/models/popular_links_edition_test.rb b/test/models/popular_links_edition_test.rb index d640be6d2..3b37a20ed 100644 --- a/test/models/popular_links_edition_test.rb +++ b/test/models/popular_links_edition_test.rb @@ -15,12 +15,12 @@ class PopularLinksEditionTest < ActiveSupport::TestCase end should "validate all links have url and title" do - link_items = [{ url: "https://www.url1.com", title: "" }, + link_items = [{ url: "/url1", title: "" }, { title: "title2" }, - { url: "https://www.url3.com", title: "title3" }, - { url: "https://www.url4.com", title: "title4" }, - { url: "https://www.url5.com", title: "title5" }, - { url: "https://www.url6.com", title: "title6" }] + { url: "/url3", title: "title3" }, + { url: "/url4", title: "title4" }, + { url: "/url5", title: "title5" }, + { url: "/url6", title: "title6" }] popular_links = FactoryBot.build(:popular_links, link_items:) assert_not popular_links.valid? @@ -31,14 +31,14 @@ class PopularLinksEditionTest < ActiveSupport::TestCase should "validate all urls are valid" do link_items = [{ url: "", title: "" }, { url: "invalid", title: "title2" }, - { url: "www.abc.co.uk", title: "title3" }, - { url: "www.cde.co.uk", title: "title4" }, - { url: "www.efg.co.uk", title: "title5" }, - { url: "www.ijk.com", title: "title6" }] + { url: "/abc", title: "title3" }, + { url: "/cde", title: "title4" }, + { url: "/efg", title: "title5" }, + { url: "/hij", title: "title6" }] popular_links = FactoryBot.build(:popular_links, link_items:) assert_not popular_links.valid? - assert popular_links.errors.messages[:url2].include?("URL is invalid for Link 2, all URLs should have at least one '.' and no spaces.") + assert popular_links.errors.messages[:url2].include?("URL is invalid for Link 2, all URLs should start with '/'") assert popular_links.errors.messages[:url1].include?("URL is required for Link 1") end diff --git a/test/support/factories.rb b/test/support/factories.rb index 77bdac0b6..9ea6d1fa5 100644 --- a/test/support/factories.rb +++ b/test/support/factories.rb @@ -183,7 +183,7 @@ factory :popular_links, class: "PopularLinksEdition" do title { "Homepage Popular Links" } - link_items { [{ url: "https://www.url1.com", title: "title1" }, { url: "https://www.url2.com", title: "title2" }, { url: "https://www.url3.com", title: "title3" }, { url: "https://www.url4.com", title: "title4" }, { url: "https://www.url5.com", title: "title5" }, { url: "https://www.url6.com", title: "title6" }] } + link_items { [{ url: "/url1", title: "title1" }, { url: "/url2", title: "title2" }, { url: "/url3", title: "title3" }, { url: "/url4", title: "title4" }, { url: "/url5", title: "title5" }, { url: "/url6", title: "title6" }] } end factory :programme_edition, parent: :edition, class: "ProgrammeEdition" do diff --git a/test/unit/presenters/formats/popular_links_presenter_test.rb b/test/unit/presenters/formats/popular_links_presenter_test.rb index 275c641f5..478e96651 100644 --- a/test/unit/presenters/formats/popular_links_presenter_test.rb +++ b/test/unit/presenters/formats/popular_links_presenter_test.rb @@ -20,12 +20,12 @@ def subject assert_equal "link_collection", @result[:document_type] assert_equal "publisher", @result[:publishing_app] assert_equal "frontend", @result[:rendering_app] - assert_equal ({ link_items: [{ url: "https://www.url1.com", title: "title1" }, - { url: "https://www.url2.com", title: "title2" }, - { url: "https://www.url3.com", title: "title3" }, - { url: "https://www.url4.com", title: "title4" }, - { url: "https://www.url5.com", title: "title5" }, - { url: "https://www.url6.com", title: "title6" }] }), + assert_equal ({ link_items: [{ url: "/url1", title: "title1" }, + { url: "/url2", title: "title2" }, + { url: "/url3", title: "title3" }, + { url: "/url4", title: "title4" }, + { url: "/url5", title: "title5" }, + { url: "/url6", title: "title6" }] }), @result[:details] end