diff options
-rw-r--r-- | app/controllers/concerns/routable_actions.rb | 16 | ||||
-rw-r--r-- | app/controllers/groups/application_controller.rb | 6 | ||||
-rw-r--r-- | app/controllers/projects/application_controller.rb | 19 | ||||
-rw-r--r-- | app/models/user.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/17361-redirect-renamed-paths.yml | 4 | ||||
-rw-r--r-- | spec/controllers/groups_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/controllers/projects_controller_spec.rb | 1 | ||||
-rw-r--r-- | spec/controllers/users_controller_spec.rb | 4 |
8 files changed, 31 insertions, 25 deletions
diff --git a/app/controllers/concerns/routable_actions.rb b/app/controllers/concerns/routable_actions.rb index ba236fa5459..d4ab6782444 100644 --- a/app/controllers/concerns/routable_actions.rb +++ b/app/controllers/concerns/routable_actions.rb @@ -1,10 +1,10 @@ module RoutableActions extend ActiveSupport::Concern - def find_routable!(routable_klass, requested_full_path, extra_authorization_method: nil) + def find_routable!(routable_klass, requested_full_path, extra_authorization_proc: nil) routable = routable_klass.find_by_full_path(requested_full_path, follow_redirects: request.get?) - if authorized?(routable_klass, routable, extra_authorization_method) + if routable_authorized?(routable_klass, routable, extra_authorization_proc) ensure_canonical_path(routable, requested_full_path) routable else @@ -13,12 +13,12 @@ module RoutableActions end end - def authorized?(routable_klass, routable, extra_authorization_method) + def routable_authorized?(routable_klass, routable, extra_authorization_proc) action = :"read_#{routable_klass.to_s.underscore}" return false unless can?(current_user, action, routable) - if extra_authorization_method - send(extra_authorization_method, routable) + if extra_authorization_proc + extra_authorization_proc.call(routable) else true end @@ -27,9 +27,11 @@ module RoutableActions def ensure_canonical_path(routable, requested_path) return unless request.get? - canonical_path = routable.try(:full_path) || routable.namespace.full_path + canonical_path = routable.full_path if canonical_path != requested_path - flash[:notice] = 'This project has moved to this location. Please update your links and bookmarks.' + if canonical_path.casecmp(requested_path) != 0 + flash[:notice] = "Project '#{requested_path}' was moved to '#{canonical_path}'. Please update any links and bookmarks that may still have the old path." + end redirect_to request.original_url.sub(requested_path, canonical_path) end end diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 2157a56dea2..afffb813b44 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -9,11 +9,7 @@ class Groups::ApplicationController < ApplicationController private def group - @group ||= find_routable!(Group, requested_full_path) - end - - def requested_full_path - params[:group_id] || params[:id] + @group ||= find_routable!(Group, params[:group_id] || params[:id]) end def group_projects diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 2301e1cca77..25232fc9457 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -17,24 +17,17 @@ class Projects::ApplicationController < ApplicationController # to # localhost/group/project # - if params[:format] == 'git' - redirect_to request.original_url.gsub(/\.git\/?\Z/, '') - return - end + redirect_to url_for(params.merge(format: nil)) if params[:format] == 'git' end def project - @project ||= find_routable!(Project, requested_full_path, extra_authorization_method: :project_not_being_deleted?) - end - - def requested_full_path - namespace = params[:namespace_id] - id = params[:project_id] || params[:id] - "#{namespace}/#{id}" + @project ||= find_routable!(Project, + File.join(params[:namespace_id], params[:project_id] || params[:id]), + extra_authorization_proc: project_not_being_deleted?) end - def project_not_being_deleted?(project) - !project.pending_delete? + def project_not_being_deleted? + ->(project) { !project.pending_delete? } end def repository diff --git a/app/models/user.rb b/app/models/user.rb index 7da92d03427..dd2c8f1b6ef 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -360,6 +360,10 @@ class User < ActiveRecord::Base end end + def full_path + username + end + def self.internal_attributes [:ghost] end diff --git a/changelogs/unreleased/17361-redirect-renamed-paths.yml b/changelogs/unreleased/17361-redirect-renamed-paths.yml new file mode 100644 index 00000000000..7a33c9fb3ec --- /dev/null +++ b/changelogs/unreleased/17361-redirect-renamed-paths.yml @@ -0,0 +1,4 @@ +--- +title: Redirect old links after renaming a user/group/project. +merge_request: 10370 +author: diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index b5fd611747a..073b87a1cb4 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -55,6 +55,7 @@ describe GroupsController do get :issues, id: group.to_param.upcase expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).not_to set_flash[:notice] end end @@ -99,6 +100,7 @@ describe GroupsController do get :merge_requests, id: group.to_param.upcase expect(response).to redirect_to(merge_requests_group_path(group.to_param)) + expect(controller).not_to set_flash[:notice] end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 5e3e943c124..e46ef447df2 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -185,6 +185,7 @@ describe ProjectsController do expect(assigns(:project)).to eq(public_project) expect(response).to redirect_to("/#{public_project.full_path}") + expect(controller).not_to set_flash[:notice] end end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9b6b9358a40..74c5aa44ba9 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -71,6 +71,7 @@ describe UsersController do get :show, username: user.username.downcase expect(response).to redirect_to(user) + expect(controller).not_to set_flash[:notice] end end end @@ -149,6 +150,7 @@ describe UsersController do get :calendar, username: user.username.downcase expect(response).to redirect_to(user_calendar_path(user)) + expect(controller).not_to set_flash[:notice] end end end @@ -202,6 +204,7 @@ describe UsersController do get :calendar_activities, username: user.username.downcase expect(response).to redirect_to(user_calendar_activities_path(user)) + expect(controller).not_to set_flash[:notice] end end end @@ -255,6 +258,7 @@ describe UsersController do get :snippets, username: user.username.downcase expect(response).to redirect_to(user_snippets_path(user)) + expect(controller).not_to set_flash[:notice] end end end |