Commit 6b90ccb9 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Change user & group landing page routing from /u/:name & /groups/:name to /:name

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 7c8c8088
...@@ -82,6 +82,7 @@ v 8.12.4 ...@@ -82,6 +82,7 @@ v 8.12.4
- Fix failed project deletion when feature visibility set to private. !6688 - Fix failed project deletion when feature visibility set to private. !6688
- Prevent claiming associated model IDs via import. - Prevent claiming associated model IDs via import.
- Set GitLab project exported file permissions to owner only - Set GitLab project exported file permissions to owner only
- Change user & group landing page routing from /u/:username to /:username
v 8.12.3 v 8.12.3
- Update Gitlab Shell to support low IO priority for storage moves - Update Gitlab Shell to support low IO priority for storage moves
......
...@@ -89,7 +89,7 @@ content on the Users#show page. ...@@ -89,7 +89,7 @@ content on the Users#show page.
const action = $target.data('action'); const action = $target.data('action');
const source = $target.attr('href'); const source = $target.attr('href');
this.setTab(source, action); this.setTab(source, action);
return this.setCurrentAction(action); return this.setCurrentAction(source, action);
} }
activateTab(action) { activateTab(action) {
...@@ -142,14 +142,9 @@ content on the Users#show page. ...@@ -142,14 +142,9 @@ content on the Users#show page.
.toggle(status); .toggle(status);
} }
setCurrentAction(action) { setCurrentAction(source, action) {
const regExp = new RegExp(`\/(${this.actions.join('|')})(\.html)?\/?$`); let new_state = source
let new_state = this._location.pathname;
new_state = new_state.replace(/\/+$/, ''); 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; new_state += this._location.search + this._location.hash;
history.replaceState({ history.replaceState({
turbolinks: true, turbolinks: true,
......
...@@ -71,8 +71,8 @@ ...@@ -71,8 +71,8 @@
return $collapsedSidebar.html(collapsedAssigneeTemplate(user)); 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> <% } %>'); 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="/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> <% } %>'); 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({ return $dropdown.glDropdown({
showMenuAbove: showMenuAbove, showMenuAbove: showMenuAbove,
data: function(term, callback) { data: function(term, callback) {
......
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
":title" => "label.description", ":title" => "label.description",
data: { container: 'body' } } data: { container: 'body' } }
{{ label.title }} {{ label.title }}
%a.has-tooltip{ ":href" => "'/u/' + issue.assignee.username", %a.has-tooltip{ ":href" => "'/' + issue.assignee.username",
":title" => "'Assigned to ' + issue.assignee.name", ":title" => "'Assigned to ' + issue.assignee.name",
"v-if" => "issue.assignee", "v-if" => "issue.assignee",
data: { container: 'body' } } data: { container: 'body' } }
......
...@@ -82,7 +82,7 @@ ...@@ -82,7 +82,7 @@
%ul.nav-links.center.user-profile-nav %ul.nav-links.center.user-profile-nav
%li.js-activity-tab %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 Activity
%li.js-groups-tab %li.js-groups-tab
= link_to user_groups_path, data: {target: 'div#groups', action: 'groups', toggle: 'tab'} do = link_to user_groups_path, data: {target: 'div#groups', action: 'groups', toggle: 'tab'} do
......
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 resources :groups, constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ } do
member do member do
get :issues get :issues
......
scope(path: 'u/:username', require 'constraints/user_url_constrainer'
as: :user,
constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }, get '/u/:username', to: redirect('/%{username}'),
controller: :users) do constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }
get :calendar
get :calendar_activities
get :groups
get :projects
get :contributed, as: :contributed_projects
get :snippets
get '/', action: :show
end
devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks,
registrations: :registrations, registrations: :registrations,
...@@ -21,3 +13,22 @@ devise_scope :user do ...@@ -21,3 +13,22 @@ devise_scope :user do
get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error
get '/users/almost_there' => 'confirmations#almost_there' get '/users/almost_there' => 'confirmations#almost_there'
end 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
require 'constraints/namespace_url_constrainer'
class GroupUrlConstrainer < NamespaceUrlConstrainer
def find_resource(id)
Group.find_by_path(id)
end
end
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
require 'constraints/namespace_url_constrainer'
class UserUrlConstrainer < NamespaceUrlConstrainer
def find_resource(id)
User.find_by_username(id)
end
end
...@@ -40,6 +40,23 @@ feature 'Users', feature: true do ...@@ -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}' expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}'
end 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) def errors_on_page(page)
page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n") page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n")
end end
...@@ -47,4 +64,9 @@ feature 'Users', feature: true do ...@@ -47,4 +64,9 @@ feature 'Users', feature: true do
def number_of_errors_on_page(page) def number_of_errors_on_page(page)
page.find('#error_explanation').find('ul').all('li').count page.find('#error_explanation').find('ul').all('li').count
end end
def expect_user_show_page
expect(current_path).to eq user_path(user)
expect(page).to have_text(user.name)
end
end end
...@@ -72,7 +72,7 @@ describe ProjectsHelper do ...@@ -72,7 +72,7 @@ describe ProjectsHelper do
it 'returns an HTML link to the user' do it 'returns an HTML link to the user' do
link = helper.link_to_member(project, user) 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 end
end end
......
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
...@@ -9,7 +9,9 @@ require 'spec_helper' ...@@ -9,7 +9,9 @@ require 'spec_helper'
# user_calendar_activities GET /u/:username/calendar_activities(.:format) # user_calendar_activities GET /u/:username/calendar_activities(.:format)
describe UsersController, "routing" do describe UsersController, "routing" do
it "to #show" 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 end
it "to #groups" do it "to #groups" do
......
...@@ -561,7 +561,7 @@ describe SystemNoteService, services: true do ...@@ -561,7 +561,7 @@ describe SystemNoteService, services: true do
describe "existing reference" do describe "existing reference" do
before 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}"}]})) WebMock.stub_request(:get, jira_api_comment_url).to_return(body: %Q({"comments":[{"body":"#{message}"}]}))
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment