Add latest changes from gitlab-org/gitlab@master

13 files changed, 174 insertions, 34 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 5e52062ef40..3bf19399cec 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -217,6 +217,8 @@ module Ci
     scope :for_sha, -> (sha) { where(sha: sha) }
     scope :for_source_sha, -> (source_sha) { where(source_sha: source_sha) }
     scope :for_sha_or_source_sha, -> (sha) { for_sha(sha).or(for_source_sha(sha)) }
+    scope :for_ref, -> (ref) { where(ref: ref) }
+    scope :for_id, -> (id) { where(id: id) }
     scope :created_after, -> (time) { where('ci_pipelines.created_at > ?', time) }
 
     scope :triggered_by_merge_request, -> (merge_request) do
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 94a0de556f4..852576dbbc2 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -25,12 +25,20 @@ module Issuable
   include UpdatedAtFilterable
   include IssuableStates
   include ClosedAtFilterable
+  include VersionedDescription
 
   TITLE_LENGTH_MAX = 255
   TITLE_HTML_LENGTH_MAX = 800
   DESCRIPTION_LENGTH_MAX = 1.megabyte
   DESCRIPTION_HTML_LENGTH_MAX = 5.megabytes
 
+  STATE_ID_MAP = {
+    opened: 1,
+    closed: 2,
+    merged: 3,
+    locked: 4
+  }.with_indifferent_access.freeze
+
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests
   # lists avoiding n+1 queries and improving performance.
@@ -172,13 +180,17 @@ module Issuable
       fuzzy_search(query, [:title])
     end
 
-    # Available state values persisted in state_id column using state machine
+    def available_states
+      @available_states ||= STATE_ID_MAP.slice(*available_state_names)
+    end
+
+    # Available state names used to persist state_id column using state machine
     #
     # Override this on subclasses if different states are needed
     #
-    # Check MergeRequest.available_states for example
-    def available_states
-      @available_states ||= { opened: 1, closed: 2 }.with_indifferent_access
+    # Check MergeRequest.available_states_names for example
+    def available_state_names
+      [:opened, :closed]
     end
 
     # Searches for records with a matching title or description.
@@ -297,6 +309,14 @@ module Issuable
     end
   end
 
+  def state
+    self.class.available_states.key(state_id)
+  end
+
+  def state=(value)
+    self.state_id = self.class.available_states[value]
+  end
+
   def resource_parent
     project
   end
diff --git a/app/models/concerns/issuable_states.rb b/app/models/concerns/issuable_states.rb
index 33bc41d7f44..f0b9f0d1f3a 100644
--- a/app/models/concerns/issuable_states.rb
+++ b/app/models/concerns/issuable_states.rb
@@ -4,22 +4,20 @@ module IssuableStates
   extend ActiveSupport::Concern
 
   # The state:string column is being migrated to state_id:integer column
-  # This is a temporary hook to populate state_id column with new values
-  # and should be removed after the state column is removed.
-  # Check https://gitlab.com/gitlab-org/gitlab-foss/issues/51789 for more information
+  # This is a temporary hook to keep state column in sync until it is removed.
+  # Check https: https://gitlab.com/gitlab-org/gitlab/issues/33814 for more information
+  # The state column can be safely removed after 2019-10-27
   included do
-    before_save :set_state_id
+    before_save :sync_issuable_deprecated_state
   end
 
-  def set_state_id
-    return if state.nil? || state.empty?
+  def sync_issuable_deprecated_state
+    return if self.is_a?(Epic)
+    return unless respond_to?(:state)
+    return if state_id.nil?
 
-    # Needed to prevent breaking some migration specs that
-    # rollback database to a point where state_id does not exist.
-    # We can use this guard clause for now since this file will
-    # be removed in the next release.
-    return unless self.has_attribute?(:state_id)
+    deprecated_state = self.class.available_states.key(state_id)
 
-    self.state_id = self.class.available_states[state]
+    self.write_attribute(:state, deprecated_state)
   end
 end
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 3deb86da6cf..42b370990ac 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -6,7 +6,9 @@ module Milestoneish
   end
 
   def closed_issues_count(user)
-    count_issues_by_state(user)['closed'].to_i
+    closed_state_id = Issue.available_states[:closed]
+
+    count_issues_by_state(user)[closed_state_id].to_i
   end
 
   def complete?(user)
@@ -117,7 +119,7 @@ module Milestoneish
 
   def count_issues_by_state(user)
     memoize_per_user(user, :count_issues_by_state) do
-      issues_visible_to_user(user).reorder(nil).group(:state).count
+      issues_visible_to_user(user).reorder(nil).group(:state_id).count
     end
   end
 
diff --git a/app/models/concerns/versioned_description.rb b/app/models/concerns/versioned_description.rb
new file mode 100644
index 00000000000..63a24aadc8a
--- /dev/null
+++ b/app/models/concerns/versioned_description.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module VersionedDescription
+  extend ActiveSupport::Concern
+
+  included do
+    attr_accessor :saved_description_version
+
+    has_many :description_versions
+
+    after_update :save_description_version
+  end
+
+  private
+
+  def save_description_version
+    self.saved_description_version = nil
+
+    return unless Feature.enabled?(:save_description_versions, issuing_parent)
+    return unless saved_change_to_description?
+
+    unless description_versions.exists?
+      description_versions.create!(
+        description: description_before_last_save,
+        created_at: created_at
+      )
+    end
+
+    self.saved_description_version = description_versions.create!(description: description)
+  end
+end
diff --git a/app/models/concerns/worker_attributes.rb b/app/models/concerns/worker_attributes.rb
new file mode 100644
index 00000000000..af40e9e3b19
--- /dev/null
+++ b/app/models/concerns/worker_attributes.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module WorkerAttributes
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def feature_category(value)
+      raise "Invalid category. Use `feature_category_not_owned!` to mark a worker as not owned" if value == :not_owned
+
+      worker_attributes[:feature_category] = value
+    end
+
+    # Special case: mark this work as not associated with a feature category
+    # this should be used for cross-cutting concerns, such as mailer workers.
+    def feature_category_not_owned!
+      worker_attributes[:feature_category] = :not_owned
+    end
+
+    def get_feature_category
+      get_worker_attribute(:feature_category)
+    end
+
+    def feature_category_not_owned?
+      get_worker_attribute(:feature_category) == :not_owned
+    end
+
+    protected
+
+    # Returns a worker attribute declared on this class or its parent class.
+    # This approach allows declared attributes to be inherited by
+    # child classes.
+    def get_worker_attribute(name)
+      worker_attributes[name] || superclass_worker_attributes(name)
+    end
+
+    private
+
+    def worker_attributes
+      @attributes ||= {}
+    end
+
+    def superclass_worker_attributes(name)
+      return unless superclass.include? WorkerAttributes
+
+      superclass.get_worker_attribute(name)
+    end
+  end
+end
diff --git a/app/models/description_version.rb b/app/models/description_version.rb
new file mode 100644
index 00000000000..abab7f94212
--- /dev/null
+++ b/app/models/description_version.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class DescriptionVersion < ApplicationRecord
+  belongs_to :issue
+  belongs_to :merge_request
+
+  validate :exactly_one_issuable
+
+  def self.issuable_attrs
+    %i(issue merge_request).freeze
+  end
+
+  private
+
+  def exactly_one_issuable
+    issuable_count = self.class.issuable_attrs.count { |attr| self["#{attr}_id"] }
+
+    errors.add(:base, "Exactly one of #{self.class.issuable_attrs.join(', ')} is required") if issuable_count != 1
+  end
+end
+
+DescriptionVersion.prepend_if_ee('EE::DescriptionVersion')
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d0b2165fcc7..b9b481ac29b 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -71,7 +71,7 @@ class Issue < ApplicationRecord
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true
 
-  state_machine :state, initial: :opened do
+  state_machine :state_id, initial: :opened do
     event :close do
       transition [:opened] => :closed
     end
@@ -80,8 +80,8 @@ class Issue < ApplicationRecord
       transition closed: :opened
     end
 
-    state :opened
-    state :closed
+    state :opened, value: Issue.available_states[:opened]
+    state :closed, value: Issue.available_states[:closed]
 
     before_transition any => :closed do |issue|
       issue.closed_at = issue.system_note_timestamp
@@ -93,6 +93,13 @@ class Issue < ApplicationRecord
     end
   end
 
+  # Alias to state machine .with_state_id method
+  # This needs to be defined after the state machine block to avoid errors
+  class << self
+    alias_method :with_state, :with_state_id
+    alias_method :with_states, :with_state_ids
+  end
+
   def self.relative_positioning_query_base(issue)
     in_projects(issue.parent_ids)
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 50efe5d6aa0..7cdaa3e3ca7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -85,7 +85,13 @@ class MergeRequest < ApplicationRecord
   # when creating new merge request
   attr_accessor :can_be_created, :compare_commits, :diff_options, :compare
 
-  state_machine :state, initial: :opened do
+  # Keep states definition to be evaluated before the state_machine block to avoid spec failures.
+  # If this gets evaluated after, the `merged` and `locked` states which are overrided can be nil.
+  def self.available_state_names
+    super + [:merged, :locked]
+  end
+
+  state_machine :state_id, initial: :opened do
     event :close do
       transition [:opened] => :closed
     end
@@ -116,10 +122,17 @@ class MergeRequest < ApplicationRecord
       end
     end
 
-    state :opened
-    state :closed
-    state :merged
-    state :locked
+    state :opened, value: MergeRequest.available_states[:opened]
+    state :closed, value: MergeRequest.available_states[:closed]
+    state :merged, value: MergeRequest.available_states[:merged]
+    state :locked, value: MergeRequest.available_states[:locked]
+  end
+
+  # Alias to state machine .with_state_id method
+  # This needs to be defined after the state machine block to avoid errors
+  class << self
+    alias_method :with_state, :with_state_id
+    alias_method :with_states, :with_state_ids
   end
 
   state_machine :merge_status, initial: :unchecked do
@@ -211,10 +224,6 @@ class MergeRequest < ApplicationRecord
     '!'
   end
 
-  def self.available_states
-    @available_states ||= super.merge(merged: 3, locked: 4)
-  end
-
   # Returns the top 100 target branches
   #
   # The returned value is a Array containing branch names
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index fe8ba9765b7..735ad046f22 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -83,7 +83,7 @@ class MergeRequestDiff < ApplicationRecord
 
     metrics_join = mr_diffs.join(mr_metrics).on(metrics_join_condition)
 
-    condition = MergeRequest.arel_table[:state].eq(:merged)
+    condition = MergeRequest.arel_table[:state_id].eq(MergeRequest.available_states[:merged])
       .and(MergeRequest::Metrics.arel_table[:merged_at].lteq(before))
       .and(MergeRequest::Metrics.arel_table[:merged_at].not_eq(nil))
 
@@ -91,7 +91,7 @@ class MergeRequestDiff < ApplicationRecord
   end
 
   scope :old_closed_diffs, -> (before) do
-    condition = MergeRequest.arel_table[:state].eq(:closed)
+    condition = MergeRequest.arel_table[:state_id].eq(MergeRequest.available_states[:closed])
       .and(MergeRequest::Metrics.arel_table[:latest_closed_at].lteq(before))
 
     joins(merge_request: :metrics).where(condition)
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 1d0b37abf72..019bd54f48c 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -161,7 +161,7 @@ class HipchatService < Service
     obj_attr = data[:object_attributes]
     obj_attr = HashWithIndifferentAccess.new(obj_attr)
     title = render_line(obj_attr[:title])
-    state = obj_attr[:state]
+    state = Issue.available_states.key(obj_attr[:state_id])
     issue_iid = obj_attr[:iid]
     issue_url = obj_attr[:url]
     description = obj_attr[:description]
diff --git a/app/models/push_event.rb b/app/models/push_event.rb
index 6f7365a2763..5cab686f20b 100644
--- a/app/models/push_event.rb
+++ b/app/models/push_event.rb
@@ -54,7 +54,7 @@ class PushEvent < Event
       .select(1)
       .where('merge_requests.source_project_id = events.project_id')
       .where('merge_requests.source_branch = push_event_payloads.ref')
-      .where(state: :opened)
+      .with_state(:opened)
 
     # For reasons unknown the use of #eager_load will result in the
     # "push_event_payload" association not being set. Because of this we're
diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb
index 8ec90ca25d3..11cbeb60bba 100644
--- a/app/models/system_note_metadata.rb
+++ b/app/models/system_note_metadata.rb
@@ -23,6 +23,7 @@ class SystemNoteMetadata < ApplicationRecord
   validates :action, inclusion: { in: :icon_types }, allow_nil: true
 
   belongs_to :note
+  belongs_to :description_version
 
   def icon_types
     ICON_TYPES