Merge branch 'dz-root-url' into 'master'
Move user#show and group#show routing under root url ## What does this MR do? Changes routing so user#show and group#show routing is under root url ## Are there points in the code the reviewer needs to double check? no ## Why was this MR needed? For nice and consistent routing for user/group show pages: | before | after| |---|---| | `/u/dzaporozhets` | `/dzaporozhets`| | `/dzaporozhets/gitlab-ce` |`/dzaporozhets/gitlab-ce`| | `/u/dzaporozhets/snippets` |`/u/dzaporozhets/snippets`| | `/groups/gitlab-org`| `/gitlab-org`| | `/gitlab-org/gitlab-ce`| `/gitlab-org/gitlab-ce`| | `/groups/gitlab-org/issues `| `/groups/gitlab-org/issues `| So show page for user and group shown as `/:name` which is much nicer than before. At same time all nested pages like group issues stays under REST `/groups/:name/issues` so we don't need to introduce extra name restrictions ( ex. case when group has project with name "issues") ## Screenshots (if relevant) ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - [ ] ~~API support added~~ - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/3620 See merge request !6686
This commit is contained in:
commit
49d6ca6267
17 changed files with 127 additions and 27 deletions
|
@ -85,6 +85,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.
|
||||||
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.
|
||||||
.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 @@
|
||||||
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 @@
|
||||||
":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 @@
|
||||||
|
|
||||||
%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
|
||||||
|
|
|
@ -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
|
resources :groups, constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ } do
|
||||||
member do
|
member do
|
||||||
get :issues
|
get :issues
|
||||||
|
|
|
@ -1,15 +1,7 @@
|
||||||
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
|
||||||
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
|
||||||
|
|
7
lib/constraints/group_url_constrainer.rb
Normal file
7
lib/constraints/group_url_constrainer.rb
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
require 'constraints/namespace_url_constrainer'
|
||||||
|
|
||||||
|
class GroupUrlConstrainer < NamespaceUrlConstrainer
|
||||||
|
def find_resource(id)
|
||||||
|
Group.find_by_path(id)
|
||||||
|
end
|
||||||
|
end
|
13
lib/constraints/namespace_url_constrainer.rb
Normal file
13
lib/constraints/namespace_url_constrainer.rb
Normal file
|
@ -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
|
7
lib/constraints/user_url_constrainer.rb
Normal file
7
lib/constraints/user_url_constrainer.rb
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
require 'constraints/namespace_url_constrainer'
|
||||||
|
|
||||||
|
class UserUrlConstrainer < NamespaceUrlConstrainer
|
||||||
|
def find_resource(id)
|
||||||
|
User.find_by('lower(username) = ?', id.downcase)
|
||||||
|
end
|
||||||
|
end
|
|
@ -40,6 +40,17 @@ 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(current_path).to eq user_path(user)
|
||||||
|
expect(page).to have_text(user.name)
|
||||||
|
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
|
||||||
|
|
|
@ -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
|
||||||
|
|
10
spec/lib/constraints/group_url_constrainer_spec.rb
Normal file
10
spec/lib/constraints/group_url_constrainer_spec.rb
Normal file
|
@ -0,0 +1,10 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe GroupUrlConstrainer, lib: true do
|
||||||
|
let!(:username) { create(:group, path: 'gitlab-org') }
|
||||||
|
|
||||||
|
describe '#find_resource' do
|
||||||
|
it { expect(!!subject.find_resource('gitlab-org')).to be_truthy }
|
||||||
|
it { expect(!!subject.find_resource('gitlab-com')).to be_falsey }
|
||||||
|
end
|
||||||
|
end
|
25
spec/lib/constraints/namespace_url_constrainer_spec.rb
Normal file
25
spec/lib/constraints/namespace_url_constrainer_spec.rb
Normal file
|
@ -0,0 +1,25 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe NamespaceUrlConstrainer, lib: true do
|
||||||
|
let!(:group) { create(:group, path: 'gitlab') }
|
||||||
|
|
||||||
|
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
|
10
spec/lib/constraints/user_url_constrainer_spec.rb
Normal file
10
spec/lib/constraints/user_url_constrainer_spec.rb
Normal file
|
@ -0,0 +1,10 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe UserUrlConstrainer, lib: true do
|
||||||
|
let!(:username) { create(:user, username: 'dz') }
|
||||||
|
|
||||||
|
describe '#find_resource' do
|
||||||
|
it { expect(!!subject.find_resource('dz')).to be_truthy }
|
||||||
|
it { expect(!!subject.find_resource('john')).to be_falsey }
|
||||||
|
end
|
||||||
|
end
|
|
@ -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).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
|
||||||
|
|
||||||
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
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue