diff options
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/user_tabs.js.es6 | 11 | ||||
-rw-r--r-- | app/assets/javascripts/users_select.js | 4 | ||||
-rw-r--r-- | app/views/projects/boards/components/_card.html.haml | 2 | ||||
-rw-r--r-- | app/views/users/show.html.haml | 2 | ||||
-rw-r--r-- | config/routes/group.rb | 8 | ||||
-rw-r--r-- | config/routes/user.rb | 35 | ||||
-rw-r--r-- | lib/constraints/group_url_constrainer.rb | 7 | ||||
-rw-r--r-- | lib/constraints/namespace_url_constrainer.rb | 13 | ||||
-rw-r--r-- | lib/constraints/user_url_constrainer.rb | 7 | ||||
-rw-r--r-- | spec/features/users_spec.rb | 22 | ||||
-rw-r--r-- | spec/helpers/projects_helper_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/constraints/namespace_url_constrainer_spec.rb | 26 | ||||
-rw-r--r-- | spec/routing/routing_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/system_note_service_spec.rb | 2 |
15 files changed, 119 insertions, 27 deletions
diff --git a/CHANGELOG b/CHANGELOG index 30c8e801ea1..43713d9c9e2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -82,6 +82,7 @@ v 8.12.4 - Fix failed project deletion when feature visibility set to private. !6688 - Prevent claiming associated model IDs via import. - Set GitLab project exported file permissions to owner only + - Change user & group landing page routing from /u/:username to /:username v 8.12.3 - Update Gitlab Shell to support low IO priority for storage moves diff --git a/app/assets/javascripts/user_tabs.js.es6 b/app/assets/javascripts/user_tabs.js.es6 index 63bce0a6f6f..dfdfa1e7f75 100644 --- a/app/assets/javascripts/user_tabs.js.es6 +++ b/app/assets/javascripts/user_tabs.js.es6 @@ -89,7 +89,7 @@ content on the Users#show page. const action = $target.data('action'); const source = $target.attr('href'); this.setTab(source, action); - return this.setCurrentAction(action); + return this.setCurrentAction(source, action); } activateTab(action) { @@ -142,14 +142,9 @@ content on the Users#show page. .toggle(status); } - setCurrentAction(action) { - const regExp = new RegExp(`\/(${this.actions.join('|')})(\.html)?\/?$`); - let new_state = this._location.pathname; + setCurrentAction(source, action) { + let new_state = source new_state = new_state.replace(/\/+$/, ''); - new_state = new_state.replace(regExp, ''); - if (action !== this.defaultAction) { - new_state += `/${action}`; - } new_state += this._location.search + this._location.hash; history.replaceState({ turbolinks: true, diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 05056a73aaf..bcabda3ceb2 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -71,8 +71,8 @@ return $collapsedSidebar.html(collapsedAssigneeTemplate(user)); }); }; - collapsedAssigneeTemplate = _.template('<% if( avatar ) { %> <a class="author_link" href="/u/<%- username %>"> <img width="24" class="avatar avatar-inline s24" alt="" src="<%- avatar %>"> </a> <% } else { %> <i class="fa fa-user"></i> <% } %>'); - assigneeTemplate = _.template('<% if (username) { %> <a class="author_link bold" href="/u/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> No assignee - <a href="#" class="js-assign-yourself"> assign yourself </a> </span> <% } %>'); + collapsedAssigneeTemplate = _.template('<% if( avatar ) { %> <a class="author_link" href="/<%- username %>"> <img width="24" class="avatar avatar-inline s24" alt="" src="<%- avatar %>"> </a> <% } else { %> <i class="fa fa-user"></i> <% } %>'); + assigneeTemplate = _.template('<% if (username) { %> <a class="author_link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> No assignee - <a href="#" class="js-assign-yourself"> assign yourself </a> </span> <% } %>'); return $dropdown.glDropdown({ showMenuAbove: showMenuAbove, data: function(term, callback) { diff --git a/app/views/projects/boards/components/_card.html.haml b/app/views/projects/boards/components/_card.html.haml index f15c87c8185..d8f16022407 100644 --- a/app/views/projects/boards/components/_card.html.haml +++ b/app/views/projects/boards/components/_card.html.haml @@ -26,7 +26,7 @@ ":title" => "label.description", data: { container: 'body' } } {{ label.title }} - %a.has-tooltip{ ":href" => "'/u/' + issue.assignee.username", + %a.has-tooltip{ ":href" => "'/' + issue.assignee.username", ":title" => "'Assigned to ' + issue.assignee.name", "v-if" => "issue.assignee", data: { container: 'body' } } diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 60fc0c0daf6..19759a33add 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -82,7 +82,7 @@ %ul.nav-links.center.user-profile-nav %li.js-activity-tab - = link_to user_calendar_activities_path, data: {target: 'div#activity', action: 'activity', toggle: 'tab'} do + = link_to user_path, data: {target: 'div#activity', action: 'activity', toggle: 'tab'} do Activity %li.js-groups-tab = link_to user_groups_path, data: {target: 'div#groups', action: 'groups', toggle: 'tab'} do diff --git a/config/routes/group.rb b/config/routes/group.rb index 5b3e25d5e3d..47a8a0a53d4 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -1,3 +1,11 @@ +require 'constraints/group_url_constrainer' + +constraints(GroupUrlConstrainer.new) do + scope(path: ':id', as: :group, controller: :groups) do + get '/', action: :show + end +end + resources :groups, constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ } do member do get :issues diff --git a/config/routes/user.rb b/config/routes/user.rb index bbb30cedd4d..c287039ba26 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -1,15 +1,7 @@ -scope(path: 'u/:username', - as: :user, - constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }, - controller: :users) do - get :calendar - get :calendar_activities - get :groups - get :projects - get :contributed, as: :contributed_projects - get :snippets - get '/', action: :show -end +require 'constraints/user_url_constrainer' + +get '/u/:username', to: redirect('/%{username}'), + constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ } devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, registrations: :registrations, @@ -21,3 +13,22 @@ devise_scope :user do get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error get '/users/almost_there' => 'confirmations#almost_there' end + +constraints(UserUrlConstrainer.new) do + scope(path: ':username', as: :user, controller: :users) do + get '/', action: :show + end +end + +scope(path: 'u/:username', + as: :user, + constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }, + controller: :users) do + get :calendar + get :calendar_activities + get :groups + get :projects + get :contributed, as: :contributed_projects + get :snippets + get '/', to: redirect('/%{username}') +end diff --git a/lib/constraints/group_url_constrainer.rb b/lib/constraints/group_url_constrainer.rb new file mode 100644 index 00000000000..ca39b1961ae --- /dev/null +++ b/lib/constraints/group_url_constrainer.rb @@ -0,0 +1,7 @@ +require 'constraints/namespace_url_constrainer' + +class GroupUrlConstrainer < NamespaceUrlConstrainer + def find_resource(id) + Group.find_by_path(id) + end +end diff --git a/lib/constraints/namespace_url_constrainer.rb b/lib/constraints/namespace_url_constrainer.rb new file mode 100644 index 00000000000..23920193743 --- /dev/null +++ b/lib/constraints/namespace_url_constrainer.rb @@ -0,0 +1,13 @@ +class NamespaceUrlConstrainer + def matches?(request) + id = request.path.sub(/\A\/+/, '').split('/').first.sub(/.atom\z/, '') + + if id =~ Gitlab::Regex.namespace_regex + find_resource(id) + end + end + + def find_resource(id) + Namespace.find_by_path(id) + end +end diff --git a/lib/constraints/user_url_constrainer.rb b/lib/constraints/user_url_constrainer.rb new file mode 100644 index 00000000000..ef38d47d5c6 --- /dev/null +++ b/lib/constraints/user_url_constrainer.rb @@ -0,0 +1,7 @@ +require 'constraints/namespace_url_constrainer' + +class UserUrlConstrainer < NamespaceUrlConstrainer + def find_resource(id) + User.find_by_username(id) + end +end diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index b5a94fe0383..f2c0aa7784a 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -40,6 +40,23 @@ feature 'Users', feature: true do expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}' end + describe 'redirect alias routes' do + before { user } + + scenario '/u/user1 redirects to user page' do + visit '/u/user1' + + expect_user_show_page + end + + + scenario '/users/user1 redirects to user page' do + visit '/users/user1' + + expect_user_show_page + end + end + def errors_on_page(page) page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n") end @@ -47,4 +64,9 @@ feature 'Users', feature: true do def number_of_errors_on_page(page) page.find('#error_explanation').find('ul').all('li').count end + + def expect_user_show_page + expect(current_path).to eq user_path(user) + expect(page).to have_text(user.name) + end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index bcd53440cb4..8113742923b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -72,7 +72,7 @@ describe ProjectsHelper do it 'returns an HTML link to the user' do link = helper.link_to_member(project, user) - expect(link).to match(%r{/u/#{user.username}}) + expect(link).to match(%r{/#{user.username}}) end end end diff --git a/spec/lib/constraints/namespace_url_constrainer_spec.rb b/spec/lib/constraints/namespace_url_constrainer_spec.rb new file mode 100644 index 00000000000..8940fd6b94e --- /dev/null +++ b/spec/lib/constraints/namespace_url_constrainer_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe NamespaceUrlConstrainer, lib: true do + let!(:group) { create(:group, path: 'gitlab') } + subject { NamespaceUrlConstrainer.new } + + describe '#matches?' do + context 'existing namespace' do + it { expect(subject.matches?(request '/gitlab')).to be_truthy } + it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy } + it { expect(subject.matches?(request '/gitlab/')).to be_truthy } + it { expect(subject.matches?(request '//gitlab/')).to be_truthy } + end + + context 'non-existing namespace' do + it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey } + it { expect(subject.matches?(request '/gitlab.ce')).to be_falsey } + it { expect(subject.matches?(request '/g/gitlab')).to be_falsey } + it { expect(subject.matches?(request '/.gitlab')).to be_falsey } + end + end + + def request(path) + OpenStruct.new(path: path) + end +end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 4bc3cddd9c2..3608aa70f65 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -9,7 +9,9 @@ require 'spec_helper' # user_calendar_activities GET /u/:username/calendar_activities(.:format) describe UsersController, "routing" do it "to #show" do - expect(get("/u/User")).to route_to('users#show', username: 'User') + allow(User).to receive(:find_by_username).and_return(true) + + expect(get("/User")).to route_to('users#show', username: 'User') end it "to #groups" do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index c22dd9ab77a..d1a47ea9b6f 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -561,7 +561,7 @@ describe SystemNoteService, services: true do describe "existing reference" do before do - message = %Q{[#{author.name}|http://localhost/u/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]:\\n'#{commit.title}'} + message = %Q{[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]:\\n'#{commit.title}'} WebMock.stub_request(:get, jira_api_comment_url).to_return(body: %Q({"comments":[{"body":"#{message}"}]})) end |