From 361e8461d2e34f47540689d5202247f857182875 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 3 Feb 2026 16:18:55 +0100 Subject: [PATCH] fix problems uncovered in routing specs --- app/views/user_mailer/news_added.html.erb | 2 +- app/views/user_mailer/news_added.text.erb | 2 +- .../grids/widgets/news/item.html.erb | 2 +- .../components/grids/widgets/news_spec.rb | 4 +- spec/lib/open_project/static_routing_spec.rb | 4 +- spec/requests/news/comments_destroy_spec.rb | 4 +- spec/routing/messages_spec.rb | 34 +++--- spec/routing/news_comments_spec.rb | 23 ++-- spec/routing/news_spec.rb | 115 +++++++++--------- 9 files changed, 98 insertions(+), 92 deletions(-) diff --git a/app/views/user_mailer/news_added.html.erb b/app/views/user_mailer/news_added.html.erb index 28ef3745901..3062fbe6750 100644 --- a/app/views/user_mailer/news_added.html.erb +++ b/app/views/user_mailer/news_added.html.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -

<%= link_to @news.title, news_url(@news) %>

+

<%= link_to @news.title, project_news_url(@news.project, @news) %>

<%= @news.author&.name %> <%= format_text @news.description, only_path: false %> diff --git a/app/views/user_mailer/news_added.text.erb b/app/views/user_mailer/news_added.text.erb index 90701cd335e..d9ff46760f5 100644 --- a/app/views/user_mailer/news_added.text.erb +++ b/app/views/user_mailer/news_added.text.erb @@ -28,7 +28,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= @news.title %> -<%= news_url(@news) %> +<%= project_news_url(@news.project, @news) %> <%= @news.author&.name %> <%= @news.description %> diff --git a/modules/grids/app/components/grids/widgets/news/item.html.erb b/modules/grids/app/components/grids/widgets/news/item.html.erb index 5ef5a51cd40..343959c4dda 100644 --- a/modules/grids/app/components/grids/widgets/news/item.html.erb +++ b/modules/grids/app/components/grids/widgets/news/item.html.erb @@ -46,7 +46,7 @@ See COPYRIGHT and LICENSE files for more details. end layout.with_column do - render(Primer::Beta::Link.new(font_weight: :bold, href: news_path(item))) { item.title } + render(Primer::Beta::Link.new(font_weight: :bold, href: project_news_path(item.project, item))) { item.title } end end ) diff --git a/modules/grids/spec/components/grids/widgets/news_spec.rb b/modules/grids/spec/components/grids/widgets/news_spec.rb index 0be4c7326b0..31c9b61859c 100644 --- a/modules/grids/spec/components/grids/widgets/news_spec.rb +++ b/modules/grids/spec/components/grids/widgets/news_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Grids::Widgets::News, type: :component do it "renders news items from all projects", :aggregate_failures do expect(rendered_component).to have_list_item(count: 2) expect(rendered_component).to have_list_item(position: 2) do |item| - expect(item).to have_link href: news_path(news_red) + expect(item).to have_link href: project_news_path(project_red, news_red) expect(item).to have_content(/Added by .+ on \d{2}\/\d{2}\/\d{4} \d{2}:\d{2} [AP]M/) expect(item).to have_link href: user_path(author) end @@ -105,7 +105,7 @@ RSpec.describe Grids::Widgets::News, type: :component do it "renders only this project’s news" do expect(rendered_component).to have_list_item(count: 3) expect(rendered_component).to have_list_item(position: 3) do |item| - expect(item).to have_link href: news_path(news_items.first) + expect(item).to have_link href: project_news_path(project, news_items.first) expect(item).to have_content(/Added by .+ on \d{2}\/\d{2}\/\d{4} \d{2}:\d{2} [AP]M/) expect(item).to have_link href: user_path(author) end diff --git a/spec/lib/open_project/static_routing_spec.rb b/spec/lib/open_project/static_routing_spec.rb index d0151d64c1f..a1af0aa2dde 100644 --- a/spec/lib/open_project/static_routing_spec.rb +++ b/spec/lib/open_project/static_routing_spec.rb @@ -35,7 +35,7 @@ RSpec.describe OpenProject::StaticRouting do subject { described_class.recognize_route path } context "with no relative URL root", with_config: { rails_relative_url_root: nil } do - let(:path) { "/news/1" } + let(:path) { "/projects/foo/news/1" } it "detects the route" do expect(subject).to be_present @@ -44,7 +44,7 @@ RSpec.describe OpenProject::StaticRouting do end context "with a relative URL root", with_config: { rails_relative_url_root: "/foobar" } do - let(:path) { "/foobar/news/1" } + let(:path) { "/foobar/projects/foo/news/1" } it "detects the route" do expect(subject).to be_present diff --git a/spec/requests/news/comments_destroy_spec.rb b/spec/requests/news/comments_destroy_spec.rb index 5036007c028..52deef708cc 100644 --- a/spec/requests/news/comments_destroy_spec.rb +++ b/spec/requests/news/comments_destroy_spec.rb @@ -40,7 +40,7 @@ RSpec.describe "News comments destroy redirect", context "when an admin deletes a news comment" do current_user { create(:admin) } - let(:request) { delete "/comments/#{comment.id}" } + let(:request) { delete "/projects/#{project.identifier}/news/#{news.id}/comments/#{comment.id}" } subject do request @@ -49,7 +49,7 @@ RSpec.describe "News comments destroy redirect", it "responds with 303 See Other and redirects to the news page" do expect(subject).to have_http_status(:see_other) - expect(response).to redirect_to(news_path(news)) + expect(response).to redirect_to(project_news_path(project, news)) expect { Comment.find(comment.id) }.to raise_error(ActiveRecord::RecordNotFound) expect { News.find(news.id) }.not_to raise_error diff --git a/spec/routing/messages_spec.rb b/spec/routing/messages_spec.rb index 7ae59d4dd8b..3069a81e732 100644 --- a/spec/routing/messages_spec.rb +++ b/spec/routing/messages_spec.rb @@ -31,67 +31,67 @@ require "spec_helper" RSpec.describe MessagesController, "routing" do - context "project scoped" do - it { + context "with projects scoped forums" do + it do expect(subject).to route(:get, "/projects/some-project/forums/lala/topics/new").to(controller: "messages", action: "new", project_id: "some-project", forum_id: "lala") - } + end - it { + it do expect(subject).to route(:post, "/projects/some-project/forums/lala/topics").to(controller: "messages", action: "create", project_id: "some-project", forum_id: "lala") - } + end end - it { + it do expect(subject).to route(:get, "/projects/some-project/forums/lala/topics/2").to(controller: "messages", action: "show", project_id: "some-project", forum_id: "lala", id: "2") - } + end - it { + it do expect(subject).to route(:get, "/projects/some-project/forums/lala/topics/22/edit").to(controller: "messages", action: "edit", project_id: "some-project", forum_id: "lala", id: "22") - } + end - it { + it do expect(subject).to route(:put, "/projects/some-project/forums/lala/topics/22").to(controller: "messages", action: "update", project_id: "some-project", forum_id: "lala", id: "22") - } + end - it { + it do expect(subject).to route(:delete, "/projects/some-project/forums/lala/topics/555").to(controller: "messages", action: "destroy", project_id: "some-project", forum_id: "lala", id: "555") - } + end - it { + it do expect(subject).to route(:get, "/projects/some-project/forums/lala/topics/22/quote").to(controller: "messages", action: "quote", project_id: "some-project", forum_id: "lala", id: "22") - } + end - it { + it do expect(subject).to route(:post, "/projects/some-project/forums/lala/topics/555/reply").to(controller: "messages", action: "reply", project_id: "some-project", forum_id: "lala", id: "555") - } + end end diff --git a/spec/routing/news_comments_spec.rb b/spec/routing/news_comments_spec.rb index 37bf39a5781..f6158acd17a 100644 --- a/spec/routing/news_comments_spec.rb +++ b/spec/routing/news_comments_spec.rb @@ -32,16 +32,19 @@ require "spec_helper" RSpec.describe News::CommentsController, "routing" do context "news scoped" do - it { - expect(subject).to route(:post, "/news/567/comments").to(controller: "news/comments", - action: "create", - news_id: "567") - } + it do + expect(subject).to route(:post, "/projects/123/news/567/comments").to(controller: "news/comments", + action: "create", + project_id: "123", + news_id: "567") + end end - it { - expect(subject).to route(:delete, "/comments/15").to(controller: "news/comments", - action: "destroy", - id: "15") - } + it do + expect(subject).to route(:delete, "/projects/123/news/567/comments/15").to(controller: "news/comments", + action: "destroy", + project_id: "123", + news_id: "567", + id: "15") + end end diff --git a/spec/routing/news_spec.rb b/spec/routing/news_spec.rb index 30abb4ed0e4..02c34f0871e 100644 --- a/spec/routing/news_spec.rb +++ b/spec/routing/news_spec.rb @@ -31,73 +31,76 @@ require "spec_helper" RSpec.describe NewsController, "routing" do - context "project scoped" do - it { + it do + expect(subject).to route(:get, "/news").to(controller: "news", + action: "index") + end + + it do + expect(get("/news.atom")).to route_to(controller: "news", + action: "index", + format: "atom") + end + + context "with project scoped routes" do + it do expect(subject).to route(:get, "/projects/567/news").to(controller: "news", action: "index", project_id: "567") - } - - it do - expect(get("/projects/567/news.atom")) - .to route_to(controller: "news", - action: "index", - format: "atom", - project_id: "567") end - it { + it do + expect(get("/projects/567/news.atom")).to route_to(controller: "news", + action: "index", + format: "atom", + project_id: "567") + end + + it do expect(subject).to route(:get, "/projects/567/news/new").to(controller: "news", action: "new", project_id: "567") - } + end - it { + it do expect(subject).to route(:post, "/projects/567/news").to(controller: "news", action: "create", project_id: "567") - } + end + + it do + expect(subject).to route(:get, "/projects/567/news/2").to(controller: "news", + action: "show", + project_id: "567", + id: "2") + end + + it do + expect(subject).to route(:get, "/projects/567/news/234").to(controller: "news", + action: "show", + project_id: "567", + id: "234") + end + + it do + expect(subject).to route(:get, "/projects/567/news/567/edit").to(controller: "news", + action: "edit", + project_id: "567", + id: "567") + end + + it do + expect(subject).to route(:put, "/projects/567/news/567").to(controller: "news", + action: "update", + project_id: "567", + id: "567") + end + + it do + expect(subject).to route(:delete, "/projects/567/news/567").to(controller: "news", + action: "destroy", + project_id: "567", + id: "567") + end end - - it { - expect(subject).to route(:get, "/news").to(controller: "news", - action: "index") - } - - it do - expect(get("/news.atom")) - .to route_to(controller: "news", - action: "index", - format: "atom") - end - - it { - expect(subject).to route(:get, "/news/2").to(controller: "news", - action: "show", - id: "2") - } - - it { - expect(subject).to route(:get, "/news/234").to(controller: "news", - action: "show", - id: "234") - } - - it { - expect(subject).to route(:get, "/news/567/edit").to(controller: "news", - action: "edit", - id: "567") - } - - it { - expect(subject).to route(:put, "/news/567").to(controller: "news", - action: "update", - id: "567") - } - - it { - expect(subject).to route(:delete, "/news/567").to(controller: "news", - action: "destroy", - id: "567") - } end