From c8148ab79971a208d59c0ae5d8ddffda14650335 Mon Sep 17 00:00:00 2001 From: Mir Bhatia Date: Wed, 22 Apr 2026 16:44:50 +0200 Subject: [PATCH] Fix more redirection HTTP statuses --- .../admin/import/jira/instances_controller.rb | 2 +- .../settings/enumerations_controller_base.rb | 4 +- app/controllers/forums_controller.rb | 2 +- app/controllers/roles_controller.rb | 2 +- .../controllers/boards/boards_controller.rb | 2 +- .../boards/boards_controller_spec.rb | 1 + .../app/controllers/costlog_controller.rb | 4 +- .../controllers/costlog_controller_spec.rb | 13 +++++ .../controllers/deploy_targets_controller.rb | 2 +- .../spec/requests/deploy_targets_spec.rb | 48 +++++++++++++++++ .../synchronized_filters_controller.rb | 2 +- .../requests/synchronized_filters_spec.rb | 48 +++++++++++++++++ .../storages/admin/storages_controller.rb | 2 +- .../admin/storages_controller_spec.rb | 17 ++++++ .../team_planner/team_planner_controller.rb | 2 +- .../spec/requests/team_planner_spec.rb | 52 +++++++++++++++++++ .../import/jira/instances_controller_spec.rb | 2 + spec/controllers/forums_controller_spec.rb | 1 + spec/controllers/roles_controller_spec.rb | 2 + .../settings/work_package_priorities_spec.rb | 1 + 20 files changed, 197 insertions(+), 12 deletions(-) create mode 100644 modules/github_integration/spec/requests/deploy_targets_spec.rb create mode 100644 modules/ldap_groups/spec/requests/synchronized_filters_spec.rb create mode 100644 modules/team_planner/spec/requests/team_planner_spec.rb diff --git a/app/controllers/admin/import/jira/instances_controller.rb b/app/controllers/admin/import/jira/instances_controller.rb index 68616e85ffd..f4964aa9e96 100644 --- a/app/controllers/admin/import/jira/instances_controller.rb +++ b/app/controllers/admin/import/jira/instances_controller.rb @@ -70,7 +70,7 @@ module Admin::Import::Jira @jira.destroy! flash[:notice] = t(:notice_successful_delete) end - redirect_to action: :index + redirect_to action: :index, status: :see_other end def delete_token diff --git a/app/controllers/admin/settings/enumerations_controller_base.rb b/app/controllers/admin/settings/enumerations_controller_base.rb index c3346c8baf1..4b9e2853877 100644 --- a/app/controllers/admin/settings/enumerations_controller_base.rb +++ b/app/controllers/admin/settings/enumerations_controller_base.rb @@ -74,10 +74,10 @@ module Admin handle_reassignment_on_deletion elsif @enumeration.destroy flash[:notice] = I18n.t(:notice_successful_delete) - redirect_to(action: :index) + redirect_to(action: :index, status: :see_other) else flash.now[:error] = I18n.t(:error_can_not_delete_entry) - redirect_to(action: :index) + redirect_to(action: :index, status: :see_other) end end diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb index 9ff04248152..1249b6a45a1 100644 --- a/app/controllers/forums_controller.rb +++ b/app/controllers/forums_controller.rb @@ -120,7 +120,7 @@ class ForumsController < ApplicationController @forum.destroy! flash[:notice] = I18n.t(:notice_successful_delete) - redirect_to project_forums_path(@project) + redirect_to project_forums_path(@project), status: :see_other end private diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 8ece566240e..bd9b465aceb 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -94,7 +94,7 @@ class RolesController < ApplicationController else flash[:error] = I18n.t(:error_can_not_remove_role) end - redirect_to action: "index" + redirect_to action: "index", status: :see_other end def report diff --git a/modules/boards/app/controllers/boards/boards_controller.rb b/modules/boards/app/controllers/boards/boards_controller.rb index 914eed69e8d..f1b7e270eba 100644 --- a/modules/boards/app/controllers/boards/boards_controller.rb +++ b/modules/boards/app/controllers/boards/boards_controller.rb @@ -63,7 +63,7 @@ module ::Boards render json: { redirect_url: project_work_package_boards_path(@project) } end format.html do - redirect_to action: "index", project_id: @project + redirect_to action: "index", project_id: @project, status: :see_other end end end diff --git a/modules/boards/spec/controllers/boards/boards_controller_spec.rb b/modules/boards/spec/controllers/boards/boards_controller_spec.rb index c761a3f6f19..19a5959c035 100644 --- a/modules/boards/spec/controllers/boards/boards_controller_spec.rb +++ b/modules/boards/spec/controllers/boards/boards_controller_spec.rb @@ -49,6 +49,7 @@ RSpec.describe Boards::BoardsController do expect(flash[:notice]).to eq(I18n.t(:notice_successful_delete)) expect(response).to redirect_to(project_work_package_boards_path(project)) + expect(response).to have_http_status(:see_other) end end diff --git a/modules/costs/app/controllers/costlog_controller.rb b/modules/costs/app/controllers/costlog_controller.rb index 611720e2336..0eea746cb9d 100644 --- a/modules/costs/app/controllers/costlog_controller.rb +++ b/modules/costs/app/controllers/costlog_controller.rb @@ -88,9 +88,9 @@ class CostlogController < ApplicationController flash[:notice] = t(:notice_successful_delete) if request.referer.include?("cost_reports") - redirect_to controller: "/cost_reports", action: :index + redirect_to controller: "/cost_reports", action: :index, status: :see_other else - redirect_back_or_to(polymorphic_path(@cost_entry.entity)) + redirect_back_or_to(polymorphic_path(@cost_entry.entity), status: :see_other) end end diff --git a/modules/costs/spec/controllers/costlog_controller_spec.rb b/modules/costs/spec/controllers/costlog_controller_spec.rb index cb62c508057..4025860bf93 100644 --- a/modules/costs/spec/controllers/costlog_controller_spec.rb +++ b/modules/costs/spec/controllers/costlog_controller_spec.rb @@ -528,6 +528,19 @@ RSpec.describe CostlogController do end end + describe "DELETE destroy" do + before do + cost_entry.save(validate: false) + grant_current_user_permissions user, %i[view_project view_work_packages view_cost_entries edit_cost_entries] + request.env["HTTP_REFERER"] = "http://example.com/work_packages" + end + + it "redirects with see_other status" do + delete :destroy, params: { id: cost_entry.id } + expect(response).to have_http_status(:see_other) + end + end + describe "PUT update" do let(:params) do { "id" => cost_entry.id.to_s, diff --git a/modules/github_integration/app/controllers/deploy_targets_controller.rb b/modules/github_integration/app/controllers/deploy_targets_controller.rb index c4743a04db3..f825feb2b89 100644 --- a/modules/github_integration/app/controllers/deploy_targets_controller.rb +++ b/modules/github_integration/app/controllers/deploy_targets_controller.rb @@ -65,6 +65,6 @@ class DeployTargetsController < ApplicationController flash[:success] = I18n.t(:notice_deploy_target_destroyed) - redirect_to deploy_targets_path + redirect_to deploy_targets_path, status: :see_other end end diff --git a/modules/github_integration/spec/requests/deploy_targets_spec.rb b/modules/github_integration/spec/requests/deploy_targets_spec.rb new file mode 100644 index 00000000000..b791c83c2f0 --- /dev/null +++ b/modules/github_integration/spec/requests/deploy_targets_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe "Deploy targets", :skip_csrf, type: :rails_request do + let(:admin) { create(:admin) } + let(:deploy_target) { create(:deploy_target) } + + before do + login_as(admin) + end + + describe "DELETE /deploy_targets/:id" do + it "redirects with 303 See Other" do + delete deploy_target_path(deploy_target) + expect(response).to have_http_status(:see_other) + expect(response).to redirect_to(deploy_targets_path) + end + end +end diff --git a/modules/ldap_groups/app/controllers/ldap_groups/synchronized_filters_controller.rb b/modules/ldap_groups/app/controllers/ldap_groups/synchronized_filters_controller.rb index b386f89992e..5f57f998b09 100644 --- a/modules/ldap_groups/app/controllers/ldap_groups/synchronized_filters_controller.rb +++ b/modules/ldap_groups/app/controllers/ldap_groups/synchronized_filters_controller.rb @@ -56,7 +56,7 @@ module LdapGroups flash[:error] = I18n.t(:error_can_not_delete_entry) end - redirect_to ldap_groups_synchronized_groups_path + redirect_to ldap_groups_synchronized_groups_path, status: :see_other end def synchronize diff --git a/modules/ldap_groups/spec/requests/synchronized_filters_spec.rb b/modules/ldap_groups/spec/requests/synchronized_filters_spec.rb new file mode 100644 index 00000000000..ecba45e23d1 --- /dev/null +++ b/modules/ldap_groups/spec/requests/synchronized_filters_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require_relative "../spec_helper" + +RSpec.describe "LDAP synchronized filters", :skip_csrf, type: :rails_request do + let(:admin) { create(:admin) } + let(:filter) { create(:ldap_synchronized_filter) } + + before do + login_as(admin) + end + + describe "DELETE /ldap_groups/synchronized_filters/:ldap_filter_id" do + it "redirects with 303 See Other" do + delete ldap_groups_synchronized_filter_path(ldap_filter_id: filter.id) + expect(response).to have_http_status(:see_other) + expect(response).to redirect_to(ldap_groups_synchronized_groups_path) + end + end +end diff --git a/modules/storages/app/controllers/storages/admin/storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages_controller.rb index c88af851abb..75afb7b687f 100644 --- a/modules/storages/app/controllers/storages/admin/storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages_controller.rb @@ -190,7 +190,7 @@ module Storages service_result.on_success do flash[:notice] = I18n.t(:notice_successful_delete) end - redirect_to admin_settings_storages_path + redirect_to admin_settings_storages_path, status: :see_other end def replace_oauth_application diff --git a/modules/storages/spec/controllers/storages/admin/storages_controller_spec.rb b/modules/storages/spec/controllers/storages/admin/storages_controller_spec.rb index e50bcf09b71..99ca23132c1 100644 --- a/modules/storages/spec/controllers/storages/admin/storages_controller_spec.rb +++ b/modules/storages/spec/controllers/storages/admin/storages_controller_spec.rb @@ -65,4 +65,21 @@ RSpec.describe Storages::Admin::StoragesController do end end end + + describe "DELETE #destroy" do + let(:storage) { build_stubbed(:nextcloud_storage) } + let(:service_result) { ServiceResult.success } + let(:delete_service) { instance_double(Storages::Storages::DeleteService, call: service_result) } + + before do + allow(Storages::Storage).to receive(:visible).and_return(instance_double(ActiveRecord::Relation, find: storage)) + allow(Storages::Storages::DeleteService).to receive(:new).and_return(delete_service) + end + + it "redirects to storages index with see_other" do + delete :destroy, params: { id: storage.id } + expect(response).to redirect_to(admin_settings_storages_path) + expect(response).to have_http_status(:see_other) + end + end end diff --git a/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb b/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb index 2e02e579725..e8edeaefd25 100644 --- a/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb +++ b/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb @@ -50,7 +50,7 @@ module ::TeamPlanner flash[:error] = t(:error_can_not_delete_entry) end - redirect_to action: :index + redirect_to action: :index, status: :see_other end current_menu_item :index do diff --git a/modules/team_planner/spec/requests/team_planner_spec.rb b/modules/team_planner/spec/requests/team_planner_spec.rb new file mode 100644 index 00000000000..d8cb015ed20 --- /dev/null +++ b/modules/team_planner/spec/requests/team_planner_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe "Team planners", :skip_csrf, type: :rails_request, with_ee: %i[team_planner_view] do + let(:project) { create(:project) } + let(:user) do + create(:user, member_with_permissions: { project => %i[view_work_packages view_team_planner manage_team_planner] }) + end + let(:query) { create(:query, project:, user:) } + + before do + create(:view_team_planner, query:) + login_as(user) + end + + describe "DELETE /projects/:project_id/team_planners/:id" do + it "redirects with 303 See Other" do + delete project_team_planner_path(project, query) + expect(response).to have_http_status(:see_other) + expect(response).to redirect_to(project_team_planners_path(project)) + end + end +end diff --git a/spec/controllers/admin/import/jira/instances_controller_spec.rb b/spec/controllers/admin/import/jira/instances_controller_spec.rb index fa6ae6e6e6d..70ca2c8706f 100644 --- a/spec/controllers/admin/import/jira/instances_controller_spec.rb +++ b/spec/controllers/admin/import/jira/instances_controller_spec.rb @@ -358,6 +358,7 @@ RSpec.describe Admin::Import::Jira::InstancesController do jira_to_delete = create(:jira) delete :destroy, params: { id: jira_to_delete.id } expect(response).to redirect_to(action: :index) + expect(response).to have_http_status(:see_other) end end @@ -378,6 +379,7 @@ RSpec.describe Admin::Import::Jira::InstancesController do it "redirects to index" do delete :destroy, params: { id: jira.id } expect(response).to redirect_to(action: :index) + expect(response).to have_http_status(:see_other) end end diff --git a/spec/controllers/forums_controller_spec.rb b/spec/controllers/forums_controller_spec.rb index 4b54e9b2014..f4ab75d11c2 100644 --- a/spec/controllers/forums_controller_spec.rb +++ b/spec/controllers/forums_controller_spec.rb @@ -200,6 +200,7 @@ RSpec.describe ForumsController do end.to change(Forum, :count).by(-1) expect(response).to redirect_to project_forums_path(project) + expect(response).to have_http_status(:see_other) expect(flash[:notice]).to eq(I18n.t(:notice_successful_delete)) end end diff --git a/spec/controllers/roles_controller_spec.rb b/spec/controllers/roles_controller_spec.rb index 53c573d183f..16178361276 100644 --- a/spec/controllers/roles_controller_spec.rb +++ b/spec/controllers/roles_controller_spec.rb @@ -352,6 +352,7 @@ RSpec.describe RolesController do expect(enqueued_jobs.count).to eq(1) expect(enqueued_jobs[0][:job]).to eq(Storages::ManageStorageIntegrationsJob) expect(response).to redirect_to roles_path + expect(response).to have_http_status(:see_other) expect(Role.count).to eq(0) end end @@ -368,6 +369,7 @@ RSpec.describe RolesController do expect(enqueued_jobs.count).to eq(0) expect(Role.count).to eq(1) expect(response).to redirect_to roles_path + expect(response).to have_http_status(:see_other) expect(flash[:error]).to eq I18n.t(:error_can_not_remove_role) end end diff --git a/spec/requests/admin/settings/work_package_priorities_spec.rb b/spec/requests/admin/settings/work_package_priorities_spec.rb index 0c02e87e344..7b7ea93ae09 100644 --- a/spec/requests/admin/settings/work_package_priorities_spec.rb +++ b/spec/requests/admin/settings/work_package_priorities_spec.rb @@ -58,6 +58,7 @@ RSpec.describe "Work package priorities", it "redirects to the priorities index" do delete(admin_settings_work_package_priority_path(priority), params:) expect(response).to redirect_to admin_settings_work_package_priorities_path + expect(response).to have_http_status(:see_other) expect { priority.reload }.to raise_error(ActiveRecord::RecordNotFound) end end