diff options
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/login.scss | 17 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/pipelines.scss | 14 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 4 | ||||
-rw-r--r-- | app/helpers/merge_requests_helper.rb | 10 | ||||
-rw-r--r-- | app/models/merge_request.rb | 25 | ||||
-rw-r--r-- | app/views/devise/sessions/_new_base.html.haml | 4 | ||||
-rw-r--r-- | app/views/projects/ci/builds/_build_pipeline.html.haml | 4 | ||||
-rw-r--r-- | app/views/projects/commit/_pipeline_status_group.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml | 13 | ||||
-rw-r--r-- | app/views/projects/merge_requests/_show.html.haml | 24 | ||||
-rw-r--r-- | spec/features/merge_requests/created_from_fork_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 40 |
13 files changed, 87 insertions, 85 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9135b851f96..0bfdf23b959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Please view this file on the master branch, on stable branches it's out of date. - ProjectCacheWorker updates caches at most once per 15 minutes per project - Fix Error 500 when viewing old merge requests with bad diff data - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar) + - Fix viewing merged MRs when the source project has been removed !6991 - Speed-up group milestones show page - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps) - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index e6d9be5185d..bdb13bee178 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -53,6 +53,7 @@ margin: 0 0 10px; } + .login-footer { margin-top: 10px; @@ -246,3 +247,19 @@ padding: 65px; // height of footer + bottom padding of email confirmation link } } + +// For sign in pane only, to improve tab order, the following removes the submit button from +// normal document flow and pins it to the bottom of the form. For context, see !6867 & !6928 + +.login-box { + .new_user { + position: relative; + padding-bottom: 35px; + } + + .move-submit-down { + position: absolute; + width: 100%; + bottom: 0; + } +} diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index dfaf1ab732d..5b8dc7f8c40 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -369,10 +369,6 @@ &:hover { background-color: $gray-lighter; - - .dropdown-menu-toggle { - background-color: transparent; - } } &.playable { @@ -402,6 +398,15 @@ } } + .tooltip { + white-space: nowrap; + + .tooltip-inner { + overflow: hidden; + text-overflow: ellipsis; + } + } + .ci-status-text { width: 135px; white-space: nowrap; @@ -419,6 +424,7 @@ } .dropdown-menu-toggle { + background-color: transparent; border: none; width: auto; padding: 0; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0f593d1a936..2ee53f7ceda 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -398,7 +398,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController status ||= "preparing" else - ci_service = @merge_request.source_project.ci_service + ci_service = @merge_request.source_project.try(:ci_service) status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service if ci_service.respond_to?(:commit_coverage) @@ -554,7 +554,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_pipelines_vars @pipelines = @merge_request.all_pipelines - if @pipelines.any? + if @pipelines.present? @pipeline = @pipelines.first @statuses = @pipeline.statuses.relevant end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 249cb44e9d5..a6659ea2fd1 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -86,11 +86,15 @@ module MergeRequestsHelper end def source_branch_with_namespace(merge_request) - branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch)) + namespace = merge_request.source_project_namespace + branch = merge_request.source_branch + + if merge_request.source_branch_exists? + namespace = link_to(namespace, project_path(merge_request.source_project)) + branch = link_to(branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch)) + end if merge_request.for_fork? - namespace = link_to(merge_request.source_project_namespace, - project_path(merge_request.source_project)) namespace + ":" + branch else branch diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0cc0b3c2a0e..c476a3bb14e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -326,21 +326,17 @@ class MergeRequest < ActiveRecord::Base def validate_fork return true unless target_project && source_project return true if target_project == source_project - return true unless forked_source_project_missing? + return true unless source_project_missing? errors.add :validate_fork, 'Source project is not a fork of the target project' end def closed_without_fork? - closed? && forked_source_project_missing? + closed? && source_project_missing? end - def closed_without_source_project? - closed? && !source_project - end - - def forked_source_project_missing? + def source_project_missing? return false unless for_fork? return true unless source_project @@ -348,9 +344,7 @@ class MergeRequest < ActiveRecord::Base end def reopenable? - return false if closed_without_fork? || closed_without_source_project? || merged? - - closed? + closed? && !source_project_missing? && source_branch_exists? end def ensure_merge_request_diff @@ -662,7 +656,7 @@ class MergeRequest < ActiveRecord::Base end def has_ci? - source_project.ci_service && commits.any? + source_project.try(:ci_service) && commits.any? end def branch_missing? @@ -694,12 +688,9 @@ class MergeRequest < ActiveRecord::Base @environments ||= begin - environments = source_project.environments_for( - source_branch, diff_head_commit) - environments += target_project.environments_for( - target_branch, diff_head_commit, with_tags: true) - - environments.uniq + envs = target_project.environments_for(target_branch, diff_head_commit, with_tags: true) + envs.concat(source_project.environments_for(source_branch, diff_head_commit)) if source_project + envs.uniq end end diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index a96b579c593..525e7d99d71 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -5,6 +5,8 @@ %div.form-group = f.label :password = f.password_field :password, class: "form-control bottom", required: true, title: "This field is required." + %div.submit-container.move-submit-down + = f.submit "Sign in", class: "btn btn-save" - if devise_mapping.rememberable? .remember-me.checkbox %label{for: "user_remember_me"} @@ -12,5 +14,3 @@ %span Remember me .pull-right = link_to "Forgot your password?", new_password_path(resource_name) - %div.submit-container - = f.submit "Sign in", class: "btn btn-save" diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml index 017d3ff6af2..55965172d3f 100644 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ b/app/views/projects/ci/builds/_build_pipeline.html.haml @@ -1,10 +1,10 @@ - is_playable = subject.playable? && can?(current_user, :update_build, @project) - if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: 'Play' do + = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.pipeline-graph', placement: 'bottom' } do = render_status_with_link('build', 'play') .ci-status-text= subject.name - elsif can?(current_user, :read_build, @project) - = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject) do + = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do %span.ci-status-icon = render_status_with_link('build', subject.status) .ci-status-text= subject.name diff --git a/app/views/projects/commit/_pipeline_status_group.html.haml b/app/views/projects/commit/_pipeline_status_group.html.haml index 5d0d5ba0262..f2d71fa6989 100644 --- a/app/views/projects/commit/_pipeline_status_group.html.haml +++ b/app/views/projects/commit/_pipeline_status_group.html.haml @@ -1,5 +1,5 @@ - group_status = CommitStatus.where(id: subject).status -%button.dropdown-menu-toggle{ type: 'button', data: { toggle: 'dropdown' } } +%button.dropdown-menu-toggle.has-tooltip{ type: 'button', data: { toggle: 'dropdown', title: "#{name} - #{group_status}" } } %span.ci-status-icon = render_status_with_link('build', group_status) %span.ci-status-text diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml index 0a66d60accc..c45b73e4225 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml @@ -1,9 +1,10 @@ -- if subject.target_url - = link_to subject.target_url do +%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } } + - if subject.target_url + = link_to subject.target_url do + %span.ci-status-icon + = render_status_with_link('commit status', subject.status) + %span.ci-status-text= subject.name + - else %span.ci-status-icon = render_status_with_link('commit status', subject.status) %span.ci-status-text= subject.name -- else - %span.ci-status-icon - = render_status_with_link('commit status', subject.status) - %span.ci-status-text= subject.name diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index cd98aaf8d75..0e19d224fcd 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -26,19 +26,19 @@ %ul.dropdown-menu.dropdown-menu-align-right %li= link_to "Email Patches", merge_request_path(@merge_request, format: :patch) %li= link_to "Plain Diff", merge_request_path(@merge_request, format: :diff) - - unless @merge_request.closed_without_fork? - .normal - %span Request to merge - %span.label-branch= source_branch_with_namespace(@merge_request) - %span into - %span.label-branch - = link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch) - - if @merge_request.open? && @merge_request.diverged_from_target_branch? - %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) + .normal + %span Request to merge + %span.label-branch= source_branch_with_namespace(@merge_request) + %span into + %span.label-branch + = link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch) + - if @merge_request.open? && @merge_request.diverged_from_target_branch? + %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) - - unless @merge_request.closed_without_source_project? + - if @merge_request.source_branch_exists? = render "projects/merge_requests/show/how_to_merge" - = render "projects/merge_requests/widget/show.html.haml" + + = render "projects/merge_requests/widget/show.html.haml" - if @merge_request.source_branch_exists? && @merge_request.mergeable? && @merge_request.can_be_merged_by?(current_user) .light.prepend-top-default.append-bottom-default @@ -52,7 +52,7 @@ = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do Discussion %span.badge= @merge_request.mr_and_commit_notes.user.count - - unless @merge_request.closed_without_source_project? + - if @merge_request.source_project %li.commits-tab = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do Commits diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index a506624b30d..cfc1244429f 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -25,6 +25,20 @@ feature 'Merge request created from fork' do expect(page).to have_content 'Test merge request' end + context 'source project is deleted' do + background do + MergeRequests::MergeService.new(project, user).execute(merge_request) + fork_project.destroy! + end + + scenario 'user can access merge request' do + visit_merge_request(merge_request) + + expect(page).to have_content 'Test merge request' + expect(page).to have_content "(removed):#{merge_request.source_branch}" + end + end + context 'pipeline present in source project' do include WaitForAjax diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6db5e7f7d80..6e5137602aa 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1198,7 +1198,7 @@ describe MergeRequest, models: true do end end - describe "#forked_source_project_missing?" do + describe "#source_project_missing?" do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } let(:user) { create(:user) } @@ -1211,13 +1211,13 @@ describe MergeRequest, models: true do target_project: project) end - it { expect(merge_request.forked_source_project_missing?).to be_falsey } + it { expect(merge_request.source_project_missing?).to be_falsey } end context "when the source project is the same as the target project" do let(:merge_request) { create(:merge_request, source_project: project) } - it { expect(merge_request.forked_source_project_missing?).to be_falsey } + it { expect(merge_request.source_project_missing?).to be_falsey } end context "when the fork does not exist" do @@ -1231,7 +1231,7 @@ describe MergeRequest, models: true do unlink_project.execute merge_request.reload - expect(merge_request.forked_source_project_missing?).to be_truthy + expect(merge_request.source_project_missing?).to be_truthy end end end @@ -1274,38 +1274,6 @@ describe MergeRequest, models: true do end end - describe '#closed_without_source_project?' do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) } - let(:destroy_service) { Projects::DestroyService.new(fork_project, user) } - - context 'when the merge request is closed' do - let(:closed_merge_request) do - create(:closed_merge_request, - source_project: fork_project, - target_project: project) - end - - it 'returns false if the source project exists' do - expect(closed_merge_request.closed_without_source_project?).to be_falsey - end - - it 'returns true if the source project does not exist' do - destroy_service.execute - closed_merge_request.reload - - expect(closed_merge_request.closed_without_source_project?).to be_truthy - end - end - - context 'when the merge request is open' do - it 'returns false' do - expect(subject.closed_without_source_project?).to be_falsey - end - end - end - describe '#reopenable?' do context 'when the merge request is closed' do it 'returns true' do |