diff options
30 files changed, 607 insertions, 666 deletions
diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index 4c9ad128e6c..77e92ff8caf 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -22,6 +22,7 @@ $(() => { } $('body').on('click', '.js-toggle-button', function toggleButton(e) { + e.target.classList.toggle('open'); toggleContainer($(this).closest('.js-toggle-container')); const targetTag = e.currentTarget.tagName.toLowerCase(); diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index abb092623c0..2071d4dcfce 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -155,7 +155,7 @@ header { .header-logo { display: inline-block; - margin: 0 7px 0 2px; + margin: 0 12px 0 2px; position: relative; top: 10px; transition-duration: .3s; @@ -186,7 +186,7 @@ header { display: flex; align-items: flex-start; flex: 1 1 auto; - padding-top: (($header-height - 19) / 2); + padding-top: 14px; overflow: hidden; } diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 0dad91ba128..9e3142c8aa3 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -135,7 +135,7 @@ .text-expander { display: inline-block; - background: $gray-light; + background: $white-light; color: $gl-text-color-secondary; padding: 0 5px; cursor: pointer; @@ -146,6 +146,11 @@ line-height: $gl-font-size; outline: none; + &.open { + background: $gray-light; + box-shadow: inset 0 0 2px rgba($black, 0.2); + } + &:hover { background-color: darken($gray-light, 10%); text-decoration: none; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 220b6d6f141..2ea2ff8362b 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -123,6 +123,9 @@ ul.notes { } .note-emoji-button { + position: relative; + line-height: 1; + .fa-spinner { display: none; } @@ -443,7 +446,8 @@ ul.notes { .award-control-icon-positive, .award-control-icon-super-positive { position: absolute; - margin-left: -20px; + top: 0; + left: 0; opacity: 0; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 717ebb44a23..a9e2a034790 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -929,27 +929,23 @@ pre.light-well { } .variable-key { - width: 300px; - max-width: 300px; + max-width: 120px; overflow: hidden; word-wrap: break-word; - - // override bootstrap - white-space: normal!important; - - @media (max-width: $screen-sm-max) { - width: 150px; - max-width: 150px; - } + white-space: nowrap; + text-overflow: ellipsis; } .variable-value { - @media(max-width: $screen-xs-max) { - width: 150px; - max-width: 150px; - overflow: hidden; - word-wrap: break-word; - } + max-width: 150px; + overflow: hidden; + word-wrap: break-word; + white-space: nowrap; + text-overflow: ellipsis; + } + + .variable-menu { + text-align: right; } } diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 6c3f3a61e0a..4b3ab03a69c 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -34,7 +34,7 @@ module BlobHelper if !on_top_of_branch?(project, ref) button_tag 'Edit', class: "#{common_classes} disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' } # This condition applies to anonymous or users who can edit directly - elsif !current_user || (current_user && can_edit_blob?(blob, project, ref)) + elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) link_to 'Edit', edit_path(project, ref, path, options), class: "#{common_classes} btn-sm" elsif current_user && can?(current_user, :fork_project, project) button_tag 'Edit', class: "#{common_classes} js-edit-blob-link-fork-toggler" @@ -52,7 +52,7 @@ module BlobHelper button_tag label, class: "btn btn-#{btn_class} disabled has-tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' } elsif blob.lfs_pointer? button_tag label, class: "btn btn-#{btn_class} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } - elsif can_edit_blob?(blob, project, ref) + elsif can_modify_blob?(blob, project, ref) button_tag label, class: "btn btn-#{btn_class}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' elsif can?(current_user, :fork_project, project) continue_params = { @@ -90,7 +90,7 @@ module BlobHelper ) end - def can_edit_blob?(blob, project = @project, ref = @ref) + def can_modify_blob?(blob, project = @project, ref = @ref) !blob.lfs_pointer? && can_edit_tree?(project, ref) end diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index 8c02b4061ca..979264c9421 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -42,7 +42,7 @@ module SnippetsHelper 0, lined_content.size, surrounding_lines - ) if line.include?(query) + ) if line.downcase.include?(query.downcase) end used_lines.uniq.sort diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 4c72d5e117d..eea17e24903 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -59,7 +59,6 @@ module Projects project.repository.add_remote(project.import_type, project.import_url) project.repository.set_remote_as_mirror(project.import_type) project.repository.fetch_remote(project.import_type, forced: true) - project.repository.remove_remote(project.import_type) end def import_data diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index b6738c3380f..b9b3f3ec7a3 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -8,7 +8,7 @@ #tree-holder.tree-holder = render 'blob', blob: @blob - - if can_edit_blob?(@blob) + - if can_modify_blob?(@blob) = render 'projects/blob/remove' - title = "Replace #{@blob.name}" diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml index 5c38b5ad9c0..438a98c3e95 100644 --- a/app/views/projects/diffs/_content.html.haml +++ b/app/views/projects/diffs/_content.html.haml @@ -4,7 +4,7 @@ - if diff_file.too_large? .nothing-here-block This diff could not be displayed because it is too large. - elsif blob.only_display_raw? - .nothing-here-block This file is too large to display. + .nothing-here-block The file could not be displayed because it is too large. - elsif blob_text_viewable?(blob) - if !project.repository.diffable?(blob) .nothing-here-block This diff was suppressed by a .gitattributes entry. diff --git a/app/views/projects/snippets/edit.html.haml b/app/views/projects/snippets/edit.html.haml index fb39028529d..24b92094b7d 100644 --- a/app/views/projects/snippets/edit.html.haml +++ b/app/views/projects/snippets/edit.html.haml @@ -1,4 +1,4 @@ -- page_title "Edit", @snippet.title, "Snippets" +- page_title "Edit", "#{@snippet.title} (#{@snippet.to_reference})", "Snippets" %h3.page-title Edit Snippet diff --git a/app/views/projects/snippets/show.html.haml b/app/views/projects/snippets/show.html.haml index e35385f4cab..7c6be003d4c 100644 --- a/app/views/projects/snippets/show.html.haml +++ b/app/views/projects/snippets/show.html.haml @@ -1,4 +1,4 @@ -- page_title @snippet.title, "Snippets" +- page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets" = render 'shared/snippets/header' diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml index c7cebf45160..0ce597dcf21 100644 --- a/app/views/projects/variables/_table.html.haml +++ b/app/views/projects/variables/_table.html.haml @@ -14,7 +14,7 @@ %tr %td.variable-key= variable.key %td.variable-value{ "data-value" => variable.value }****** - %td + %td.variable-menu = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent btn-variable-edit" do %span.sr-only Update diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml index e010f21de5a..fc4385865a4 100644 --- a/app/views/search/results/_issue.html.haml +++ b/app/views/search/results/_issue.html.haml @@ -3,6 +3,8 @@ = confidential_icon(issue) = link_to [issue.project.namespace.becomes(Namespace), issue.project, issue] do %span.term.str-truncated= issue.title + - if issue.closed? + %span.label.label-danger.prepend-left-5 Closed .pull-right ##{issue.iid} - if issue.description.present? .description.term @@ -10,6 +12,3 @@ = search_md_sanitize(issue, :description) %span.light #{issue.project.name_with_namespace} - - if issue.closed? - .pull-right - %span.label.label-danger Closed diff --git a/app/views/search/results/_merge_request.html.haml b/app/views/search/results/_merge_request.html.haml index 2e6adf3027c..9b583285d02 100644 --- a/app/views/search/results/_merge_request.html.haml +++ b/app/views/search/results/_merge_request.html.haml @@ -2,6 +2,10 @@ %h4 = link_to [merge_request.target_project.namespace.becomes(Namespace), merge_request.target_project, merge_request] do %span.term.str-truncated= merge_request.title + - if merge_request.merged? + %span.label.label-primary.prepend-left-5 Merged + - elsif merge_request.closed? + %span.label.label-danger.prepend-left-5 Closed .pull-right= merge_request.to_reference - if merge_request.description.present? .description.term @@ -9,8 +13,3 @@ = search_md_sanitize(merge_request, :description) %span.light #{merge_request.project.name_with_namespace} - .pull-right - - if merge_request.merged? - %span.label.label-primary Merged - - elsif merge_request.closed? - %span.label.label-danger Closed diff --git a/app/views/snippets/edit.html.haml b/app/views/snippets/edit.html.haml index 915bf98eb3e..18ebeb78f87 100644 --- a/app/views/snippets/edit.html.haml +++ b/app/views/snippets/edit.html.haml @@ -1,4 +1,4 @@ -- page_title "Edit", @snippet.title, "Snippets" +- page_title "Edit", "#{@snippet.title} (#{@snippet.to_reference})", "Snippets" %h3.page-title Edit Snippet %hr diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index da9fb755a36..e5711ca79c7 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -1,4 +1,4 @@ -- page_title @snippet.title, "Snippets" +- page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets" = render 'shared/snippets/header' diff --git a/changelogs/unreleased/28575-expand-collapse-look.yml b/changelogs/unreleased/28575-expand-collapse-look.yml new file mode 100644 index 00000000000..d8943316300 --- /dev/null +++ b/changelogs/unreleased/28575-expand-collapse-look.yml @@ -0,0 +1,4 @@ +--- +title: Expand/collapse button -> Change to make it look like a toggle +merge_request: 10720 +author: Jacopo Beschi @jacopo-beschi diff --git a/changelogs/unreleased/move-search-labels.yml b/changelogs/unreleased/move-search-labels.yml new file mode 100644 index 00000000000..3a1d23d622e --- /dev/null +++ b/changelogs/unreleased/move-search-labels.yml @@ -0,0 +1,4 @@ +--- +title: Move labels of search results from bottom to title +merge_request: 10705 +author: dr diff --git a/config/routes/project.rb b/config/routes/project.rb index f5009186344..fa92202c1ea 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -42,29 +42,6 @@ constraints(ProjectUrlConstrainer.new) do resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains', constraints: { id: /[^\/]+/ } end - resources :compare, only: [:index, :create] do - collection do - get :diff_for_path - end - end - - get '/compare/:from...:to', to: 'compare#show', as: 'compare', constraints: { from: /.+/, to: /.+/ } - - # Don't use format parameter as file extension (old 3.0.x behavior) - # See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments - scope format: false do - resources :network, only: [:show], constraints: { id: Gitlab::Regex.git_reference_regex } - - resources :graphs, only: [:show], constraints: { id: Gitlab::Regex.git_reference_regex } do - member do - get :charts - get :commits - get :ci - get :languages - end - end - end - resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do member do get 'raw' @@ -128,15 +105,6 @@ constraints(ProjectUrlConstrainer.new) do end end - resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } - delete :merged_branches, controller: 'branches', action: :destroy_all_merged - resources :tags, only: [:index, :show, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do - resource :release, only: [:edit, :update] - end - - resources :protected_branches, only: [:index, :show, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } - resources :protected_tags, only: [:index, :show, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } - resources :variables, only: [:index, :show, :update, :create, :destroy] resources :triggers, only: [:index, :create, :edit, :update, :destroy] do member do diff --git a/config/routes/repository.rb b/config/routes/repository.rb index f8966c5ae75..5cf37a06e97 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -1,4 +1,4 @@ -# All routing related to repositoty browsing +# All routing related to repository browsing resource :repository, only: [:create] do member do @@ -6,83 +6,84 @@ resource :repository, only: [:create] do end end -resources :refs, only: [] do - collection do - get 'switch' +# Don't use format parameter as file extension (old 3.0.x behavior) +# See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments +scope format: false do + get '/compare/:from...:to', to: 'compare#show', as: 'compare', constraints: { from: /.+/, to: /.+/ } + + resources :compare, only: [:index, :create] do + collection do + get :diff_for_path + end end - member do - # tree viewer logs - get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex } - # Directories with leading dots erroneously get rejected if git - # ref regex used in constraints. Regex verification now done in controller. - get 'logs_tree/*path' => 'refs#logs_tree', as: :logs_file, constraints: { - id: /.*/, - path: /.*/ - } + resources :refs, only: [] do + collection do + get 'switch' + end + + member do + # tree viewer logs + get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex } + # Directories with leading dots erroneously get rejected if git + # ref regex used in constraints. Regex verification now done in controller. + get 'logs_tree/*path', action: :logs_tree, as: :logs_file, format: false, constraints: { + id: /.*/, + path: /.*/ + } + end end -end -get '/new/*id', to: 'blob#new', constraints: { id: /.+/ }, as: 'new_blob' -post '/create/*id', to: 'blob#create', constraints: { id: /.+/ }, as: 'create_blob' -get '/edit/*id', to: 'blob#edit', constraints: { id: /.+/ }, as: 'edit_blob' -put '/update/*id', to: 'blob#update', constraints: { id: /.+/ }, as: 'update_blob' -post '/preview/*id', to: 'blob#preview', constraints: { id: /.+/ }, as: 'preview_blob' + scope constraints: { id: Gitlab::Regex.git_reference_regex } do + resources :network, only: [:show] -scope('/blob/*id', as: :blob, controller: :blob, constraints: { id: /.+/, format: false }) do - get :diff - get '/', action: :show - delete '/', action: :destroy - post '/', action: :create - put '/', action: :update -end + resources :graphs, only: [:show] do + member do + get :charts + get :commits + get :ci + get :languages + end + end -get( - '/raw/*id', - to: 'raw#show', - constraints: { id: /.+/, format: /(html|js)/ }, - as: :raw -) + resources :branches, only: [:index, :new, :create, :destroy] + delete :merged_branches, controller: 'branches', action: :destroy_all_merged + resources :tags, only: [:index, :show, :new, :create, :destroy] do + resource :release, only: [:edit, :update] + end -get( - '/tree/*id', - to: 'tree#show', - constraints: { id: /.+/, format: /(html|js)/ }, - as: :tree -) + resources :protected_branches, only: [:index, :show, :create, :update, :destroy] + resources :protected_tags, only: [:index, :show, :create, :update, :destroy] + end + + scope constraints: { id: /.+/ } do + scope controller: :blob do + get '/new/*id', action: :new, as: :new_blob + post '/create/*id', action: :create, as: :create_blob + get '/edit/*id', action: :edit, as: :edit_blob + put '/update/*id', action: :update, as: :update_blob + post '/preview/*id', action: :preview, as: :preview_blob -get( - '/find_file/*id', - to: 'find_file#show', - constraints: { id: /.+/, format: /html/ }, - as: :find_file -) + scope path: '/blob/*id', as: :blob do + get :diff + get '/', action: :show + delete '/', action: :destroy + post '/', action: :create + put '/', action: :update + end + end -get( - '/files/*id', - to: 'find_file#list', - constraints: { id: /(?:[^.]|\.(?!json$))+/, format: /json/ }, - as: :files -) + get '/tree/*id', to: 'tree#show', as: :tree + get '/raw/*id', to: 'raw#show', as: :raw + get '/blame/*id', to: 'blame#show', as: :blame + get '/commits/*id', to: 'commits#show', as: :commits -post( - '/create_dir/*id', - to: 'tree#create_dir', - constraints: { id: /.+/ }, - as: 'create_dir' -) + post '/create_dir/*id', to: 'tree#create_dir', as: :create_dir -get( - '/blame/*id', - to: 'blame#show', - constraints: { id: /.+/, format: /(html|js)/ }, - as: :blame -) + scope controller: :find_file do + get '/find_file/*id', action: :show, as: :find_file -# File/dir history -get( - '/commits/*id', - to: 'commits#show', - constraints: { id: /.+/, format: false }, - as: :commits -) + get '/files/*id', action: :list, as: :files + end + end +end diff --git a/doc/workflow/importing/import_projects_from_github.md b/doc/workflow/importing/import_projects_from_github.md index aece4ab34ba..8ed1d98d05b 100644 --- a/doc/workflow/importing/import_projects_from_github.md +++ b/doc/workflow/importing/import_projects_from_github.md @@ -10,6 +10,11 @@ in your GitLab instance sitewide. This configuration is optional, users will still be able to import their GitHub repositories with a
[personal access token][gh-token].
+>**Note:**
+Administrators of a GitLab instance (Community or Enterprise Edition) can also
+use the [GitHub rake task][gh-rake] to import projects from GitHub without the
+constrains of a Sidekiq worker.
+
- At its current state, GitHub importer can import:
- the repository description (GitLab 7.7+)
- the Git repository data (GitLab 7.7+)
@@ -112,5 +117,6 @@ You can also choose a different name for the project and a different namespace, if you have the privileges to do so.
[gh-import]: ../../integration/github.md "GitHub integration"
+[gh-rake]: ../../administration/raketasks/github_import.md "GitHub rake task"
[gh-integration]: #authorize-access-to-your-repositories-using-the-github-integration
[gh-token]: #authorize-access-to-your-repositories-using-a-personal-access-token
diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index 15131fbf755..a9dad6a1bf0 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -52,7 +52,6 @@ class NewImporter < ::Gitlab::GithubImport::Importer project.repository.add_remote(project.import_type, project.import_url) project.repository.set_remote_as_mirror(project.import_type) project.repository.fetch_remote(project.import_type, forced: true) - project.repository.remove_remote(project.import_type) rescue => e # Expire cache to prevent scenarios such as: # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true diff --git a/spec/features/merge_requests/diff_notes_spec.rb b/spec/features/merge_requests/diff_notes_spec.rb deleted file mode 100644 index 06fad1007e8..00000000000 --- a/spec/features/merge_requests/diff_notes_spec.rb +++ /dev/null @@ -1,238 +0,0 @@ -require 'spec_helper' - -feature 'Diff notes', js: true, feature: true do - include WaitForAjax - - before do - login_as :admin - @merge_request = create(:merge_request) - @project = @merge_request.source_project - end - - context 'merge request diffs' do - let(:comment_button_class) { '.add-diff-note' } - let(:notes_holder_input_class) { 'js-temp-notes-holder' } - let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' } - let(:test_note_comment) { 'this is a test note!' } - - context 'when hovering over a parallel view diff file' do - before(:each) do - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'parallel') - end - - context 'with an old line on the left and no line on the right' do - it 'should allow commenting on the left side' do - should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left') - end - - it 'should not allow commenting on the right side' do - should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right') - end - end - - context 'with no line on the left and a new line on the right' do - it 'should not allow commenting on the left side' do - should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left') - end - - it 'should allow commenting on the right side' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right') - end - end - - context 'with an old line on the left and a new line on the right' do - it 'should allow commenting on the left side' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left') - end - - it 'should allow commenting on the right side' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right') - end - end - - context 'with an unchanged line on the left and an unchanged line on the right' do - it 'should allow commenting on the left side' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left') - end - - it 'should allow commenting on the right side' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'right') - end - end - - context 'with a match line' do - it 'should not allow commenting on the left side' do - should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'left') - end - - it 'should not allow commenting on the right side' do - should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'right') - end - end - - context 'with an unfolded line' do - before(:each) do - find('.js-unfold', match: :first).click - wait_for_ajax - end - - # The first `.js-unfold` unfolds upwards, therefore the first - # `.line_holder` will be an unfolded line. - let(:line_holder) { first('.line_holder[id="1"]') } - - it 'should not allow commenting on the left side' do - should_not_allow_commenting(line_holder, 'left') - end - - it 'should not allow commenting on the right side' do - should_not_allow_commenting(line_holder, 'right') - end - end - end - - context 'when hovering over an inline view diff file' do - before do - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'inline') - end - - context 'with a new line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) - end - end - - context 'with an old line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) - end - end - - context 'with an unchanged line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) - end - end - - context 'with a match line' do - it 'should not allow commenting' do - should_not_allow_commenting(find('.match', match: :first)) - end - end - - context 'with an unfolded line' do - before(:each) do - find('.js-unfold', match: :first).click - wait_for_ajax - end - - # The first `.js-unfold` unfolds upwards, therefore the first - # `.line_holder` will be an unfolded line. - let(:line_holder) { first('.line_holder[id="1"]') } - - it 'should not allow commenting' do - should_not_allow_commenting line_holder - end - end - - context 'when hovering over a diff discussion' do - before do - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'inline') - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) - visit namespace_project_merge_request_path(@project.namespace, @project, @merge_request) - end - - it 'should not allow commenting' do - should_not_allow_commenting(find('.line_holder', match: :first)) - end - end - end - - context 'when the MR only supports legacy diff notes' do - before do - @merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) - visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'inline') - end - - context 'with a new line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) - end - end - - context 'with an old line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) - end - end - - context 'with an unchanged line' do - it 'should allow commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) - end - end - - context 'with a match line' do - it 'should not allow commenting' do - should_not_allow_commenting(find('.match', match: :first)) - end - end - end - - def should_allow_commenting(line_holder, diff_side = nil) - line = get_line_components(line_holder, diff_side) - line[:content].hover - expect(line[:num]).to have_css comment_button_class - - comment_on_line(line_holder, line) - - assert_comment_persistence(line_holder) - end - - def should_not_allow_commenting(line_holder, diff_side = nil) - line = get_line_components(line_holder, diff_side) - line[:content].hover - expect(line[:num]).not_to have_css comment_button_class - end - - def get_line_components(line_holder, diff_side = nil) - if diff_side.nil? - get_inline_line_components(line_holder) - else - get_parallel_line_components(line_holder, diff_side) - end - end - - def get_inline_line_components(line_holder) - { content: line_holder.find('.line_content', match: :first), num: line_holder.find('.diff-line-num', match: :first) } - end - - def get_parallel_line_components(line_holder, diff_side = nil) - side_index = diff_side == 'left' ? 0 : 1 - # Wait for `.line_content` - line_holder.find('.line_content', match: :first) - # Wait for `.diff-line-num` - line_holder.find('.diff-line-num', match: :first) - { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] } - end - - def comment_on_line(line_holder, line) - line[:num].find(comment_button_class).trigger 'click' - line_holder.find(:xpath, notes_holder_input_xpath) - - notes_holder_input = line_holder.find(:xpath, notes_holder_input_xpath) - expect(notes_holder_input[:class]).to include(notes_holder_input_class) - - notes_holder_input.fill_in 'note[note]', with: test_note_comment - click_button 'Comment' - wait_for_ajax - end - - def assert_comment_persistence(line_holder) - expect(line_holder).to have_xpath notes_holder_input_xpath - - notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath) - expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class) - expect(notes_holder_saved).to have_content test_note_comment - end - end -end diff --git a/spec/features/merge_requests/user_posts_diff_notes_spec.rb b/spec/features/merge_requests/user_posts_diff_notes_spec.rb new file mode 100644 index 00000000000..7756202e3f5 --- /dev/null +++ b/spec/features/merge_requests/user_posts_diff_notes_spec.rb @@ -0,0 +1,294 @@ +require 'spec_helper' + +feature 'Merge requests > User posts diff notes', :js do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.source_project } + + before do + project.add_developer(user) + login_as(user) + end + + let(:comment_button_class) { '.add-diff-note' } + let(:notes_holder_input_class) { 'js-temp-notes-holder' } + let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' } + let(:test_note_comment) { 'this is a test note!' } + + context 'when hovering over a parallel view diff file' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'parallel') + end + + context 'with an old line on the left and no line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right') + end + end + + context 'with no line on the left and a new line on the right' do + it 'does not allow commenting on the left side' do + should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right') + end + end + + context 'with an old line on the left and a new line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right') + end + end + + context 'with an unchanged line on the left and an unchanged line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'right') + end + end + + context 'with a match line' do + it 'does not allow commenting on the left side' do + should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'right') + end + end + + context 'with an unfolded line' do + before(:each) do + find('.js-unfold', match: :first).click + wait_for_ajax + end + + # The first `.js-unfold` unfolds upwards, therefore the first + # `.line_holder` will be an unfolded line. + let(:line_holder) { first('.line_holder[id="1"]') } + + it 'does not allow commenting on the left side' do + should_not_allow_commenting(line_holder, 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(line_holder, 'right') + end + end + end + + context 'when hovering over an inline view diff file' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'inline') + end + + context 'with a new line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + + context 'with an old line' do + it 'allows commenting' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + end + + context 'with an unchanged line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + end + end + + context 'with a match line' do + it 'does not allow commenting' do + should_not_allow_commenting(find('.match', match: :first)) + end + end + + context 'with an unfolded line' do + before(:each) do + find('.js-unfold', match: :first).click + wait_for_ajax + end + + # The first `.js-unfold` unfolds upwards, therefore the first + # `.line_holder` will be an unfolded line. + let(:line_holder) { first('.line_holder[id="1"]') } + + it 'does not allow commenting' do + should_not_allow_commenting line_holder + end + end + + context 'when hovering over a diff discussion' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'inline') + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not allow commenting' do + should_not_allow_commenting(find('.line_holder', match: :first)) + end + end + end + + context 'when cancelling the comment addition' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'inline') + end + + context 'with a new line' do + it 'allows dismissing a comment' do + should_allow_dismissing_a_comment(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + end + + describe 'with muliple note forms' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'inline') + click_diff_line(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + click_diff_line(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + + describe 'posting a note' do + it 'adds as discussion' do + expect(page).to have_css('.js-temp-notes-holder', count: 2) + + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false) + expect(page).to have_css('.notes_holder .note', count: 1) + expect(page).to have_css('.js-temp-notes-holder', count: 1) + expect(page).to have_button('Reply...') + end + end + end + + context 'when the MR only supports legacy diff notes' do + before do + merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: 'inline') + end + + context 'with a new line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + + context 'with an old line' do + it 'allows commenting' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + end + + context 'with an unchanged line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + end + end + + context 'with a match line' do + it 'does not allow commenting' do + should_not_allow_commenting(find('.match', match: :first)) + end + end + end + + def should_allow_commenting(line_holder, diff_side = nil, asset_form_reset: true) + write_comment_on_line(line_holder, diff_side) + + click_button 'Comment' + wait_for_ajax + + assert_comment_persistence(line_holder, asset_form_reset: asset_form_reset) + end + + def should_allow_dismissing_a_comment(line_holder, diff_side = nil) + write_comment_on_line(line_holder, diff_side) + + find('.js-close-discussion-note-form').trigger('click') + + assert_comment_dismissal(line_holder) + end + + def should_not_allow_commenting(line_holder, diff_side = nil) + line = get_line_components(line_holder, diff_side) + line[:content].hover + expect(line[:num]).not_to have_css comment_button_class + end + + def get_line_components(line_holder, diff_side = nil) + if diff_side.nil? + get_inline_line_components(line_holder) + else + get_parallel_line_components(line_holder, diff_side) + end + end + + def get_inline_line_components(line_holder) + { content: line_holder.find('.line_content', match: :first), num: line_holder.find('.diff-line-num', match: :first) } + end + + def get_parallel_line_components(line_holder, diff_side = nil) + side_index = diff_side == 'left' ? 0 : 1 + # Wait for `.line_content` + line_holder.find('.line_content', match: :first) + # Wait for `.diff-line-num` + line_holder.find('.diff-line-num', match: :first) + { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] } + end + + def click_diff_line(line_holder, diff_side = nil) + line = get_line_components(line_holder, diff_side) + line[:content].hover + + expect(line[:num]).to have_css comment_button_class + + line[:num].find(comment_button_class).trigger 'click' + end + + def write_comment_on_line(line_holder, diff_side) + click_diff_line(line_holder, diff_side) + + notes_holder_input = line_holder.find(:xpath, notes_holder_input_xpath) + + expect(notes_holder_input[:class]).to include(notes_holder_input_class) + + notes_holder_input.fill_in 'note[note]', with: test_note_comment + end + + def assert_comment_persistence(line_holder, asset_form_reset:) + notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath) + + expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class) + expect(notes_holder_saved).to have_content test_note_comment + + assert_form_is_reset if asset_form_reset + end + + def assert_comment_dismissal(line_holder) + expect(line_holder).not_to have_xpath notes_holder_input_xpath + expect(page).not_to have_content test_note_comment + + assert_form_is_reset + end + + def assert_form_is_reset + expect(page).to have_no_css('.js-temp-notes-holder') + end +end diff --git a/spec/features/merge_requests/user_posts_notes.rb b/spec/features/merge_requests/user_posts_notes.rb new file mode 100644 index 00000000000..c7cc4d6bc72 --- /dev/null +++ b/spec/features/merge_requests/user_posts_notes.rb @@ -0,0 +1,145 @@ +require 'spec_helper' + +describe 'Merge requests > User posts notes', :js do + let(:project) { create(:project) } + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + let!(:note) do + create(:note_on_merge_request, :with_attachment, noteable: merge_request, + project: project) + end + + before do + login_as :admin + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + subject { page } + + describe 'the note form' do + it 'is valid' do + is_expected.to have_css('.js-main-target-form', visible: true, count: 1) + expect(find('.js-main-target-form .js-comment-button').value). + to eq('Comment') + page.within('.js-main-target-form') do + expect(page).not_to have_link('Cancel') + end + end + + describe 'with text' do + before do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: 'This is awesome' + end + end + + it 'has enable submit button and preview button' do + page.within('.js-main-target-form') do + expect(page).not_to have_css('.js-comment-button[disabled]') + expect(page).to have_css('.js-md-preview-button', visible: true) + end + end + end + end + + describe 'when posting a note' do + before do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: 'This is awesome!' + find('.js-md-preview-button').click + click_button 'Comment' + end + end + + it 'is added and form reset' do + is_expected.to have_content('This is awesome!') + page.within('.js-main-target-form') do + expect(page).to have_no_field('note[note]', with: 'This is awesome!') + expect(page).to have_css('.js-md-preview', visible: :hidden) + end + page.within('.js-main-target-form') do + is_expected.to have_css('.js-note-text', visible: true) + end + end + end + + describe 'when editing a note' do + it 'there should be a hidden edit form' do + is_expected.to have_css('.note-edit-form:not(.mr-note-edit-form)', visible: false, count: 1) + is_expected.to have_css('.note-edit-form.mr-note-edit-form', visible: false, count: 1) + end + + describe 'editing the note' do + before do + find('.note').hover + find('.js-note-edit').click + end + + it 'shows the note edit form and hide the note body' do + page.within("#note_#{note.id}") do + expect(find('.current-note-edit-form', visible: true)).to be_visible + expect(find('.note-edit-form', visible: true)).to be_visible + expect(find(:css, '.note-body > .note-text', visible: false)).not_to be_visible + end + end + + it 'resets the edit note form textarea with the original content of the note if cancelled' do + within('.current-note-edit-form') do + fill_in 'note[note]', with: 'Some new content' + find('.btn-cancel').click + expect(find('.js-note-text', visible: false).text).to eq '' + end + end + + it 'allows using markdown buttons after saving a note and then trying to edit it again' do + page.within('.current-note-edit-form') do + fill_in 'note[note]', with: 'This is the new content' + find('.btn-save').click + end + + find('.note').hover + find('.js-note-edit').click + + page.within('.current-note-edit-form') do + expect(find('#note_note').value).to eq('This is the new content') + find('.js-md:first-child').click + expect(find('#note_note').value).to eq('This is the new content****') + end + end + + it 'appends the edited at time to the note' do + page.within('.current-note-edit-form') do + fill_in 'note[note]', with: 'Some new content' + find('.btn-save').click + end + + page.within("#note_#{note.id}") do + is_expected.to have_css('.note_edited_ago') + expect(find('.note_edited_ago').text). + to match(/less than a minute ago/) + end + end + end + + describe 'deleting an attachment' do + before do + find('.note').hover + find('.js-note-edit').click + end + + it 'shows the delete link' do + page.within('.note-attachment') do + is_expected.to have_css('.js-note-attachment-delete') + end + end + + it 'removes the attachment div and resets the edit form' do + find('.js-note-attachment-delete').click + is_expected.not_to have_css('.note-attachment') + is_expected.not_to have_css('.current-note-edit-form') + wait_for_ajax + end + end + end +end diff --git a/spec/features/merge_requests/user_sees_system_notes_spec.rb b/spec/features/merge_requests/user_sees_system_notes_spec.rb new file mode 100644 index 00000000000..55d0f9d728c --- /dev/null +++ b/spec/features/merge_requests/user_sees_system_notes_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +feature 'Merge requests > User sees system notes' do + let(:public_project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:issue) { create(:issue, project: private_project) } + let(:merge_request) { create(:merge_request, source_project: public_project, source_branch: 'markdown') } + let!(:note) { create(:note_on_merge_request, :system, noteable: merge_request, project: public_project, note: "mentioned in #{issue.to_reference(public_project)}") } + + context 'when logged-in as a member of the private project' do + before do + user = create(:user) + private_project.add_developer(user) + login_as(user) + end + + it 'shows the system note' do + visit namespace_project_merge_request_path(public_project.namespace, public_project, merge_request) + + expect(page).to have_css('.system-note') + end + end + + context 'when not logged-in' do + it 'hides the system note' do + visit namespace_project_merge_request_path(public_project.namespace, public_project, merge_request) + + expect(page).not_to have_css('.system-note') + end + end +end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb deleted file mode 100644 index 783f2e93909..00000000000 --- a/spec/features/notes_on_merge_requests_spec.rb +++ /dev/null @@ -1,285 +0,0 @@ -require 'spec_helper' - -describe 'Comments', feature: true do - include RepoHelpers - include WaitForAjax - - describe 'On a merge request', js: true, feature: true do - let!(:project) { create(:project) } - let!(:merge_request) do - create(:merge_request, source_project: project, target_project: project) - end - - let!(:note) do - create(:note_on_merge_request, :with_attachment, noteable: merge_request, - project: project) - end - - before do - login_as :admin - visit namespace_project_merge_request_path(project.namespace, project, merge_request) - end - - subject { page } - - describe 'the note form' do - it 'is valid' do - is_expected.to have_css('.js-main-target-form', visible: true, count: 1) - expect(find('.js-main-target-form .js-comment-button').value). - to eq('Comment') - page.within('.js-main-target-form') do - expect(page).not_to have_link('Cancel') - end - end - - describe 'with text' do - before do - page.within('.js-main-target-form') do - fill_in 'note[note]', with: 'This is awesome' - end - end - - it 'has enable submit button and preview button' do - page.within('.js-main-target-form') do - expect(page).not_to have_css('.js-comment-button[disabled]') - expect(page).to have_css('.js-md-preview-button', visible: true) - end - end - end - end - - describe 'when posting a note' do - before do - page.within('.js-main-target-form') do - fill_in 'note[note]', with: 'This is awsome!' - find('.js-md-preview-button').click - click_button 'Comment' - end - end - - it 'is added and form reset' do - is_expected.to have_content('This is awsome!') - page.within('.js-main-target-form') do - expect(page).to have_no_field('note[note]', with: 'This is awesome!') - expect(page).to have_css('.js-md-preview', visible: :hidden) - end - page.within('.js-main-target-form') do - is_expected.to have_css('.js-note-text', visible: true) - end - end - end - - describe 'when editing a note', js: true do - it 'there should be a hidden edit form' do - is_expected.to have_css('.note-edit-form:not(.mr-note-edit-form)', visible: false, count: 1) - is_expected.to have_css('.note-edit-form.mr-note-edit-form', visible: false, count: 1) - end - - describe 'editing the note' do - before do - find('.note').hover - find('.js-note-edit').click - end - - it 'shows the note edit form and hide the note body' do - page.within("#note_#{note.id}") do - expect(find('.current-note-edit-form', visible: true)).to be_visible - expect(find('.note-edit-form', visible: true)).to be_visible - expect(find(:css, '.note-body > .note-text', visible: false)).not_to be_visible - end - end - - it 'resets the edit note form textarea with the original content of the note if cancelled' do - within('.current-note-edit-form') do - fill_in 'note[note]', with: 'Some new content' - find('.btn-cancel').click - expect(find('.js-note-text', visible: false).text).to eq '' - end - end - - it 'allows using markdown buttons after saving a note and then trying to edit it again' do - page.within('.current-note-edit-form') do - fill_in 'note[note]', with: 'This is the new content' - find('.btn-save').click - end - - find('.note').hover - find('.js-note-edit').click - - page.within('.current-note-edit-form') do - expect(find('#note_note').value).to eq('This is the new content') - find('.js-md:first-child').click - expect(find('#note_note').value).to eq('This is the new content****') - end - end - - it 'appends the edited at time to the note' do - page.within('.current-note-edit-form') do - fill_in 'note[note]', with: 'Some new content' - find('.btn-save').click - end - - page.within("#note_#{note.id}") do - is_expected.to have_css('.note_edited_ago') - expect(find('.note_edited_ago').text). - to match(/less than a minute ago/) - end - end - end - - describe 'deleting an attachment' do - before do - find('.note').hover - find('.js-note-edit').click - end - - it 'shows the delete link' do - page.within('.note-attachment') do - is_expected.to have_css('.js-note-attachment-delete') - end - end - - it 'removes the attachment div and resets the edit form' do - find('.js-note-attachment-delete').click - is_expected.not_to have_css('.note-attachment') - is_expected.not_to have_css('.current-note-edit-form') - wait_for_ajax - end - end - end - end - - describe 'Handles cross-project system notes', js: true, feature: true do - let(:user) { create(:user) } - let(:project) { create(:project, :public) } - let(:project2) { create(:project, :private) } - let(:issue) { create(:issue, project: project2) } - let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'markdown') } - let!(:note) { create(:note_on_merge_request, :system, noteable: merge_request, project: project, note: "mentioned in #{issue.to_reference(project)}") } - - it 'shows the system note' do - login_as :admin - visit namespace_project_merge_request_path(project.namespace, project, merge_request) - - expect(page).to have_css('.system-note') - end - - it 'hides redacted system note' do - visit namespace_project_merge_request_path(project.namespace, project, merge_request) - - expect(page).not_to have_css('.system-note') - end - end - - describe 'On a merge request diff', js: true, feature: true do - let(:merge_request) { create(:merge_request) } - let(:project) { merge_request.source_project } - - before do - login_as :admin - visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) - end - - subject { page } - - describe 'when adding a note' do - before do - click_diff_line - end - - describe 'the notes holder' do - it { is_expected.to have_css('.js-temp-notes-holder') } - - it 'has .new_note css class' do - page.within('.js-temp-notes-holder') do - expect(subject).to have_css('.new-note') - end - end - end - - describe 'the note form' do - it "does not add a second form for same row" do - click_diff_line - - is_expected. - to have_css("form[data-line-code='#{line_code}']", - count: 1) - end - - it 'is removed when canceled' do - is_expected.to have_css('.js-temp-notes-holder') - - page.within("form[data-line-code='#{line_code}']") do - find('.js-close-discussion-note-form').trigger('click') - end - - is_expected.to have_no_css('.js-temp-notes-holder') - end - end - end - - describe 'with muliple note forms' do - before do - click_diff_line - click_diff_line(line_code_2) - end - - it { is_expected.to have_css('.js-temp-notes-holder', count: 2) } - - describe 'previewing them separately' do - before do - # add two separate texts and trigger previews on both - page.within("tr[id='#{line_code}'] + .js-temp-notes-holder") do - fill_in 'note[note]', with: 'One comment on line 7' - find('.js-md-preview-button').click - end - page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do - fill_in 'note[note]', with: 'Another comment on line 10' - find('.js-md-preview-button').click - end - end - end - - describe 'posting a note' do - before do - page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do - fill_in 'note[note]', with: 'Another comment on line 10' - click_button('Comment') - end - end - - it 'adds as discussion' do - is_expected.to have_content('Another comment on line 10') - is_expected.to have_css('.notes_holder') - is_expected.to have_css('.notes_holder .note', count: 1) - is_expected.to have_button('Reply...') - end - - it 'adds code to discussion' do - click_button 'Reply...' - - page.within(first('.js-discussion-note-form')) do - fill_in 'note[note]', with: '```{{ test }}```' - - click_button('Comment') - end - - expect(page).to have_content('{{ test }}') - end - end - end - end - - def line_code - sample_compare.changes.first[:line_code] - end - - def line_code_2 - sample_compare.changes.last[:line_code] - end - - def click_diff_line(data = line_code) - find(".line_holder[id='#{data}'] td.line_content").hover - find(".line_holder[id='#{data}'] button").trigger('click') - end -end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 4baccacd448..a3de022d242 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -484,7 +484,7 @@ describe 'project routing' do end it 'to #list' do - expect(get('/gitlab/gitlabhq/files/master.json')).to route_to('projects/find_file#list', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') + expect(get('/gitlab/gitlabhq/files/master.json')).to route_to('projects/find_file#list', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master.json') end end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 09cfa36b3b9..852a4ac852f 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -54,6 +54,15 @@ describe Projects::ImportService, services: true do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Failed to import the repository" end + + it 'does not remove the GitHub remote' do + expect_any_instance_of(Repository).to receive(:fetch_remote).and_return(true) + expect_any_instance_of(Gitlab::GithubImport::Importer).to receive(:execute).and_return(true) + + subject.execute + + expect(project.repository.raw_repository.remote_names).to include('github') + end end context 'with a non Github repository' do |