diff --git a/changelogs/unreleased/dblessing-profile-scoped-routes.yml b/changelogs/unreleased/dblessing-profile-scoped-routes.yml new file mode 100644 index 0000000000000000000000000000000000000000..41c60ae2d517606e9f007cc3d3758dbdbc4faccd --- /dev/null +++ b/changelogs/unreleased/dblessing-profile-scoped-routes.yml @@ -0,0 +1,5 @@ +--- +title: Move existing `/profile` paths to scoped `/-/profile` +merge_request: 44644 +author: +type: changed diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 3eda53318e3461430b5e6f6a86a9e87e5d753146..71aa6ec64f6d101e844a9a565bd0fcb5aabd891b 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -1,73 +1,77 @@ # frozen_string_literal: true # for secondary email confirmations - uses the same confirmation controller as :users -devise_for :emails, path: 'profile/emails', controllers: { confirmations: :confirmations } +devise_for :emails, path: '/-/profile/emails', controllers: { confirmations: :confirmations } -resource :profile, only: [:show, :update] do - member do - get :audit_log - get :applications, to: 'oauth/applications#index' +scope '-' do + resource :profile, only: [:show, :update] do + member do + get :audit_log + get :applications, to: 'oauth/applications#index' - put :reset_incoming_email_token - put :reset_feed_token - put :reset_static_object_token - put :update_username - end + put :reset_incoming_email_token + put :reset_feed_token + put :reset_static_object_token + put :update_username + end - scope module: :profiles do - resource :account, only: [:show] do - member do - delete :unlink + scope module: :profiles do + resource :account, only: [:show] do + member do + delete :unlink + end end - end - resource :notifications, only: [:show, :update] do - resources :groups, only: :update, constraints: { id: Gitlab::PathRegex.full_namespace_route_regex } - end + resource :notifications, only: [:show, :update] do + resources :groups, only: :update, constraints: { id: Gitlab::PathRegex.full_namespace_route_regex } + end - resource :password, only: [:new, :create, :edit, :update] do - member do - put :reset + resource :password, only: [:new, :create, :edit, :update] do + member do + put :reset + end end - end - resource :preferences, only: [:show, :update] - resources :keys, only: [:index, :show, :create, :destroy] - resources :gpg_keys, only: [:index, :create, :destroy] do - member do - put :revoke + resource :preferences, only: [:show, :update] + resources :keys, only: [:index, :show, :create, :destroy] + resources :gpg_keys, only: [:index, :create, :destroy] do + member do + put :revoke + end end - end - resources :active_sessions, only: [:index, :destroy] - resources :emails, only: [:index, :create, :destroy] do - member do - put :resend_confirmation_instructions + resources :active_sessions, only: [:index, :destroy] + resources :emails, only: [:index, :create, :destroy] do + member do + put :resend_confirmation_instructions + end end - end - resources :chat_names, only: [:index, :new, :create, :destroy] do - collection do - delete :deny + resources :chat_names, only: [:index, :new, :create, :destroy] do + collection do + delete :deny + end end - end - resource :avatar, only: [:destroy] + resource :avatar, only: [:destroy] - resources :personal_access_tokens, only: [:index, :create] do - member do - put :revoke + resources :personal_access_tokens, only: [:index, :create] do + member do + put :revoke + end end - end - resource :two_factor_auth, only: [:show, :create, :destroy] do - member do - post :create_u2f - post :codes - patch :skip - post :create_webauthn + resource :two_factor_auth, only: [:show, :create, :destroy] do + member do + post :create_u2f + post :codes + patch :skip + post :create_webauthn + end end - end - resources :u2f_registrations, only: [:destroy] - resources :webauthn_registrations, only: [:destroy] + resources :u2f_registrations, only: [:destroy] + resources :webauthn_registrations, only: [:destroy] + end end end + +Gitlab::Routing.redirect_legacy_paths(self, :profile) diff --git a/ee/config/routes/profile.rb b/ee/config/routes/profile.rb index e93f7b57e3d149909616ea8acaf1ccdafe67aede..8ffff831754990dd6e697131208da5d7ee73f325 100644 --- a/ee/config/routes/profile.rb +++ b/ee/config/routes/profile.rb @@ -1,14 +1,16 @@ # frozen_string_literal: true -resource :profile, only: [] do - scope module: :profiles do - resource :slack, only: [:edit] do - member do - get :slack_link +scope '-' do + resource :profile, only: [] do + scope module: :profiles do + resource :slack, only: [:edit] do + member do + get :slack_link + end end - end - resources :usage_quotas, only: [:index] - resources :billings, only: [:index] + resources :usage_quotas, only: [:index] + resources :billings, only: [:index] + end end end diff --git a/ee/spec/features/security/dashboard_access_spec.rb b/ee/spec/features/security/dashboard_access_spec.rb index 90e06a5d2b14f5bb051f4a3aefb07bcaa5d6fee4..29270ee54ced5c601f78b47662681eb01004007a 100644 --- a/ee/spec/features/security/dashboard_access_spec.rb +++ b/ee/spec/features/security/dashboard_access_spec.rb @@ -43,7 +43,7 @@ it { expect(new_group_path).to be_allowed_for :auditor } end - describe "GET /profile/groups" do + describe "GET /-/profile/groups" do subject { dashboard_groups_path } it { is_expected.to be_allowed_for :auditor } diff --git a/ee/spec/features/security/profile_access_spec.rb b/ee/spec/features/security/profile_access_spec.rb index 7e85b77618b57a5facb8b2564d1affe547f355f8..3793fb0f827957db094b7838397956203fbff209 100644 --- a/ee/spec/features/security/profile_access_spec.rb +++ b/ee/spec/features/security/profile_access_spec.rb @@ -5,37 +5,37 @@ RSpec.describe "Profile access" do include AccessMatchers - describe "GET /profile/keys" do + describe "GET /-/profile/keys" do subject { profile_keys_path } it { is_expected.to be_allowed_for :auditor } end - describe "GET /profile" do + describe "GET /-/profile" do subject { profile_path } it { is_expected.to be_allowed_for :auditor } end - describe "GET /profile/account" do + describe "GET /-/profile/account" do subject { profile_account_path } it { is_expected.to be_allowed_for :auditor } end - describe "GET /profile/preferences" do + describe "GET /-/profile/preferences" do subject { profile_preferences_path } it { is_expected.to be_allowed_for :auditor } end - describe "GET /profile/audit_log" do + describe "GET /-/profile/audit_log" do subject { audit_log_profile_path } it { is_expected.to be_allowed_for :auditor } end - describe "GET /profile/notifications" do + describe "GET /-/profile/notifications" do subject { profile_notifications_path } it { is_expected.to be_allowed_for :auditor } diff --git a/ee/spec/serializers/dashboard_operations_project_entity_spec.rb b/ee/spec/serializers/dashboard_operations_project_entity_spec.rb index 9959661ce11e92b259eea737a8cb68399b78e9d1..cd182a32a905affacaf7b286a056cc8cd6d3f73d 100644 --- a/ee/spec/serializers/dashboard_operations_project_entity_spec.rb +++ b/ee/spec/serializers/dashboard_operations_project_entity_spec.rb @@ -99,7 +99,7 @@ let(:user) { build(:user, :admin) } it 'shows the profile upgrade path' do - expect(subject[:upgrade_path]).to eq '/profile/billings' + expect(subject[:upgrade_path]).to eq '/-/profile/billings' end end diff --git a/spec/features/profiles/account_spec.rb b/spec/features/profiles/account_spec.rb index e8caa2159a45f0a4d55827db55ff49798ce9ccd1..13ec12c50ead533de14fb45ffb078c9b7fa64d90 100644 --- a/spec/features/profiles/account_spec.rb +++ b/spec/features/profiles/account_spec.rb @@ -33,7 +33,7 @@ end it 'allows the user to disconnect when there is an existing identity' do - expect(page).to have_link('Disconnect Twitter', href: '/profile/account/unlink?provider=twitter') + expect(page).to have_link('Disconnect Twitter', href: '/-/profile/account/unlink?provider=twitter') end it 'shows active for a provider that is not allowed to unlink' do diff --git a/spec/features/security/dashboard_access_spec.rb b/spec/features/security/dashboard_access_spec.rb index 5ac4a5c1840317f6258f6be9025a363278afbacc..0d442919ca507e0d17e3df27be1d18aff7d45cf4 100644 --- a/spec/features/security/dashboard_access_spec.rb +++ b/spec/features/security/dashboard_access_spec.rb @@ -57,7 +57,7 @@ it { expect(new_group_path).to be_denied_for :visitor } end - describe "GET /profile/groups" do + describe "GET /-/profile/groups" do subject { dashboard_groups_path } it { is_expected.to be_allowed_for :admin } diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 3aa8278866c9967d5d16e8a904472992fa3910c1..301efd2d99bcf2c10142e16ae470a0b7138c71b3 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -5,7 +5,7 @@ RSpec.describe "Profile access" do include AccessMatchers - describe "GET /profile/keys" do + describe "GET /-/profile/keys" do subject { profile_keys_path } it { is_expected.to be_allowed_for :admin } @@ -13,7 +13,7 @@ it { is_expected.to be_denied_for :visitor } end - describe "GET /profile" do + describe "GET /-/profile" do subject { profile_path } it { is_expected.to be_allowed_for :admin } @@ -21,7 +21,7 @@ it { is_expected.to be_denied_for :visitor } end - describe "GET /profile/account" do + describe "GET /-/profile/account" do subject { profile_account_path } it { is_expected.to be_allowed_for :admin } @@ -29,7 +29,7 @@ it { is_expected.to be_denied_for :visitor } end - describe "GET /profile/preferences" do + describe "GET /-/profile/preferences" do subject { profile_preferences_path } it { is_expected.to be_allowed_for :admin } @@ -37,7 +37,7 @@ it { is_expected.to be_denied_for :visitor } end - describe "GET /profile/audit_log" do + describe "GET /-/profile/audit_log" do subject { audit_log_profile_path } it { is_expected.to be_allowed_for :admin } @@ -45,7 +45,7 @@ it { is_expected.to be_denied_for :visitor } end - describe "GET /profile/notifications" do + describe "GET /-/profile/notifications" do subject { profile_notifications_path } it { is_expected.to be_allowed_for :admin } diff --git a/spec/requests/profiles/notifications_controller_spec.rb b/spec/requests/profiles/notifications_controller_spec.rb index 87669b3594cbf0575d080dc7f753806c830db717..d7dfb1c675d159af90879f2079d8e1a4036ca3cb 100644 --- a/spec/requests/profiles/notifications_controller_spec.rb +++ b/spec/requests/profiles/notifications_controller_spec.rb @@ -24,7 +24,7 @@ def get_profile_notifications get profile_notifications_path end - describe 'GET /profile/notifications' do + describe 'GET /-/profile/notifications' do it 'does not have an N+1 due to an additional groups (with no parent group)' do get_profile_notifications diff --git a/spec/routing/notifications_routing_spec.rb b/spec/routing/notifications_routing_spec.rb index 007e8ff4816b5b74b02938c6ac3d1b210f510c1f..d66aa7f219f71b0ed79d08ddec00dadd3198b5a8 100644 --- a/spec/routing/notifications_routing_spec.rb +++ b/spec/routing/notifications_routing_spec.rb @@ -4,15 +4,15 @@ RSpec.describe "notifications routing" do it "routes to #show" do - expect(get("/profile/notifications")).to route_to("profiles/notifications#show") + expect(get("/-/profile/notifications")).to route_to("profiles/notifications#show") end it "routes to #update" do - expect(put("/profile/notifications")).to route_to("profiles/notifications#update") + expect(put("/-/profile/notifications")).to route_to("profiles/notifications#update") end it 'routes to group #update' do - expect(put("/profile/notifications/groups/gitlab-org")).to route_to("profiles/groups#update", id: 'gitlab-org') - expect(put("/profile/notifications/groups/gitlab.org")).to route_to("profiles/groups#update", id: 'gitlab.org') + expect(put("/-/profile/notifications/groups/gitlab-org")).to route_to("profiles/groups#update", id: 'gitlab-org') + expect(put("/-/profile/notifications/groups/gitlab.org")).to route_to("profiles/groups#update", id: 'gitlab.org') end end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 6150a8b26ccdd9eec542fab36849ce5c55a67fd3..cb437820ef0f3c8117ed50545db67daca211d708 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -126,42 +126,46 @@ end end -# profile_account GET /profile/account(.:format) profile#account -# profile_history GET /profile/history(.:format) profile#history -# profile_password PUT /profile/password(.:format) profile#password_update -# profile_token GET /profile/token(.:format) profile#token -# profile GET /profile(.:format) profile#show -# profile_update PUT /profile/update(.:format) profile#update +# profile_account GET /-/profile/account(.:format) profile#account +# profile_history GET /-/profile/history(.:format) profile#history +# profile_password PUT /-/profile/password(.:format) profile#password_update +# profile_token GET /-/profile/token(.:format) profile#token +# profile GET /-/profile(.:format) profile#show +# profile_update PUT /-/profile/update(.:format) profile#update RSpec.describe ProfilesController, "routing" do it "to #account" do - expect(get("/profile/account")).to route_to('profiles/accounts#show') + expect(get("/-/profile/account")).to route_to('profiles/accounts#show') end it "to #audit_log" do - expect(get("/profile/audit_log")).to route_to('profiles#audit_log') + expect(get("/-/profile/audit_log")).to route_to('profiles#audit_log') end it "to #reset_feed_token" do - expect(put("/profile/reset_feed_token")).to route_to('profiles#reset_feed_token') + expect(put("/-/profile/reset_feed_token")).to route_to('profiles#reset_feed_token') end it "to #show" do - expect(get("/profile")).to route_to('profiles#show') + expect(get("/-/profile")).to route_to('profiles#show') end + + it_behaves_like 'redirecting a legacy path to scoped', '/profile/account', '/profile/audit_log', '/profile' end -# profile_preferences GET /profile/preferences(.:format) profiles/preferences#show -# PATCH /profile/preferences(.:format) profiles/preferences#update -# PUT /profile/preferences(.:format) profiles/preferences#update +# profile_preferences GET /-/profile/preferences(.:format) profiles/preferences#show +# PATCH /-/profile/preferences(.:format) profiles/preferences#update +# PUT /-/profile/preferences(.:format) profiles/preferences#update RSpec.describe Profiles::PreferencesController, 'routing' do it 'to #show' do - expect(get('/profile/preferences')).to route_to('profiles/preferences#show') + expect(get('/-/profile/preferences')).to route_to('profiles/preferences#show') end it 'to #update' do - expect(put('/profile/preferences')).to route_to('profiles/preferences#update') - expect(patch('/profile/preferences')).to route_to('profiles/preferences#update') + expect(put('/-/profile/preferences')).to route_to('profiles/preferences#update') + expect(patch('/-/profile/preferences')).to route_to('profiles/preferences#update') end + + it_behaves_like 'redirecting a legacy path to scoped', '/profile/preferences' end # keys GET /keys(.:format) keys#index @@ -172,19 +176,19 @@ # DELETE /keys/:id(.:format) keys#destroy RSpec.describe Profiles::KeysController, "routing" do it "to #index" do - expect(get("/profile/keys")).to route_to('profiles/keys#index') + expect(get("/-/profile/keys")).to route_to('profiles/keys#index') end it "to #create" do - expect(post("/profile/keys")).to route_to('profiles/keys#create') + expect(post("/-/profile/keys")).to route_to('profiles/keys#create') end it "to #show" do - expect(get("/profile/keys/1")).to route_to('profiles/keys#show', id: '1') + expect(get("/-/profile/keys/1")).to route_to('profiles/keys#show', id: '1') end it "to #destroy" do - expect(delete("/profile/keys/1")).to route_to('profiles/keys#destroy', id: '1') + expect(delete("/-/profile/keys/1")).to route_to('profiles/keys#destroy', id: '1') end it "to #get_keys" do @@ -192,6 +196,8 @@ expect(get("/foo.keys")).to route_to('profiles/keys#get_keys', username: 'foo') end + + it_behaves_like 'redirecting a legacy path to scoped', '/profile/keys' end # emails GET /emails(.:format) emails#index @@ -199,22 +205,24 @@ # DELETE /keys/:id(.:format) keys#destroy RSpec.describe Profiles::EmailsController, "routing" do it "to #index" do - expect(get("/profile/emails")).to route_to('profiles/emails#index') + expect(get("/-/profile/emails")).to route_to('profiles/emails#index') end it "to #create" do - expect(post("/profile/emails")).to route_to('profiles/emails#create') + expect(post("/-/profile/emails")).to route_to('profiles/emails#create') end it "to #destroy" do - expect(delete("/profile/emails/1")).to route_to('profiles/emails#destroy', id: '1') + expect(delete("/-/profile/emails/1")).to route_to('profiles/emails#destroy', id: '1') end + + it_behaves_like 'redirecting a legacy path to scoped', '/profile/emails' end -# profile_avatar DELETE /profile/avatar(.:format) profiles/avatars#destroy +# profile_avatar DELETE /-/profile/avatar(.:format) profiles/avatars#destroy RSpec.describe Profiles::AvatarsController, "routing" do it "to #destroy" do - expect(delete("/profile/avatar")).to route_to('profiles/avatars#destroy') + expect(delete("/-/profile/avatar")).to route_to('profiles/avatars#destroy') end end diff --git a/spec/support/shared_examples/routing/legacy_path_redirect_shared_examples.rb b/spec/support/shared_examples/routing/legacy_path_redirect_shared_examples.rb index 808336db7b166978646bffe7aed20f041d8eb8c6..c07c42a32f1c914fa568f12d75d4a5c610dd1998 100644 --- a/spec/support/shared_examples/routing/legacy_path_redirect_shared_examples.rb +++ b/spec/support/shared_examples/routing/legacy_path_redirect_shared_examples.rb @@ -7,3 +7,15 @@ expect(get(source)).to redirect_to(target) end end + +RSpec.shared_examples 'redirecting a legacy path to scoped' do |*paths| + include RSpec::Rails::RequestExampleGroup + + paths.each do |path| + target = "/-#{path}" + + it "redirects #{path} to #{target}" do + expect(get(path)).to redirect_to(target) + end + end +end