Merge branch 'ce-to-ee-2018-08-10' into 'master'

CE upstream - 2018-08-10 03:21 UTC

Closes gitlab-development-kit#387

See merge request gitlab-org/gitlab-ee!6863

.gitlab-ci.yml

@@ -587,6 +587,7 @@ danger-review:
       - master
     variables:
       - $CI_COMMIT_REF_NAME =~ /^ce-to-ee-.*/
+      - $CI_COMMIT_REF_NAME =~ /.*-stable(-ee)?-prepare-.*/
   script:
     - git version
     - danger --fail-on-errors=true

GITLAB_SHELL_VERSION

-8.1.0
+8.1.1

Gemfile.lock

@@ -131,7 +131,7 @@ GEM
       numerizer (~> 0.1.1)
     chunky_png (1.3.5)
     citrus (3.0.2)
-    coderay (1.1.1)
+    coderay (1.1.2)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
     commonmarker (0.17.8)
@@ -522,7 +522,7 @@ GEM
     memoist (0.16.0)
     memoizable (0.4.2)
       thread_safe (~> 0.3, >= 0.3.1)
-    method_source (0.8.2)
+    method_source (0.9.0)
     mime-types (3.1)
       mime-types-data (~> 3.2015)
     mime-types-data (3.2016.0521)
@@ -665,12 +665,11 @@ GEM
       unparser
     procto (0.0.3)
     prometheus-client-mmap (0.9.4)
-    pry (0.10.4)
+    pry (0.11.3)
       coderay (~> 1.1.0)
-      method_source (~> 0.8.1)
-      slop (~> 3.4)
-    pry-byebug (3.4.2)
-      byebug (~> 9.0)
+      method_source (~> 0.9.0)
+    pry-byebug (3.4.3)
+      byebug (>= 9.0, < 9.1)
       pry (~> 0.10)
     pry-rails (0.3.5)
       pry (>= 0.9.10)
@@ -901,7 +900,6 @@ GEM
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.0)
     slack-notifier (1.5.1)
-    slop (3.6.0)
     spring (2.0.1)
       activesupport (>= 4.2)
     spring-commands-rspec (1.0.4)
@@ -1244,4 +1242,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.16.2
+   1.16.3

app/assets/javascripts/ide/ide_router.js

@@ -117,7 +117,7 @@ router.beforeEach((to, from, next) => {
               mergeRequestId: to.params.mrid,
             })
             .then(mr => {
-              store.dispatch('updateActivityBarView', activityBarViews.review);
+              store.dispatch('setCurrentBranchId', mr.source_branch);
 
               store.dispatch('getBranchData', {
                 projectId: fullProjectId,
@@ -144,6 +144,10 @@ router.beforeEach((to, from, next) => {
               }),
             )
             .then(mrChanges => {
+              if (mrChanges.changes.length) {
+                store.dispatch('updateActivityBarView', activityBarViews.review);
+              }
+
               mrChanges.changes.forEach((change, ind) => {
                 const changeTreeEntry = store.state.entries[change.new_path];
 

app/assets/javascripts/ide/stores/actions/file.js

@@ -54,9 +54,6 @@ export const setFileActive = ({ commit, state, getters, dispatch }, path) => {
 
   commit(types.SET_FILE_ACTIVE, { path, active: true });
   dispatch('scrollToTab');
-
-  commit(types.SET_CURRENT_PROJECT, file.projectId);
-  commit(types.SET_CURRENT_BRANCH, file.branchId);
 };
 
 export const getFileData = ({ state, commit, dispatch }, { path, makeFileActive = true }) => {

app/assets/stylesheets/framework/avatar.scss

@@ -107,6 +107,7 @@
   display: flex;
 
   a {
+    width: 100%;
     display: flex;
   }
 

app/assets/stylesheets/pages/labels.scss

@@ -253,7 +253,7 @@
   text-align: right;
   padding: 0;
   position: relative;
-  top: -3px;
+  margin: 0;
 }
 
 .label-badge {
@@ -274,6 +274,7 @@
 
 .label-links {
   list-style: none;
+  margin: 0;
   padding: 0;
   white-space: nowrap;
 }

app/assets/stylesheets/pages/projects.scss

@@ -824,10 +824,6 @@ pre.light-well {
 
   .avatar-container {
     align-self: flex-start;
-
-    > a {
-      width: 100%;
-    }
   }
 
   .project-details {

app/helpers/avatars_helper.rb

 module AvatarsHelper
   def project_icon(project_id, options = {})
-    project =
-      if project_id.respond_to?(:avatar_url)
-        project_id
-      else
-        Project.find_by_full_path(project_id)
-      end
-
-    if project.avatar_url
-      image_tag project.avatar_url, options
-    else # generated icon
-      project_identicon(project, options)
-    end
+    source_icon(Project, project_id, options)
   end
 
-  def project_identicon(project, options = {})
-    bg_key = (project.id % 7) + 1
-    options[:class] ||= ''
-    options[:class] << ' identicon'
-    options[:class] << " bg#{bg_key}"
-
-    content_tag(:div, class: options[:class]) do
-      project.name[0, 1].upcase
-    end
+  def group_icon(group_id, options = {})
+    source_icon(Group, group_id, options)
   end
 
   # Takes both user and email and returns the avatar_icon by
@@ -123,4 +105,32 @@ module AvatarsHelper
       mail_to(options[:user_email], avatar)
     end
   end
+
+  private
+
+  def source_icon(klass, source_id, options = {})
+    source =
+      if source_id.respond_to?(:avatar_url)
+        source_id
+      else
+        klass.find_by_full_path(source_id)
+      end
+
+    if source.avatar_url
+      image_tag source.avatar_url, options
+    else
+      source_identicon(source, options)
+    end
+  end
+
+  def source_identicon(source, options = {})
+    bg_key = (source.id % 7) + 1
+    options[:class] ||= ''
+    options[:class] << ' identicon'
+    options[:class] << " bg#{bg_key}"
+
+    content_tag(:div, class: options[:class].strip) do
+      source.name[0, 1].upcase
+    end
+  end
 end

app/helpers/groups_helper.rb

@@ -35,11 +35,6 @@ module GroupsHelper
       .count
   end
 
-  def group_icon(group, options = {})
-    img_path = group_icon_url(group, options)
-    image_tag img_path, options
-  end
-
   def group_icon_url(group, options = {})
     if group.is_a?(String)
       group = Group.find_by_full_path(group)

app/views/projects/forks/_fork_button.html.haml

@@ -5,7 +5,7 @@
   .bordered-box.fork-thumbnail.text-center.prepend-left-default.append-right-default.prepend-top-default.append-bottom-default.forked
     = link_to project_path(forked_project) do
       - if /no_((\w*)_)*avatar/.match(avatar)
-        = project_identicon(namespace, class: "avatar s100 identicon")
+        = project_icon(namespace, class: "avatar s100 identicon")
       - else
         .avatar-container.s100
           = image_tag(avatar, class: "avatar s100")
@@ -18,7 +18,7 @@
         class: ("disabled has-tooltip" unless can_create_project),
         title: (_('You have reached your project limit') unless can_create_project) do
       - if /no_((\w*)_)*avatar/.match(avatar)
-        = project_identicon(namespace, class: "avatar s100 identicon")
+        = project_icon(namespace, class: "avatar s100 identicon")
       - else
         .avatar-container.s100
           = image_tag(avatar, class: "avatar s100")

changelogs/unreleased/feat-add-default-avatar-to-group.yml

+---
+title: Add default avatar to group
+merge_request: 17271
+author: George Tsiolis
+type: changed

changelogs/unreleased/fix-labels-list-item-height-with-no-description.yml

+---
+title: Fix label list item container height when there is no label description
+merge_request: 21106
+author:
+type: fixed

changelogs/unreleased/ide-open-empty-merge-request.yml

+---
+title: Fix empty merge requests not opening in the Web IDE
+merge_request: 21102
+author:
+type: fixed

changelogs/unreleased/sh-fix-bitbucket-cloud-importer-replies.yml

+---
+title: Fix Bitbucket Cloud importer omitting replies
+merge_request: 21076
+author:
+type: fixed

db/fixtures/development/23_spam_logs.rb

+# frozen_string_literal: true
+
+module Db
+  module Fixtures
+    module Development
+      class SpamLog
+        def self.seed
+          Gitlab::Seeder.quiet do
+            (::SpamLog.default_per_page + 3).times do |i|
+              ::SpamLog.create(
+                user: self.random_user,
+                user_agent: FFaker::Lorem.sentence,
+                source_ip: FFaker::Internet.ip_v4_address,
+                title: FFaker::Lorem.sentence,
+                description: FFaker::Lorem.paragraph,
+                via_api: FFaker::Boolean.random,
+                submitted_as_ham: FFaker::Boolean.random,
+                recaptcha_verified: FFaker::Boolean.random)
+              print '.'
+            end
+          end
+        end
+
+        def self.random_user
+          User.find(User.pluck(:id).sample)
+        end
+      end
+    end
+  end
+end
+
+Db::Fixtures::Development::SpamLog.seed

db/migrate/20160317092222_add_moved_to_to_issue.rb

 class AddMovedToToIssue < ActiveRecord::Migration
   def change
-    add_reference :issues, :moved_to, references: :issues
+    add_reference :issues, :moved_to, references: :issues # rubocop:disable Migration/AddReference
   end
 end

doc/api/README.md

@@ -408,14 +408,13 @@ GET /api/v4/projects/1/branches/my%2Fbranch/commits
 
 ## Encoding API parameters of `array` and `hash` types
 
-When making an API call with parameters of type `array` and/or `hash`, the parameters may be
-specified as shown below.
+We can call the API with `array` and `hash` types parameters as shown below:
 
 ### `array`
 
 `import_sources` is a parameter of type `array`:
 
-```
+```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" \
 -d "import_sources[]=github" \
 -d "import_sources[]=bitbucket" \
@@ -426,7 +425,7 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" \
 
 `override_params` is a parameter of type `hash`:
 
-```
+```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" \
 --form "namespace=email" \
 --form "path=impapi" \
@@ -436,6 +435,20 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" \
 https://gitlab.example.com/api/v4/projects/import
 ```
 
+### Array of hashes
+
+`variables` is a parameter of type `array` containing hash key/value pairs `[{ 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }]`:
+
+```bash
+curl --globoff --request POST --header "PRIVATE-TOKEN: ********************" \
+"https://gitlab.example.com/api/v4/projects/169/pipeline?ref=master&variables[][key]=VAR1&variables[][value]=hello&variables[][key]=VAR2&variables[][value]=world"
+
+curl --request POST --header "PRIVATE-TOKEN: ********************" \
+--header "Content-Type: application/json" \
+--data '{ "ref": "master", "variables": [ {"key": "VAR1", "value": "hello"}, {"key": "VAR2", "value": "world"} ] }' \
+"https://gitlab.example.com/api/v4/projects/169/pipeline"
+```
+
 ## `id` vs `iid`
 
 When you work with the API, you may notice two similar fields in API entities:

doc/development/migration_style_guide.md

@@ -182,6 +182,34 @@ class MyMigration < ActiveRecord::Migration
 end
 ```
 
+## Adding foreign-key constraints
+
+When adding a foreign-key constraint to either an existing or new
+column remember to also add a index on the column.
+
+This is _required_ if the foreign-key constraint specifies
+`ON DELETE CASCADE` or `ON DELETE SET NULL` behavior. On a cascading
+delete, the [corresponding record needs to be retrieved using an
+index](https://www.cybertec-postgresql.com/en/postgresql-indexes-and-foreign-keys/)
+(otherwise, we'd need to scan the whole table) for subsequent update or
+deletion.
+
+Here's an example where we add a new column with a foreign key
+constraint. Note it includes `index: true` to create an index for it.
+
+```ruby
+class Migration < ActiveRecord::Migration
+
+  def change
+    add_reference :model, :other_model, index: true, foreign_key: { on_delete: :cascade }
+  end
+end
+```
+
+When adding a foreign-key constraint to an existing column, we
+have to employ `add_concurrent_foreign_key` and `add_concurrent_index`
+instead of `add_reference`.
+
 ## Adding Columns With Default Values
 
 When adding columns with default values you must use the method

doc/integration/bitbucket.md

-# Integrate your GitLab server with Bitbucket
+# Integrate your GitLab server with Bitbucket Cloud
 
 NOTE: **Note:**
 You need to [enable OmniAuth](omniauth.md) in order to use this.

doc/integration/oauth_provider.md

@@ -78,8 +78,8 @@ in the **Authorized applications** section under **Profile Settings > Applicatio
 ---
 
 GitLab's OAuth applications support scopes, which allow various actions that any given
-application can perform. Although there are only two scopes available at the
-moment – `read_user` and `api` – the groundwork has been laid to add more scopes easily.
+application can perform such as `read_user` and `api`. There are many more scopes
+available.
 
 At any time you can revoke any access by just clicking **Revoke**.
 

doc/user/project/import/bitbucket.md

-# Import your project from Bitbucket to GitLab
+# Import your project from Bitbucket Cloud to GitLab
 
-Import your projects from Bitbucket to GitLab with minimal effort.
+NOTE: **Note:**
+The Bitbucket Cloud importer works only with Bitbucket.org, not with Bitbucket
+Server (aka Stash). If you are trying to import projects from Bitbucket Server, use
+[the Bitbucket Server importer](bitbucket_server.md).
 
-## Overview
-
->**Note:**
-The [Bitbucket integration][bb-import] must be first enabled in order to be
-able to import your projects from Bitbucket. Ask your GitLab administrator
-to enable this if not already.
+Import your projects from Bitbucket Cloud to GitLab with minimal effort.
 
->**Note:**  
-The BitBucket importer currently only works with BitBucket's cloud offering 
-(bitbucket.org) and does not work with BitBucket Server (aka Stash).
+## Overview
 
 - At its current state, the Bitbucket importer can import:
   - the repository description (GitLab 7.7+)
@@ -26,6 +22,11 @@ The BitBucket importer currently only works with BitBucket's cloud offering
 - Repository public access is retained. If a repository is private in Bitbucket
   it will be created as private in GitLab as well.
 
+## Requirements
+
+The [Bitbucket Cloud integration][bb-import] must be first enabled in order to be
+able to import your projects from Bitbucket Cloud. Ask your GitLab administrator
+to enable this if not already.
 
 ## How it works
 
@@ -46,9 +47,7 @@ namespace that started the import process.
 1. Sign in to GitLab and go to your dashboard.
 1. Click on **New project**.
 
-    ![New project in GitLab](img/bitbucket_import_new_project.png)
-
-1. Click on the "Bitbucket" button
+1. Click on the "Bitbucket Cloud" button.
 
     ![Bitbucket](img/import_projects_from_new_project_page.png)
 

doc/user/project/import/bitbucket_server.md

+# Import your project from Bitbucket Server to GitLab
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20164)
+in GitLab 11.2.
+
+NOTE: **Note:**
+The Bitbucket Server importer does not work with Bitbucket Cloud (aka bitbucket.org).
+Use the [Bitbucket Cloud importer](bitbucket.md) for that.
+
+Import your projects from Bitbucket Server to GitLab with minimal effort.
+
+## Overview
+
+- In its current state, the Bitbucket importer can import:
+  - the repository description (GitLab 11.2+)
+  - the Git repository data (GitLab 11.2+)
+  - the pull requests (GitLab 11.2+)
+  - the pull request comments (GitLab 11.2+)
+- Repository public access is retained. If a repository is private in Bitbucket
+  it will be created as private in GitLab as well.
+
+## Limitations
+
+1. Currently GitLab doesn't allow comments on arbitrary lines of code, so any
+Bitbucket comments out of bounds will be inserted as comments in the merge
+request.
+1. Bitbucket Server allows multiple levels of threading. GitLab
+import will collapse this into one discussion and quote part of the original
+comment.
+1. Declined pull requests have unrecahable commits, which prevents the GitLab
+importer from generating a proper diff. These pull requests will show up as
+empty changes.
+1. Attachments in Markdown are currently not imported.
+1. Task lists are not imported.
+1. Emoji reactions are not imported
+
+## How it works
+
+The Bitbucket Server importer works as follows:
+
+1. The user will be prompted to enter the URl, username, and password or personal access token to login to Bitbucket.
+   These credentials are preserved only as long as the importer is running.
+1. The importer will attempt to list all the current repositories on the Bitbucket Server.
+1. Upon selection, the importer will clone the repository and import pull requests and comments.
+
+### User assignment
+
+When issues/pull requests are being imported, the Bitbucket importer tries to
+find the author's e-mail address with a confirmed e-mail address in the GitLab
+user database.  If no such user is available, the project creator is set as
+the author. The importer will append a note in the comment to mark the original
+creator.
+
+The importer will create any new namespaces (groups) if they don't exist or in
+the case the namespace is taken, the repository will be imported under the user's
+namespace that started the import process.
+
+## Importing your Bitbucket repositories
+
+1. Sign in to GitLab and go to your dashboard.
+1. Click on **New project**.
+1. Click on the "Bitbucket Server" button. If the button is not present, enable the importer in
+    **Admin > Application Settings > Visibility and access controls > Import sources**.
+
+    ![Bitbucket](img/import_projects_from_new_project_page.png)
+
+1. Enter your Bitbucket Server credentials.
+
+    ![Grant access](img/bitbucket_server_import_credentials.png)
+
+1. Click on the projects that you'd like to import or **Import all projects**.
+   You can also select the namespace under which each project will be
+   imported.
+
+    ![Import projects](img/bitbucket_server_import_select_project.png)

ee/db/migrate/20170808155912_add_plan_id_to_namespaces.rb

@@ -17,7 +17,7 @@ class AddPlanIdToNamespaces < ActiveRecord::Migration
   end
 
   def up
-    add_reference :namespaces, :plan
+    add_reference :namespaces, :plan # rubocop:disable Migration/AddReference
     add_concurrent_foreign_key :namespaces, :plans, column: :plan_id, on_delete: :nullify
     add_concurrent_index :namespaces, :plan_id
 

lib/gitlab/bitbucket_import/importer.rb

@@ -188,7 +188,8 @@ module Gitlab
       end
 
       def import_inline_comments(inline_comments, pull_request, merge_request)
-        line_code_map = {}
+        position_map = {}
+        discussion_map = {}
 
         children, parents = inline_comments.partition(&:has_parent?)
 
@@ -196,22 +197,28 @@ module Gitlab
         # relationships. We assume that the child can appear in any order in
         # the JSON.
         parents.each do |comment|
-          line_code_map[comment.iid] = generate_line_code(comment)
+          position_map[comment.iid] = build_position(merge_request, comment)
         end
 
         children.each do |comment|
-          line_code_map[comment.iid] = line_code_map.fetch(comment.parent_id, nil)
+          position_map[comment.iid] = position_map.fetch(comment.parent_id, nil)
         end
 
         inline_comments.each do |comment|
           begin
             attributes = pull_request_comment_attributes(comment)
+            attributes[:discussion_id] = discussion_map[comment.parent_id] if comment.has_parent?
+
             attributes.merge!(
-              position: build_position(merge_request, comment),
-              line_code: line_code_map.fetch(comment.iid),
+              position: position_map[comment.iid],
               type: 'DiffNote')
 
-            merge_request.notes.create!(attributes)
+            note = merge_request.notes.create!(attributes)
+
+            # We can't store a discussion ID until a note is created, so if
+            # replies are created before the parent the discussion ID won't be
+            # linked properly.
+            discussion_map[comment.iid] = note.discussion_id
           rescue StandardError => e
             errors << { type: :pull_request, iid: comment.iid, errors: e.message }
           end
@@ -240,10 +247,6 @@ module Gitlab
         end
       end
 
-      def generate_line_code(pr_comment)
-        Gitlab::Git.diff_line_code(pr_comment.file_path, pr_comment.new_pos, pr_comment.old_pos)
-      end
-
       def pull_request_comment_attributes(comment)
         {
           project: project,

rubocop/cop/migration/add_reference.rb

+# frozen_string_literal: true
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if a foreign key constraint is added and require a index for it
+      class AddReference < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = '`add_reference` requires `index: true`'
+
+        def on_send(node)
+          return unless in_migration?(node)
+
+          name = node.children[1]
+
+          return unless name == :add_reference
+
+          opts = node.children.last
+
+          add_offense(node, location: :selector) unless opts && opts.type == :hash
+
+          index_present = false
+
+          opts.each_node(:pair) do |pair|
+            index_present ||= index_enabled?(pair)
+          end
+
+          add_offense(node, location: :selector) unless index_present
+        end
+
+        private
+
+        def index_enabled?(pair)
+          hash_key_type(pair) == :sym && hash_key_name(pair) == :index && pair.children[1].true_type?
+        end
+
+        def hash_key_type(pair)
+          pair.children[0].type
+        end
+
+        def hash_key_name(pair)
+          pair.children[0].children[0]
+        end
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -11,6 +11,7 @@ require_relative 'cop/migration/add_column'
 require_relative 'cop/migration/add_concurrent_foreign_key'
 require_relative 'cop/migration/add_concurrent_index'
 require_relative 'cop/migration/add_index'
+require_relative 'cop/migration/add_reference'
 require_relative 'cop/migration/add_timestamps'
 require_relative 'cop/migration/datetime'
 require_relative 'cop/migration/hash_index'

spec/helpers/avatars_helper_spec.rb

@@ -5,12 +5,67 @@ describe AvatarsHelper do
 
   let(:user) { create(:user) }
 
-  describe '#project_icon' do
-    it 'returns an url for the avatar' do
-      project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
+  describe '#project_icon & #group_icon' do
+    shared_examples 'resource with a default avatar' do |source_type|
+      it 'returns a default avatar div' do
+        expect(public_send("#{source_type}_icon", *helper_args))
+          .to match(%r{<div class="identicon bg\d+">F</div>})
+      end
+    end
+
+    shared_examples 'resource with a custom avatar' do |source_type|
+      it 'returns a custom avatar image' do
+        expect(public_send("#{source_type}_icon", *helper_args))
+          .to eq "<img src=\"#{resource.avatar.url}\" alt=\"Banana sample\" />"
+      end
+    end
+
+    context 'when providing a project' do
+      it_behaves_like 'resource with a default avatar', 'project' do
+        let(:resource) { create(:project, name: 'foo') }
+        let(:helper_args) { [resource] }
+      end
+
+      it_behaves_like 'resource with a custom avatar', 'project' do
+        let(:resource) { create(:project, :public, avatar: File.open(uploaded_image_temp_path)) }
+        let(:helper_args) { [resource] }
+      end
+    end
+
+    context 'when providing a project path' do
+      it_behaves_like 'resource with a default avatar', 'project' do
+        let(:resource) { create(:project, name: 'foo') }
+        let(:helper_args) { [resource.full_path] }
+      end
 
-      expect(helper.project_icon(project.full_path).to_s)
-        .to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
+      it_behaves_like 'resource with a custom avatar', 'project' do
+        let(:resource) { create(:project, :public, avatar: File.open(uploaded_image_temp_path)) }
+        let(:helper_args) { [resource.full_path] }
+      end
+    end
+
+    context 'when providing a group' do
+      it_behaves_like 'resource with a default avatar', 'group' do
+        let(:resource) { create(:group, name: 'foo') }
+        let(:helper_args) { [resource] }
+      end
+
+      it_behaves_like 'resource with a custom avatar', 'group' do
+        let(:resource) { create(:group, avatar: File.open(uploaded_image_temp_path)) }
+        let(:helper_args) { [resource] }
+      end
+    end
+
+    context 'when providing a group path' do
+      it_behaves_like 'resource with a default avatar', 'group' do
+        let(:resource) { create(:group, name: 'foo') }
+        let(:helper_args) { [resource.full_path] }
+      end
+
+      it_behaves_like 'resource with a custom avatar', 'group' do
+        let(:resource) { create(:group, avatar: File.open(uploaded_image_temp_path)) }
+        let(:helper_args) { [resource.full_path] }
+      end
     end
   end
 

spec/helpers/groups_helper_spec.rb

@@ -3,19 +3,6 @@ require 'spec_helper'
 describe GroupsHelper do
   include ApplicationHelper
 
-  describe 'group_icon' do
-    it 'returns an url for the avatar' do
-      avatar_file_path = File.join('spec', 'fixtures', 'banana_sample.gif')
-
-      group = create(:group)
-      group.avatar = fixture_file_upload(avatar_file_path)
-      group.save!
-
-      expect(helper.group_icon(group).to_s)
-        .to eq "<img data-src=\"#{group.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
-    end
-  end
-
   describe 'group_icon_url' do
     it 'returns an url for the avatar' do
       avatar_file_path = File.join('spec', 'fixtures', 'banana_sample.gif')

spec/lib/gitlab/bitbucket_import/importer_spec.rb

@@ -69,6 +69,7 @@ describe Gitlab::BitbucketImport::Importer do
   let(:project) do
     create(
       :project,
+      :repository,
       import_source: project_identifier,
       import_url: "https://bitbucket.org/#{project_identifier}.git",
       import_data_attributes: { credentials: data }
@@ -85,10 +86,84 @@ describe Gitlab::BitbucketImport::Importer do
     }
   end
 
+  let(:sample) { RepoHelpers.sample_compare }
+
   before do
     allow(importer).to receive(:gitlab_shell) { gitlab_shell }
   end
 
+  subject { described_class.new(project) }
+
+  describe '#import_pull_requests' do
+    before do
+      allow(subject).to receive(:import_wiki)
+      allow(subject).to receive(:import_issues)
+
+      pull_request = instance_double(
+        Bitbucket::Representation::PullRequest,
+        iid: 10,
+        source_branch_sha: sample.commits.last,
+        source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch,
+        target_branch_sha: sample.commits.first,
+        target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch,
+        title: 'This is a title',
+        description: 'This is a test pull request',
+        state: 'merged',
+        author: 'other',
+        created_at: Time.now,
+        updated_at: Time.now)
+
+      # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
+      @inline_note = instance_double(
+        Bitbucket::Representation::PullRequestComment,
+        iid: 2,
+        file_path: '.gitmodules',
+        old_pos: nil,
+        new_pos: 4,
+        note: 'Hello world',
+        author: 'root',
+        created_at: Time.now,
+        updated_at: Time.now,
+        inline?: true,
+        has_parent?: false)
+
+      @reply = instance_double(
+        Bitbucket::Representation::PullRequestComment,
+        iid: 3,
+        file_path: '.gitmodules',
+        note: 'Hello world',
+        author: 'root',
+        created_at: Time.now,
+        updated_at: Time.now,
+        inline?: true,
+        has_parent?: true,
+        parent_id: 2)
+
+      comments = [@inline_note, @reply]
+
+      allow(subject.client).to receive(:repo)
+      allow(subject.client).to receive(:pull_requests).and_return([pull_request])
+      allow(subject.client).to receive(:pull_request_comments).with(anything, pull_request.iid).and_return(comments)
+    end
+
+    it 'imports threaded discussions' do
+      expect { subject.execute }.to change { MergeRequest.count }.by(1)
+
+      merge_request = MergeRequest.first
+      expect(merge_request.notes.count).to eq(2)
+      expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
+
+      notes = merge_request.notes.order(:id).to_a
+      start_note = notes.first
+      expect(start_note).to be_a(DiffNote)
+      expect(start_note.note).to eq(@inline_note.note)
+
+      reply_note = notes.last
+      expect(reply_note).to be_a(DiffNote)
+      expect(reply_note.note).to eq(@reply.note)
+    end
+  end
+
   context 'issues statuses' do
     before do
       # HACK: Bitbucket::Representation.const_get('Issue') seems to return ::Issue without this

spec/rubocop/cop/migration/add_reference_spec.rb

+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/add_reference'
+
+describe RuboCop::Cop::Migration::AddReference do
+  include CopHelper
+
+  let(:cop) { described_class.new }
+
+  context 'outside of a migration' do
+    it 'does not register any offenses' do
+      expect_no_offenses(<<~RUBY)
+        def up
+          add_reference(:projects, :users)
+        end
+      RUBY
+    end
+  end
+
+  context 'in a migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    it 'registers an offense when using add_reference without index' do
+      expect_offense(<<~RUBY)
+        call do
+          add_reference(:projects, :users)
+          ^^^^^^^^^^^^^ `add_reference` requires `index: true`
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using add_reference index disabled' do
+      expect_offense(<<~RUBY)
+        def up
+          add_reference(:projects, :users, index: false)
+          ^^^^^^^^^^^^^ `add_reference` requires `index: true`
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using add_reference with index enabled' do
+      expect_no_offenses(<<~RUBY)
+        def up
+          add_reference(:projects, :users, index: true)
+        end
+      RUBY
+    end
+  end
+end