diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-18 14:11:44 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-18 14:11:44 +0300 |
commit | 25989ab7ef1a444ed2abd5479f176d58e1d9462a (patch) | |
tree | 271bb24f3c7178f320cb9de0be0833a285327d09 /app/models | |
parent | 9bbb32b29703f3ce33dd35d5101145774b793a6d (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/ci/pipeline.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 28 | ||||
-rw-r--r-- | app/models/concerns/issuable_states.rb | 22 | ||||
-rw-r--r-- | app/models/concerns/milestoneish.rb | 6 | ||||
-rw-r--r-- | app/models/concerns/versioned_description.rb | 31 | ||||
-rw-r--r-- | app/models/concerns/worker_attributes.rb | 48 | ||||
-rw-r--r-- | app/models/description_version.rb | 22 | ||||
-rw-r--r-- | app/models/issue.rb | 13 | ||||
-rw-r--r-- | app/models/merge_request.rb | 27 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 4 | ||||
-rw-r--r-- | app/models/project_services/hipchat_service.rb | 2 | ||||
-rw-r--r-- | app/models/push_event.rb | 2 | ||||
-rw-r--r-- | app/models/system_note_metadata.rb | 1 |
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 |