diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-06-05 13:51:59 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-06-05 13:51:59 +0300 |
commit | 79380fe5fb5943179960f4879e174eb977b918d3 (patch) | |
tree | 94953113c8bf5174cf52fb78d90e28a8c14c20c7 /app/models | |
parent | 3801a0df80306a76dc340ca74427a124a1514dbb (diff) | |
parent | c34107608ecc5c36e80a748eb4c9b88d2b1157cf (diff) |
Merge branch 'master' into feature/gb/persist-pipeline-stages
* master: (524 commits)
Improve user experience around slash commands in instant comments
Fix LFS timeouts when trying to save large files
retryable? is now available for CommitStatus
Resolve "Documentation of `.gitlab-ci.yml` states incorrect example for variables"
Fix test failures
Add slugify project path to CI enviroment variables
Fixed typo: craeted -> created
32118 Make New environment empty state btn lowercase
Expose import_status in Projects API
32832 Remove overflow from comment form for confidential issues and vertically aligns confidential issue icon
Fix test failures
Allow manual bypass of auto_sign_in_with_provider
Fix keys seed
Allow users to be hard-deleted from the API
fixup some classnames and media queries
Enable the Style/PreferredHashMethods cop
Lint our factories creation in addition to their build
Don’t schedule workers from inside transactions
Allow scheduling from after_commit hooks
Forbid Sidekiq scheduling in transactions
...
Conflicts:
app/serializers/pipeline_entity.rb
db/schema.rb
spec/factories/ci/stages.rb
spec/lib/gitlab/import_export/safe_model_attributes.yml
spec/services/ci/create_pipeline_service_spec.rb
spec/spec_helper.rb
Diffstat (limited to 'app/models')
80 files changed, 522 insertions, 479 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 043f57241a3..9e04976e8fd 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -13,13 +13,13 @@ class ApplicationSetting < ActiveRecord::Base [\r\n] # any number of newline characters }x - serialize :restricted_visibility_levels - serialize :import_sources - serialize :disabled_oauth_sign_in_sources, Array - serialize :domain_whitelist, Array - serialize :domain_blacklist, Array - serialize :repository_storages - serialize :sidekiq_throttling_queues, Array + serialize :restricted_visibility_levels # rubocop:disable Cop/ActiverecordSerialize + serialize :import_sources # rubocop:disable Cop/ActiverecordSerialize + serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiverecordSerialize + serialize :domain_whitelist, Array # rubocop:disable Cop/ActiverecordSerialize + serialize :domain_blacklist, Array # rubocop:disable Cop/ActiverecordSerialize + serialize :repository_storages # rubocop:disable Cop/ActiverecordSerialize + serialize :sidekiq_throttling_queues, Array # rubocop:disable Cop/ActiverecordSerialize cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -143,7 +143,7 @@ class ApplicationSetting < ActiveRecord::Base validates_each :restricted_visibility_levels do |record, attr, value| value&.each do |level| - unless Gitlab::VisibilityLevel.options.has_value?(level) + unless Gitlab::VisibilityLevel.options.value?(level) record.errors.add(attr, "'#{level}' is not a valid visibility level") end end @@ -151,7 +151,7 @@ class ApplicationSetting < ActiveRecord::Base validates_each :import_sources do |record, attr, value| value&.each do |source| - unless Gitlab::ImportSources.options.has_value?(source) + unless Gitlab::ImportSources.options.value?(source) record.errors.add(attr, "'#{source}' is not a import source") end end diff --git a/app/models/audit_event.rb b/app/models/audit_event.rb index 967ffd46db0..46d412fbd72 100644 --- a/app/models/audit_event.rb +++ b/app/models/audit_event.rb @@ -1,5 +1,5 @@ class AuditEvent < ActiveRecord::Base - serialize :details, Hash + serialize :details, Hash # rubocop:disable Cop/ActiverecordSerialize belongs_to :user, foreign_key: :author_id diff --git a/app/models/blob.rb b/app/models/blob.rb index e75926241ba..6a42a12891c 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -102,10 +102,6 @@ class Blob < SimpleDelegator raw_size == 0 end - def too_large? - size && truncated? - end - def external_storage_error? if external_storage == :lfs !project&.lfs_enabled? @@ -160,7 +156,7 @@ class Blob < SimpleDelegator end def readable_text? - text? && !stored_externally? && !too_large? + text? && !stored_externally? && !truncated? end def simple_viewer @@ -187,9 +183,9 @@ class Blob < SimpleDelegator rendered_as_text? && rich_viewer end - def override_max_size! - simple_viewer&.override_max_size = true - rich_viewer&.override_max_size = true + def expand! + simple_viewer&.expanded = true + rich_viewer&.expanded = true end private diff --git a/app/models/blob_viewer/auxiliary.rb b/app/models/blob_viewer/auxiliary.rb index 07a207730cf..1bea225f17c 100644 --- a/app/models/blob_viewer/auxiliary.rb +++ b/app/models/blob_viewer/auxiliary.rb @@ -7,8 +7,8 @@ module BlobViewer included do self.loading_partial_name = 'loading_auxiliary' self.type = :auxiliary - self.overridable_max_size = 100.kilobytes - self.max_size = 100.kilobytes + self.collapse_limit = 100.kilobytes + self.size_limit = 100.kilobytes end def visible_to?(current_user) diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb index 26a3778c2a3..e6119d25fab 100644 --- a/app/models/blob_viewer/base.rb +++ b/app/models/blob_viewer/base.rb @@ -2,14 +2,14 @@ module BlobViewer class Base PARTIAL_PATH_PREFIX = 'projects/blob/viewers'.freeze - class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_types, :load_async, :binary, :switcher_icon, :switcher_title, :overridable_max_size, :max_size + class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_types, :load_async, :binary, :switcher_icon, :switcher_title, :collapse_limit, :size_limit self.loading_partial_name = 'loading' delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class attr_reader :blob - attr_accessor :override_max_size + attr_accessor :expanded delegate :project, to: :blob @@ -61,24 +61,16 @@ module BlobViewer self.class.load_async? && render_error.nil? end - def exceeds_overridable_max_size? - overridable_max_size && blob.raw_size > overridable_max_size - end - - def exceeds_max_size? - max_size && blob.raw_size > max_size - end + def collapsed? + return @collapsed if defined?(@collapsed) - def can_override_max_size? - exceeds_overridable_max_size? && !exceeds_max_size? + @collapsed = !expanded && collapse_limit && blob.raw_size > collapse_limit end def too_large? - if override_max_size - exceeds_max_size? - else - exceeds_overridable_max_size? - end + return @too_large if defined?(@too_large) + + @too_large = size_limit && blob.raw_size > size_limit end # This method is used on the server side to check whether we can attempt to @@ -95,6 +87,8 @@ module BlobViewer def render_error if too_large? :too_large + elsif collapsed? + :collapsed end end diff --git a/app/models/blob_viewer/client_side.rb b/app/models/blob_viewer/client_side.rb index cc68236f92b..079cfbe3616 100644 --- a/app/models/blob_viewer/client_side.rb +++ b/app/models/blob_viewer/client_side.rb @@ -4,8 +4,8 @@ module BlobViewer included do self.load_async = false - self.overridable_max_size = 10.megabytes - self.max_size = 50.megabytes + self.collapse_limit = 10.megabytes + self.size_limit = 50.megabytes end end end diff --git a/app/models/blob_viewer/server_side.rb b/app/models/blob_viewer/server_side.rb index 87884dcd6bf..05a3dd7d913 100644 --- a/app/models/blob_viewer/server_side.rb +++ b/app/models/blob_viewer/server_side.rb @@ -4,8 +4,8 @@ module BlobViewer included do self.load_async = true - self.overridable_max_size = 2.megabytes - self.max_size = 5.megabytes + self.collapse_limit = 2.megabytes + self.size_limit = 5.megabytes end def prepare! diff --git a/app/models/blob_viewer/text.rb b/app/models/blob_viewer/text.rb index eddca50b4d4..f68cbb7e212 100644 --- a/app/models/blob_viewer/text.rb +++ b/app/models/blob_viewer/text.rb @@ -5,7 +5,7 @@ module BlobViewer self.partial_name = 'text' self.binary = false - self.overridable_max_size = 1.megabyte - self.max_size = 10.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 10.megabytes end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 760ec8e5919..cec1ca89a6a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -19,8 +19,8 @@ module Ci ) end - serialize :options - serialize :yaml_variables, Gitlab::Serializer::Ci::Variables + serialize :options # rubocop:disable Cop/ActiverecordSerialize + serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiverecordSerialize delegate :name, to: :project, prefix: true @@ -47,10 +47,16 @@ module Ci before_destroy { unscoped_project } after_create :execute_hooks - after_save :update_project_statistics, if: :artifacts_size_changed? - after_destroy :update_project_statistics + after_commit :update_project_statistics_after_save, on: [:create, :update] + after_commit :update_project_statistics, on: :destroy class << self + # This is needed for url_for to work, + # as the controller is JobsController + def model_name + ActiveModel::Name.new(self, nil, 'job') + end + def first_pending pending.unstarted.order('created_at ASC').first end @@ -132,6 +138,17 @@ module Ci ExpandVariables.expand(environment, simple_variables) if environment end + def environment_url + return @environment_url if defined?(@environment_url) + + @environment_url = + if unexpanded_url = options&.dig(:environment, :url) + ExpandVariables.expand(unexpanded_url, simple_variables) + else + persisted_environment&.external_url + end + end + def has_environment? environment.present? end @@ -185,27 +202,30 @@ module Ci variables += project.deployment_variables if has_environment? variables += yaml_variables variables += user_variables - variables += project.secret_variables + variables += project.secret_variables_for(ref).map(&:to_runner_variable) variables += trigger_request.user_variables if trigger_request variables end # All variables, including those dependent on other variables def variables - variables = simple_variables - variables += persisted_environment.predefined_variables if persisted_environment.present? - variables + simple_variables.concat(persisted_environment_variables) end def merge_request - merge_requests = MergeRequest.includes(:merge_request_diff) - .where(source_branch: ref, - source_project: pipeline.project) - .reorder(iid: :asc) - - merge_requests.find do |merge_request| - merge_request.commits_sha.include?(pipeline.sha) - end + return @merge_request if defined?(@merge_request) + + @merge_request ||= + begin + merge_requests = MergeRequest.includes(:merge_request_diff) + .where(source_branch: ref, + source_project: pipeline.project) + .reorder(iid: :desc) + + merge_requests.find do |merge_request| + merge_request.commits_sha.include?(pipeline.sha) + end + end end def repo_url @@ -249,38 +269,6 @@ module Ci Time.now - updated_at > 15.minutes.to_i end - ## - # Deprecated - # - # This contains a hotfix for CI build data integrity, see #4246 - # - # This method is used by `ArtifactUploader` to create a store_dir. - # Warning: Uploader uses it after AND before file has been stored. - # - # This method returns old path to artifacts only if it already exists. - # - def artifacts_path - # We need the project even if it's soft deleted, because whenever - # we're really deleting the project, we'll also delete the builds, - # and in order to delete the builds, we need to know where to find - # the artifacts, which is depending on the data of the project. - # We need to retain the project in this case. - the_project = project || unscoped_project - - old = File.join(created_at.utc.strftime('%Y_%m'), - the_project.ci_id.to_s, - id.to_s) - - old_store = File.join(ArtifactUploader.artifacts_path, old) - return old if the_project.ci_id && File.directory?(old_store) - - File.join( - created_at.utc.strftime('%Y_%m'), - the_project.id.to_s, - id.to_s - ) - end - def valid_token?(token) self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) end @@ -361,7 +349,7 @@ module Ci end def has_expiring_artifacts? - artifacts_expire_at.present? + artifacts_expire_at.present? && artifacts_expire_at > Time.now end def keep_artifacts! @@ -488,6 +476,18 @@ module Ci variables.concat(legacy_variables) end + def persisted_environment_variables + return [] unless persisted_environment + + variables = persisted_environment.predefined_variables + + if url = environment_url + variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true } + end + + variables + end + def legacy_variables variables = [ { key: 'CI_BUILD_ID', value: id.to_s, public: true }, @@ -517,5 +517,11 @@ module Ci ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size]) end + + def update_project_statistics_after_save + if previous_changes.include?('artifacts_size') + update_project_statistics + end + end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e84693bcf3a..e28ba91bd64 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -31,6 +31,7 @@ module Ci delegate :id, to: :project, prefix: true + validates :source, exclusion: { in: %w(unknown), unless: :importing? }, on: :create validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :status, presence: { unless: :importing? } @@ -38,6 +39,16 @@ module Ci after_create :keep_around_commits, unless: :importing? + enum source: { + unknown: nil, + push: 1, + web: 2, + trigger: 3, + schedule: 4, + api: 5, + external: 6 + } + state_machine :status, initial: :created do event :enqueue do transition created: :pending @@ -270,10 +281,6 @@ module Ci commit.sha == sha end - def triggered? - trigger_requests.any? - end - def retried @retried ||= (statuses.order(id: :desc) - statuses.latest) end diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index cf6e53c4ca4..45d8cd34359 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -10,9 +10,9 @@ module Ci has_one :last_pipeline, -> { order(id: :desc) }, class_name: 'Ci::Pipeline' has_many :pipelines - validates :cron, unless: :importing_or_inactive?, cron: true, presence: { unless: :importing_or_inactive? } - validates :cron_timezone, cron_timezone: true, presence: { unless: :importing_or_inactive? } - validates :ref, presence: { unless: :importing_or_inactive? } + validates :cron, unless: :importing?, cron: true, presence: { unless: :importing? } + validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? } + validates :ref, presence: { unless: :importing? } validates :description, presence: true before_save :set_next_run_at @@ -24,6 +24,10 @@ module Ci owner == current_user end + def own!(user) + update(owner: user) + end + def inactive? !active? end @@ -32,10 +36,6 @@ module Ci update_attribute(:active, false) end - def importing_or_inactive? - importing? || inactive? - end - def runnable_by_owner? Ability.allowed?(owner, :create_pipeline, project) end diff --git a/app/models/ci/trigger_request.rb b/app/models/ci/trigger_request.rb index 2b807731d0d..564334ad1ad 100644 --- a/app/models/ci/trigger_request.rb +++ b/app/models/ci/trigger_request.rb @@ -6,7 +6,7 @@ module Ci belongs_to :pipeline, foreign_key: :commit_id has_many :builds - serialize :variables + serialize :variables # rubocop:disable Cop/ActiverecordSerialize def user_variables return [] unless variables diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 6c6586110c5..f235260208f 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -12,11 +12,16 @@ module Ci message: "can contain only letters, digits and '_'." } scope :order_key_asc, -> { reorder(key: :asc) } + scope :unprotected, -> { where(protected: false) } attr_encrypted :value, mode: :per_attribute_iv_and_salt, insecure_mode: true, key: Gitlab::Application.secrets.db_key_base, algorithm: 'aes-256-cbc' + + def to_runner_variable + { key: key, value: value, public: false } + end end end diff --git a/app/models/commit.rb b/app/models/commit.rb index dbc0a22829e..bfa3a624e70 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -14,7 +14,7 @@ class Commit participant :committer participant :notes_with_associations - attr_accessor :project + attr_accessor :project, :author DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] @@ -177,7 +177,7 @@ class Commit if RequestStore.active? key = "commit_author:#{author_email.downcase}" # nil is a valid value since no author may exist in the system - if RequestStore.store.has_key?(key) + if RequestStore.store.key?(key) @author = RequestStore.store[key] else @author = find_author_by_any_email @@ -326,11 +326,12 @@ class Commit end def raw_diffs(*args) - if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs) - Gitlab::GitalyClient::Commit.new(project.repository).diff_from_parent(self, *args) - else - raw.diffs(*args) - end + # Uncomment when https://gitlab.com/gitlab-org/gitaly/merge_requests/170 is merged + # if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs) + # Gitlab::GitalyClient::Commit.new(project.repository).diff_from_parent(self, *args) + # else + raw.diffs(*args) + # end end def raw_deltas diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 82f6ce8e484..55c16f7e1fd 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -83,14 +83,15 @@ class CommitStatus < ActiveRecord::Base next if transition.loopback? commit_status.run_after_commit do - pipeline.try do |pipeline| + if pipeline if complete? || manual? PipelineProcessWorker.perform_async(pipeline.id) else PipelineUpdateWorker.perform_async(pipeline.id) end - ExpireJobCacheWorker.perform_async(commit_status.id) end + + ExpireJobCacheWorker.perform_async(commit_status.id) end end @@ -126,6 +127,11 @@ class CommitStatus < ActiveRecord::Base false end + # To be overriden when inherrited from + def retryable? + false + end + def stuck? false end diff --git a/app/models/concerns/editable.rb b/app/models/concerns/editable.rb new file mode 100644 index 00000000000..c62c7e1e936 --- /dev/null +++ b/app/models/concerns/editable.rb @@ -0,0 +1,7 @@ +module Editable + extend ActiveSupport::Concern + + def is_edited? + last_edited_at.present? && last_edited_at != created_at + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 075ec575f9d..ea10d004c9c 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -15,6 +15,7 @@ module Issuable include Taskable include TimeTrackable include Importable + include Editable # This object is used to gather issuable meta data for displaying # upvotes, downvotes, notes and closing merge requests count for issues and merge requests diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb index 6359f7596b1..f734952fa6c 100644 --- a/app/models/concerns/note_on_diff.rb +++ b/app/models/concerns/note_on_diff.rb @@ -33,14 +33,4 @@ module NoteOnDiff def created_at_diff?(diff_refs) false end - - private - - def noteable_diff_refs - if noteable.respond_to?(:diff_sha_refs) - noteable.diff_sha_refs - else - noteable.diff_refs - end - end end diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index dd1e6630642..c7bdc997eca 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -43,7 +43,12 @@ module Noteable end def resolvable_discussions - @resolvable_discussions ||= discussion_notes.resolvable.discussions(self) + @resolvable_discussions ||= + if defined?(@discussions) + @discussions.select(&:resolvable?) + else + discussion_notes.resolvable.discussions(self) + end end def discussions_resolvable? diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index c4463abdfe6..63d02b76f6b 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -84,89 +84,6 @@ module Routable joins(:route).where(wheres.join(' OR ')) end end - - # Builds a relation to find multiple objects that are nested under user membership - # - # Usage: - # - # Klass.member_descendants(1) - # - # Returns an ActiveRecord::Relation. - def member_descendants(user_id) - joins(:route). - joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%') - INNER JOIN members ON members.source_id = r2.source_id - AND members.source_type = r2.source_type"). - where('members.user_id = ?', user_id) - end - - # Builds a relation to find multiple objects that are nested under user - # membership. Includes the parent, as opposed to `#member_descendants` - # which only includes the descendants. - # - # Usage: - # - # Klass.member_self_and_descendants(1) - # - # Returns an ActiveRecord::Relation. - def member_self_and_descendants(user_id) - joins(:route). - joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%') - OR routes.path = r2.path - INNER JOIN members ON members.source_id = r2.source_id - AND members.source_type = r2.source_type"). - where('members.user_id = ?', user_id) - end - - # Returns all objects in a hierarchy, where any node in the hierarchy is - # under the user membership. - # - # Usage: - # - # Klass.member_hierarchy(1) - # - # Examples: - # - # Given the following group tree... - # - # _______group_1_______ - # | | - # | | - # nested_group_1 nested_group_2 - # | | - # | | - # nested_group_1_1 nested_group_2_1 - # - # - # ... the following results are returned: - # - # * the user is a member of group 1 - # => 'group_1', - # 'nested_group_1', nested_group_1_1', - # 'nested_group_2', 'nested_group_2_1' - # - # * the user is a member of nested_group_2 - # => 'group1', - # 'nested_group_2', 'nested_group_2_1' - # - # * the user is a member of nested_group_2_1 - # => 'group1', - # 'nested_group_2', 'nested_group_2_1' - # - # Returns an ActiveRecord::Relation. - def member_hierarchy(user_id) - paths = member_self_and_descendants(user_id).pluck('routes.path') - - return none if paths.empty? - - wheres = paths.map do |path| - "#{connection.quote(path)} = routes.path - OR - #{connection.quote(path)} LIKE CONCAT(routes.path, '/%')" - end - - joins(:route).where(wheres.join(' OR ')) - end end def full_name diff --git a/app/models/concerns/select_for_project_authorization.rb b/app/models/concerns/select_for_project_authorization.rb index 50a1d7fc3e1..58194b0ea13 100644 --- a/app/models/concerns/select_for_project_authorization.rb +++ b/app/models/concerns/select_for_project_authorization.rb @@ -3,7 +3,11 @@ module SelectForProjectAuthorization module ClassMethods def select_for_project_authorization - select("members.user_id, projects.id AS project_id, members.access_level") + select("projects.id AS project_id, members.access_level") + end + + def select_as_master_for_project_authorization + select(["projects.id AS project_id", "#{Gitlab::Access::MASTER} AS access_level"]) end end end diff --git a/app/models/conversational_development_index/card.rb b/app/models/conversational_development_index/card.rb new file mode 100644 index 00000000000..e8f09dc9161 --- /dev/null +++ b/app/models/conversational_development_index/card.rb @@ -0,0 +1,26 @@ +module ConversationalDevelopmentIndex + class Card + attr_accessor :metric, :title, :description, :feature, :blog, :docs + + def initialize(metric:, title:, description:, feature:, blog:, docs: nil) + self.metric = metric + self.title = title + self.description = description + self.feature = feature + self.blog = blog + self.docs = docs + end + + def instance_score + metric.instance_score(feature) + end + + def leader_score + metric.leader_score(feature) + end + + def percentage_score + metric.percentage_score(feature) + end + end +end diff --git a/app/models/conversational_development_index/idea_to_production_step.rb b/app/models/conversational_development_index/idea_to_production_step.rb new file mode 100644 index 00000000000..6e1753c9f30 --- /dev/null +++ b/app/models/conversational_development_index/idea_to_production_step.rb @@ -0,0 +1,19 @@ +module ConversationalDevelopmentIndex + class IdeaToProductionStep + attr_accessor :metric, :title, :features + + def initialize(metric:, title:, features:) + self.metric = metric + self.title = title + self.features = features + end + + def percentage_score + sum = features.sum do |feature| + metric.percentage_score(feature) + end + + sum / features.size.to_f + end + end +end diff --git a/app/models/conversational_development_index/metric.rb b/app/models/conversational_development_index/metric.rb new file mode 100644 index 00000000000..f42f516f99a --- /dev/null +++ b/app/models/conversational_development_index/metric.rb @@ -0,0 +1,21 @@ +module ConversationalDevelopmentIndex + class Metric < ActiveRecord::Base + include Presentable + + self.table_name = 'conversational_development_index_metrics' + + def instance_score(feature) + self["instance_#{feature}"] + end + + def leader_score(feature) + self["leader_#{feature}"] + end + + def percentage_score(feature) + return 100 if leader_score(feature).zero? + + 100 * instance_score(feature) / leader_score(feature) + end + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 216cec751e3..304179c0a97 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -12,6 +12,7 @@ class Deployment < ActiveRecord::Base delegate :name, to: :environment, prefix: true after_create :create_ref + after_create :invalidate_cache def commit project.commit(sha) @@ -33,6 +34,10 @@ class Deployment < ActiveRecord::Base project.repository.create_ref(ref, ref_path) end + def invalidate_cache + environment.expire_etag_cache + end + def manual_actions @manual_actions ||= deployable.try(:other_actions) end diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 800574d8be0..07c4846e2ac 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -10,6 +10,7 @@ class DiffDiscussion < Discussion delegate :position, :original_position, + :change_position, to: :first_note diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 1764004078e..20ef1378500 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -6,9 +6,9 @@ class DiffNote < Note NOTEABLE_TYPES = %w(MergeRequest Commit).freeze - serialize :original_position, Gitlab::Diff::Position - serialize :position, Gitlab::Diff::Position - serialize :change_position, Gitlab::Diff::Position + serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize + serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize + serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize validates :original_position, presence: true validates :position, presence: true @@ -63,7 +63,7 @@ class DiffNote < Note return false unless supported? return true if for_commit? - diff_refs ||= noteable_diff_refs + diff_refs ||= noteable.diff_refs self.position.diff_refs == diff_refs end @@ -95,13 +95,21 @@ class DiffNote < Note return if active? - Notes::DiffPositionUpdateService.new( - self.project, - nil, + tracer = Gitlab::Diff::PositionTracer.new( + project: self.project, old_diff_refs: self.position.diff_refs, - new_diff_refs: noteable_diff_refs, + new_diff_refs: self.noteable.diff_refs, paths: self.position.paths - ).execute(self) + ) + + result = tracer.trace(self.position) + return unless result + + if result[:outdated] + self.change_position = result[:position] + else + self.position = result[:position] + end end def verify_supported diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 0b6b920ed66..d1cec7613af 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -21,7 +21,8 @@ class Discussion end def self.build_collection(notes, context_noteable = nil) - notes.group_by { |n| n.discussion_id(context_noteable) }.values.map { |notes| build(notes, context_noteable) } + grouped_notes = notes.group_by { |n| n.discussion_id(context_noteable) } + grouped_notes.values.map { |notes| build(notes, context_noteable) } end # Returns an alphanumeric discussion ID based on `build_discussion_id` @@ -84,6 +85,12 @@ class Discussion first_note.discussion_id(context_noteable) end + def reply_id + # To reply to this discussion, we need the actual discussion_id from the database, + # not the potentially overwritten one based on the noteable. + first_note.discussion_id + end + alias_method :to_param, :id def diff_discussion? diff --git a/app/models/environment.rb b/app/models/environment.rb index 61572d8d69a..6211a5c1e63 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -57,6 +57,10 @@ class Environment < ActiveRecord::Base state :available state :stopped + + after_transition do |environment| + environment.expire_etag_cache + end end def predefined_variables @@ -196,6 +200,18 @@ class Environment < ActiveRecord::Base [external_url, public_path].join('/') end + def expire_etag_cache + Gitlab::EtagCaching::Store.new.tap do |store| + store.touch(etag_cache_key) + end + end + + def etag_cache_key + Gitlab::Routing.url_helpers.namespace_project_environments_path( + project.namespace, + project) + end + private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/app/models/event.rb b/app/models/event.rb index e6fad46077a..46e89388bc1 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -26,7 +26,7 @@ class Event < ActiveRecord::Base belongs_to :target, polymorphic: true # For Hash only - serialize :data + serialize :data # rubocop:disable Cop/ActiverecordSerialize # Callbacks after_create :reset_project_activity diff --git a/app/models/group.rb b/app/models/group.rb index 6aab477f431..be944da5a67 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -38,6 +38,10 @@ class Group < Namespace after_save :update_two_factor_requirement class << self + def supports_nested_groups? + Gitlab::Database.postgresql? + end + # Searches for groups matching the given query. # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. @@ -78,7 +82,7 @@ class Group < Namespace if current_scope.joins_values.include?(:shared_projects) joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id') .where('project_namespace.share_with_group_lock = ?', false) - .select("members.user_id, projects.id AS project_id, LEAST(project_group_links.group_access, members.access_level) AS access_level") + .select("projects.id AS project_id, LEAST(project_group_links.group_access, members.access_level) AS access_level") else super end diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index eef24052a06..40e43c27f91 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -2,6 +2,6 @@ class ServiceHook < WebHook belongs_to :service def execute(data) - super(data, 'service_hook') + WebHookService.new(self, data, 'service_hook').execute end end diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index c645805c6da..1584235ab00 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -3,8 +3,4 @@ class SystemHook < WebHook default_value_for :push_events, false default_value_for :repository_update_events, true - - def async_execute(data, hook_name) - Sidekiq::Client.enqueue(SystemHookWorker, id, data, hook_name) - end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index a165fdc312f..7503f3739c3 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -1,6 +1,5 @@ class WebHook < ActiveRecord::Base include Sortable - include HTTParty default_value_for :push_events, true default_value_for :issues_events, false @@ -13,52 +12,18 @@ class WebHook < ActiveRecord::Base default_value_for :repository_update_events, false default_value_for :enable_ssl_verification, true + has_many :web_hook_logs, dependent: :destroy + scope :push_hooks, -> { where(push_events: true) } scope :tag_push_hooks, -> { where(tag_push_events: true) } - # HTTParty timeout - default_timeout Gitlab.config.gitlab.webhook_timeout - validates :url, presence: true, url: true def execute(data, hook_name) - parsed_url = URI.parse(url) - if parsed_url.userinfo.blank? - response = WebHook.post(url, - body: data.to_json, - headers: build_headers(hook_name), - verify: enable_ssl_verification) - else - post_url = url.gsub("#{parsed_url.userinfo}@", '') - auth = { - username: CGI.unescape(parsed_url.user), - password: CGI.unescape(parsed_url.password) - } - response = WebHook.post(post_url, - body: data.to_json, - headers: build_headers(hook_name), - verify: enable_ssl_verification, - basic_auth: auth) - end - - [response.code, response.to_s] - rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e - logger.error("WebHook Error => #{e}") - [false, e.to_s] + WebHookService.new(self, data, hook_name).execute end def async_execute(data, hook_name) - Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name) - end - - private - - def build_headers(hook_name) - headers = { - 'Content-Type' => 'application/json', - 'X-Gitlab-Event' => hook_name.singularize.titleize - } - headers['X-Gitlab-Token'] = token if token.present? - headers + WebHookService.new(self, data, hook_name).async_execute end end diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb new file mode 100644 index 00000000000..d73cfcf630d --- /dev/null +++ b/app/models/hooks/web_hook_log.rb @@ -0,0 +1,13 @@ +class WebHookLog < ActiveRecord::Base + belongs_to :web_hook + + serialize :request_headers, Hash # rubocop:disable Cop/ActiverecordSerialize + serialize :request_data, Hash # rubocop:disable Cop/ActiverecordSerialize + serialize :response_headers, Hash # rubocop:disable Cop/ActiverecordSerialize + + validates :web_hook, presence: true + + def success? + response_status =~ /^2/ + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index a88dbb3e065..693cc21bb40 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -251,9 +251,9 @@ class Issue < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| - json[:subscribed] = subscribed?(options[:user], project) if options.has_key?(:user) && options[:user] + json[:subscribed] = subscribed?(options[:user], project) if options.key?(:user) && options[:user] - if options.has_key?(:labels) + if options.key?(:labels) json[:labels] = labels.as_json( project: project, only: [:id, :title, :description, :color, :priority], diff --git a/app/models/key.rb b/app/models/key.rb index b7956052c3f..cb8f10f6d55 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -1,7 +1,6 @@ require 'digest/md5' class Key < ActiveRecord::Base - include AfterCommitQueue include Sortable LAST_USED_AT_REFRESH_TIME = 1.day.to_i @@ -25,10 +24,10 @@ class Key < ActiveRecord::Base delegate :name, :email, to: :user, prefix: true - after_create :add_to_shell - after_create :notify_user + after_commit :add_to_shell, on: :create + after_commit :notify_user, on: :create after_create :post_create_hook - after_destroy :remove_from_shell + after_commit :remove_from_shell, on: :destroy after_destroy :post_destroy_hook def key=(value) @@ -93,6 +92,6 @@ class Key < ActiveRecord::Base end def notify_user - run_after_commit { NotificationService.new.new_key(self) } + NotificationService.new.new_key(self) end end diff --git a/app/models/label.rb b/app/models/label.rb index ddddb6bdf8f..955d6b4079b 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -133,6 +133,10 @@ class Label < ActiveRecord::Base template end + def color + super || DEFAULT_COLOR + end + def text_color LabelsHelper.text_color_for_bg(self.color) end @@ -168,7 +172,7 @@ class Label < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| - json[:priority] = priority(options[:project]) if options.has_key?(:project) + json[:priority] = priority(options[:project]) if options.key?(:project) end end diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index d7c627432d2..7126de2d488 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -7,7 +7,7 @@ class LegacyDiffNote < Note include NoteOnDiff - serialize :st_diff + serialize :st_diff # rubocop:disable Cop/ActiverecordSerialize validates :line_code, presence: true, line_code: true @@ -61,7 +61,7 @@ class LegacyDiffNote < Note return true if for_commit? return true unless diff_line return false unless noteable - return false if diff_refs && diff_refs != noteable_diff_refs + return false if diff_refs && diff_refs != noteable.diff_refs noteable_diff = find_noteable_diff diff --git a/app/models/lfs_objects_project.rb b/app/models/lfs_objects_project.rb index 007eed5600a..b0625c52b62 100644 --- a/app/models/lfs_objects_project.rb +++ b/app/models/lfs_objects_project.rb @@ -6,8 +6,7 @@ class LfsObjectsProject < ActiveRecord::Base validates :lfs_object_id, uniqueness: { scope: [:project_id], message: "already exists in project" } validates :project_id, presence: true - after_create :update_project_statistics - after_destroy :update_project_statistics + after_commit :update_project_statistics, on: [:create, :destroy] private diff --git a/app/models/list.rb b/app/models/list.rb index fbd19acd1f5..ba7353a1325 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -28,7 +28,7 @@ class List < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| - if options.has_key?(:label) + if options.key?(:label) json[:label] = label.as_json( project: board.project, only: [:id, :title, :description, :color] diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2eec013fa9d..dd155252ad5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -21,7 +21,7 @@ class MergeRequest < ActiveRecord::Base belongs_to :assignee, class_name: "User" - serialize :merge_params, Hash + serialize :merge_params, Hash # rubocop:disable Cop/ActiverecordSerialize after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed @@ -220,10 +220,10 @@ class MergeRequest < ActiveRecord::Base def diffs(diff_options = {}) if compare - # When saving MR diffs, `no_collapse` is implicitly added (because we need + # When saving MR diffs, `expanded` is implicitly added (because we need # to save the entire contents to the DB), so add that here for # consistency. - compare.diffs(diff_options.merge(no_collapse: true)) + compare.diffs(diff_options.merge(expanded: true)) else merge_request_diff.diffs(diff_options) end @@ -245,19 +245,6 @@ class MergeRequest < ActiveRecord::Base end end - # MRs created before 8.4 don't store a MergeRequestDiff#base_commit_sha, - # but we need to get a commit for the "View file @ ..." link by deleted files, - # so we find the likely one if we can't get the actual one. - # This will not be the actual base commit if the target branch was merged into - # the source branch after the merge request was created, but it is good enough - # for the specific purpose of linking to a commit. - # It is not good enough for use in `Gitlab::Git::DiffRefs`, which needs the - # true base commit, so we can't simply have `#diff_base_commit` fall back on - # this method. - def likely_diff_base_commit - first_commit.try(:parent) || first_commit - end - def diff_start_commit if persisted? merge_request_diff.start_commit @@ -322,21 +309,14 @@ class MergeRequest < ActiveRecord::Base end def diff_refs - return unless diff_start_commit || diff_base_commit - - Gitlab::Diff::DiffRefs.new( - base_sha: diff_base_sha, - start_sha: diff_start_sha, - head_sha: diff_head_sha - ) - end - - # Return diff_refs instance trying to not touch the git repository - def diff_sha_refs - if merge_request_diff && merge_request_diff.diff_refs_by_sha? + if persisted? merge_request_diff.diff_refs else - diff_refs + Gitlab::Diff::DiffRefs.new( + base_sha: diff_base_sha, + start_sha: diff_start_sha, + head_sha: diff_head_sha + ) end end @@ -441,7 +421,7 @@ class MergeRequest < ActiveRecord::Base MergeRequests::MergeRequestDiffCacheService.new.execute(self) new_diff_refs = self.diff_refs - update_diff_notes_positions( + update_diff_discussion_positions( old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs, current_user: current_user @@ -870,22 +850,21 @@ class MergeRequest < ActiveRecord::Base end def has_complete_diff_refs? - diff_sha_refs && diff_sha_refs.complete? + diff_refs && diff_refs.complete? end - def update_diff_notes_positions(old_diff_refs:, new_diff_refs:, current_user: nil) + def update_diff_discussion_positions(old_diff_refs:, new_diff_refs:, current_user: nil) return unless has_complete_diff_refs? return if new_diff_refs == old_diff_refs - active_diff_notes = self.notes.new_diff_notes.select do |note| - note.active?(old_diff_refs) + active_diff_discussions = self.notes.new_diff_notes.discussions.select do |discussion| + discussion.active?(old_diff_refs) end + return if active_diff_discussions.empty? - return if active_diff_notes.empty? - - paths = active_diff_notes.flat_map { |n| n.diff_file.paths }.uniq + paths = active_diff_discussions.flat_map { |n| n.diff_file.paths }.uniq - service = Notes::DiffPositionUpdateService.new( + service = Discussions::UpdateDiffPositionService.new( self.project, current_user, old_diff_refs: old_diff_refs, @@ -893,11 +872,8 @@ class MergeRequest < ActiveRecord::Base paths: paths ) - transaction do - active_diff_notes.each do |note| - service.execute(note) - Gitlab::Timeless.timeless(note, &:save) - end + active_diff_discussions.each do |discussion| + service.execute(discussion) end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 6e3917a10a3..99dd2130188 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -1,7 +1,7 @@ class MergeRequestDiff < ActiveRecord::Base include Sortable include Importable - include Gitlab::Git::EncodingHelper + include Gitlab::EncodingHelper # Prevent store of diff if commits amount more then 500 COMMITS_SAFE_SIZE = 100 @@ -11,8 +11,8 @@ class MergeRequestDiff < ActiveRecord::Base belongs_to :merge_request - serialize :st_commits - serialize :st_diffs + serialize :st_commits # rubocop:disable Cop/ActiverecordSerialize + serialize :st_diffs # rubocop:disable Cop/ActiverecordSerialize state_machine :state, initial: :empty do state :collected @@ -150,6 +150,29 @@ class MergeRequestDiff < ActiveRecord::Base ) end + # MRs created before 8.4 don't store their true diff refs (start and base), + # but we need to get a commit SHA for the "View file @ ..." link by a file, + # so we use an approximation of the diff refs if we can't get the actual one. + # + # These will not be the actual diff refs if the target branch was merged into + # the source branch after the merge request was created, but it is good enough + # for the specific purpose of linking to a commit. + # + # It is not good enough for highlighting diffs, so we can't simply pass + # these as `diff_refs.` + def fallback_diff_refs + real_refs = diff_refs + return real_refs if real_refs + + likely_base_commit_sha = (first_commit&.parent || first_commit)&.sha + + Gitlab::Diff::DiffRefs.new( + base_sha: likely_base_commit_sha, + start_sha: safe_start_commit_sha, + head_sha: head_commit_sha + ) + end + def diff_refs_by_sha? base_commit_sha? && head_commit_sha? && start_commit_sha? end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index c06bfe0ccdd..b04bed4c014 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -107,7 +107,7 @@ class Milestone < ActiveRecord::Base end def participants - User.joins(assigned_issues: :milestone).where("milestones.id = ?", id) + User.joins(assigned_issues: :milestone).where("milestones.id = ?", id).uniq end def self.sort(method) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 4d59267f71d..b48d73dcae7 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -6,6 +6,7 @@ class Namespace < ActiveRecord::Base include Gitlab::ShellAdapter include Gitlab::CurrentSettings include Routable + include AfterCommitQueue # Prevent users from creating unreasonably deep level of nesting. # The number 20 was taken based on maximum nesting level of @@ -176,26 +177,20 @@ class Namespace < ActiveRecord::Base projects.with_shared_runners.any? end - # Scopes the model on ancestors of the record + # Returns all the ancestors of the current namespaces. def ancestors - if parent_id - path = route ? route.path : full_path - paths = [] + return self.class.none unless parent_id - until path.blank? - path = path.rpartition('/').first - paths << path - end - - self.class.joins(:route).where('routes.path IN (?)', paths).reorder('routes.path ASC') - else - self.class.none - end + Gitlab::GroupHierarchy. + new(self.class.where(id: parent_id)). + base_and_ancestors end - # Scopes the model on direct and indirect children of the record + # Returns all the descendants of the current namespace. def descendants - self.class.joins(:route).merge(Route.inside_path(route.path)).reorder('routes.path ASC') + Gitlab::GroupHierarchy. + new(self.class.where(parent_id: id)). + base_and_descendants end def user_ids_for_project_authorizations @@ -248,7 +243,9 @@ class Namespace < ActiveRecord::Base # Remove namespace directroy async with delay so # GitLab has time to remove all projects first - GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path) + run_after_commit do + GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path) + end end end diff --git a/app/models/note.rb b/app/models/note.rb index 60257aac93b..563af47f314 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -13,6 +13,7 @@ class Note < ActiveRecord::Base include AfterCommitQueue include ResolvableNote include IgnorableColumn + include Editable ignore_column :original_discussion_id @@ -110,7 +111,7 @@ class Note < ActiveRecord::Base end def discussions(context_noteable = nil) - Discussion.build_collection(fresh, context_noteable) + Discussion.build_collection(all.includes(:noteable).fresh, context_noteable) end def find_discussion(discussion_id) diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index e8b000ddad6..ae9f71e7747 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -3,7 +3,7 @@ class PersonalAccessToken < ActiveRecord::Base include TokenAuthenticatable add_authentication_token_field :token - serialize :scopes, Array + serialize :scopes, Array # rubocop:disable Cop/ActiverecordSerialize belongs_to :user diff --git a/app/models/project.rb b/app/models/project.rb index cfca0dcd2f2..d21ff274b6e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -205,8 +205,8 @@ class Project < ActiveRecord::Base presence: true, dynamic_path: true, length: { maximum: 255 }, - format: { with: Gitlab::Regex.project_path_format_regex, - message: Gitlab::Regex.project_path_regex_message }, + format: { with: Gitlab::PathRegex.project_path_format_regex, + message: Gitlab::PathRegex.project_path_format_message }, uniqueness: { scope: :namespace_id } validates :namespace, presence: true @@ -242,6 +242,7 @@ class Project < ActiveRecord::Base scope :in_namespace, ->(namespace_ids) { where(namespace_id: namespace_ids) } scope :personal, ->(user) { where(namespace_id: user.namespace_id) } scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) } + scope :starred_by, ->(user) { joins(:users_star_projects).where('users_star_projects.user_id': user.id) } scope :visible_to_user, ->(user) { where(id: user.authorized_projects.select(:id).reorder(nil)) } scope :non_archived, -> { where(archived: false) } scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } @@ -271,6 +272,7 @@ class Project < ActiveRecord::Base scope :with_builds_enabled, -> { with_feature_enabled(:builds) } scope :with_issues_enabled, -> { with_feature_enabled(:issues) } + scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) } enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } @@ -349,10 +351,6 @@ class Project < ActiveRecord::Base where("projects.id IN (#{union.to_sql})") end - def search_by_visibility(level) - where(visibility_level: Gitlab::VisibilityLevel.string_options[level]) - end - def search_by_title(query) pattern = "%#{query}%" table = Project.arel_table @@ -380,11 +378,9 @@ class Project < ActiveRecord::Base end def reference_pattern - name_pattern = Gitlab::Regex::FULL_NAMESPACE_REGEX_STR - %r{ - ((?<namespace>#{name_pattern})\/)? - (?<project>#{name_pattern}) + ((?<namespace>#{Gitlab::PathRegex::FULL_NAMESPACE_FORMAT_REGEX})\/)? + (?<project>#{Gitlab::PathRegex::PROJECT_PATH_FORMAT_REGEX}) }x end @@ -475,7 +471,9 @@ class Project < ActiveRecord::Base end def reset_cache_and_import_attrs - ProjectCacheWorker.perform_async(self.id) + run_after_commit do + ProjectCacheWorker.perform_async(self.id) + end self.import_data&.destroy end @@ -875,10 +873,8 @@ class Project < ActiveRecord::Base url_to_repo end - def http_url_to_repo(user = nil) - credentials = Gitlab::UrlSanitizer.http_credentials_for_user(user) - - Gitlab::UrlSanitizer.new("#{web_url}.git", credentials: credentials).full_url + def http_url_to_repo + "#{web_url}.git" end def user_can_push_to_empty_repo?(user) @@ -1067,11 +1063,6 @@ class Project < ActiveRecord::Base pipelines.order(id: :desc).find_by(sha: sha, ref: ref) end - def ensure_pipeline(ref, sha, current_user = nil) - pipeline_for(ref, sha) || - pipelines.create(sha: sha, ref: ref, user: current_user) - end - def enable_ci project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED) end @@ -1237,6 +1228,7 @@ class Project < ActiveRecord::Base { key: 'CI_PROJECT_ID', value: id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: path, public: true }, { key: 'CI_PROJECT_PATH', value: path_with_namespace, public: true }, + { key: 'CI_PROJECT_PATH_SLUG', value: path_with_namespace.parameterize, public: true }, { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: web_url, public: true } ] @@ -1256,12 +1248,19 @@ class Project < ActiveRecord::Base variables end - def secret_variables - variables.map do |variable| - { key: variable.key, value: variable.value, public: false } + def secret_variables_for(ref) + if protected_for?(ref) + variables + else + variables.unprotected end end + def protected_for?(ref) + ProtectedBranch.protected?(self, ref) || + ProtectedTag.protected?(self, ref) + end + def deployment_variables return [] unless deployment_service diff --git a/app/models/project_authorization.rb b/app/models/project_authorization.rb index 4c7f4f5a429..def09675253 100644 --- a/app/models/project_authorization.rb +++ b/app/models/project_authorization.rb @@ -6,6 +6,12 @@ class ProjectAuthorization < ActiveRecord::Base validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true + def self.select_from_union(union) + select(['project_id', 'MAX(access_level) AS access_level']). + from("(#{union.to_sql}) #{ProjectAuthorization.table_name}"). + group(:project_id) + end + def self.insert_authorizations(rows, per_batch = 1000) rows.each_slice(per_batch) do |slice| tuples = slice.map do |tuple| diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 331123a5a5b..e3cafd4d1c6 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -10,7 +10,7 @@ class ProjectImportData < ActiveRecord::Base insecure_mode: true, algorithm: 'aes-256-cbc' - serialize :data, JSON + serialize :data, JSON # rubocop:disable Cop/ActiverecordSerialize validates :project, presence: true diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 3728f5642e4..9ce2d1153a7 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -34,7 +34,8 @@ http://app.asana.com/-/account_api' { type: 'text', name: 'api_key', - placeholder: 'User Personal Access Token. User must have access to task, all comments will be attributed to this user.' + placeholder: 'User Personal Access Token. User must have access to task, all comments will be attributed to this user.', + required: true }, { type: 'text', diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb index aeeff8917bf..ae6af732ed4 100644 --- a/app/models/project_services/assembla_service.rb +++ b/app/models/project_services/assembla_service.rb @@ -18,7 +18,7 @@ class AssemblaService < Service def fields [ - { type: 'text', name: 'token', placeholder: '' }, + { type: 'text', name: 'token', placeholder: '', required: true }, { type: 'text', name: 'subdomain', placeholder: '' } ] end diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index 3f5b3eb159b..42939ea0ec8 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -47,9 +47,9 @@ class BambooService < CiService def fields [ { type: 'text', name: 'bamboo_url', - placeholder: 'Bamboo root URL like https://bamboo.example.com' }, + placeholder: 'Bamboo root URL like https://bamboo.example.com', required: true }, { type: 'text', name: 'build_key', - placeholder: 'Bamboo build plan key like KEY' }, + placeholder: 'Bamboo build plan key like KEY', required: true }, { type: 'text', name: 'username', placeholder: 'A user with API access, if applicable' }, { type: 'password', name: 'password' } diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb index 5fb95050b83..fc30f6e3365 100644 --- a/app/models/project_services/buildkite_service.rb +++ b/app/models/project_services/buildkite_service.rb @@ -58,11 +58,11 @@ class BuildkiteService < CiService [ { type: 'text', name: 'token', - placeholder: 'Buildkite project GitLab token' }, + placeholder: 'Buildkite project GitLab token', required: true }, { type: 'text', name: 'project_url', - placeholder: "#{ENDPOINT}/example/project" }, + placeholder: "#{ENDPOINT}/example/project", required: true }, { type: 'checkbox', name: 'enable_ssl_verification', diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 0de59af5652..c3f5b310619 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -18,7 +18,7 @@ class CampfireService < Service def fields [ - { type: 'text', name: 'token', placeholder: '' }, + { type: 'text', name: 'token', placeholder: '', required: true }, { type: 'text', name: 'subdomain', placeholder: '' }, { type: 'text', name: 'room', placeholder: '' } ] @@ -76,7 +76,7 @@ class CampfireService < Service # Returns a list of rooms, or []. # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms def rooms(auth) - res = self.class.get("/rooms.json", auth) + res = self.class.get("/rooms.json", auth) res.code == 200 ? res["rooms"] : [] end diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index 779ef54cfcb..6d1a321f651 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -21,10 +21,6 @@ class ChatNotificationService < Service end end - def can_test? - valid? - end - def self.supported_events %w[push issue confidential_issue merge_request note tag_push pipeline wiki_page] @@ -36,7 +32,7 @@ class ChatNotificationService < Service def default_fields [ - { type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}" }, + { type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}", required: true }, { type: 'text', name: 'username', placeholder: 'e.g. GitLab' }, { type: 'checkbox', name: 'notify_only_broken_pipelines' }, { type: 'checkbox', name: 'notify_only_default_branch' } diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb index dea915a4d05..b9e3e982b64 100644 --- a/app/models/project_services/custom_issue_tracker_service.rb +++ b/app/models/project_services/custom_issue_tracker_service.rb @@ -31,9 +31,9 @@ class CustomIssueTrackerService < IssueTrackerService [ { type: 'text', name: 'title', placeholder: title }, { type: 'text', name: 'description', placeholder: description }, - { type: 'text', name: 'project_url', placeholder: 'Project url' }, - { type: 'text', name: 'issues_url', placeholder: 'Issue url' }, - { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url' } + { type: 'text', name: 'project_url', placeholder: 'Project url', required: true }, + { type: 'text', name: 'issues_url', placeholder: 'Issue url', required: true }, + { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url', required: true } ] end end diff --git a/app/models/project_services/deployment_service.rb b/app/models/project_services/deployment_service.rb index 91a55514a9a..5b8320158fc 100644 --- a/app/models/project_services/deployment_service.rb +++ b/app/models/project_services/deployment_service.rb @@ -30,4 +30,8 @@ class DeploymentService < Service def terminals(environment) raise NotImplementedError end + + def can_test? + false + end end diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index 2717c240f05..f6cade9c290 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -93,8 +93,8 @@ class DroneCiService < CiService def fields [ - { type: 'text', name: 'token', placeholder: 'Drone CI project specific token' }, - { type: 'text', name: 'drone_url', placeholder: 'http://drone.example.com' }, + { type: 'text', name: 'token', placeholder: 'Drone CI project specific token', required: true }, + { type: 'text', name: 'drone_url', placeholder: 'http://drone.example.com', required: true }, { type: 'checkbox', name: 'enable_ssl_verification', title: "Enable SSL verification" } ] end diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb index b4d7c977ce4..720ad61162e 100644 --- a/app/models/project_services/external_wiki_service.rb +++ b/app/models/project_services/external_wiki_service.rb @@ -19,7 +19,7 @@ class ExternalWikiService < Service def fields [ - { type: 'text', name: 'external_wiki_url', placeholder: 'The URL of the external Wiki' } + { type: 'text', name: 'external_wiki_url', placeholder: 'The URL of the external Wiki', required: true } ] end diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb index 2a05d757eb4..2db95b9aaa3 100644 --- a/app/models/project_services/flowdock_service.rb +++ b/app/models/project_services/flowdock_service.rb @@ -18,7 +18,7 @@ class FlowdockService < Service def fields [ - { type: 'text', name: 'token', placeholder: 'Flowdock Git source token' } + { type: 'text', name: 'token', placeholder: 'Flowdock Git source token', required: true } ] end diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index f271e1f1739..017a9b2df6e 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -18,8 +18,8 @@ class GemnasiumService < Service def fields [ - { type: 'text', name: 'api_key', placeholder: 'Your personal API KEY on gemnasium.com ' }, - { type: 'text', name: 'token', placeholder: 'The project\'s slug on gemnasium.com' } + { type: 'text', name: 'api_key', placeholder: 'Your personal API KEY on gemnasium.com ', required: true }, + { type: 'text', name: 'token', placeholder: 'The project\'s slug on gemnasium.com', required: true } ] end diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index c19fed339ba..e3906943ecd 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -33,7 +33,7 @@ class HipchatService < Service def fields [ - { type: 'text', name: 'token', placeholder: 'Room token' }, + { type: 'text', name: 'token', placeholder: 'Room token', required: true }, { type: 'text', name: 'room', placeholder: 'Room name or ID' }, { type: 'checkbox', name: 'notify' }, { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) }, diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb index a51d43adcb9..19357f90810 100644 --- a/app/models/project_services/irker_service.rb +++ b/app/models/project_services/irker_service.rb @@ -49,7 +49,7 @@ class IrkerService < Service help: 'A default IRC URI to prepend before each recipient (optional)', placeholder: 'irc://irc.network.net:6697/' }, { type: 'textarea', name: 'recipients', - placeholder: 'Recipients/channels separated by whitespaces', + placeholder: 'Recipients/channels separated by whitespaces', required: true, help: 'Recipients have to be specified with a full URI: '\ 'irc[s]://irc.network.net[:port]/#channel. Special cases: if '\ 'you want the channel to be a nickname instead, append ",isnick" to ' \ diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index eddf308eae3..ff138b9066d 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -32,9 +32,9 @@ class IssueTrackerService < Service def fields [ { type: 'text', name: 'description', placeholder: description }, - { type: 'text', name: 'project_url', placeholder: 'Project url' }, - { type: 'text', name: 'issues_url', placeholder: 'Issue url' }, - { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url' } + { type: 'text', name: 'project_url', placeholder: 'Project url', required: true }, + { type: 'text', name: 'issues_url', placeholder: 'Issue url', required: true }, + { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url', required: true } ] end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index a91a986e195..2450fb43212 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -2,9 +2,10 @@ class JiraService < IssueTrackerService include Gitlab::Routing.url_helpers validates :url, url: true, presence: true, if: :activated? + validates :api_url, url: true, allow_blank: true validates :project_key, presence: true, if: :activated? - prop_accessor :username, :password, :url, :project_key, + prop_accessor :username, :password, :url, :api_url, :project_key, :jira_issue_transition_id, :title, :description before_update :reset_password @@ -25,20 +26,18 @@ class JiraService < IssueTrackerService super do self.properties = { title: issues_tracker['title'], - url: issues_tracker['url'] + url: issues_tracker['url'], + api_url: issues_tracker['api_url'] } end end def reset_password - # don't reset the password if a new one is provided - if url_changed? && !password_touched? - self.password = nil - end + self.password = nil if reset_password? end def options - url = URI.parse(self.url) + url = URI.parse(client_url) { username: self.username, @@ -87,10 +86,11 @@ class JiraService < IssueTrackerService def fields [ - { type: 'text', name: 'url', title: 'URL', placeholder: 'https://jira.example.com' }, - { type: 'text', name: 'project_key', placeholder: 'Project Key' }, - { type: 'text', name: 'username', placeholder: '' }, - { type: 'password', name: 'password', placeholder: '' }, + { type: 'text', name: 'url', title: 'Web URL', placeholder: 'https://jira.example.com', required: true }, + { type: 'text', name: 'api_url', title: 'JIRA API URL', placeholder: 'If different from Web URL' }, + { type: 'text', name: 'project_key', placeholder: 'Project Key', required: true }, + { type: 'text', name: 'username', placeholder: '', required: true }, + { type: 'password', name: 'password', placeholder: '', required: true }, { type: 'text', name: 'jira_issue_transition_id', placeholder: '' } ] end @@ -175,10 +175,6 @@ class JiraService < IssueTrackerService { success: result.present?, result: result } end - def can_test? - username.present? && password.present? - end - # JIRA does not need test data. # We are requesting the project that belongs to the project key. def test_data(user = nil, project = nil) @@ -186,7 +182,7 @@ class JiraService < IssueTrackerService end def test_settings - return unless url.present? + return unless client_url.present? # Test settings by getting the project jira_request { jira_project.present? } end @@ -236,20 +232,29 @@ class JiraService < IssueTrackerService end def send_message(issue, message, remote_link_props) - return unless url.present? + return unless client_url.present? jira_request do - if issue.comments.build.save!(body: message) - remote_link = issue.remotelink.build + remote_link = find_remote_link(issue, remote_link_props[:object][:url]) + if remote_link remote_link.save!(remote_link_props) - result_message = "#{self.class.name} SUCCESS: Successfully posted to #{url}." + elsif issue.comments.build.save!(body: message) + new_remote_link = issue.remotelink.build + new_remote_link.save!(remote_link_props) end + result_message = "#{self.class.name} SUCCESS: Successfully posted to #{client_url}." Rails.logger.info(result_message) result_message end end + def find_remote_link(issue, url) + links = jira_request { issue.remotelink.all } + + links.find { |link| link.object["url"] == url } + end + def build_remote_link_props(url:, title:, resolved: false) status = { resolved: resolved @@ -295,7 +300,20 @@ class JiraService < IssueTrackerService yield rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError, OpenSSL::SSL::SSLError => e - Rails.logger.info "#{self.class.name} Send message ERROR: #{url} - #{e.message}" + Rails.logger.info "#{self.class.name} Send message ERROR: #{client_url} - #{e.message}" nil end + + def client_url + api_url.present? ? api_url : url + end + + def reset_password? + # don't reset the password if a new one is provided + return false if password_touched? + return true if api_url_changed? + return false if api_url.present? + + url_changed? + end end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index b2494a0be6e..8977a7cdafe 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -77,6 +77,14 @@ class KubernetesService < DeploymentService ] end + def actual_namespace + if namespace.present? + namespace + else + default_namespace + end + end + # Check we can connect to the Kubernetes API def test(*args) kubeclient = build_kubeclient! @@ -91,7 +99,7 @@ class KubernetesService < DeploymentService variables = [ { key: 'KUBE_URL', value: api_url, public: true }, { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: namespace_variable, public: true } + { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true } ] if ca_pem.present? @@ -110,7 +118,7 @@ class KubernetesService < DeploymentService with_reactive_cache do |data| pods = data.fetch(:pods, nil) filter_pods(pods, app: environment.slug). - flat_map { |pod| terminals_for_pod(api_url, namespace, pod) }. + flat_map { |pod| terminals_for_pod(api_url, actual_namespace, pod) }. each { |terminal| add_terminal_auth(terminal, terminal_auth) } end end @@ -124,7 +132,7 @@ class KubernetesService < DeploymentService # Store as hashes, rather than as third-party types pods = begin - kubeclient.get_pods(namespace: namespace).as_json + kubeclient.get_pods(namespace: actual_namespace).as_json rescue KubeException => err raise err unless err.error_code == 404 [] @@ -142,20 +150,12 @@ class KubernetesService < DeploymentService default_namespace || TEMPLATE_PLACEHOLDER end - def namespace_variable - if namespace.present? - namespace - else - default_namespace - end - end - def default_namespace "#{project.path}-#{project.id}" if project.present? end def build_kubeclient!(api_path: 'api', api_version: 'v1') - raise "Incomplete settings" unless api_url && namespace && token + raise "Incomplete settings" unless api_url && actual_namespace && token ::Kubeclient::Client.new( join_api_url(api_path), diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb index 546b6e0a498..72ddf9a4be3 100644 --- a/app/models/project_services/mock_ci_service.rb +++ b/app/models/project_services/mock_ci_service.rb @@ -21,7 +21,8 @@ class MockCiService < CiService [ { type: 'text', name: 'mock_service_url', - placeholder: 'http://localhost:4004' } + placeholder: 'http://localhost:4004', + required: true } ] end @@ -79,4 +80,8 @@ class MockCiService < CiService :error end end + + def can_test? + false + end end diff --git a/app/models/project_services/mock_monitoring_service.rb b/app/models/project_services/mock_monitoring_service.rb index dd04e04e198..ed0318c6b27 100644 --- a/app/models/project_services/mock_monitoring_service.rb +++ b/app/models/project_services/mock_monitoring_service.rb @@ -14,4 +14,8 @@ class MockMonitoringService < MonitoringService def metrics(environment) JSON.parse(File.read(Rails.root + 'spec/fixtures/metrics.json')) end + + def can_test? + false + end end diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index f824171ad09..9d37184be2c 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -53,7 +53,8 @@ class PipelinesEmailService < Service [ { type: 'textarea', name: 'recipients', - placeholder: 'Emails separated by comma' }, + placeholder: 'Emails separated by comma', + required: true }, { type: 'checkbox', name: 'notify_only_broken_pipelines' } ] diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index d86f4f6f448..f9dfa2e91c3 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -23,7 +23,8 @@ class PivotaltrackerService < Service { type: 'text', name: 'token', - placeholder: 'Pivotal Tracker API token.' + placeholder: 'Pivotal Tracker API token.', + required: true }, { type: 'text', diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index ec72cb6856d..110b8bc209b 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -49,7 +49,8 @@ class PrometheusService < MonitoringService type: 'text', name: 'api_url', title: 'API URL', - placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/' + placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/', + required: true } ] end diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index fc29a5277bb..aa7bd4c3c84 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -19,10 +19,10 @@ class PushoverService < Service def fields [ - { type: 'text', name: 'api_key', placeholder: 'Your application key' }, - { type: 'text', name: 'user_key', placeholder: 'Your user key' }, + { type: 'text', name: 'api_key', placeholder: 'Your application key', required: true }, + { type: 'text', name: 'user_key', placeholder: 'Your user key', required: true }, { type: 'text', name: 'device', placeholder: 'Leave blank for all active devices' }, - { type: 'select', name: 'priority', choices: + { type: 'select', name: 'priority', required: true, choices: [ ['Lowest Priority', -2], ['Low Priority', -1], diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index b16beb406b9..cbe137452bd 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -50,9 +50,9 @@ class TeamcityService < CiService def fields [ { type: 'text', name: 'teamcity_url', - placeholder: 'TeamCity root URL like https://teamcity.example.com' }, + placeholder: 'TeamCity root URL like https://teamcity.example.com', required: true }, { type: 'text', name: 'build_type', - placeholder: 'Build configuration ID' }, + placeholder: 'Build configuration ID', required: true }, { type: 'text', name: 'username', placeholder: 'A user with permissions to trigger a manual build' }, { type: 'password', name: 'password' } diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 543b9b293e0..e1cc56551ba 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -167,7 +167,7 @@ class ProjectTeam access = RequestStore.store[key] end - # Lookup only the IDs we need + # Look up only the IDs we need user_ids = user_ids - access.keys return access if user_ids.empty? @@ -178,6 +178,13 @@ class ProjectTeam maximum(:access_level) access.merge!(users_access) + + missing_user_ids = user_ids - users_access.keys + + missing_user_ids.each do |user_id| + access[user_id] = Gitlab::Access::NO_ACCESS + end + access end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 189c106b70b..f38fbda7839 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -42,11 +42,8 @@ class ProjectWiki url_to_repo end - def http_url_to_repo(user = nil) - url = "#{Gitlab.config.gitlab.url}/#{path_with_namespace}.git" - credentials = Gitlab::UrlSanitizer.http_credentials_for_user(user) - - Gitlab::UrlSanitizer.new(url, credentials: credentials).full_url + def http_url_to_repo + "#{Gitlab.config.gitlab.url}/#{path_with_namespace}.git" end def wiki_base_path diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index 0ae5864615a..eed3ca7e179 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -1,5 +1,5 @@ class SentNotification < ActiveRecord::Base - serialize :position, Gitlab::Diff::Position + serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize belongs_to :project belongs_to :noteable, polymorphic: true diff --git a/app/models/service.rb b/app/models/service.rb index 8916f88076e..6a0b0a5c522 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -2,7 +2,7 @@ # and implement a set of methods class Service < ActiveRecord::Base include Sortable - serialize :properties, JSON + serialize :properties, JSON # rubocop:disable Cop/ActiverecordSerialize default_value_for :active, false default_value_for :push_events, true diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 882e2fa0594..6c3358685fe 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -8,6 +8,7 @@ class Snippet < ActiveRecord::Base include Awardable include Mentionable include Spammable + include Editable cache_markdown_field :title, pipeline: :single_line cache_markdown_field :content diff --git a/app/models/user.rb b/app/models/user.rb index cf3914568a6..e6eb9d09656 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,9 +10,12 @@ class User < ActiveRecord::Base include Sortable include CaseSensitivity include TokenAuthenticatable + include IgnorableColumn DEFAULT_NOTIFICATION_LEVEL = :participating + ignore_column :authorized_projects_populated + add_authentication_token_field :authentication_token add_authentication_token_field :incoming_email_token add_authentication_token_field :rss_token @@ -37,7 +40,7 @@ class User < ActiveRecord::Base otp_secret_encryption_key: Gitlab::Application.secrets.otp_key_base devise :two_factor_backupable, otp_number_of_backup_codes: 10 - serialize :otp_backup_codes, JSON + serialize :otp_backup_codes, JSON # rubocop:disable Cop/ActiverecordSerialize devise :lockable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable @@ -98,6 +101,7 @@ class User < ActiveRecord::Base has_many :snippets, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id + has_many :issues, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id has_many :events, dependent: :destroy, foreign_key: :author_id has_many :subscriptions, dependent: :destroy @@ -117,11 +121,6 @@ class User < ActiveRecord::Base has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" - # Issues that a user owns are expected to be moved to the "ghost" user before - # the user is destroyed. If the user owns any issues during deletion, this - # should be treated as an exceptional condition. - has_many :issues, dependent: :restrict_with_exception, foreign_key: :author_id - # # Validations # @@ -218,7 +217,6 @@ class User < ActiveRecord::Base scope :blocked, -> { with_states(:blocked, :ldap_blocked) } scope :external, -> { where(external: true) } scope :active, -> { with_state(:active).non_internal } - scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members WHERE user_id IS NOT NULL AND requested_at IS NULL)') } scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('last_sign_in_at', 'DESC')) } @@ -367,8 +365,9 @@ class User < ActiveRecord::Base # Pattern used to extract `@user` user references from text def reference_pattern %r{ + (?<!\w) #{Regexp.escape(reference_prefix)} - (?<user>#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}) + (?<user>#{Gitlab::PathRegex::FULL_NAMESPACE_FORMAT_REGEX}) }x end @@ -510,23 +509,16 @@ class User < ActiveRecord::Base Group.where("namespaces.id IN (#{union.to_sql})") end - def nested_groups - Group.member_descendants(id) - end - + # Returns a relation of groups the user has access to, including their parent + # and child groups (recursively). def all_expanded_groups - Group.member_hierarchy(id) + Gitlab::GroupHierarchy.new(groups).all_groups end def expanded_groups_requiring_two_factor_authentication all_expanded_groups.where(require_two_factor_authentication: true) end - def nested_groups_projects - Project.joins(:namespace).where('namespaces.parent_id IS NOT NULL'). - member_descendants(id) - end - def refresh_authorized_projects Users::RefreshAuthorizedProjectsService.new(self).execute end @@ -535,18 +527,15 @@ class User < ActiveRecord::Base project_authorizations.where(project_id: project_ids).delete_all end - def set_authorized_projects_column - unless authorized_projects_populated - update_column(:authorized_projects_populated, true) - end - end - def authorized_projects(min_access_level = nil) - refresh_authorized_projects unless authorized_projects_populated - - # We're overriding an association, so explicitly call super with no arguments or it would be passed as `force_reload` to the association + # We're overriding an association, so explicitly call super with no + # arguments or it would be passed as `force_reload` to the association projects = super() - projects = projects.where('project_authorizations.access_level >= ?', min_access_level) if min_access_level + + if min_access_level + projects = projects. + where('project_authorizations.access_level >= ?', min_access_level) + end projects end @@ -565,12 +554,6 @@ class User < ActiveRecord::Base authorized_projects(Gitlab::Access::REPORTER).where(id: projects) end - def viewable_starred_projects - starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (?)", - [Project::PUBLIC, Project::INTERNAL], - authorized_projects.select(:project_id)) - end - def owned_projects @owned_projects ||= Project.where('namespace_id IN (?) OR namespace_id = ?', @@ -794,7 +777,7 @@ class User < ActiveRecord::Base def avatar_url(size: nil, scale: 2, **args) # We use avatar_path instead of overriding avatar_url because of carrierwave. # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864 - avatar_path(args) || GravatarService.new.execute(email, size, scale) + avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username) end def all_emails @@ -919,13 +902,13 @@ class User < ActiveRecord::Base end def assigned_open_merge_requests_count(force: false) - Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force) do + Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force, expires_in: 20.minutes) do MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened').execute.count end end def assigned_open_issues_count(force: false) - Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force) do + Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: 20.minutes) do IssuesFinder.new(self, assignee_id: self.id, state: 'opened').execute.count end end |