Merge remote-tracking branch 'ce/master' into ce-to-ee

Signed-off-by: Rémy Coutable <remy@rymai.me>

CHANGELOG

@@ -15,6 +15,7 @@ v 8.10.0 (unreleased)
   - Make images fit to the size of the viewport !4810
   - Fix check for New Branch button on Issue page !4630 (winniehell)
   - Fix MR-auto-close text added to description. !4836
+  - Fix issue, preventing users w/o push access to sort tags !5105 (redetection)
   - Add Spring EmojiOne updates.
   - Fix pagination when sorting by columns with lots of ties (like priority)
   - Updated project header design
@@ -28,6 +29,7 @@ v 8.10.0 (unreleased)
   - PipelinesFinder uses git cache data
   - Throttle the update of `project.pushes_since_gc` to 1 minute.
   - Check for conflicts with existing Project's wiki path when creating a new project.
+  - Show last push widget in upstream after push to fork
   - Don't instantiate a git tree on Projects show default view
   - Bump Rinku to 2.0.0
   - Remove unused front-end variable -> default_issues_tracker
@@ -44,17 +46,23 @@ v 8.10.0 (unreleased)
   - More descriptive message for git hooks and file locks
   - Handle custom Git hook result in GitLab UI
   - Allow '?', or '&' for label names
+  - Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests
+  - Add date when user joined the team on the member page
 
-v 8.9.5 (unreleased)
-  - Improve the request / withdraw access button. !4860
-  - Fix assigning shared runners as admins. !4961
-  - Show "locked" label for locked runners on runners admin. !4961
-  - Downgrade to Redis 3.2.2 due to massive memory leak with Sidekiq
-  - Add index on the user and emoji name on AwardEmoji table !5061
-  - Fixes issues importing events in Import/Export. Import/Export version bumped to 0.1.1
-  - Fix import button disabled when import process fail due to the namespace already been taken.
+v 8.9.5
+  - Add more debug info to import/export and memory killer. !5108
+  - Fixed avatar alignment in new MR view. !5095
   - Fix diff comments not showing up in activity feed. !5069
-  - Security: Update RedCloth to 4.3.2 (Takuya Noguchi)
+  - Add index on both Award Emoji user and name. !5061
+  - Downgrade to Redis 3.2.2 due to massive memory leak with Sidekiq. !5056
+  - Re-enable import button when import process fails due to namespace already being taken. !5053
+  - Fix snippets comments not displayed. !5045
+  - Fix emoji paths in relative root configurations. !5027
+  - Fix issues importing events in Import/Export. !4987
+  - Fixed 'use shortcuts' button on docs. !4979
+  - Admin should be able to turn shared runners into specific ones. !4961
+  - Update RedCloth to 4.3.2 for CVE-2012-6684. !4929 (Takuya Noguchi)
+  - Improve the request / withdraw access button. !4860
 
 v 8.9.4
   - Fix privilege escalation issue with OAuth external users.

app/assets/javascripts/ci/build.coffee

@@ -2,7 +2,7 @@ class @CiBuild
   @interval: null
   @state: null
 
-  constructor: (@build_url, @build_status, @state) ->
+  constructor: (@page_url, @build_url, @build_status, @state) ->
     clearInterval(CiBuild.interval)
 
     # Init breakpoint checker
@@ -41,7 +41,7 @@ class @CiBuild
       # Only valid for runnig build when output changes during time
       #
       CiBuild.interval = setInterval =>
-        if window.location.href.split("#").first() is @build_url
+        if window.location.href.split("#").first() is @page_url
           @getBuildTrace()
       , 4000
 
@@ -57,7 +57,7 @@ class @CiBuild
 
   getBuildTrace: ->
     $.ajax
-      url: "#{@build_url}/trace.json?state=#{encodeURIComponent(@state)}"
+      url: "#{@page_url}/trace.json?state=#{encodeURIComponent(@state)}"
       dataType: "json"
       success: (log) =>
         if log.state
@@ -70,7 +70,7 @@ class @CiBuild
             $('.js-build-output').html log.html
           @checkAutoscroll()
         else if log.status isnt @build_status
-          Turbolinks.visit @build_url
+          Turbolinks.visit @page_url
 
   checkAutoscroll: ->
     $("html,body").scrollTop $("#build-trace").height()  if "enabled" is $("#autoscroll-button").data("state")

app/assets/stylesheets/framework/dropdowns.scss

@@ -23,6 +23,9 @@
   .dropdown-menu,
   .dropdown-menu-nav {
     display: block;
+    @media (max-width: $screen-xs-max) {
+      width: 100%;
+    }
   }
 
   .dropdown-menu-toggle {

app/assets/stylesheets/framework/lists.scss

@@ -137,6 +137,15 @@ ul.content-list {
       padding-top: 1px;
       float: right;
 
+      > .control-text {
+        margin-right: $gl-padding-top;
+        line-height: 40px;
+
+        &:last-child {
+          margin-right: 0;
+        }
+      }
+
       > .btn,
       > .btn-group {
         margin-right: $gl-padding-top;

app/helpers/projects_helper.rb

@@ -310,7 +310,11 @@ module ProjectsHelper
   end
 
   def last_push_event
-    if current_user
+    return unless current_user
+
+    if fork = current_user.fork_of(@project)
+      current_user.recent_push(fork.id)
+    else
       current_user.recent_push(@project.id)
     end
   end

app/services/audit_event_service.rb

@@ -75,7 +75,7 @@ class AuditEventService
     @details = {
       with: @details[:with],
       target_id: @author.id,
-      target_type: "User",
+      target_type: 'User',
       target_details: @author.name,
     }
 

app/services/auth/container_registry_authentication_service.rb

@@ -20,9 +20,11 @@ module Auth
       token.issuer = registry.issuer
       token.audience = AUDIENCE
       token.expire_time = token_expire_at
+
       token[:access] = names.map do |name|
         { type: 'repository', name: name, actions: %w(*) }
       end
+      
       token.encoded
     end
 

app/services/create_branch_service.rb

@@ -3,17 +3,20 @@ require_relative 'base_service'
 class CreateBranchService < BaseService
   def execute(branch_name, ref, source_project: @project)
     valid_branch = Gitlab::GitRefValidator.validate(branch_name)
-    if valid_branch == false
+
+    unless valid_branch
       return error('Branch name is invalid')
     end
 
     repository = project.repository
     existing_branch = repository.find_branch(branch_name)
+
     if existing_branch
       return error('Branch already exists')
     end
 
     new_branch = nil
+
     if source_project != @project
       repository.with_tmp_ref do |tmp_ref|
         repository.fetch_ref(
@@ -29,7 +32,6 @@ class CreateBranchService < BaseService
     end
 
     if new_branch
-      # GitPushService handles execution of services and hooks for branch pushes
       success(new_branch)
     else
       error('Invalid reference name')
@@ -39,8 +41,6 @@ class CreateBranchService < BaseService
   end
 
   def success(branch)
-    out = super()
-    out[:branch] = branch
-    out
+    super().merge(branch: branch)
   end
 end

app/services/create_release_service.rb

@@ -23,8 +23,6 @@ class CreateReleaseService < BaseService
   end
 
   def success(release)
-    out = super()
-    out[:release] = release
-    out
+    super().merge(release: release)
   end
 end

app/services/create_snippet_service.rb

 class CreateSnippetService < BaseService
   def execute
-    if project.nil?
-      snippet = PersonalSnippet.new(params)
-    else
-      snippet = project.snippets.build(params)
-    end
+    snippet = if project
+                project.snippets.build(params)
+              else
+                PersonalSnippet.new(params)
+              end
 
     unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
       deny_visibility_level(snippet)

app/services/create_tag_service.rb

@@ -9,6 +9,7 @@ class CreateTagService < BaseService
     message.strip! if message
 
     new_tag = nil
+
     begin
       new_tag = repository.add_tag(current_user, tag_name, target, message)
     rescue Rugged::TagError
@@ -22,6 +23,7 @@ class CreateTagService < BaseService
         CreateReleaseService.new(@project, @current_user).
           execute(tag_name, release_description)
       end
+      
       success.merge(tag: new_tag)
     else
       error("Target #{target} is invalid")

app/services/delete_branch_service.rb

@@ -5,7 +5,6 @@ class DeleteBranchService < BaseService
     repository = project.repository
     branch = repository.find_branch(branch_name)
 
-    # No such branch
     unless branch
       return error('No such branch', 404)
     end
@@ -14,18 +13,15 @@ class DeleteBranchService < BaseService
       return error('Cannot remove HEAD branch', 405)
     end
 
-    # Dont allow remove of protected branch
     if project.protected_branch?(branch_name)
       return error('Protected branch cant be removed', 405)
     end
 
-    # Dont allow user to remove branch if he is not allowed to push
     unless current_user.can?(:push_code, project)
       return error('You dont have push access to repo', 405)
     end
 
     if repository.rm_branch(current_user, branch_name)
-      # GitPushService handles execution of services and hooks for branch pushes
       success('Branch was removed')
     else
       error('Failed to remove branch')
@@ -35,15 +31,11 @@ class DeleteBranchService < BaseService
   end
 
   def error(message, return_code = 400)
-    out = super(message)
-    out[:return_code] = return_code
-    out
+    super(message).merge(return_code: return_code)
   end
 
   def success(message)
-    out = super()
-    out[:message] = message
-    out
+    super().merge(message: message)
   end
 
   def build_push_data(branch)

app/services/delete_tag_service.rb

@@ -5,7 +5,6 @@ class DeleteTagService < BaseService
     repository = project.repository
     tag = repository.find_tag(tag_name)
 
-    # No such tag
     unless tag
       return error('No such tag', 404)
     end
@@ -26,15 +25,11 @@ class DeleteTagService < BaseService
   end
 
   def error(message, return_code = 400)
-    out = super(message)
-    out[:return_code] = return_code
-    out
+    super(message).merge(return_code: return_code)
   end
 
   def success(message)
-    out = super()
-    out[:message] = message
-    out
+    super().merge(message: message)
   end
 
   def build_push_data(tag)

app/services/files/base_service.rb

@@ -15,7 +15,6 @@ module Files
                           params[:file_content]
                         end
 
-      # Validate parameters
       validate
 
       # Create new branch if it different from source_branch
@@ -26,7 +25,7 @@ module Files
       if commit
         success
       else
-        error("Something went wrong. Your changes were not committed")
+        error('Something went wrong. Your changes were not committed')
       end
     rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex
       error(ex.message)
@@ -51,12 +50,12 @@ module Files
 
       unless project.empty_repo?
         unless @source_project.repository.branch_names.include?(@source_branch)
-          raise_error("You can only create or edit files when you are on a branch")
+          raise_error('You can only create or edit files when you are on a branch')
         end
 
         if different_branch?
           if repository.branch_names.include?(@target_branch)
-            raise_error("Branch with such name already exists. You need to switch to this branch in order to make changes")
+            raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes')
           end
         end
       end

app/services/files/create_service.rb

@@ -29,7 +29,7 @@ module Files
         blob = repository.blob_at_branch(@source_branch, @file_path)
 
         if blob
-          raise_error("Your changes could not be committed because a file with the same name already exists")
+          raise_error('Your changes could not be committed because a file with the same name already exists')
         end
       end
     end

app/services/git_tag_push_service.rb

@@ -31,6 +31,7 @@ class GitTagPushService < BaseService
     unless Gitlab::Git.blank_ref?(params[:newrev])
       tag_name = Gitlab::Git.ref_name(params[:ref])
       tag = project.repository.find_tag(tag_name)
+      
       if tag && tag.target == params[:newrev]
         commit = project.commit(tag.target)
         commits = [commit].compact

app/services/issues/bulk_update_service.rb

@@ -9,6 +9,7 @@ module Issues
       end
 
       issues = Issue.where(id: issues_ids)
+
       issues.each do |issue|
         next unless can?(current_user, :update_issue, issue)
 

app/services/merge_requests/post_merge_service.rb

@@ -20,6 +20,7 @@ module MergeRequests
       return unless merge_request.target_branch == project.default_branch
 
       closed_issues = merge_request.closes_issues(current_user)
+
       closed_issues.each do |issue|
         if can?(current_user, :update_issue, issue)
           Issues::CloseService.new(project, current_user, {}).execute(issue, commit: merge_request)

app/services/notification_service.rb

@@ -157,6 +157,7 @@ class NotificationService
       else
         mentioned_users
       end
+
     recipients = recipients.concat(participants)
 
     # Merge project watchers
@@ -180,6 +181,7 @@ class NotificationService
 
     # build notify method like 'note_commit_email'
     notify_method = "note_#{note.noteable_type.underscore}_email".to_sym
+    
     recipients.each do |recipient|
       mailer.send(notify_method, recipient.id, note.id).deliver_later
     end

app/services/projects/update_service.rb

@@ -3,10 +3,11 @@ module Projects
     def execute
       # check that user is allowed to set specified visibility_level
       new_visibility = params[:visibility_level]
+      
       if new_visibility && new_visibility.to_i != project.visibility_level
         unless can?(current_user, :change_visibility_level, project) &&
           Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
-          
+
           deny_visibility_level(project, new_visibility)
           return project
         end

app/services/system_note_service.rb

@@ -82,7 +82,7 @@ class SystemNoteService
     end
 
     body << ' ' << 'label'.pluralize(labels_count)
-    body = "#{body.capitalize}"
+    body = body.capitalize
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
@@ -125,7 +125,7 @@ class SystemNoteService
   # Returns the created Note object
   def self.change_status(noteable, project, author, status, source)
     body = "Status changed to #{status}"
-    body += " by #{source.gfm_reference(project)}" if source
+    body << " by #{source.gfm_reference(project)}" if source
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
@@ -139,7 +139,7 @@ class SystemNoteService
 
   # Called when 'merge when build succeeds' is canceled
   def self.cancel_merge_when_build_succeeds(noteable, project, author)
-    body = "Canceled the automatic merge"
+    body = 'Canceled the automatic merge'
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
@@ -236,6 +236,7 @@ class SystemNoteService
       else
         'deleted'
       end
+
     body = "#{verb} #{branch_type.to_s} branch `#{branch}`".capitalize
     create_note(noteable: noteable, project: project, author: author, note: body)
   end

app/services/update_release_service.rb

@@ -21,8 +21,6 @@ class UpdateReleaseService < BaseService
   end
 
   def success(release)
-    out = super()
-    out[:release] = release
-    out
+    super().merge(release: release)
   end
 end

app/services/update_snippet_service.rb

@@ -9,6 +9,7 @@ class UpdateSnippetService < BaseService
   def execute
     # check that user is allowed to set specified visibility_level
     new_visibility = params[:visibility_level]
+    
     if new_visibility && new_visibility.to_i != snippet.visibility_level
       unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
         deny_visibility_level(snippet, new_visibility)

app/views/projects/_last_push.html.haml

@@ -7,7 +7,9 @@
             %span You pushed to
             = link_to namespace_project_commits_path(event.project.namespace, event.project, event.ref_name) do
               %strong= event.ref_name
-            branch
+            - if @project && event.project != @project
+              %span at
+              %strong= link_to_project event.project
             #{time_ago_with_tooltip(event.created_at)}
 
           .pull-right

app/views/projects/builds/show.html.haml

@@ -67,4 +67,4 @@
 = render "sidebar"
 
 :javascript
-  new CiBuild("#{namespace_project_build_url(@project.namespace, @project, @build, :json)}", "#{@build.status}", "#{trace_with_state[:state]}")
+  new CiBuild("#{namespace_project_build_url(@project.namespace, @project, @build)}", "#{namespace_project_build_url(@project.namespace, @project, @build, :json)}", "#{@build.status}", "#{trace_with_state[:state]}")

app/views/projects/commit/_ci_stage.html.haml

@@ -7,7 +7,7 @@
         = ci_icon_for_status(status)
       - if stage
          
-        = stage.titleize.pluralize
+        = stage.titleize
   = render statuses.latest.ordered, coverage: @project.build_coverage_enabled?, stage: false, ref: false, allow_retry: true
   = render statuses.retried.ordered, coverage: @project.build_coverage_enabled?, stage: false, ref: false, retried: true
   %tr

app/views/projects/pipelines/index.html.haml

@@ -50,7 +50,7 @@
             - stages.each do |stage|
               %th.stage
                 %span.has-tooltip{ title: "#{stage.titleize}" }
-                  = stage.titleize.pluralize
+                  = stage.titleize
             %th Duration
             %th
           = render @pipelines, commit_sha: true, stage: true, allow_retry: true, stages: stages

app/views/projects/tags/index.html.haml

@@ -7,22 +7,22 @@
     .nav-text
       Tags give the ability to mark specific points in history as being important
 
-    - if can? current_user, :push_code, @project
-      .nav-controls
+    .nav-controls
+      - if can? current_user, :push_code, @project
         = link_to new_namespace_project_tag_path(@project.namespace, @project), class: 'btn btn-create new-tag-btn' do
           New tag
-        .dropdown.inline
-          %button.dropdown-toggle.btn{ type: 'button', data: { toggle: 'dropdown'} }
-            %span.light= @sort.humanize
-            %b.caret
-          %ul.dropdown-menu.dropdown-menu-align-right
-            %li
-              = link_to namespace_project_tags_path(sort: nil) do
-                Name
-              = link_to namespace_project_tags_path(sort: sort_value_recently_updated) do
-                = sort_title_recently_updated
-              = link_to namespace_project_tags_path(sort: sort_value_oldest_updated) do
-                = sort_title_oldest_updated
+      .dropdown.inline
+        %button.dropdown-toggle.btn{ type: 'button', data: { toggle: 'dropdown'} }
+          %span.light= @sort.humanize
+          %b.caret
+        %ul.dropdown-menu.dropdown-menu-align-right
+          %li
+            = link_to namespace_project_tags_path(sort: nil) do
+              Name
+            = link_to namespace_project_tags_path(sort: sort_value_recently_updated) do
+              = sort_title_recently_updated
+            = link_to namespace_project_tags_path(sort: sort_value_oldest_updated) do
+              = sort_title_oldest_updated
 
   = render 'projects/commits/mirror_status'
 

app/views/shared/members/_member.html.haml

@@ -3,71 +3,74 @@
 - user = member.user
 
 %li.js-toggle-container{ class: dom_class(member), id: dom_id(member) }
-  %span{ class: ("list-item-name" if show_controls) }
-    - if user
-      = image_tag avatar_icon(user, 24), class: "avatar s24", alt: ''
-      %strong
-        = link_to user.name, user_path(user)
-      %span.cgray= user.username
-
-      - if user == current_user
-        %span.label.label-success It's you
-
-      - if user.blocked?
-        %label.label.label-danger
-          %strong Blocked
-
-      - if member.request?
-        %span.cgray
-          – Requested
-          = time_ago_with_tooltip(member.requested_at)
-    - else
-      = image_tag avatar_icon(member.invite_email, 24), class: "avatar s24", alt: ''
-      %strong= member.invite_email
-      %span.cgray
-        – Invited
-        - if member.created_by
-          by
-          = link_to member.created_by.name, user_path(member.created_by)
-        = time_ago_with_tooltip(member.created_at)
-
-      - if show_controls && can?(current_user, action_member_permission(:admin, member), member.source)
-        = link_to 'Resend invite', polymorphic_path([:resend_invite, member]),
-                  method: :post,
-                  class: 'btn-xs btn'
-
   - if show_roles
-    %span.pull-right
-      %strong= member.human_access
+    .controls
+      %strong.control-text= member.human_access
       - if show_controls
+        - if !user && can?(current_user, action_member_permission(:admin, member), member.source)
+          = link_to 'Resend invite', polymorphic_path([:resend_invite, member]),
+                    method: :post,
+                    class: 'btn'
+
         - if can?(current_user, action_member_permission(:update, member), member)
           = button_tag icon('pencil'),
                        type: 'button',
-                       class: 'btn-xs btn btn-grouped inline js-toggle-button',
+                       class: 'btn inline js-toggle-button',
                        title: 'Edit access level'
 
           - if member.request?
-             
             = link_to icon('check inverse'), polymorphic_path([:approve_access_request, member]),
                       method: :post,
-                      class: 'btn-xs btn btn-success',
+                      class: 'btn btn-success',
                       title: 'Grant access'
 
         - if can?(current_user, action_member_permission(:destroy, member), member)
-           
           - if current_user == user
             = link_to icon('sign-out', text: 'Leave'), polymorphic_path([:leave, member.source, :members]),
                       method: :delete,
                       data: { confirm: leave_confirmation_message(member.source) },
-                      class: 'btn-xs btn btn-remove'
+                      class: 'btn btn-remove'
           - else
             = link_to icon('trash'), member,
                       remote: true,
                       method: :delete,
                       data: { confirm: remove_member_message(member) },
-                      class: 'btn-xs btn btn-remove',
+                      class: 'btn btn-remove',
                       title: remove_member_title(member)
 
+
+  %span{ class: ("list-item-name" if show_controls) }
+    - if user
+      = image_tag avatar_icon(user, 40), class: "avatar s40", alt: ''
+      %strong
+        = link_to user.name, user_path(user)
+      %span.cgray= user.username
+
+      - if user == current_user
+        %span.label.label-success It's you
+
+      - if user.blocked?
+        %label.label.label-danger
+          %strong Blocked
+
+      .cgray
+        - if member.request?
+          Requested
+          = time_ago_with_tooltip(member.requested_at)
+        - else
+          Joined #{time_ago_with_tooltip(member.created_at)}
+
+    - else
+      = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: ''
+      %strong= member.invite_email
+      .cgray
+        Invited
+        - if member.created_by
+          by
+          = link_to member.created_by.name, user_path(member.created_by)
+        = time_ago_with_tooltip(member.created_at)
+
+  - if show_roles
     .edit-member.hide.js-toggle-content
       %br
       = form_for member, remote: true do |f|

features/steps/admin/groups.rb

@@ -62,7 +62,8 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps
 
   step 'I should see "johndoe@gitlab.com" in team list in every project as "Reporter"' do
     page.within ".group-users-list" do
-      expect(page).to have_content "johndoe@gitlab.com – Invited by"
+      expect(page).to have_content "johndoe@gitlab.com"
+      expect(page).to have_content "Invited by"
       expect(page).to have_content "Reporter"
     end
   end

lib/gitlab/github_import/branch_formatter.rb

@@ -4,7 +4,7 @@ module Gitlab
       delegate :repo, :sha, :ref, to: :raw_data
 
       def exists?
-        project.repository.branch_exists?(ref)
+        branch_exists? && commit_exists?
       end
 
       def name
@@ -15,11 +15,15 @@ module Gitlab
         repo.present?
       end
 
-      def valid?
-        repo.present?
+      private
+
+      def branch_exists?
+        project.repository.branch_exists?(ref)
       end
 
-      private
+      def commit_exists?
+        project.repository.commit(sha).present?
+      end
 
       def short_id
         sha.to_s[0..7]

lib/gitlab/github_import/importer.rb

@@ -131,8 +131,10 @@ module Gitlab
       def clean_up_restored_branches(branches)
         branches.each do |name, _|
           client.delete_ref(repo, "heads/#{name}")
-          project.repository.rm_branch(project.creator, name)
+          project.repository.delete_branch(name) rescue Rugged::ReferenceError
         end
+
+        project.repository.after_remove_branch
       end
 
       def apply_labels(issuable)

spec/features/pipelines_spec.rb

@@ -123,7 +123,7 @@ describe "Pipelines" do
     before { visit namespace_project_pipeline_path(project.namespace, project, pipeline) }
 
     it 'showing a list of builds' do
-      expect(page).to have_content('Tests')
+      expect(page).to have_content('Test')
       expect(page).to have_content(@success.id)
       expect(page).to have_content('Deploy')
       expect(page).to have_content(@failed.id)

spec/lib/gitlab/github_import/branch_formatter_spec.rb

@@ -2,17 +2,18 @@ require 'spec_helper'
 
 describe Gitlab::GithubImport::BranchFormatter, lib: true do
   let(:project) { create(:project) }
+  let(:commit) { create(:commit, project: project) }
   let(:repo) { double }
   let(:raw) do
     {
       ref: 'feature',
       repo: repo,
-      sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+      sha: commit.id
     }
   end
 
   describe '#exists?' do
-    it 'returns true when branch exists' do
+    it 'returns true when both branch, and commit exists' do
       branch = described_class.new(project, double(raw))
 
       expect(branch.exists?).to eq true
@@ -23,6 +24,12 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
 
       expect(branch.exists?).to eq false
     end
+
+    it 'returns false when commit does not exist' do
+      branch = described_class.new(project, double(raw.merge(sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b')))
+
+      expect(branch.exists?).to eq false
+    end
   end
 
   describe '#name' do
@@ -33,7 +40,7 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
     end
 
     it 'returns formatted ref when branch does not exist' do
-      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch')))
+      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b')))
 
       expect(branch.name).to eq 'removed-branch-2e5d3239'
     end
@@ -51,18 +58,18 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
     it 'returns raw sha' do
       branch = described_class.new(project, double(raw))
 
-      expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+      expect(branch.sha).to eq commit.id
     end
   end
 
   describe '#valid?' do
-    it 'returns true when repository exists' do
+    it 'returns true when raw repo is present' do
       branch = described_class.new(project, double(raw))
 
       expect(branch.valid?).to eq true
     end
 
-    it 'returns false when repository does not exist' do
+    it 'returns false when raw repo is blank' do
       branch = described_class.new(project, double(raw.merge(repo: nil)))
 
       expect(branch.valid?).to eq false

spec/lib/gitlab/github_import/pull_request_formatter_spec.rb

@@ -2,11 +2,13 @@ require 'spec_helper'
 
 describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
   let(:project) { create(:project) }
+  let(:source_sha) { create(:commit, project: project).id }
+  let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id }
   let(:repository) { double(id: 1, fork: false) }
   let(:source_repo) { repository }
-  let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') }
+  let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: source_sha) }
   let(:target_repo) { repository }
-  let(:target_branch) { double(ref: 'master', repo: target_repo, sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7') }
+  let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) }
   let(:octocat) { double(id: 123456, login: 'octocat') }
   let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') }
   let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') }
@@ -41,10 +43,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'opened',
           milestone: nil,
           author_id: project.creator_id,
@@ -68,10 +70,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'closed',
           milestone: nil,
           author_id: project.creator_id,
@@ -95,10 +97,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'merged',
           milestone: nil,
           author_id: project.creator_id,