diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/assets/javascripts/notes/components/note_header.vue | 5 | ||||
| -rw-r--r-- | app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue | 4 | ||||
| -rw-r--r-- | app/assets/stylesheets/pages/notes.scss | 4 | ||||
| -rw-r--r-- | app/controllers/projects/settings/ci_cd_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/appearance.rb | 3 | ||||
| -rw-r--r-- | app/models/ci/runner.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/with_uploads.rb | 39 | ||||
| -rw-r--r-- | app/models/group.rb | 3 | ||||
| -rw-r--r-- | app/models/project.rb | 3 | ||||
| -rw-r--r-- | app/models/user.rb | 22 | ||||
| -rw-r--r-- | app/policies/ci/runner_policy.rb | 15 | ||||
| -rw-r--r-- | app/views/shared/notes/_note.html.haml | 5 |
12 files changed, 83 insertions, 24 deletions
diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 7183d0b50b2..a4081957207 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -93,10 +93,13 @@ export default { v-html="actionTextHtml" class="system-note-message"> </span> + <span class="system-note-separator"> + · + </span> <a :href="noteTimestampLink" @click="updateTargetNoteHash" - class="note-timestamp"> + class="note-timestamp system-note-separator"> <time-ago-tooltip :time="createdAt" tooltip-placement="bottom" diff --git a/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue index bec4e7c99b6..368eeb6c453 100644 --- a/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue +++ b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue @@ -40,7 +40,7 @@ export default { :class="cssClass" :title="tooltipTitle(time)" :data-placement="tooltipPlacement" - data-container="body"> - {{ timeFormated(time) }} + data-container="body" + v-text="timeFormated(time)"> </time> </template> diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 42fe319b931..feee964f9bb 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -455,6 +455,10 @@ ul.notes { white-space: normal; } + .system-note-separator { + color: $gl-text-color-disabled; + } + a:hover { text-decoration: underline; } diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 177c8a54099..1d850baf012 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -69,7 +69,7 @@ module Projects @project_runners = @project.runners.ordered @assignable_runners = current_user - .ci_authorized_runners + .ci_owned_runners .assignable_for(project) .ordered .page(params[:page]).per(20) diff --git a/app/models/appearance.rb b/app/models/appearance.rb index fb66dd0b766..f8713138a93 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -2,6 +2,7 @@ class Appearance < ActiveRecord::Base include CacheMarkdownField include AfterCommitQueue include ObjectStorage::BackgroundMove + include WithUploads cache_markdown_field :description cache_markdown_field :new_project_guidelines @@ -14,8 +15,6 @@ class Appearance < ActiveRecord::Base mount_uploader :logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - CACHE_KEY = "current_appearance:#{Gitlab::VERSION}".freeze after_commit :flush_redis_cache diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index bda69f85a78..e6f1ed519be 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -52,7 +52,7 @@ module Ci # Without that, placeholders would miss one and couldn't match. where(locked: false) .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})") - .specific + .project_type end validate :tag_constraints diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb new file mode 100644 index 00000000000..e7cfffb775b --- /dev/null +++ b/app/models/concerns/with_uploads.rb @@ -0,0 +1,39 @@ +# Mounted uploaders are destroyed by carrierwave's after_commit +# hook. This hook fetches upload location (local vs remote) from +# Upload model. So it's neccessary to make sure that during that +# after_commit hook model's associated uploads are not deleted yet. +# IOW we can not use dependent: :destroy : +# has_many :uploads, as: :model, dependent: :destroy +# +# And because not-mounted uploads require presence of upload's +# object model when destroying them (FileUploader's `build_upload` method +# references `model` on delete), we can not use after_commit hook for these +# uploads. +# +# Instead FileUploads are destroyed in before_destroy hook and remaining uploads +# are destroyed by the carrierwave's after_commit hook. + +module WithUploads + extend ActiveSupport::Concern + + # Currently there is no simple way how to select only not-mounted + # uploads, it should be all FileUploaders so we select them by + # `uploader` class + FILE_UPLOADERS = %w(PersonalFileUploader NamespaceFileUploader FileUploader).freeze + + included do + has_many :uploads, as: :model + + before_destroy :destroy_file_uploads + end + + # mounted uploads are deleted in carrierwave's after_commit hook, + # but FileUploaders which are not mounted must be deleted explicitly and + # it can not be done in after_commit because FileUploader requires loads + # associated model on destroy (which is already deleted in after_commit) + def destroy_file_uploads + self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload| + upload.destroy + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index cefca316399..8fb77a7869d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -10,6 +10,7 @@ class Group < Namespace include LoadedInGroupList include GroupDescendant include TokenAuthenticatable + include WithUploads has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent alias_method :members, :group_members @@ -30,8 +31,6 @@ class Group < Namespace has_many :variables, class_name: 'Ci::GroupVariable' has_many :custom_attributes, class_name: 'GroupCustomAttribute' - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :boards has_many :badges, class_name: 'GroupBadge' diff --git a/app/models/project.rb b/app/models/project.rb index 534a0e630af..0975e64e995 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -23,6 +23,7 @@ class Project < ActiveRecord::Base include ::Gitlab::Utils::StrongMemoize include ChronicDurationAttribute include FastDestroyAll::Helpers + include WithUploads extend Gitlab::ConfigHelper @@ -301,8 +302,6 @@ class Project < ActiveRecord::Base inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } validates :variables, variable_duplicates: { scope: :environment_scope } - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - # Scopes scope :pending_delete, -> { where(pending_delete: true) } scope :without_deleted, -> { where(pending_delete: false) } diff --git a/app/models/user.rb b/app/models/user.rb index 173ab38e20c..474fde36c02 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,7 @@ class User < ActiveRecord::Base include IgnorableColumn include BulkMemberAccessLoad include BlocksJsonSerialization + include WithUploads DEFAULT_NOTIFICATION_LEVEL = :participating @@ -137,7 +138,6 @@ class User < ActiveRecord::Base has_many :custom_attributes, class_name: 'UserCustomAttribute' has_many :callouts, class_name: 'UserCallout' - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :term_agreements belongs_to :accepted_term, class_name: 'ApplicationSetting::Term' @@ -999,12 +999,19 @@ class User < ActiveRecord::Base !solo_owned_groups.present? end - def ci_authorized_runners - @ci_authorized_runners ||= begin - runner_ids = Ci::RunnerProject + def ci_owned_runners + @ci_owned_runners ||= begin + project_runner_ids = Ci::RunnerProject .where(project: authorized_projects(Gitlab::Access::MASTER)) .select(:runner_id) - Ci::Runner.specific.where(id: runner_ids) + + group_runner_ids = Ci::RunnerNamespace + .where(namespace_id: owned_or_masters_groups.select(:id)) + .select(:runner_id) + + union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids]) + + Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end end @@ -1205,6 +1212,11 @@ class User < ActiveRecord::Base !terms_accepted? end + def owned_or_masters_groups + union = Gitlab::SQL::Union.new([owned_groups, masters_groups]) + Group.from("(#{union.to_sql}) namespaces") + end + protected # override, from Devise::Validatable diff --git a/app/policies/ci/runner_policy.rb b/app/policies/ci/runner_policy.rb index 7dff8470e23..895abe87d86 100644 --- a/app/policies/ci/runner_policy.rb +++ b/app/policies/ci/runner_policy.rb @@ -1,16 +1,19 @@ module Ci class RunnerPolicy < BasePolicy with_options scope: :subject, score: 0 - condition(:shared) { @subject.is_shared? } - - with_options scope: :subject, score: 0 condition(:locked, scope: :subject) { @subject.locked? } - condition(:authorized_runner) { @user.ci_authorized_runners.include?(@subject) } + condition(:owned_runner) { @user.ci_owned_runners.exists?(@subject.id) } rule { anonymous }.prevent_all - rule { admin | authorized_runner }.enable :assign_runner - rule { ~admin & shared }.prevent :assign_runner + + rule { admin | owned_runner }.policy do + enable :assign_runner + enable :read_runner + enable :update_runner + enable :delete_runner + end + rule { ~admin & locked }.prevent :assign_runner end end diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 893a7f26ebd..d4e67b5e7e3 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -41,8 +41,9 @@ - if note.system %span.system-note-message = markdown_field(note, :note) - %a{ href: "##{dom_id(note)}" } - = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') + %span.system-note-separator + · + %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') - unless note.system? .note-actions - if note.for_personal_snippet? |