From c5280434399ee489eebda254b2d246252df68f2b Mon Sep 17 00:00:00 2001 From: Cristian Bica Date: Thu, 1 Oct 2015 17:05:20 +0300 Subject: [PATCH 01/28] Allow users to select the Files view as default project view --- app/assets/stylesheets/pages/projects.scss | 84 ++++++++++---------- app/controllers/projects_controller.rb | 5 +- app/helpers/preferences_helper.rb | 8 +- app/models/user.rb | 2 +- app/views/projects/_files.html.haml | 11 +++ app/views/projects/show.html.haml | 9 +-- app/views/projects/tree/_blob_item.html.haml | 2 +- app/views/projects/tree/_tree_item.html.haml | 2 +- spec/controllers/projects_controller_spec.rb | 25 ++++++ 9 files changed, 96 insertions(+), 52 deletions(-) create mode 100644 app/views/projects/_files.html.haml diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 31051785676..7396de88cff 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -1,6 +1,6 @@ .alert_holder { margin: -16px; - + .alert-link { font-weight: normal; } @@ -31,20 +31,20 @@ margin: -$gl-padding; padding: $gl-padding; padding: 44px 0 17px 0; - + .project-identicon-holder { margin-bottom: 16px; - + .avatar, .identicon { margin: 0 auto; float: none; } - + .identicon { @include border-radius(50%); } } - + .project-home-dropdown { margin: 11px 3px 0; } @@ -86,15 +86,15 @@ top: 17px; margin-bottom: 44px; } - + .project-repo-buttons { margin-top: 12px; margin-bottom: 0px; - + .btn { @include bnt-project; @include btn-info; - + .count { display: inline-block; } @@ -105,7 +105,7 @@ .split-one { display: inline-table; margin-right: 12px; - + a { margin: -1px !important; } @@ -129,10 +129,10 @@ &.git-protocols { padding: 0; border: none; - + .input-group-btn:last-child > .btn { @include border-radius-right(0); - + border-left: 1px solid #c6cacf; margin-left: -2px !important; } @@ -141,55 +141,55 @@ } .projects-search-form { - + .input-group .form-control { height: 42px; } } .input-group-btn { - .btn { + .btn { @include bnt-project; @include btn-middle; - + &:hover { outline: none; } - + &:focus { outline: none; } - + &:active { outline: none; } } - + .active { @include box-shadow(inset 0 0 4px rgba(0, 0, 0, 0.12)); - + border: 1px solid #c6cacf !important; background-color: #e4e7ed !important; } - + .btn-green { @include btn-green } - + } .split-repo-buttons { display: inline-table; margin: 0 12px 0 12px; - + .btn{ @include bnt-project; @include btn-info; } - + .dropdown-toggle { margin: -5px; - } + } } #notification-form { @@ -202,7 +202,7 @@ .open > .dropdown-new.btn { @include box-shadow(inset 0 0 4px rgba(0, 0, 0, 0.12)); - + border: 1px solid #c6cacf !important; background-color: #e4e7ed !important; text-transform: uppercase; @@ -214,21 +214,21 @@ .dropdown-menu { @include box-shadow(rgba(76, 86, 103, 0.247059) 0px 0px 1px 0px, rgba(31, 37, 50, 0.317647) 0px 2px 18px 0px); @include border-radius (0px); - + border: none; padding: 16px 0; font-size: 14px; font-weight: 100; - + li a { color: #5f697a; line-height: 30px; - + &:hover { - background-color: #3084bb !important; + background-color: #3084bb !important; } } - + .fa-fw { margin-right: 8px; } @@ -370,7 +370,7 @@ table.table.protected-branches-list tr.no-border { ul.nav-pills { display:inline-block; } - + .nav-pills li { display:inline; } @@ -378,12 +378,12 @@ table.table.protected-branches-list tr.no-border { .nav > li > a { @include btn-info; @include bnt-project; - + background-color: transparent; border: 1px solid #f7f8fa; margin-left: 12px; } - + li { display:inline; } @@ -418,27 +418,27 @@ pre.light-well { .git-empty { margin: 0 7px 0 7px; - + h5 { color: #5c5d5e; } - + .light-well { @include border-radius (2px); - + color: #5b6169; - font-size: 13px; - line-height: 1.6em; + font-size: 13px; + line-height: 1.6em; } } .prepend-top-20 { margin-top: 20px; - + .btn-remove { @include btn-middle; @include btn-remove; - + float: left !important; } } @@ -446,7 +446,7 @@ pre.light-well { /* * Projects list rendered on dashboard and user page */ - + .projects-list { @include basic-list; @@ -507,6 +507,10 @@ pre.light-well { } } +.project-show-files { + padding-top: 20px; +} + .inline-form { display: inline-block; } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 213c2a7173b..7158d4b49ac 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -89,7 +89,10 @@ class ProjectsController < ApplicationController if current_user @membership = @project.project_member_by_id(current_user.id) end - + @ref = "master" + @id = "master" + @commit = @project.repository.commit(@ref) + @tree = @project.repository.tree(@commit.id) render :show end else diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 1b1f4162df4..f888c4a829b 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -27,7 +27,8 @@ module PreferencesHelper def project_view_choices [ ['Readme (default)', :readme], - ['Activity view', :activity] + ['Activity view', :activity], + ['Files view', :files] ] end @@ -43,4 +44,9 @@ module PreferencesHelper !current_user || current_user.project_view == 'readme' end + + def current_user_default_project_view + (current_user && current_user.project_view) || + 'readme' + end end diff --git a/app/models/user.rb b/app/models/user.rb index 3879f3fd381..f7a1589f5d0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -176,7 +176,7 @@ class User < ActiveRecord::Base # User's Project preference # Note: When adding an option, it MUST go on the end of the array. - enum project_view: [:readme, :activity] + enum project_view: [:readme, :activity, :files] alias_attribute :private_token, :authentication_token diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml new file mode 100644 index 00000000000..2a99708eb43 --- /dev/null +++ b/app/views/projects/_files.html.haml @@ -0,0 +1,11 @@ += render 'projects/last_push' + +.tree-ref-holder + = render 'shared/ref_switcher', destination: 'tree', path: @path + +- if can? current_user, :download_code, @project + .tree-download-holder + = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'btn-group pull-right hidden-xs hidden-sm', split_button: true + +#tree-holder.tree-holder.clearfix + = render "projects/tree/tree", tree: @tree diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 6a5fc689803..54afb7de15d 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -64,13 +64,8 @@ Archived project! Repository is read-only %section - - if prefer_readme? - .project-show-readme - = render 'projects/readme' - - else - .project-show-activity - = render 'projects/activity' - + %div{class: "project-show-#{current_user_default_project_view}"} + = render current_user_default_project_view - if current_user - access = user_max_access_in_project(current_user, @project) diff --git a/app/views/projects/tree/_blob_item.html.haml b/app/views/projects/tree/_blob_item.html.haml index 02ecbade219..2ddc5d504fa 100644 --- a/app/views/projects/tree/_blob_item.html.haml +++ b/app/views/projects/tree/_blob_item.html.haml @@ -4,5 +4,5 @@ %span.str-truncated = link_to blob_item.name, namespace_project_blob_path(@project.namespace, @project, tree_join(@id || @commit.id, blob_item.name)) %td.tree_time_ago.cgray - = render 'spinner' + = render 'projects/tree/spinner' %td.hidden-xs.tree_commit diff --git a/app/views/projects/tree/_tree_item.html.haml b/app/views/projects/tree/_tree_item.html.haml index e87138bf980..cf65057e704 100644 --- a/app/views/projects/tree/_tree_item.html.haml +++ b/app/views/projects/tree/_tree_item.html.haml @@ -5,5 +5,5 @@ - path = flatten_tree(tree_item) = link_to path, namespace_project_tree_path(@project.namespace, @project, tree_join(@id || @commit.id, path)) %td.tree_time_ago.cgray - = render 'spinner' + = render 'projects/tree/spinner' %td.hidden-xs.tree_commit diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 29233e9fae6..1e018acf42a 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -21,6 +21,31 @@ describe ProjectsController do expect(response.body).to include("content='#{content}'") end end + + context "rendering default project view" do + render_views + + it "shold render the activity view", focus: true do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('activity') + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_activity') + end + + it "shold render the readme view", focus: true do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('readme') + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_readme') + end + + it "shold render the files view", focus: true do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('files') + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_files') + end + end end describe "POST #toggle_star" do From df99ddbba13db4a7699bf1d585675f921cf382ce Mon Sep 17 00:00:00 2001 From: Han Loong Liauw Date: Tue, 13 Oct 2015 21:24:44 +1100 Subject: [PATCH 02/28] Adds ability to remove the forked relationship This was previously possible through the API but can now be done through the project#edit settings screen if the current user is the owner of the project. Update changelog --- CHANGELOG | 2 + app/controllers/projects_controller.rb | 9 ++++- app/helpers/projects_helper.rb | 4 ++ app/models/ability.rb | 3 +- app/views/projects/edit.html.haml | 16 ++++++++ app/views/projects/remove_fork.js.haml | 2 + config/routes.rb | 1 + lib/api/projects.rb | 2 +- spec/controllers/projects_controller_spec.rb | 40 ++++++++++++++++++++ spec/features/projects_spec.rb | 29 ++++++++++++-- 10 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 app/views/projects/remove_fork.js.haml diff --git a/CHANGELOG b/CHANGELOG index a3d796bea66..3c730aef5e4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -45,6 +45,8 @@ v 8.1.0 (unreleased) - Fix position of hamburger in header for smaller screens (Han Loong Liauw) - Fix bug where Emojis in Markdown would truncate remaining text (Sakata Sinji) - Persist filters when sorting on admin user page (Jerry Lukins) + - Adds ability to remove the forked relationship from project settings + screen. #2578 (Han Loong Liauw) v 8.0.4 - Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 213c2a7173b..1fb83d0eb6d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -5,7 +5,7 @@ class ProjectsController < ApplicationController before_action :repository, except: [:new, :create] # Authorize - before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive] + before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive, :remove_fork] before_action :event_filter, only: [:show, :activity] layout :determine_layout @@ -64,6 +64,13 @@ class ProjectsController < ApplicationController end end + def remove_fork + if @project.forked? + @project.forked_project_link.destroy + flash[:notice] = 'Fork relationship has been removed.' + end + end + def activity respond_to do |format| format.html diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index a0220af4c30..b0a3a20fa0a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -70,6 +70,10 @@ module ProjectsHelper "You are going to transfer #{project.name_with_namespace} to another owner. Are you ABSOLUTELY sure?" end + def remove_fork_project_message(project) + "You are going to remove the fork relationship to the source project from #{@project.forked_from_project.namespace.try(:name)}. Are you ABSOLUTELY sure?" + end + def project_nav_tabs @nav_tabs ||= get_project_nav_tabs(@project, current_user) end diff --git a/app/models/ability.rb b/app/models/ability.rb index a020b24a550..11ada46610b 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -185,7 +185,8 @@ class Ability :change_visibility_level, :rename_project, :remove_project, - :archive_project + :archive_project, + :remove_fork_project ] end diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 1882a82fba5..ce77a9242fc 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -189,6 +189,21 @@ - else .nothing-here-block Only the project owner can transfer a project + - if @project.forked? && can?(current_user, :remove_fork_project, @project) + = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :put, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f| + .panel.panel-default.panel.panel-danger + .panel-heading Remove forked relationship + .panel-body + %p + This will remove the relationship to the source project from + = link_to project_path(@project.forked_from_project) do + = @project.forked_from_project.namespace.try(:name) + %br + %strong Once removed it cannot be reversed through this interface + = button_to 'Remove forked relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) } + - elsif @project.forked? + .nothing-here-block Only the project owner can remove the fork relationship + - if can?(current_user, :remove_project, @project) .panel.panel-default.panel.panel-danger .panel-heading Remove project @@ -203,6 +218,7 @@ - else .nothing-here-block Only project owner can remove a project + .save-project-loader.hide .center %h2 diff --git a/app/views/projects/remove_fork.js.haml b/app/views/projects/remove_fork.js.haml new file mode 100644 index 00000000000..17b9fecfeb1 --- /dev/null +++ b/app/views/projects/remove_fork.js.haml @@ -0,0 +1,2 @@ +:plain + location.href = "#{edit_namespace_project_path(@project.namespace, @project)}"; diff --git a/config/routes.rb b/config/routes.rb index 8e6fbf6340c..3ac4342b23a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -378,6 +378,7 @@ Gitlab::Application.routes.draw do [:new, :create, :index], path: "/") do member do put :transfer + put :remove_fork post :archive post :unarchive post :toggle_star diff --git a/lib/api/projects.rb b/lib/api/projects.rb index c2fb36b4143..e8a0e7f3ec9 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -247,7 +247,7 @@ module API # DELETE /projects/:id/fork delete ":id/fork" do authenticated_as_admin! - unless user_project.forked_project_link.nil? + if user_project.forked? user_project.forked_project_link.destroy end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 21beaf37fce..626b56cc789 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -62,4 +62,44 @@ describe ProjectsController do expect(user.starred?(public_project)).to be_falsey end end + + describe "PUT remove_fork" do + context 'when signed in' do + before do + sign_in(user) + end + + context 'with forked project' do + let(:project_fork) { create(:project, namespace: user.namespace) } + + it 'should remove fork from project' do + create(:forked_project_link, forked_to_project: project_fork) + put(:remove_fork, + namespace_id: project_fork.namespace.to_param, + id: project_fork.to_param, format: :js) + + expect(project_fork.forked?).to be_falsey + expect(flash[:notice]).to eq('Fork relationship has been removed.') + expect(response).to render_template(:remove_fork) + end + end + + it 'should do nothing if project was not forked' do + unforked_project = create(:project, namespace: user.namespace) + put(:remove_fork, + namespace_id: unforked_project.namespace.to_param, + id: unforked_project.to_param, format: :js) + + expect(flash[:notice]).to be_nil + expect(response).to render_template(:remove_fork) + end + end + + it "does nothing if user is not signed in" do + put(:remove_fork, + namespace_id: project.namespace.to_param, + id: project.to_param, format: :js) + expect(response.status).to eq(401) + end + end end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index aac93b17a38..df0dcb2bb21 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -34,6 +34,27 @@ feature 'Project', feature: true do end end + describe 'remove forked relationship', js: true do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + + before do + login_with user + create(:forked_project_link, forked_to_project: project) + visit edit_namespace_project_path(project.namespace, project) + end + + it 'should remove fork' do + expect(page).to have_content 'Remove forked relationship' + + remove_with_confirm('Remove forked relationship', project.path) + + expect(page).to have_content 'Fork relationship has been removed.' + expect(project.forked?).to be_falsey + expect(page).not_to have_content 'Remove forked relationship' + end + end + describe 'removal', js: true do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } @@ -45,13 +66,13 @@ feature 'Project', feature: true do end it 'should remove project' do - expect { remove_project }.to change {Project.count}.by(-1) + expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1) end end - def remove_project - click_button "Remove project" - fill_in 'confirm_name_input', with: project.path + def remove_with_confirm(button_text, confirm_with) + click_button button_text + fill_in 'confirm_name_input', with: confirm_with click_button 'Confirm' end end From 0bea5ced8bf4c9306f8f8e912313731a43d16f4c Mon Sep 17 00:00:00 2001 From: Han Loong Liauw Date: Wed, 14 Oct 2015 09:04:22 +1100 Subject: [PATCH 03/28] Made suggested content changes based on MR Review Changed the authentication method for removing fork through API Reflected changes to new auth method in API specs --- CHANGELOG | 2 +- app/controllers/projects_controller.rb | 6 ++- app/views/projects/edit.html.haml | 8 ++-- config/routes.rb | 2 +- lib/api/projects.rb | 2 +- spec/controllers/projects_controller_spec.rb | 26 ++++++---- spec/features/projects_spec.rb | 6 +-- spec/requests/api/projects_spec.rb | 50 +++++++++++++------- 8 files changed, 63 insertions(+), 39 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index cf9e5b43c4d..e45fd2ef5a1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,7 +46,7 @@ v 8.1.0 (unreleased) - Fix bug where Emojis in Markdown would truncate remaining text (Sakata Sinji) - Persist filters when sorting on admin user page (Jerry Lukins) - Adds ability to remove the forked relationship from project settings - screen. #2578 (Han Loong Liauw) + screen. (Han Loong Liauw) - Add spellcheck=false to certain input fields - Invalidate stored service password if the endpoint URL is changed diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1fb83d0eb6d..77b3af9a5d0 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -5,7 +5,7 @@ class ProjectsController < ApplicationController before_action :repository, except: [:new, :create] # Authorize - before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive, :remove_fork] + before_action :authorize_admin_project!, only: [:edit, :update] before_action :event_filter, only: [:show, :activity] layout :determine_layout @@ -56,6 +56,8 @@ class ProjectsController < ApplicationController end def transfer + return access_denied! unless can?(current_user, :change_namespace, @project) + namespace = Namespace.find_by(id: params[:new_namespace_id]) ::Projects::TransferService.new(project, current_user).execute(namespace) @@ -65,6 +67,8 @@ class ProjectsController < ApplicationController end def remove_fork + return access_denied! unless can?(current_user, :remove_fork_project, @project) + if @project.forked? @project.forked_project_link.destroy flash[:notice] = 'Fork relationship has been removed.' diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index ce77a9242fc..ec58d0924b0 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -190,17 +190,17 @@ .nothing-here-block Only the project owner can transfer a project - if @project.forked? && can?(current_user, :remove_fork_project, @project) - = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :put, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f| + = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :delete, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f| .panel.panel-default.panel.panel-danger - .panel-heading Remove forked relationship + .panel-heading Remove fork relationship .panel-body %p This will remove the relationship to the source project from = link_to project_path(@project.forked_from_project) do = @project.forked_from_project.namespace.try(:name) %br - %strong Once removed it cannot be reversed through this interface - = button_to 'Remove forked relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) } + %strong Once removed it cannot be reversed through this interface. + = button_to 'Remove fork relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) } - elsif @project.forked? .nothing-here-block Only the project owner can remove the fork relationship diff --git a/config/routes.rb b/config/routes.rb index 3ac4342b23a..64bdd189f53 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -378,7 +378,7 @@ Gitlab::Application.routes.draw do [:new, :create, :index], path: "/") do member do put :transfer - put :remove_fork + delete :remove_fork post :archive post :unarchive post :toggle_star diff --git a/lib/api/projects.rb b/lib/api/projects.rb index e8a0e7f3ec9..67ee66a2058 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -246,7 +246,7 @@ module API # Example Request: # DELETE /projects/:id/fork delete ":id/fork" do - authenticated_as_admin! + authorize! :remove_fork_project, user_project if user_project.forked? user_project.forked_project_link.destroy end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 626b56cc789..e963e913512 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -72,9 +72,12 @@ describe ProjectsController do context 'with forked project' do let(:project_fork) { create(:project, namespace: user.namespace) } - it 'should remove fork from project' do + before do create(:forked_project_link, forked_to_project: project_fork) - put(:remove_fork, + end + + it 'should remove fork from project' do + delete(:remove_fork, namespace_id: project_fork.namespace.to_param, id: project_fork.to_param, format: :js) @@ -84,19 +87,22 @@ describe ProjectsController do end end - it 'should do nothing if project was not forked' do - unforked_project = create(:project, namespace: user.namespace) - put(:remove_fork, - namespace_id: unforked_project.namespace.to_param, - id: unforked_project.to_param, format: :js) + context 'when project not forked' do + let(:unforked_project) { create(:project, namespace: user.namespace) } - expect(flash[:notice]).to be_nil - expect(response).to render_template(:remove_fork) + it 'should do nothing if project was not forked' do + delete(:remove_fork, + namespace_id: unforked_project.namespace.to_param, + id: unforked_project.to_param, format: :js) + + expect(flash[:notice]).to be_nil + expect(response).to render_template(:remove_fork) + end end end it "does nothing if user is not signed in" do - put(:remove_fork, + delete(:remove_fork, namespace_id: project.namespace.to_param, id: project.to_param, format: :js) expect(response.status).to eq(401) diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index df0dcb2bb21..f3d51641ece 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -45,13 +45,13 @@ feature 'Project', feature: true do end it 'should remove fork' do - expect(page).to have_content 'Remove forked relationship' + expect(page).to have_content 'Remove fork relationship' - remove_with_confirm('Remove forked relationship', project.path) + remove_with_confirm('Remove fork relationship', project.path) expect(page).to have_content 'Fork relationship has been removed.' expect(project.forked?).to be_falsey - expect(page).not_to have_content 'Remove forked relationship' + expect(page).not_to have_content 'Remove fork relationship' end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 580bbec77d1..e9de9e0826d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -606,28 +606,42 @@ describe API::API, api: true do describe 'DELETE /projects/:id/fork' do - it "shouldn't available for non admin users" do + it "shouldn't be visible to users outside group" do delete api("/projects/#{project_fork_target.id}/fork", user) - expect(response.status).to eq(403) + expect(response.status).to eq(404) end - it 'should make forked project unforked' do - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) - project_fork_target.reload - expect(project_fork_target.forked_from_project).not_to be_nil - expect(project_fork_target.forked?).to be_truthy - delete api("/projects/#{project_fork_target.id}/fork", admin) - expect(response.status).to eq(200) - project_fork_target.reload - expect(project_fork_target.forked_from_project).to be_nil - expect(project_fork_target.forked?).not_to be_truthy - end + context 'when users belong to project group' do + let(:project_fork_target) { create(:project, group: create(:group)) } - it 'should be idempotent if not forked' do - expect(project_fork_target.forked_from_project).to be_nil - delete api("/projects/#{project_fork_target.id}/fork", admin) - expect(response.status).to eq(200) - expect(project_fork_target.reload.forked_from_project).to be_nil + before do + project_fork_target.group.add_owner user + project_fork_target.group.add_developer user2 + end + + it 'should be forbidden to non-owner users' do + delete api("/projects/#{project_fork_target.id}/fork", user2) + expect(response.status).to eq(403) + end + + it 'should make forked project unforked' do + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + project_fork_target.reload + expect(project_fork_target.forked_from_project).not_to be_nil + expect(project_fork_target.forked?).to be_truthy + delete api("/projects/#{project_fork_target.id}/fork", admin) + expect(response.status).to eq(200) + project_fork_target.reload + expect(project_fork_target.forked_from_project).to be_nil + expect(project_fork_target.forked?).not_to be_truthy + end + + it 'should be idempotent if not forked' do + expect(project_fork_target.forked_from_project).to be_nil + delete api("/projects/#{project_fork_target.id}/fork", admin) + expect(response.status).to eq(200) + expect(project_fork_target.reload.forked_from_project).to be_nil + end end end end From ddeb766e9638d14dd86d966ef097fbeebb9dffab Mon Sep 17 00:00:00 2001 From: Mike Kelly Date: Wed, 14 Oct 2015 18:42:59 +0000 Subject: [PATCH 04/28] Remove some padding from code blocks This creates a weird "leading indent" on the first line of code blocks, at least in Chrome 46, and ends up making the first row not line up nicely with everything else. --- app/assets/stylesheets/framework/typography.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index bf36f96cc97..b56758a97d3 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -17,7 +17,6 @@ font-family: $monospace_font; white-space: pre; word-wrap: normal; - padding: 1px 2px; } kbd { @@ -268,4 +267,4 @@ textarea.js-gfm-input { .strikethrough { text-decoration: line-through; -} +} \ No newline at end of file From 9f9f0c35ecd9f7a5a057030253791d051f832f6d Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 12 Oct 2015 12:04:20 +0200 Subject: [PATCH 05/28] Show merge requests which close current issue --- CHANGELOG | 1 + app/assets/stylesheets/pages/issues.scss | 5 +++++ app/controllers/projects/issues_controller.rb | 7 +++++++ app/controllers/projects/merge_requests_controller.rb | 2 +- app/helpers/issues_helper.rb | 4 ++++ app/models/issue.rb | 10 ++++++++++ app/models/merge_request.rb | 4 ++++ app/views/projects/issues/_closed_by_box.html.haml | 6 ++++++ app/views/projects/issues/show.html.haml | 3 ++- 9 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 app/views/projects/issues/_closed_by_box.html.haml diff --git a/CHANGELOG b/CHANGELOG index 39926692147..e15fc9cb24a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.1.0 (unreleased) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) - Speed up load times of issue detail pages by roughly 1.5x + - If a merge request is to close an issue, show this on the issue page (Zeger-Jan van de Weg) - Make diff file view easier to use on mobile screens (Stan Hu) - Improved performance of finding users by username or Email address - Fix bug where merge request comments created by API would not trigger notifications (Stan Hu) diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 4bf58cb4a59..41c069f0ad3 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -132,6 +132,11 @@ form.edit-issue { } } +.issue-closed-by-widget { + padding: 16px 0; + margin: 0px; +} + .issue-form .select2-container { width: 250px !important; } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 97485c101fb..eaf14009242 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -14,6 +14,9 @@ class Projects::IssuesController < Projects::ApplicationController # Allow issues bulk update before_action :authorize_admin_issues!, only: [:bulk_update] + # Cross-reference merge requests + before_action :closed_by_merge_requests, only: [:show] + respond_to :html def index @@ -112,6 +115,10 @@ class Projects::IssuesController < Projects::ApplicationController render nothing: true end + def closed_by_merge_requests + @closed_by_mr = @issue.closed_by_merge_requests(current_user) + end + protected def issue diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 98df6984bf7..0d9c5461959 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -259,7 +259,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commits = @merge_request.commits @merge_request_diff = @merge_request.merge_request_diff - + if @merge_request.locked_long_ago? @merge_request.unlock_mr @merge_request.close diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 6ddb37cd0dc..1adbf3a79b1 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -83,6 +83,10 @@ module IssuesHelper end end + def merge_requests_sentence(merge_requests) + merge_requests.map(&:to_reference).to_sentence + end + # Required for Gitlab::Markdown::IssueReferenceFilter module_function :url_for_issue end diff --git a/app/models/issue.rb b/app/models/issue.rb index fc7e9abe29e..c24a329847c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -95,4 +95,14 @@ class Issue < ActiveRecord::Base def source_project project end + + # From all notes on this issue, we'll select the system notes about linked + # merge requests. Of those, the MRs closing `self` are returned. + def closed_by_merge_requests(current_user) + notes.system.flat_map do |note| + ext = Gitlab::ReferenceExtractor.new(self.project, current_user) + ext.analyze(note.note) + ext.merge_requests + end.uniq.select { |mr| mr.closes_issue?(self) } + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c83b15c7d39..3ae74ceac68 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -294,6 +294,10 @@ class MergeRequest < ActiveRecord::Base target_project end + def closes_issue?(issue) + closes_issues.include?(issue) + end + # Return the set of issues that will be closed if this merge request is accepted. def closes_issues(current_user = self.author) if target_branch == project.default_branch diff --git a/app/views/projects/issues/_closed_by_box.html.haml b/app/views/projects/issues/_closed_by_box.html.haml new file mode 100644 index 00000000000..fe886b6d7d7 --- /dev/null +++ b/app/views/projects/issues/_closed_by_box.html.haml @@ -0,0 +1,6 @@ +.issue-closed-by-widget + %i.fa.fa-check + - if @closed_by_mr.count == 1 + This issue will be closed when #{gfm(@closed_by_mr.first.to_reference)} is accepted. + -else + This issue will be closed when any of merge requests #{gfm(merge_requests_sentence(@closed_by_mr.sort))} is accepted. diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 5cb814c9ea8..309f276882d 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -46,6 +46,7 @@ = markdown(@issue.description) %textarea.hidden.js-task-list-field = @issue.description - + - if @closed_by_mr.present? + = render 'projects/issues/closed_by_box' .issue-discussion = render 'projects/issues/discussion' From 94a788f66dfcc13ad02855b05c38826f958038af Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 13 Oct 2015 09:41:46 +0200 Subject: [PATCH 06/28] Only accept open issues and merge requests --- app/controllers/projects/issues_controller.rb | 2 +- app/helpers/issues_helper.rb | 2 +- app/models/concerns/issuable.rb | 4 ++ app/models/issue.rb | 10 ++--- app/models/merge_request.rb | 4 -- .../projects/issues/_closed_by_box.html.haml | 9 ++--- app/views/projects/issues/show.html.haml | 2 +- spec/helpers/issues_helper_spec.rb | 10 +++++ spec/models/concerns/issuable_spec.rb | 1 - spec/models/issue_spec.rb | 37 +++++++++++++++++++ 10 files changed, 62 insertions(+), 19 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index eaf14009242..cc8321d97ad 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -116,7 +116,7 @@ class Projects::IssuesController < Projects::ApplicationController end def closed_by_merge_requests - @closed_by_mr = @issue.closed_by_merge_requests(current_user) + @closed_by_merge_requests ||= @issue.closed_by_merge_requests(current_user) end protected diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 1adbf3a79b1..fda18e7b316 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -84,7 +84,7 @@ module IssuesHelper end def merge_requests_sentence(merge_requests) - merge_requests.map(&:to_reference).to_sentence + merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ') end # Required for Gitlab::Markdown::IssueReferenceFilter diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0e8bcc1a4ec..efa6a269992 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -86,6 +86,10 @@ module Issuable assignee_id_changed? end + def open? + opened? || reopened? + end + # # Votes # diff --git a/app/models/issue.rb b/app/models/issue.rb index c24a329847c..72183108033 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -98,11 +98,11 @@ class Issue < ActiveRecord::Base # From all notes on this issue, we'll select the system notes about linked # merge requests. Of those, the MRs closing `self` are returned. - def closed_by_merge_requests(current_user) + def closed_by_merge_requests(current_user = nil) + return [] unless open? + notes.system.flat_map do |note| - ext = Gitlab::ReferenceExtractor.new(self.project, current_user) - ext.analyze(note.note) - ext.merge_requests - end.uniq.select { |mr| mr.closes_issue?(self) } + note.all_references(current_user).merge_requests + end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 3ae74ceac68..0042b95c4f1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -222,10 +222,6 @@ class MergeRequest < ActiveRecord::Base self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last end - def open? - opened? || reopened? - end - def work_in_progress? !!(title =~ /\A\[?WIP\]?:? /i) end diff --git a/app/views/projects/issues/_closed_by_box.html.haml b/app/views/projects/issues/_closed_by_box.html.haml index fe886b6d7d7..aef352029d0 100644 --- a/app/views/projects/issues/_closed_by_box.html.haml +++ b/app/views/projects/issues/_closed_by_box.html.haml @@ -1,6 +1,3 @@ -.issue-closed-by-widget - %i.fa.fa-check - - if @closed_by_mr.count == 1 - This issue will be closed when #{gfm(@closed_by_mr.first.to_reference)} is accepted. - -else - This issue will be closed when any of merge requests #{gfm(merge_requests_sentence(@closed_by_mr.sort))} is accepted. +.issue-closed-by-widget + = icon('check') + This issue will be closed automatically when merge request #{gfm(merge_requests_sentence(@closed_by_merge_requests.sort))} is accepted. diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 309f276882d..f01bf2505da 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -46,7 +46,7 @@ = markdown(@issue.description) %textarea.hidden.js-task-list-field = @issue.description - - if @closed_by_mr.present? + - if @closed_by_merge_requests.present? = render 'projects/issues/closed_by_box' .issue-discussion = render 'projects/issues/discussion' diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index c08ddb4cae1..78a6b631eb2 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -117,4 +117,14 @@ describe IssuesHelper do end end + describe "#merge_requests_sentence" do + subject { merge_requests_sentence(merge_requests)} + let(:merge_requests) do + [ build(:merge_request, iid: 1), build(:merge_request, iid: 2), + build(:merge_request, iid: 3)] + end + + it { is_expected.to eq("!1, !2, or !3") } + end + end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 8f706f8934b..0f13c4410cd 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -68,7 +68,6 @@ describe Issue, "Issuable" do end end - describe "#to_hook_data" do let(:hook_data) { issue.to_hook_data(user) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 623332cd2f9..c9aa1b063c6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -68,6 +68,43 @@ describe Issue do end end + describe '#closed_by_merge_requests' do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project, state: "opened")} + let(:closed_issue) { build(:issue, project: project, state: "closed")} + + let(:mr) do + opts = { + title: 'Awesome merge_request', + description: "Fixes #{issue.to_reference}", + source_branch: 'feature', + target_branch: 'master' + } + MergeRequests::CreateService.new(project, project.owner, opts).execute + end + + let(:closed_mr) do + opts = { + title: 'Awesome merge_request 2', + description: "Fixes #{issue.to_reference}", + source_branch: 'feature', + target_branch: 'master', + state: 'closed' + } + MergeRequests::CreateService.new(project, project.owner, opts).execute + end + + it 'returns the merge request to close this issue' do + allow(mr).to receive(:closes_issue?).with(issue).and_return(true) + + expect(issue.closed_by_merge_requests).to eq([mr]) + end + + it "returns an empty array when the current issue is closed already" do + expect(closed_issue.closed_by_merge_requests).to eq([]) + end + end + it_behaves_like 'an editable mentionable' do subject { create(:issue) } From 7c85ebf6dc9c083d53642cbe2b6a5276c6b95aa6 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 16 Oct 2015 13:24:28 +0200 Subject: [PATCH 07/28] Partly implement new UI for user page Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework/blocks.scss | 45 +++++++++ app/assets/stylesheets/pages/profile.scss | 6 ++ .../{ => admin}/users/_profile.html.haml | 0 app/views/admin/users/show.html.haml | 2 +- app/views/users/calendar.html.haml | 6 +- app/views/users/show.html.haml | 94 ++++++++++++------- features/steps/abuse_reports.rb | 2 +- 7 files changed, 114 insertions(+), 41 deletions(-) rename app/views/{ => admin}/users/_profile.html.haml (100%) diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 6ce34b5c3e8..a09339050c5 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -60,3 +60,48 @@ line-height: 42px; } } + +.cover-block { + text-align: center; + background: #f7f8fa; + margin: -$gl-padding; + margin-bottom: 0; + padding: 44px $gl-padding; + border-bottom: 1px solid $border-color; + position: relative; + + .avatar-holder { + margin-bottom: 16px; + + .avatar, .identicon { + margin: 0 auto; + float: none; + } + + .identicon { + @include border-radius(50%); + } + } + + .cover-title { + color: $gl-header-color; + margin: 0; + font-size: 23px; + font-weight: normal; + margin: 16px 0 5px 0; + color: #4c4e54; + font-size: 23px; + line-height: 1.1; + } + + .cover-desc { + padding: 0 $gl-padding; + color: $gl-text-color; + } + + .cover-controls { + position: absolute; + top: 10px; + right: 10px; + } +} diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 8e4f0eb2b25..b7391e5303b 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -47,3 +47,9 @@ } } } + +.calendar-hint { + margin-top: -12px; + float: right; + font-size: 12px; +} diff --git a/app/views/users/_profile.html.haml b/app/views/admin/users/_profile.html.haml similarity index 100% rename from app/views/users/_profile.html.haml rename to app/views/admin/users/_profile.html.haml diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 231bcb0426f..0848504b7a6 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -14,7 +14,7 @@ %strong = link_to user_path(@user) do = @user.username - = render 'users/profile', user: @user + = render 'admin/users/profile', user: @user .panel.panel-default .panel-heading diff --git a/app/views/users/calendar.html.haml b/app/views/users/calendar.html.haml index 922b0c6cebf..7f29918dba3 100644 --- a/app/views/users/calendar.html.haml +++ b/app/views/users/calendar.html.haml @@ -1,7 +1,3 @@ -%h4 - Contributions calendar - .pull-right - %small Issues, merge requests and push events #cal-heatmap.calendar :javascript new Calendar( @@ -10,3 +6,5 @@ #{@starting_month}, '#{user_calendar_activities_path}' ); + +.calendar-hint Summary of issues, merge requests and push events diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 2a64708d07c..4ea4a1f92c2 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -6,47 +6,72 @@ = render 'shared/show_aside' -.row +.cover-block + .avatar-holder + = link_to avatar_icon(@user, 400), target: '_blank' do + = image_tag avatar_icon(@user, 90), class: "avatar s90", alt: '' + .cover-title + = @user.name + + .cover-desc + %span + @#{@user.username}. + - if @user.bio.present? + %span + #{@user.bio}. + %span + Member since #{@user.created_at.stamp("Aug 21, 2011")} + + .cover-desc + - unless @user.public_email.blank? + = link_to @user.public_email, "mailto:#{@user.public_email}" + - unless @user.skype.blank? + · + = link_to "Skype", "skype:#{@user.skype}" + - unless @user.linkedin.blank? + · + = link_to "LinkedIn", "http://www.linkedin.com/in/#{@user.linkedin}" + - unless @user.twitter.blank? + · + = link_to "Twitter", "http://www.twitter.com/#{@user.twitter}" + - unless @user.website_url.blank? + · + = link_to @user.short_website_url, @user.full_website_url + - unless @user.location.blank? + · + = @user.location + + + .cover-controls + - if @user == current_user + = link_to profile_path, class: 'btn btn-gray' do + = icon('pencil') + - elsif current_user + .report-abuse + - if @user.abuse_report + %button.btn.btn-danger{ title: 'Already reported for abuse', + data: { toggle: 'tooltip', placement: 'left', container: 'body' }} + = icon('exclamation-circle') + - else + = link_to new_abuse_report_path(user_id: @user.id), class: 'btn btn-gray', + title: 'Report abuse', data: {toggle: 'tooltip', placement: 'left', container: 'body'} do + = icon('exclamation-circle') + +.gray-content-block.second-block + .user-calendar + %h4.center.light + %i.fa.fa-spinner.fa-spin + .user-calendar-activities + + +.row.prepend-top-20 %section.col-md-7 - .header-with-avatar - = link_to avatar_icon(@user, 400), target: '_blank' do - = image_tag avatar_icon(@user, 90), class: "avatar avatar-tile s90", alt: '' - %h3 - = @user.name - - if @user == current_user - .pull-right.hidden-xs - = link_to profile_path, class: 'btn btn-sm' do - = icon('user') - Profile settings - - elsif current_user - .report_abuse.pull-right - - if @user.abuse_report - %span#report_abuse_btn.light.btn.btn-sm.btn-close{title: 'Already reported for abuse', data: {toggle: 'tooltip', placement: 'right', container: 'body'}} - = icon('exclamation-circle') - - else - %a.light.btn.btn-sm{href: new_abuse_report_path(user_id: @user.id), title: 'Report abuse', data: {toggle: 'tooltip', placement: 'right', container: 'body'}} - = icon('exclamation-circle') - - .username - @#{@user.username} - .description - - if @user.bio.present? - = @user.bio - - .clearfix - - if @groups.any? .prepend-top-20 %h4 Groups = render 'groups', groups: @groups %hr - .hidden-xs - .user-calendar - %h4.center.light - %i.fa.fa-spinner.fa-spin - .user-calendar-activities - %hr %h4 User Activity @@ -59,7 +84,6 @@ .content_list = spinner %aside.col-md-5 - = render 'profile', user: @user = render 'projects', projects: @projects, contributed_projects: @contributed_projects :coffeescript diff --git a/features/steps/abuse_reports.rb b/features/steps/abuse_reports.rb index 56652ff6f05..499accb0b08 100644 --- a/features/steps/abuse_reports.rb +++ b/features/steps/abuse_reports.rb @@ -23,7 +23,7 @@ class Spinach::Features::AbuseReports < Spinach::FeatureSteps end step 'I should see a red "Report abuse" button' do - expect(find(:css, '.report_abuse')).to have_selector(:css, 'span.btn-close') + expect(page).to have_button("Already reported for abuse") end def user_mike From cfa3602a1303c39e78d5bf23ad0ab34b1966d397 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:15:32 +0200 Subject: [PATCH 08/28] Move last push info at top of project page. --- app/views/projects/_activity.html.haml | 1 - app/views/projects/activity.html.haml | 2 ++ app/views/projects/show.html.haml | 3 +-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/projects/_activity.html.haml b/app/views/projects/_activity.html.haml index c2683bc6219..012858f70b4 100644 --- a/app/views/projects/_activity.html.haml +++ b/app/views/projects/_activity.html.haml @@ -1,4 +1,3 @@ -= render 'projects/last_push' .gray-content-block.activity-filter-block - if current_user .pull-right diff --git a/app/views/projects/activity.html.haml b/app/views/projects/activity.html.haml index 555ed76426d..69fa4ad37c4 100644 --- a/app/views/projects/activity.html.haml +++ b/app/views/projects/activity.html.haml @@ -1,4 +1,6 @@ - page_title "Activity" - header_title project_title(@project, "Activity", activity_project_path(@project)) += render 'projects/last_push' + = render 'projects/activity' diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index fdd3d0b322e..f8c06d8b06b 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -7,8 +7,7 @@ = render 'shared/no_ssh' = render 'shared/no_password' -- if prefer_readme? - = render 'projects/last_push' += render 'projects/last_push' = render "home_panel" From 6f0856535e242694805909b824e9e238372e3f65 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:16:04 +0200 Subject: [PATCH 09/28] Remove redundant helper method. --- app/helpers/preferences_helper.rb | 10 ++-------- app/views/projects/show.html.haml | 7 +++---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 5a49ab8195c..c73cb3028ee 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -47,13 +47,7 @@ module PreferencesHelper Gitlab::ColorSchemes.for_user(current_user).css_class end - def prefer_readme? - !current_user || - current_user.project_view == 'readme' - end - - def current_user_default_project_view - (current_user && current_user.project_view) || - 'readme' + def default_project_view + current_user ? current_user.project_view : 'readme' end end diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index f8c06d8b06b..585caf674c9 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -27,7 +27,7 @@ = link_to project_files_path(@project) do = repository_size - - if !prefer_readme? && @repository.readme + - if default_project_view != 'readme' && @repository.readme %li = link_to 'Readme', readme_path(@project) @@ -67,9 +67,8 @@ .content-block.second-block.white = render 'projects/last_commit', commit: @repository.commit, project: @project -%section - %div{class: "project-show-#{current_user_default_project_view}"} - = render current_user_default_project_view +%div{class: "project-show-#{default_project_view}"} + = render default_project_view - if current_user - access = user_max_access_in_project(current_user, @project) From 6bd9a9fbf7aa33b74ad2522466eb84fbbc6aa7b5 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:16:40 +0200 Subject: [PATCH 10/28] Set vars used by tree view in project show action. --- app/controllers/projects_controller.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 7158d4b49ac..c81c7ea59c2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -1,8 +1,11 @@ class ProjectsController < ApplicationController + include ExtractsPath + prepend_before_filter :render_go_import, only: [:show] skip_before_action :authenticate_user!, only: [:show, :activity] before_action :project, except: [:new, :create] before_action :repository, except: [:new, :create] + before_action :assign_ref_vars, :tree, only: [:show] # Authorize before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive] @@ -89,10 +92,7 @@ class ProjectsController < ApplicationController if current_user @membership = @project.project_member_by_id(current_user.id) end - @ref = "master" - @id = "master" - @commit = @project.repository.commit(@ref) - @tree = @project.repository.tree(@commit.id) + render :show end else @@ -228,4 +228,8 @@ class ProjectsController < ApplicationController render "go_import", layout: false end + + def get_id + project.repository.root_ref + end end From 7b26414c155796e60b4d6cff10a50d891f276ec2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:17:13 +0200 Subject: [PATCH 11/28] Split up projects/tree/_tree partial. --- ...tree.html.haml => _tree_content.html.haml} | 35 +------------------ .../projects/tree/_tree_header.html.haml | 32 +++++++++++++++++ app/views/projects/tree/show.html.haml | 8 ++--- 3 files changed, 37 insertions(+), 38 deletions(-) rename app/views/projects/tree/{_tree.html.haml => _tree_content.html.haml} (52%) create mode 100644 app/views/projects/tree/_tree_header.html.haml diff --git a/app/views/projects/tree/_tree.html.haml b/app/views/projects/tree/_tree_content.html.haml similarity index 52% rename from app/views/projects/tree/_tree.html.haml rename to app/views/projects/tree/_tree_content.html.haml index 7ff48e32e60..ed1f61e9077 100644 --- a/app/views/projects/tree/_tree.html.haml +++ b/app/views/projects/tree/_tree_content.html.haml @@ -1,35 +1,4 @@ -.gray-content-block - %ul.breadcrumb.repo-breadcrumb - %li - = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do - = @project.path - - tree_breadcrumbs(tree, 6) do |title, path| - %li - - if path - = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) - - else - = link_to title, '#' - - if allowed_tree_edit? - %li - %span.dropdown - %a.dropdown-toggle.btn.add-to-tree{href: '#', "data-toggle" => "dropdown"} - = icon('plus') - %ul.dropdown-menu - %li - = link_to namespace_project_new_blob_path(@project.namespace, @project, @id), title: 'Create file', id: 'new-file-link' do - = icon('pencil fw') - Create file - %li - = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'} do - = icon('file fw') - Upload file - %li.divider - %li - = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do - = icon('folder fw') - New directory - -%div#tree-content-holder.tree-content-holder +%div.tree-content-holder .tree-table-holder %table.table#tree-slider{class: "table_#{@hex_path} tree-table table-striped" } %thead @@ -60,8 +29,6 @@ - if tree.readme = render "projects/tree/readme", readme: tree.readme -%div.tree_progress - - if allowed_tree_edit? = render 'projects/blob/upload', title: 'Upload', placeholder: 'Upload new file', button_title: 'Upload file', form_path: namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post = render 'projects/blob/new_dir' diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml new file mode 100644 index 00000000000..1115ca6b4ca --- /dev/null +++ b/app/views/projects/tree/_tree_header.html.haml @@ -0,0 +1,32 @@ +.tree-ref-holder + = render 'shared/ref_switcher', destination: 'tree', path: @path + +%ul.breadcrumb.repo-breadcrumb + %li + = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do + = @project.path + - tree_breadcrumbs(tree, 6) do |title, path| + %li + - if path + = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) + - else + = link_to title, '#' + - if allowed_tree_edit? + %li + %span.dropdown + %a.dropdown-toggle.btn.add-to-tree{href: '#', "data-toggle" => "dropdown"} + = icon('plus') + %ul.dropdown-menu + %li + = link_to namespace_project_new_blob_path(@project.namespace, @project, @id), title: 'Create file', id: 'new-file-link' do + = icon('pencil fw') + Create file + %li + = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'} do + = icon('file fw') + Upload file + %li.divider + %li + = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do + = icon('folder fw') + New directory diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml index dec4677f830..ec14bd7f65a 100644 --- a/app/views/projects/tree/show.html.haml +++ b/app/views/projects/tree/show.html.haml @@ -6,12 +6,12 @@ = render 'projects/last_push' -.tree-ref-holder - = render 'shared/ref_switcher', destination: 'tree', path: @path - - if can? current_user, :download_code, @project .tree-download-holder = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'btn-group pull-right hidden-xs hidden-sm', split_button: true #tree-holder.tree-holder.clearfix - = render "tree", tree: @tree + .gray-content-block.top-block + = render 'projects/tree/tree_header', tree: @tree + + = render 'projects/tree/tree_content', tree: @tree From a22fe254c18a64b1f145593781b2505d0fbdd46c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:17:31 +0200 Subject: [PATCH 12/28] Use same style for project readme, tree readme and regular blob. --- app/assets/stylesheets/pages/tree.scss | 8 -------- app/views/projects/_readme.html.haml | 15 +++++++-------- app/views/projects/tree/_readme.html.haml | 6 +++--- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index dadd86e88cc..ace371d7695 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -4,14 +4,6 @@ margin-right: -$gl-padding; } - .tree_progress { - display: none; - margin: 20px; - &.loading { - display: block; - } - } - .tree-table { margin-bottom: 0; diff --git a/app/views/projects/_readme.html.haml b/app/views/projects/_readme.html.haml index 5bc1999ec9d..0a1cecfdcdf 100644 --- a/app/views/projects/_readme.html.haml +++ b/app/views/projects/_readme.html.haml @@ -1,12 +1,11 @@ - if readme = @repository.readme - %article.readme-holder#README - .clearfix - .pull-right -   - - if can?(current_user, :push_code, @project) - = link_to namespace_project_edit_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)), class: 'light' do - %i.fa-align.fa.fa-pencil - .wiki + %article.file-holder.readme-holder + .file-title + = blob_icon readme.mode, readme.name + = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do + %strong + = readme.name + .file-content.wiki = cache(readme_cache_key) do = render_readme(readme) - else diff --git a/app/views/projects/tree/_readme.html.haml b/app/views/projects/tree/_readme.html.haml index 7e9af19c8ba..3c5edf4b033 100644 --- a/app/views/projects/tree/_readme.html.haml +++ b/app/views/projects/tree/_readme.html.haml @@ -1,8 +1,8 @@ -%article.file-holder.readme-holder#README +%article.file-holder.readme-holder .file-title - = link_to '#README' do + = blob_icon readme.mode, readme.name + = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)) do %strong - %i.fa.fa-file = readme.name .file-content.wiki = render_readme(readme) From f009b6e256cbb9493883949abd87cd41920ba054 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:18:28 +0200 Subject: [PATCH 13/28] Tweak styling of project home files list. --- app/assets/stylesheets/pages/projects.scss | 6 +----- app/views/projects/_files.html.haml | 15 +++++---------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index db6864ed784..bc62532f8a0 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -543,10 +543,6 @@ pre.light-well { } } -.project-show-files { - padding-top: 20px; -} - .project-show-readme .readme-holder { - padding: 7px; + border-top: 0; } diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index 2a99708eb43..fa978325ddd 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -1,11 +1,6 @@ -= render 'projects/last_push' - -.tree-ref-holder - = render 'shared/ref_switcher', destination: 'tree', path: @path - -- if can? current_user, :download_code, @project - .tree-download-holder - = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'btn-group pull-right hidden-xs hidden-sm', split_button: true - #tree-holder.tree-holder.clearfix - = render "projects/tree/tree", tree: @tree + .gray-content-block.second-block + = render 'projects/tree/tree_header', tree: @tree + + = render 'projects/tree/tree_content', tree: @tree + From 33fd13c8f4909cbe52a8ec38701740f11f2fccbd Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:18:37 +0200 Subject: [PATCH 14/28] Remove border at bottom of readme. --- app/assets/stylesheets/framework/files.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 9dd77747884..8742d1c39b3 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -10,6 +10,10 @@ border-bottom: 1px solid #E7E9EE; margin-bottom: 1em; + &.readme-holder { + border-bottom: 0; + } + table { @extend .table; } From afb33acae6ed37e630b33b7ae18d516eb8c5fdfb Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:18:57 +0200 Subject: [PATCH 15/28] Put grey block around blob ref switcher and breadcrumb. --- app/views/projects/blob/_blob.html.haml | 31 ++++++++++++++----------- app/views/projects/blob/show.html.haml | 3 --- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index a1ae1397584..42f632b38ef 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -1,19 +1,22 @@ -%ul.breadcrumb.repo-breadcrumb - %li - %i.fa.fa-angle-right - = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do - = @project.path - - tree_breadcrumbs(@tree, 6) do |title, path| +.gray-content-block.top-block + .tree-ref-holder + = render 'shared/ref_switcher', destination: 'blob', path: @path + + %ul.breadcrumb.repo-breadcrumb %li - - if path - - if path.end_with?(@path) - = link_to namespace_project_blob_path(@project.namespace, @project, path) do - %strong - = truncate(title, length: 40) + = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do + = @project.path + - tree_breadcrumbs(@tree, 6) do |title, path| + %li + - if path + - if path.end_with?(@path) + = link_to namespace_project_blob_path(@project.namespace, @project, path) do + %strong + = truncate(title, length: 40) + - else + = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) - else - = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) - - else - = link_to title, '#' + = link_to title, '#' %ul.blob-commit-info.hidden-xs - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path) diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index fa4be4a1bc4..f52b89f6921 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -3,9 +3,6 @@ = render 'projects/last_push' -%div.tree-ref-holder - = render 'shared/ref_switcher', destination: 'blob', path: @path - %div#tree-holder.tree-holder = render 'blob', blob: @blob From 58cdfba9b9cc57998059f7ad78422d1d46b98fd7 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:19:33 +0200 Subject: [PATCH 16/28] Tweak help text. --- app/views/profiles/preferences/show.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 01e285a8dfa..cc41d7dd813 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -38,7 +38,7 @@ .col-sm-10 = f.select :layout, layout_choices, {}, class: 'form-control' .help-block - Choose between fixed (max. 1200px) and fluid (100%) application layout + Choose between fixed (max. 1200px) and fluid (100%) application layout. .form-group = f.label :dashboard, class: 'control-label' do Default Dashboard @@ -52,6 +52,6 @@ .col-sm-10 = f.select :project_view, project_view_choices, {}, class: 'form-control' .help-block - Choose what content you want to see when visit project page + Choose what content you want to see on a project's home page. .panel-footer = f.submit 'Save', class: 'btn btn-save' From aebe0ddc33b93a66bd52b63a114c485791f15805 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 17 Oct 2015 19:27:02 +0200 Subject: [PATCH 17/28] Make spec names more clear --- spec/controllers/projects_controller_spec.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 590a3f5b441..b7ec5e48e85 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -25,23 +25,26 @@ describe ProjectsController do context "rendering default project view" do render_views - it "shold render the activity view", focus: true do + it "renders the activity view" do allow(controller).to receive(:current_user).and_return(user) allow(user).to receive(:project_view).and_return('activity') + get :show, namespace_id: public_project.namespace.path, id: public_project.path expect(response).to render_template('_activity') end - it "shold render the readme view", focus: true do + it "renders the readme view" do allow(controller).to receive(:current_user).and_return(user) allow(user).to receive(:project_view).and_return('readme') + get :show, namespace_id: public_project.namespace.path, id: public_project.path expect(response).to render_template('_readme') end - it "shold render the files view", focus: true do + it "renders the files view" do allow(controller).to receive(:current_user).and_return(user) allow(user).to receive(:project_view).and_return('files') + get :show, namespace_id: public_project.namespace.path, id: public_project.path expect(response).to render_template('_files') end From ff866faf2fca82c7ad7e2d70cba2cae56cc5cf7f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 11:22:33 +0200 Subject: [PATCH 18/28] Only load tree when project has repository to prevent 404 --- app/controllers/projects_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c81c7ea59c2..bb2df275b77 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -5,7 +5,7 @@ class ProjectsController < ApplicationController skip_before_action :authenticate_user!, only: [:show, :activity] before_action :project, except: [:new, :create] before_action :repository, except: [:new, :create] - before_action :assign_ref_vars, :tree, only: [:show] + before_action :assign_ref_vars, :tree, only: [:show], if: :repo_exists? # Authorize before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive] @@ -229,6 +229,10 @@ class ProjectsController < ApplicationController render "go_import", layout: false end + def repo_exists? + project.repository_exists? && !project.empty_repo? + end + def get_id project.repository.root_ref end From f52b07cedcafc6cb5e92f549b7e7b4fab3b2ca83 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 11:22:40 +0200 Subject: [PATCH 19/28] Fix readme spec --- features/steps/project/project.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index 15f77734cb2..d76891d5bde 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -86,13 +86,13 @@ class Spinach::Features::Project < Spinach::FeatureSteps end step 'I should see project "Forum" README' do - page.within('#README') do + page.within('.readme-holder') do expect(page).to have_content 'Sample repo for testing gitlab features' end end step 'I should see project "Shop" README' do - page.within('#README') do + page.within('.readme-holder') do expect(page).to have_content 'testme' end end From 42cbc7f813386dbf6d28868c9972ff38f01ad095 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 12:37:50 +0200 Subject: [PATCH 20/28] Tweak wording. --- app/controllers/projects_controller.rb | 4 ++- app/helpers/projects_helper.rb | 2 +- app/views/projects/edit.html.haml | 30 ++++++++++---------- spec/controllers/projects_controller_spec.rb | 4 +-- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 77b3af9a5d0..12ef073e149 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -71,7 +71,7 @@ class ProjectsController < ApplicationController if @project.forked? @project.forked_project_link.destroy - flash[:notice] = 'Fork relationship has been removed.' + flash[:notice] = 'The fork relationship has been removed.' end end @@ -150,6 +150,7 @@ class ProjectsController < ApplicationController def archive return access_denied! unless can?(current_user, :archive_project, @project) + @project.archive! respond_to do |format| @@ -159,6 +160,7 @@ class ProjectsController < ApplicationController def unarchive return access_denied! unless can?(current_user, :archive_project, @project) + @project.unarchive! respond_to do |format| diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index e6e1bfd2c9b..472884e4ff2 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -71,7 +71,7 @@ module ProjectsHelper end def remove_fork_project_message(project) - "You are going to remove the fork relationship to the source project from #{@project.forked_from_project.namespace.try(:name)}. Are you ABSOLUTELY sure?" + "You are going to remove the fork relationship to source project #{@project.forked_from_project.name_with_namespace}. Are you ABSOLUTELY sure?" end def project_nav_tabs diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index ec58d0924b0..afbf88b5507 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -189,20 +189,20 @@ - else .nothing-here-block Only the project owner can transfer a project - - if @project.forked? && can?(current_user, :remove_fork_project, @project) - = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :delete, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f| - .panel.panel-default.panel.panel-danger - .panel-heading Remove fork relationship - .panel-body - %p - This will remove the relationship to the source project from - = link_to project_path(@project.forked_from_project) do - = @project.forked_from_project.namespace.try(:name) - %br - %strong Once removed it cannot be reversed through this interface. - = button_to 'Remove fork relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) } - - elsif @project.forked? - .nothing-here-block Only the project owner can remove the fork relationship + - if @project.forked? + - if can?(current_user, :remove_fork_project, @project) + = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :delete, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f| + .panel.panel-default.panel.panel-danger + .panel-heading Remove fork relationship + .panel-body + %p + This will remove the fork relationship to source project + #{link_to @project.forked_from_project.name_with_namespace, project_path(@project.forked_from_project)}. + %br + %strong Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source. + = button_to 'Remove fork relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) } + - else + .nothing-here-block Only the project owner can remove the fork relationship. - if can?(current_user, :remove_project, @project) .panel.panel-default.panel.panel-danger @@ -216,7 +216,7 @@ = button_to 'Remove project', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_project_message(@project) } - else - .nothing-here-block Only project owner can remove a project + .nothing-here-block Only the project owner can remove a project. .save-project-loader.hide diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e963e913512..9b0527a68d2 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -63,7 +63,7 @@ describe ProjectsController do end end - describe "PUT remove_fork" do + describe "DELETE remove_fork" do context 'when signed in' do before do sign_in(user) @@ -82,7 +82,7 @@ describe ProjectsController do id: project_fork.to_param, format: :js) expect(project_fork.forked?).to be_falsey - expect(flash[:notice]).to eq('Fork relationship has been removed.') + expect(flash[:notice]).to eq('The fork relationship has been removed.') expect(response).to render_template(:remove_fork) end end From cc2b05adf80f55c9f0fe4b453f9f45e2b402c006 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 14:05:27 +0200 Subject: [PATCH 21/28] Fix bug where a push would only create cross references from the first commit. --- app/services/git_push_service.rb | 2 +- lib/gitlab/reference_extractor.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e54044365b9..3de7bb9dcaa 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -79,7 +79,7 @@ class GitPushService authors = Hash.new do |hash, commit| email = commit.author_email - return hash[email] if hash.has_key?(email) + next hash[email] if hash.has_key?(email) hash[email] = commit_user(commit) end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 333bd059055..da8df8a3025 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -27,7 +27,7 @@ module Gitlab def references @references ||= Hash.new do |references, type| type = type.to_sym - return references[type] if references.has_key?(type) + next references[type] if references.has_key?(type) references[type] = pipeline_result(type) end From f3a74556b15f7429749384a823b73253602454cf Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 14:12:50 +0200 Subject: [PATCH 22/28] Fix spec --- spec/features/projects_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index f3d51641ece..09fcff2444a 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -49,7 +49,7 @@ feature 'Project', feature: true do remove_with_confirm('Remove fork relationship', project.path) - expect(page).to have_content 'Fork relationship has been removed.' + expect(page).to have_content 'The fork relationship has been removed.' expect(project.forked?).to be_falsey expect(page).not_to have_content 'Remove fork relationship' end From d12e49c5916a42dafba32555405878f32c7ad254 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 15:07:02 +0200 Subject: [PATCH 23/28] Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge. --- CHANGELOG | 1 + app/services/merge_requests/post_merge_service.rb | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index abf9ead6df7..699b44f00d7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.2.0 (unreleased) - Highlight comment based on anchor in URL v 8.1.0 (unreleased) + - Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge. - Fix nonatomic database update potentially causing project star counts to go negative (Stan Hu) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) - Speed up load times of issue detail pages by roughly 1.5x diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index aceb8cb9021..8f25c5e2496 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -6,6 +6,7 @@ module MergeRequests # class PostMergeService < MergeRequests::BaseService def execute(merge_request) + close_issues(merge_request) merge_request.mark_as_merged create_merge_event(merge_request, current_user) create_note(merge_request) @@ -15,6 +16,15 @@ module MergeRequests private + def close_issues(merge_request) + return unless merge_request.target_branch == project.default_branch + + closed_issues = merge_request.closes_issues(current_user) + closed_issues.each do |issue| + Issues::CloseService.new(project, current_user, {}).execute(issue, merge_request) + end + end + def create_merge_event(merge_request, current_user) EventCreateService.new.merge_mr(merge_request, current_user) end From 85264ab6a9fa11a2e325d72c02c00d4b8862f933 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 18 Oct 2015 15:24:04 +0200 Subject: [PATCH 24/28] Restore notification footer text. --- app/views/layouts/notify.html.haml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml index 2f7d7e86f56..854cda57c39 100644 --- a/app/views/layouts/notify.html.haml +++ b/app/views/layouts/notify.html.haml @@ -41,4 +41,8 @@ #{link_to "view it on GitLab", @target_url}. - else #{link_to "View it on GitLab", @target_url} + %br + You're receiving this email because of your account on #{link_to Gitlab.config.gitlab.host, root_url}. + If you'd like to receive fewer emails, you can adjust your notification settings. + = email_action @target_url From 405a10e9b8f94578b0417cb4fea0362aa139bac3 Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Sun, 18 Oct 2015 17:42:18 +0100 Subject: [PATCH 25/28] Fix spelling of Description --- app/views/projects/ci_services/index.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/ci_services/index.html.haml b/app/views/projects/ci_services/index.html.haml index c78b21884a3..c164b2d4bc0 100644 --- a/app/views/projects/ci_services/index.html.haml +++ b/app/views/projects/ci_services/index.html.haml @@ -6,7 +6,7 @@ %tr %th %th Service - %th Desription + %th Description %th Last edit - @services.sort_by(&:title).each do |service| %tr From 6ad78d3ab1fc0ea9f344810e22b4fa7e8d67b6f7 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 19 Oct 2015 11:25:21 +0200 Subject: [PATCH 26/28] Move changelog item to 8.2 [ci skip] --- CHANGELOG | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 46fec4bd921..08a5030f725 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.2.0 (unreleased) - Show last project commit to default branch on project home page - Highlight comment based on anchor in URL + - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw) v 8.1.0 (unreleased) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) @@ -59,8 +60,6 @@ v 8.1.0 (unreleased) - Fix position of hamburger in header for smaller screens (Han Loong Liauw) - Fix bug where Emojis in Markdown would truncate remaining text (Sakata Sinji) - Persist filters when sorting on admin user page (Jerry Lukins) - - Adds ability to remove the forked relationship from project settings - screen. (Han Loong Liauw) - Allow dashboard and group issues/MRs to be filtered by label - Add spellcheck=false to certain input fields - Invalidate stored service password if the endpoint URL is changed From 4ff75e317935f990b90dcc5869afe8ebb2b6fee6 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 15 Oct 2015 18:10:35 +0200 Subject: [PATCH 27/28] Improve performance of sorting milestone issues This cuts down the time it takes to sort issues of a milestone by about 10x. In the previous setup the code would run a SQL query for every issue that had to be sorted. The new setup instead runs a single SQL query to update all the given issues at once. The attached benchmark used to run at around 60 iterations per second, using the new setup this hovers around 600 iterations per second. Timing wise a request to update a milestone with 40-something issues would take about 760 ms, in the new setup this only takes about 130 ms. Fixes #3066 --- CHANGELOG | 1 + .../projects/milestones_controller.rb | 6 +--- app/models/milestone.rb | 32 +++++++++++++++++++ spec/benchmarks/models/milestone_spec.rb | 17 ++++++++++ spec/models/milestone_spec.rb | 28 ++++++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 spec/benchmarks/models/milestone_spec.rb diff --git a/CHANGELOG b/CHANGELOG index f8daa6d246c..7fefc90c541 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.2.0 (unreleased) - Show last project commit to default branch on project home page - Highlight comment based on anchor in URL - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw) + - Improved performance of sorting milestone issues v 8.1.0 (unreleased) - Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge. diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 86f4a02a6e9..15506bd677a 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -75,11 +75,7 @@ class Projects::MilestonesController < Projects::ApplicationController end def sort_issues - @issues = @milestone.issues.where(id: params['sortable_issue']) - @issues.each do |issue| - issue.position = params['sortable_issue'].index(issue.id.to_s) + 1 - issue.save - end + @milestone.sort_issues(params['sortable_issue'].map(&:to_i)) render json: { saved: true } end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 84acba30b6b..2ff16e2825c 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -105,4 +105,36 @@ class Milestone < ActiveRecord::Base def author_id nil end + + # Sorts the issues for the given IDs. + # + # This method runs a single SQL query using a CASE statement to update the + # position of all issues in the current milestone (scoped to the list of IDs). + # + # Given the ids [10, 20, 30] this method produces a SQL query something like + # the following: + # + # UPDATE issues + # SET position = CASE + # WHEN id = 10 THEN 1 + # WHEN id = 20 THEN 2 + # WHEN id = 30 THEN 3 + # ELSE position + # END + # WHERE id IN (10, 20, 30); + # + # This method expects that the IDs given in `ids` are already Fixnums. + def sort_issues(ids) + pairs = [] + + ids.each_with_index do |id, index| + pairs << id + pairs << index + 1 + end + + conditions = 'WHEN id = ? THEN ? ' * ids.length + + issues.where(id: ids). + update_all(["position = CASE #{conditions} ELSE position END", *pairs]) + end end diff --git a/spec/benchmarks/models/milestone_spec.rb b/spec/benchmarks/models/milestone_spec.rb new file mode 100644 index 00000000000..a94afc4c40d --- /dev/null +++ b/spec/benchmarks/models/milestone_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Milestone, benchmark: true do + describe '#sort_issues' do + let(:milestone) { create(:milestone) } + + let(:issue1) { create(:issue, milestone: milestone) } + let(:issue2) { create(:issue, milestone: milestone) } + let(:issue3) { create(:issue, milestone: milestone) } + + let(:issue_ids) { [issue3.id, issue2.id, issue1.id] } + + benchmark_subject { milestone.sort_issues(issue_ids) } + + it { is_expected.to iterate_per_second(500) } + end +end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index c88d5349663..77c58627322 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -140,4 +140,32 @@ describe Milestone do end end + describe '#sort_issues' do + let(:milestone) { create(:milestone) } + + let(:issue1) { create(:issue, milestone: milestone, position: 1) } + let(:issue2) { create(:issue, milestone: milestone, position: 2) } + let(:issue3) { create(:issue, milestone: milestone, position: 3) } + let(:issue4) { create(:issue, position: 42) } + + it 'sorts the given issues' do + milestone.sort_issues([issue3.id, issue2.id, issue1.id]) + + issue1.reload + issue2.reload + issue3.reload + + expect(issue1.position).to eq(3) + expect(issue2.position).to eq(2) + expect(issue3.position).to eq(1) + end + + it 'ignores issues not part of the milestone' do + milestone.sort_issues([issue3.id, issue2.id, issue1.id, issue4.id]) + + issue4.reload + + expect(issue4.position).to eq(42) + end + end end From 157d891615bbc9d1176b2149cfa5193b9aa4773c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 19 Oct 2015 11:40:13 +0200 Subject: [PATCH 28/28] Add changelog item --- CHANGELOG | 1 + app/controllers/projects_controller.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index f8daa6d246c..cfb7a581e81 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.2.0 (unreleased) - Show last project commit to default branch on project home page - Highlight comment based on anchor in URL - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw) + - Allow users to select the Files view as default project view (Cristian Bica) v 8.1.0 (unreleased) - Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge. diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 73200396ecc..9f0cce468b1 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -246,6 +246,8 @@ class ProjectsController < ApplicationController project.repository_exists? && !project.empty_repo? end + # Override get_id from ExtractsPath, which returns the branch and file path + # for the blob/tree, which in this case is just the root of the default branch. def get_id project.repository.root_ref end