Merge remote-tracking branch 'upstream/master' into pipeline-notifications

* upstream/master: (43 commits)
  Disable warming of the asset cache in Spinach tests under CI
  Trim project_path whitespace on form submit
  added skipped definition
  updated some links in definitions
  Don't use Hash#slice since it's not supported in Ruby 2.1
  Create protected branches bundle
  [ci skip] Add a comment explaining validate_board_limit callback
  Fix: Backup restore doesn't clear cache
  Fix GitLab project import when a user has access only to their default namespace.
  Test GitLab project import for a user with only their default namespace.
  We want to release this in 8.13.0
  Add CHANGELOG.md entry
  Return truncation for notification descriptions, fix minor bugs with rendering
  Use guard clause instead of if-else statement
  Tests for markdown HipChat notifications
  Clean up Banzai HTML for HipChat
  Ensure absolute URLs for single lines from Banzai for HipChat
  Absolute URLs for Banzai HTML for HipChat
  Also render commit titles in HipChat notifications
  Full Banzai rendering for HipChat notifications
  ...

CHANGELOG.md

@@ -3,6 +3,10 @@ Please view this file on the master branch, on stable branches it's out of date.
 ## 8.14.0 (2016-11-22)
   - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
   - Add CI notifications. Who triggered a pipeline would receive an email after the pipeline is succeeded or failed. Users could also update notification settings accordingly. !6342
+  - Trim leading and trailing whitespace on project_path (Linus Thiel)
+  - Fix HipChat notifications rendering (airatshigapov, eisnerd)
+  - Simpler arguments passed to named_route on toggle_award_url helper method
+  - Fix: Backup restore doesn't clear cache
 
 ## 8.13.0 (2016-10-22)
 
@@ -22,11 +26,13 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Add `/projects/visible` API endpoint (Ben Boeckel)
   - Fix centering of custom header logos (Ashley Dumaine)
   - Keep around commits only pipeline creation as pipeline data doesn't change over time
+  - Update duration at the end of pipeline
   - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup
   - Add group level labels. (!6425)
   - Add an example for testing a phoenix application with Gitlab CI in the docs (Manthan Mallikarjun)
   - Cancelled pipelines could be retried. !6927
   - Updating verbiage on git basics to be more intuitive
+  - Fix project_feature record not generated on project creation
   - Clarify documentation for Runners API (Gennady Trafimenkov)
   - The instrumentation for Banzai::Renderer has been restored
   - Change user & group landing page routing from /u/:username to /:username
@@ -35,11 +41,14 @@ Please view this file on the master branch, on stable branches it's out of date.
   - AbstractReferenceFilter caches project_refs on RequestStore when active
   - Replaced the check sign to arrow in the show build view. !6501
   - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar)
+  - ProjectCacheWorker updates caches at most once per 15 minutes per project
   - Fix Error 500 when viewing old merge requests with bad diff data
   - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar)
+  - Fix viewing merged MRs when the source project has been removed !6991
   - Speed-up group milestones show page
   - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps)
   - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService
+  - Fix discussion thread from emails for merge requests. !7010
   - Don't include archived projects when creating group milestones. !4940 (Jeroen Jacobs)
   - Add tag shortcut from the Commit page. !6543
   - Keep refs for each deployment
@@ -135,6 +144,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Delete dynamic environments
   - Fix buggy iOS tooltip layering behavior.
   - Make guests unable to view MRs on private projects
+  - Fix broken Project API docs (Takuya Noguchi)
 
 ## 8.12.7
 

app/assets/javascripts/protected_branches/protected_branches_bundle.js

+/*= require_tree . */

app/assets/stylesheets/pages/login.scss

@@ -53,6 +53,7 @@
       margin: 0 0 10px;
     }
 
+
     .login-footer {
       margin-top: 10px;
 
@@ -246,3 +247,19 @@
     padding: 65px; // height of footer + bottom padding of email confirmation link
   }
 }
+
+// For sign in pane only, to improve tab order, the following removes the submit button from
+// normal document flow and pins it to the bottom of the form. For context, see !6867 & !6928
+
+.login-box {
+  .new_user {
+    position: relative;
+    padding-bottom: 35px;
+  }
+
+  .move-submit-down {
+    position: absolute;
+    width: 100%;
+    bottom: 0;
+  }
+}

app/assets/stylesheets/pages/pipelines.scss

@@ -369,10 +369,6 @@
 
     &:hover {
       background-color: $gray-lighter;
-
-      .dropdown-menu-toggle {
-        background-color: transparent;
-      }
     }
 
     &.playable {
@@ -402,6 +398,15 @@
         }
       }
 
+      .tooltip {
+        white-space: nowrap;
+
+        .tooltip-inner {
+          overflow: hidden;
+          text-overflow: ellipsis;
+        }
+      }
+
       .ci-status-text {
         width: 135px;
         white-space: nowrap;
@@ -419,6 +424,7 @@
       }
 
       .dropdown-menu-toggle {
+        background-color: transparent;
         border: none;
         width: auto;
         padding: 0;

app/controllers/projects/commits_controller.rb

@@ -13,7 +13,7 @@ def show
 
     @commits =
       if search.present?
-        @repository.find_commits_by_message(search, @ref, @path, @limit, @offset).compact
+        @repository.find_commits_by_message(search, @ref, @path, @limit, @offset)
       else
         @repository.commits(@ref, path: @path, limit: @limit, offset: @offset)
       end

app/controllers/projects/merge_requests_controller.rb

@@ -398,7 +398,7 @@ def ci_status
 
       status ||= "preparing"
     else
-      ci_service = @merge_request.source_project.ci_service
+      ci_service = @merge_request.source_project.try(:ci_service)
       status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service
 
       if ci_service.respond_to?(:commit_coverage)
@@ -554,7 +554,7 @@ def define_diff_comment_vars
   def define_pipelines_vars
     @pipelines = @merge_request.all_pipelines
 
-    if @pipelines.any?
+    if @pipelines.present?
       @pipeline = @pipelines.first
       @statuses = @pipeline.statuses.relevant
     end

app/controllers/projects/project_members_controller.rb

@@ -32,21 +32,6 @@ def create
       current_user: current_user
     )
 
-    if params[:group_ids].present?
-      group_ids = params[:group_ids].split(',')
-      groups = Group.where(id: group_ids)
-
-      groups.each do |group|
-        next unless can?(current_user, :read_group, group)
-
-        project.project_group_links.create(
-          group: group,
-          group_access: params[:access_level],
-          expires_at: params[:expires_at]
-        )
-      end
-    end
-
     redirect_to namespace_project_project_members_path(@project.namespace, @project)
   end
 

app/helpers/award_emoji_helper.rb

@@ -3,8 +3,8 @@ def toggle_award_url(awardable)
     return url_for([:toggle_award_emoji, awardable]) unless @project
 
     if awardable.is_a?(Note)
-      # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (6.5x)
-      toggle_award_emoji_namespace_project_note_url(namespace_id: @project.namespace, project_id: @project, id: awardable.id)
+      # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x)
+      toggle_award_emoji_namespace_project_note_url(@project.namespace, @project, awardable.id)
     else
       url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable])
     end

app/helpers/merge_requests_helper.rb

@@ -86,11 +86,15 @@ def mr_assign_issues_link
   end
 
   def source_branch_with_namespace(merge_request)
-    branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch))
+    namespace = merge_request.source_project_namespace
+    branch = merge_request.source_branch
+
+    if merge_request.source_branch_exists?
+      namespace = link_to(namespace, project_path(merge_request.source_project))
+      branch = link_to(branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch))
+    end
 
     if merge_request.for_fork?
-      namespace = link_to(merge_request.source_project_namespace,
-        project_path(merge_request.source_project))
       namespace + ":" + branch
     else
       branch

app/models/ci/pipeline.rb

@@ -59,9 +59,6 @@ class Pipeline < ActiveRecord::Base
 
       before_transition any => [:success, :failed, :canceled] do |pipeline|
         pipeline.finished_at = Time.now
-      end
-
-      before_transition do |pipeline|
         pipeline.update_duration
       end
 

app/models/deployment.rb

@@ -11,7 +11,7 @@ class Deployment < ActiveRecord::Base
 
   delegate :name, to: :environment, prefix: true
 
-  after_save :create_ref
+  after_create :create_ref
 
   def commit
     project.commit(sha)
@@ -102,6 +102,6 @@ def formatted_deployment_time
   private
 
   def ref_path
-    File.join(environment.ref_path, 'deployments', id.to_s)
+    File.join(environment.ref_path, 'deployments', iid.to_s)
   end
 end

app/models/environment.rb

@@ -71,8 +71,8 @@ def first_deployment_for(commit)
 
     return nil unless ref
 
-    deployment_id = ref.split('/').last
-    deployments.find(deployment_id)
+    deployment_iid = ref.split('/').last
+    deployments.find_by(iid: deployment_iid)
   end
 
   def ref_path

app/models/merge_request.rb

@@ -326,21 +326,17 @@ def validate_branches
   def validate_fork
     return true unless target_project && source_project
     return true if target_project == source_project
-    return true unless forked_source_project_missing?
+    return true unless source_project_missing?
 
     errors.add :validate_fork,
                'Source project is not a fork of the target project'
   end
 
   def closed_without_fork?
-    closed? && forked_source_project_missing?
+    closed? && source_project_missing?
   end
 
-  def closed_without_source_project?
-    closed? && !source_project
-  end
-
-  def forked_source_project_missing?
+  def source_project_missing?
     return false unless for_fork?
     return true unless source_project
 
@@ -348,9 +344,7 @@ def forked_source_project_missing?
   end
 
   def reopenable?
-    return false if closed_without_fork? || closed_without_source_project? || merged?
-
-    closed?
+    closed? && !source_project_missing? && source_branch_exists?
   end
 
   def ensure_merge_request_diff
@@ -662,7 +656,7 @@ def locked_long_ago?
   end
 
   def has_ci?
-    source_project.ci_service && commits.any?
+    source_project.try(:ci_service) && commits.any?
   end
 
   def branch_missing?
@@ -694,12 +688,9 @@ def environments
 
     @environments ||=
       begin
-        environments = source_project.environments_for(
-          source_branch, diff_head_commit)
-        environments += target_project.environments_for(
-          target_branch, diff_head_commit, with_tags: true)
-
-        environments.uniq
+        envs = target_project.environments_for(target_branch, diff_head_commit, with_tags: true)
+        envs.concat(source_project.environments_for(source_branch, diff_head_commit)) if source_project
+        envs.uniq
       end
   end
 

app/models/project.rb

@@ -32,8 +32,8 @@ class BoardLimitExceeded < StandardError; end
   default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled }
 
   after_create :ensure_dir_exist
+  after_create :create_project_feature, unless: :project_feature
   after_save :ensure_dir_exist, if: :namespace_id_changed?
-  after_initialize :setup_project_feature
 
   # set last_activity_at to the same as created_at
   after_create :set_last_activity_at
@@ -1310,11 +1310,6 @@ def pushes_since_gc_redis_key
     "projects/#{id}/pushes_since_gc"
   end
 
-  # Prevents the creation of project_feature record for every project
-  def setup_project_feature
-    build_project_feature unless project_feature
-  end
-
   def default_branch_protected?
     current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL ||
       current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE
@@ -1344,6 +1339,13 @@ def authorized_for_user_by_shared_projects?(user, min_access_level)
     shared_projects.any?
   end
 
+  # Similar to the normal callbacks that hook into the life cycle of an
+  # Active Record object, you can also define callbacks that get triggered
+  # when you add an object to an association collection. If any of these
+  # callbacks throw an exception, the object will not be added to the
+  # collection. Before you add a new board to the boards collection if you
+  # already have 1, 2, or n it will fail, but it if you have 0 that is lower
+  # than the number of permitted boards per project it won't fail.
   def validate_board_limit(board)
     raise BoardLimitExceeded, 'Number of permitted boards exceeded' if boards.size >= NUMBER_OF_PERMITTED_BOARDS
   end

app/models/project_services/hipchat_service.rb

 class HipchatService < Service
+  include ActionView::Helpers::SanitizeHelper
+
   MAX_COMMITS = 3
+  HIPCHAT_ALLOWED_TAGS = %w[
+    a b i strong em br img pre code
+    table th tr td caption colgroup col thead tbody tfoot
+    ul ol li dl dt dd
+  ]
 
   prop_accessor :token, :room, :server, :notify, :color, :api_version
   boolean_accessor :notify_only_broken_builds
@@ -88,6 +95,10 @@ def create_message(data)
     end
   end
 
+  def render_line(text)
+    markdown(text.lines.first.chomp, pipeline: :single_line) if text
+  end
+
   def create_push_message(push)
     ref_type = Gitlab::Git.tag_ref?(push[:ref]) ? 'tag' : 'branch'
     ref = Gitlab::Git.ref_name(push[:ref])
@@ -110,7 +121,7 @@ def create_push_message(push)
       message << "(<a href=\"#{project.web_url}/compare/#{before}...#{after}\">Compare changes</a>)"
 
       push[:commits].take(MAX_COMMITS).each do |commit|
-        message << "<br /> - #{commit[:message].lines.first} (<a href=\"#{commit[:url]}\">#{commit[:id][0..5]}</a>)"
+        message << "<br /> - #{render_line(commit[:message])} (<a href=\"#{commit[:url]}\">#{commit[:id][0..5]}</a>)"
       end
 
       if push[:commits].count > MAX_COMMITS
@@ -121,12 +132,22 @@ def create_push_message(push)
     message
   end
 
-  def format_body(body)
-    if body
-      body = body.truncate(200, separator: ' ', omission: '...')
-    end
+  def markdown(text, options = {})
+    return "" unless text
+
+    context = {
+      project: project,
+      pipeline: :email
+    }
+
+    Banzai.render(text, context)
 
-    "<pre>#{body}</pre>"
+    context.merge!(options)
+
+    html = Banzai.post_process(Banzai.render(text, context), context)
+    sanitized_html = sanitize(html, tags: HIPCHAT_ALLOWED_TAGS, attributes: %w[href title alt])
+
+    sanitized_html.truncate(200, separator: ' ', omission: '...')
   end
 
   def create_issue_message(data)
@@ -134,7 +155,7 @@ def create_issue_message(data)
 
     obj_attr = data[:object_attributes]
     obj_attr = HashWithIndifferentAccess.new(obj_attr)
-    title = obj_attr[:title]
+    title = render_line(obj_attr[:title])
     state = obj_attr[:state]
     issue_iid = obj_attr[:iid]
     issue_url = obj_attr[:url]
@@ -143,10 +164,7 @@ def create_issue_message(data)
     issue_link = "<a href=\"#{issue_url}\">issue ##{issue_iid}</a>"
     message = "#{user_name} #{state} #{issue_link} in #{project_link}: <b>#{title}</b>"
 
-    if description
-      description = format_body(description)
-      message << description
-    end
+    message << "<pre>#{markdown(description)}</pre>"
 
     message
   end
@@ -159,23 +177,20 @@ def create_merge_request_message(data)
     merge_request_id = obj_attr[:iid]
     state = obj_attr[:state]
     description = obj_attr[:description]
-    title = obj_attr[:title]
+    title = render_line(obj_attr[:title])
 
     merge_request_url = "#{project_url}/merge_requests/#{merge_request_id}"
     merge_request_link = "<a href=\"#{merge_request_url}\">merge request !#{merge_request_id}</a>"
     message = "#{user_name} #{state} #{merge_request_link} in " \
       "#{project_link}: <b>#{title}</b>"
 
-    if description
-      description = format_body(description)
-      message << description
-    end
+    message << "<pre>#{markdown(description)}</pre>"
 
     message
   end
 
   def format_title(title)
-    "<b>" + title.lines.first.chomp + "</b>"
+    "<b>#{render_line(title)}</b>"
   end
 
   def create_note_message(data)
@@ -186,11 +201,13 @@ def create_note_message(data)
     note = obj_attr[:note]
     note_url = obj_attr[:url]
     noteable_type = obj_attr[:noteable_type]
+    commit_id = nil
 
     case noteable_type
     when "Commit"
       commit_attr = HashWithIndifferentAccess.new(data[:commit])
-      subject_desc = commit_attr[:id]
+      commit_id = commit_attr[:id]
+      subject_desc = commit_id
       subject_desc = Commit.truncate_sha(subject_desc)
       subject_type = "commit"
       title = format_title(commit_attr[:message])
@@ -218,10 +235,7 @@ def create_note_message(data)
     message = "#{user_name} commented on #{subject_html} in #{project_link}: "
     message << title
 
-    if note
-      note = format_body(note)
-      message << note
-    end
+    message << "<pre>#{markdown(note, ref: commit_id)}</pre>"
 
     message
   end