diff options
31 files changed, 409 insertions, 129 deletions
diff --git a/app/controllers/explore/snippets_controller.rb b/app/controllers/explore/snippets_controller.rb index d4c6aae2ca8..61068df77d1 100644 --- a/app/controllers/explore/snippets_controller.rb +++ b/app/controllers/explore/snippets_controller.rb @@ -5,7 +5,7 @@ class Explore::SnippetsController < Explore::ApplicationController include Gitlab::NoteableMetadata def index - @snippets = SnippetsFinder.new(current_user) + @snippets = SnippetsFinder.new(current_user, explore: true) .execute .page(params[:page]) .inc_author diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb index 43c4f4d220e..c97fec0a6ee 100644 --- a/app/controllers/notification_settings_controller.rb +++ b/app/controllers/notification_settings_controller.rb @@ -50,8 +50,6 @@ class NotificationSettingsController < ApplicationController end def notification_setting_params_for(source) - allowed_fields = NotificationSetting.email_events(source).dup - allowed_fields << :level - params.require(:notification_setting).permit(allowed_fields) + params.require(:notification_setting).permit(NotificationSetting.allowed_fields(source)) end end diff --git a/app/controllers/profiles/groups_controller.rb b/app/controllers/profiles/groups_controller.rb index c755bcb718a..04b5ee270dc 100644 --- a/app/controllers/profiles/groups_controller.rb +++ b/app/controllers/profiles/groups_controller.rb @@ -5,7 +5,7 @@ class Profiles::GroupsController < Profiles::ApplicationController def update group = find_routable!(Group, params[:id]) - notification_setting = current_user.notification_settings.find_by(source: group) # rubocop: disable CodeReuse/ActiveRecord + notification_setting = current_user.notification_settings_for(group) if notification_setting.update(update_params) flash[:notice] = "Notification settings for #{group.name} saved" diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index 617e5bb7cb3..5f44e55f3ef 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -3,9 +3,14 @@ class Profiles::NotificationsController < Profiles::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def show - @user = current_user - @group_notifications = current_user.notification_settings.for_groups.order(:id) - @project_notifications = current_user.notification_settings.for_projects.order(:id) + @user = current_user + @group_notifications = current_user.notification_settings.for_groups.order(:id) + @group_notifications += GroupsFinder.new( + current_user, + all_available: false, + exclude_group_ids: @group_notifications.select(:source_id) + ).execute.map { |group| current_user.notification_settings_for(group, inherit: true) } + @project_notifications = current_user.notification_settings.for_projects.order(:id) @global_notification_setting = current_user.global_notification_setting end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 6e2bb1ded43..bd6b6190fb5 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -41,13 +41,14 @@ class SnippetsFinder < UnionFinder include FinderMethods - attr_accessor :current_user, :project, :author, :scope + attr_accessor :current_user, :project, :author, :scope, :explore def initialize(current_user = nil, params = {}) @current_user = current_user @project = params[:project] @author = params[:author] @scope = params[:scope].to_s + @explore = params[:explore] if project && author raise( @@ -66,13 +67,23 @@ class SnippetsFinder < UnionFinder private def init_collection - if project + if explore + snippets_for_explore + elsif project snippets_for_a_single_project else snippets_for_multiple_projects end end + # Produces a query that retrieves snippets for the Explore page + # + # We only show personal snippets here because this page is meant for + # discovery, and project snippets are of limited interest here. + def snippets_for_explore + Snippet.public_to_user(current_user).only_personal_snippets + end + # Produces a query that retrieves snippets from multiple projects. # # The resulting query will, depending on the user's permissions, include the @@ -86,7 +97,7 @@ class SnippetsFinder < UnionFinder # Each collection is constructed in isolation, allowing for greater control # over the resulting SQL query. def snippets_for_multiple_projects - queries = [global_snippets] + queries = [personal_snippets] if Ability.allowed?(current_user, :read_cross_project) queries << snippets_of_visible_projects @@ -100,8 +111,8 @@ class SnippetsFinder < UnionFinder Snippet.for_project_with_user(project, current_user) end - def global_snippets - snippets_for_author_or_visible_to_user.only_global_snippets + def personal_snippets + snippets_for_author_or_visible_to_user.only_personal_snippets end # Returns the snippets that the current user (logged in or not) can view. diff --git a/app/models/concerns/stepable.rb b/app/models/concerns/stepable.rb index d00a049a004..dea241c5dbe 100644 --- a/app/models/concerns/stepable.rb +++ b/app/models/concerns/stepable.rb @@ -11,15 +11,15 @@ module Stepable initial_result = {} steps.inject(initial_result) do |previous_result, callback| - result = method(callback).call + result = method(callback).call(previous_result) - if result[:status] == :error - result[:failed_step] = callback + if result[:status] != :success + result[:last_step] = callback break result end - previous_result.merge(result) + result end end diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 20160da62d2..2b3443f24d7 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -47,6 +47,10 @@ class NotificationSetting < ApplicationRecord EMAIL_EVENTS end + def self.allowed_fields(source = nil) + NotificationSetting.email_events(source).dup + %i(level notification_email) + end + def email_events self.class.email_events(source) end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index b6c0976cd43..4010a3e2167 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -71,7 +71,7 @@ class Snippet < ApplicationRecord end end - def self.only_global_snippets + def self.only_personal_snippets where(project_id: nil) end diff --git a/app/models/user.rb b/app/models/user.rb index 42a5f6cebeb..fe5c13f1c68 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1317,14 +1317,27 @@ class User < ApplicationRecord notification_group&.notification_email_for(self) || notification_email end - def notification_settings_for(source) + def notification_settings_for(source, inherit: false) if notification_settings.loaded? notification_settings.find do |notification| notification.source_type == source.class.base_class.name && notification.source_id == source.id end else - notification_settings.find_or_initialize_by(source: source) + notification_settings.find_or_initialize_by(source: source) do |ns| + next unless source.is_a?(Group) && inherit + + # If we're here it means we're trying to create a NotificationSetting for a group that doesn't have one. + # Find the closest parent with a notification_setting that's not Global level, or that has an email set. + ancestor_ns = source + .notification_settings(hierarchy_order: :asc) + .where(user: self) + .find_by('level != ? OR notification_email IS NOT NULL', NotificationSetting.levels[:global]) + # Use it to seed the settings + ns.assign_attributes(ancestor_ns&.slice(*NotificationSetting.allowed_fields)) + ns.source = source + ns.user = self + end end end diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 6400b182715..df9217bea32 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -20,7 +20,11 @@ module ApplicationSettings add_to_outbound_local_requests_whitelist(@params.delete(:add_to_outbound_local_requests_whitelist)) if params.key?(:performance_bar_allowed_group_path) - params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id + group_id = process_performance_bar_allowed_group_id + + return false if application_setting.errors.any? + + params[:performance_bar_allowed_group_id] = group_id end if usage_stats_updated? && !params.delete(:skip_usage_stats_user) @@ -65,12 +69,27 @@ module ApplicationSettings @application_setting.reset_memoized_terms end - def performance_bar_allowed_group_id - performance_bar_enabled = !params.key?(:performance_bar_enabled) || params.delete(:performance_bar_enabled) + def process_performance_bar_allowed_group_id group_full_path = params.delete(:performance_bar_allowed_group_path) - return unless Gitlab::Utils.to_boolean(performance_bar_enabled) + enable_param_on = Gitlab::Utils.to_boolean(params.delete(:performance_bar_enabled)) + performance_bar_enabled = enable_param_on.nil? || enable_param_on # Default to true + + return if group_full_path.blank? + return if enable_param_on == false # Explicitly disabling + + unless performance_bar_enabled + application_setting.errors.add(:performance_bar_allowed_group_id, 'not allowed when performance bar is disabled') + return + end + + group = Group.find_by_full_path(group_full_path.chomp('/')) + + unless group + application_setting.errors.add(:performance_bar_allowed_group_id, 'not found') + return + end - Group.find_by_full_path(group_full_path)&.id if group_full_path.present? + group.id end def bypass_external_auth? diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 0e6c16f0f06..457d05b4a97 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -5,6 +5,8 @@ = auto_discovery_link_tag(:atom, group_url(@group, rss_url_options), title: "#{@group.name} activity") %div{ class: [("limit-container-width" unless fluid_layout)] } + = render_if_exists 'trials/banner', namespace: @group + = render 'groups/home_panel' .groups-listing{ data: { endpoints: { default: group_children_path(@group, format: :json), shared: group_shared_projects_path(@group, format: :json) } } } diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml index 1776d260e19..33b0aa93d84 100644 --- a/app/views/profiles/notifications/_group_settings.html.haml +++ b/app/views/profiles/notifications/_group_settings.html.haml @@ -12,5 +12,5 @@ = render 'shared/notifications/button', notification_setting: setting, emails_disabled: emails_disabled .table-section.section-30 - = form_for @user.notification_settings.find { |ns| ns.source == group }, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f| + = form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f| = f.select :notification_email, @user.all_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email' diff --git a/changelogs/unreleased/29477-notification-settings-display-all-groups.yml b/changelogs/unreleased/29477-notification-settings-display-all-groups.yml new file mode 100644 index 00000000000..a4cb6fe1643 --- /dev/null +++ b/changelogs/unreleased/29477-notification-settings-display-all-groups.yml @@ -0,0 +1,5 @@ +--- +title: Show all groups user belongs to in Notification settings +merge_request: 17303 +author: +type: fixed diff --git a/changelogs/unreleased/30877-optimize-explore-snippets.yml b/changelogs/unreleased/30877-optimize-explore-snippets.yml new file mode 100644 index 00000000000..7ca52876609 --- /dev/null +++ b/changelogs/unreleased/30877-optimize-explore-snippets.yml @@ -0,0 +1,5 @@ +--- +title: Show only personal snippets on explore page +merge_request: 18092 +author: +type: performance diff --git a/changelogs/unreleased/34120-design-system-notes-icon-does-not-appear.yml b/changelogs/unreleased/34120-design-system-notes-icon-does-not-appear.yml new file mode 100644 index 00000000000..8533067a408 --- /dev/null +++ b/changelogs/unreleased/34120-design-system-notes-icon-does-not-appear.yml @@ -0,0 +1,5 @@ +--- +title: Resolve missing design system notes icons +merge_request: 18693 +author: +type: fixed diff --git a/changelogs/unreleased/return-error-message-when-performance-bar-group-is-not-found.yml b/changelogs/unreleased/return-error-message-when-performance-bar-group-is-not-found.yml new file mode 100644 index 00000000000..1d00597ba7d --- /dev/null +++ b/changelogs/unreleased/return-error-message-when-performance-bar-group-is-not-found.yml @@ -0,0 +1,5 @@ +--- +title: Show error message when setting an invalid group ID for the performance bar +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sort-severity-then-confidence.yml b/changelogs/unreleased/sort-severity-then-confidence.yml new file mode 100644 index 00000000000..3c7ab63d60b --- /dev/null +++ b/changelogs/unreleased/sort-severity-then-confidence.yml @@ -0,0 +1,5 @@ +--- +title: Sort vulnerabilities by severity then confidence for dashboard and pipeline views +merge_request: 18675 +author: +type: changed diff --git a/db/migrate/20191014084150_add_index_on_snippets_project_id_and_visibility_level.rb b/db/migrate/20191014084150_add_index_on_snippets_project_id_and_visibility_level.rb new file mode 100644 index 00000000000..a8f40953e5d --- /dev/null +++ b/db/migrate/20191014084150_add_index_on_snippets_project_id_and_visibility_level.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexOnSnippetsProjectIdAndVisibilityLevel < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :snippets, [:project_id, :visibility_level] + end + + def down + remove_concurrent_index :snippets, [:project_id, :visibility_level] + end +end diff --git a/db/migrate/20191014132931_remove_index_on_snippets_project_id.rb b/db/migrate/20191014132931_remove_index_on_snippets_project_id.rb new file mode 100644 index 00000000000..a1d3ffdb8c8 --- /dev/null +++ b/db/migrate/20191014132931_remove_index_on_snippets_project_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveIndexOnSnippetsProjectId < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + remove_concurrent_index :snippets, [:project_id] + end + + def down + add_concurrent_index :snippets, [:project_id] + end +end diff --git a/db/schema.rb b/db/schema.rb index ef9cd696db5..b39eb408b0a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3449,7 +3449,7 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do t.index ["author_id"], name: "index_snippets_on_author_id" t.index ["content"], name: "index_snippets_on_content_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["file_name"], name: "index_snippets_on_file_name_trigram", opclass: :gin_trgm_ops, using: :gin - t.index ["project_id"], name: "index_snippets_on_project_id" + t.index ["project_id", "visibility_level"], name: "index_snippets_on_project_id_and_visibility_level" t.index ["title"], name: "index_snippets_on_title_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["updated_at"], name: "index_snippets_on_updated_at" t.index ["visibility_level"], name: "index_snippets_on_visibility_level" diff --git a/doc/user/analytics/productivity_analytics.md b/doc/user/analytics/productivity_analytics.md index 7313a25e797..09f83dcff4b 100644 --- a/doc/user/analytics/productivity_analytics.md +++ b/doc/user/analytics/productivity_analytics.md @@ -1,6 +1,6 @@ # Productivity Analytics **(PREMIUM)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12079) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.3 (enabled by feature flags `productivity_analytics`, `productivity_analytics_scatterplot_enabled`). +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12079) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.3 (enabled by default using the feature flags `productivity_analytics`, `productivity_analytics_scatterplot_enabled`). Track development velocity with Productivity Analytics. diff --git a/doc/user/project/clusters/img/kubernetes_pod_logs.png b/doc/user/project/clusters/img/kubernetes_pod_logs_v12_4.png Binary files differindex 73c2ecd182a..73c2ecd182a 100644 --- a/doc/user/project/clusters/img/kubernetes_pod_logs.png +++ b/doc/user/project/clusters/img/kubernetes_pod_logs_v12_4.png diff --git a/doc/user/project/clusters/kubernetes_pod_logs.md b/doc/user/project/clusters/kubernetes_pod_logs.md index 910ebb8a503..4036eaf0bfb 100644 --- a/doc/user/project/clusters/kubernetes_pod_logs.md +++ b/doc/user/project/clusters/kubernetes_pod_logs.md @@ -17,8 +17,14 @@ Everything you need to build, test, deploy, and run your app at scale. 1. On the **Environments** page, you should see the status of the environment's pods with [Deploy Boards](../deploy_boards.md). 1. When mousing over the list of pods, a tooltip will appear with the exact pod name and status.  -1. Click on the desired pod to bring up the logs view, which will contain the last 500 lines for that pod. You may switch between pods and environments in this view. Support for pods with multiple containers is coming [in a future release](https://gitlab.com/gitlab-org/gitlab/issues/6502). -  +1. Click on the desired pod to bring up the logs view, which will contain the last 500 lines for that pod. + You may switch between the following in this view: + - Pods. + - [From GitLab 12.4](https://gitlab.com/gitlab-org/gitlab/issues/5769), environments. + + Support for pods with multiple containers is coming [in a future release](https://gitlab.com/gitlab-org/gitlab/issues/6502). + +  ## Requirements diff --git a/lib/gitlab/database_importers/self_monitoring/project/create_service.rb b/lib/gitlab/database_importers/self_monitoring/project/create_service.rb index 5422a8631a0..dfef158cc1d 100644 --- a/lib/gitlab/database_importers/self_monitoring/project/create_service.rb +++ b/lib/gitlab/database_importers/self_monitoring/project/create_service.rb @@ -33,7 +33,7 @@ module Gitlab if result[:status] == :success result - elsif STEPS_ALLOWED_TO_FAIL.include?(result[:failed_step]) + elsif STEPS_ALLOWED_TO_FAIL.include?(result[:last_step]) success else raise StandardError, result[:message] @@ -42,121 +42,124 @@ module Gitlab private - def validate_application_settings + def validate_application_settings(_result) return success if application_settings log_error('No application_settings found') error(_('No application_settings found')) end - def validate_project_created - return success unless project_created? + def validate_project_created(result) + return success(result) unless project_created? log_error('Project already created') error(_('Project already created')) end - def validate_admins + def validate_admins(result) unless instance_admins.any? log_error('No active admin user found') return error(_('No active admin user found')) end - success + success(result) end - def create_group + def create_group(result) if project_created? log_info(_('Instance administrators group already exists')) - @group = application_settings.instance_administration_project.owner - return success(group: @group) + result[:group] = application_settings.instance_administration_project.owner + return success(result) end - @group = ::Groups::CreateService.new(group_owner, create_group_params).execute + result[:group] = ::Groups::CreateService.new(group_owner, create_group_params).execute - if @group.persisted? - success(group: @group) + if result[:group].persisted? + success(result) else error(_('Could not create group')) end end - def create_project + def create_project(result) if project_created? log_info('Instance administration project already exists') - @project = application_settings.instance_administration_project - return success(project: project) + result[:project] = application_settings.instance_administration_project + return success(result) end - @project = ::Projects::CreateService.new(group_owner, create_project_params).execute + result[:project] = ::Projects::CreateService.new(group_owner, create_project_params(result[:group])).execute - if project.persisted? - success(project: project) + if result[:project].persisted? + success(result) else - log_error("Could not create instance administration project. Errors: %{errors}" % { errors: project.errors.full_messages }) + log_error("Could not create instance administration project. Errors: %{errors}" % { errors: result[:project].errors.full_messages }) error(_('Could not create project')) end end - def save_project_id + def save_project_id(result) return success if project_created? - result = application_settings.update(instance_administration_project_id: @project.id) + response = application_settings.update( + instance_administration_project_id: result[:project].id + ) - if result - success + if response + success(result) else log_error("Could not save instance administration project ID, errors: %{errors}" % { errors: application_settings.errors.full_messages }) error(_('Could not save project ID')) end end - def add_group_members - members = @group.add_users(members_to_add, Gitlab::Access::MAINTAINER) + def add_group_members(result) + group = result[:group] + members = group.add_users(members_to_add(group), Gitlab::Access::MAINTAINER) errors = members.flat_map { |member| member.errors.full_messages } if errors.any? log_error('Could not add admins as members to self-monitoring project. Errors: %{errors}' % { errors: errors }) error(_('Could not add admins as members')) else - success + success(result) end end - def add_to_whitelist - return success unless prometheus_enabled? - return success unless prometheus_listen_address.present? + def add_to_whitelist(result) + return success(result) unless prometheus_enabled? + return success(result) unless prometheus_listen_address.present? uri = parse_url(internal_prometheus_listen_address_uri) return error(_('Prometheus listen_address in config/gitlab.yml is not a valid URI')) unless uri application_settings.add_to_outbound_local_requests_whitelist([uri.normalized_host]) - result = application_settings.save + response = application_settings.save - if result + if response # Expire the Gitlab::CurrentSettings cache after updating the whitelist. # This happens automatically in an after_commit hook, but in migrations, # the after_commit hook only runs at the end of the migration. Gitlab::CurrentSettings.expire_current_application_settings - success + success(result) else log_error("Could not add prometheus URL to whitelist, errors: %{errors}" % { errors: application_settings.errors.full_messages }) error(_('Could not add prometheus URL to whitelist')) end end - def add_prometheus_manual_configuration - return success unless prometheus_enabled? - return success unless prometheus_listen_address.present? + def add_prometheus_manual_configuration(result) + return success(result) unless prometheus_enabled? + return success(result) unless prometheus_listen_address.present? - service = project.find_or_initialize_service('prometheus') + service = result[:project].find_or_initialize_service('prometheus') unless service.update(prometheus_service_attributes) log_error('Could not save prometheus manual configuration for self-monitoring project. Errors: %{errors}' % { errors: service.errors.full_messages }) return error(_('Could not save prometheus manual configuration')) end - success + success(result) end def application_settings @@ -196,11 +199,11 @@ module Gitlab instance_admins.first end - def members_to_add + def members_to_add(group) # Exclude admins who are already members of group because - # `@group.add_users(users)` returns an error if the users parameter contains + # `group.add_users(users)` returns an error if the users parameter contains # users who are already members of the group. - instance_admins - @group.members.collect(&:user) + instance_admins - group.members.collect(&:user) end def create_group_params @@ -217,13 +220,13 @@ module Gitlab ) end - def create_project_params + def create_project_params(group) { initialize_with_readme: true, visibility_level: VISIBILITY_LEVEL, name: PROJECT_NAME, description: "This project is automatically generated and will be used to help monitor this GitLab instance. [More information](#{docs_path})", - namespace_id: @group.id + namespace_id: group.id } end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b3f4b0f4931..bf24381c031 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10947,6 +10947,9 @@ msgstr "" msgid "No data to display" msgstr "" +msgid "No deployment platform available" +msgstr "" + msgid "No deployments found" msgstr "" @@ -11013,6 +11016,9 @@ msgstr "" msgid "No parent group" msgstr "" +msgid "No pods available" +msgstr "" + msgid "No preview for this file type" msgstr "" diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index f69847119d4..dbc408bcdd9 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -20,6 +20,38 @@ describe Profiles::NotificationsController do expect(response).to render_template :show end + + context 'with groups that do not have notification preferences' do + set(:group) { create(:group) } + set(:subgroup) { create(:group, parent: group) } + + before do + group.add_developer(user) + end + + it 'still shows up in the list' do + sign_in(user) + + get :show + + expect(assigns(:group_notifications).map(&:source_id)).to include(subgroup.id) + end + + it 'has an N+1 (but should not)' do + sign_in(user) + + control = ActiveRecord::QueryRecorder.new do + get :show + end + + create_list(:group, 2, parent: group) + + # We currently have an N + 1, switch to `not_to` once fixed + expect do + get :show + end.to exceed_query_limit(control) + end + end end describe 'POST update' do diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index e7372189d17..bcb762664f7 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -17,16 +17,27 @@ describe SnippetsFinder do end describe '#execute' do - set(:user) { create(:user) } - set(:private_personal_snippet) { create(:personal_snippet, :private, author: user) } - set(:internal_personal_snippet) { create(:personal_snippet, :internal, author: user) } - set(:public_personal_snippet) { create(:personal_snippet, :public, author: user) } + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, group: group) } + + let_it_be(:private_personal_snippet) { create(:personal_snippet, :private, author: user) } + let_it_be(:internal_personal_snippet) { create(:personal_snippet, :internal, author: user) } + let_it_be(:public_personal_snippet) { create(:personal_snippet, :public, author: user) } + + let_it_be(:private_project_snippet) { create(:project_snippet, :private, project: project) } + let_it_be(:internal_project_snippet) { create(:project_snippet, :internal, project: project) } + let_it_be(:public_project_snippet) { create(:project_snippet, :public, project: project) } context 'filter by scope' do it "returns all snippets for 'all' scope" do snippets = described_class.new(user, scope: :all).execute - expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) + expect(snippets).to contain_exactly( + private_personal_snippet, internal_personal_snippet, public_personal_snippet, + internal_project_snippet, public_project_snippet + ) end it "returns all snippets for 'are_private' scope" do @@ -38,13 +49,13 @@ describe SnippetsFinder do it "returns all snippets for 'are_internal' scope" do snippets = described_class.new(user, scope: :are_internal).execute - expect(snippets).to contain_exactly(internal_personal_snippet) + expect(snippets).to contain_exactly(internal_personal_snippet, internal_project_snippet) end - it "returns all snippets for 'are_private' scope" do + it "returns all snippets for 'are_public' scope" do snippets = described_class.new(user, scope: :are_public).execute - expect(snippets).to contain_exactly(public_personal_snippet) + expect(snippets).to contain_exactly(public_personal_snippet, public_project_snippet) end end @@ -86,7 +97,6 @@ describe SnippetsFinder do end it 'returns all snippets for an admin' do - admin = create(:user, :admin) snippets = described_class.new(admin, author: user).execute expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) @@ -94,12 +104,6 @@ describe SnippetsFinder do end context 'project snippets' do - let(:group) { create(:group, :public) } - let(:project) { create(:project, :public, group: group) } - let!(:private_project_snippet) { create(:project_snippet, :private, project: project) } - let!(:internal_project_snippet) { create(:project_snippet, :internal, project: project) } - let!(:public_project_snippet) { create(:project_snippet, :public, project: project) } - it 'returns public personal and project snippets for unauthorized user' do snippets = described_class.new(nil, project: project).execute @@ -147,7 +151,6 @@ describe SnippetsFinder do end it 'returns all snippets for an admin' do - admin = create(:user, :admin) snippets = described_class.new(admin, project: project).execute expect(snippets).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet) @@ -174,6 +177,30 @@ describe SnippetsFinder do end end + context 'explore snippets' do + it 'returns only public personal snippets for unauthenticated users' do + snippets = described_class.new(nil, explore: true).execute + + expect(snippets).to contain_exactly(public_personal_snippet) + end + + it 'also returns internal personal snippets for authenticated users' do + snippets = described_class.new(user, explore: true).execute + + expect(snippets).to contain_exactly( + internal_personal_snippet, public_personal_snippet + ) + end + + it 'returns all personal snippets for admins' do + snippets = described_class.new(admin, explore: true).execute + + expect(snippets).to contain_exactly( + private_personal_snippet, internal_personal_snippet, public_personal_snippet + ) + end + end + context 'when the user cannot read cross project' do before do allow(Ability).to receive(:allowed?).and_call_original diff --git a/spec/models/concerns/stepable_spec.rb b/spec/models/concerns/stepable_spec.rb index 5685de6a9bf..51356c3eaf6 100644 --- a/spec/models/concerns/stepable_spec.rb +++ b/spec/models/concerns/stepable_spec.rb @@ -7,6 +7,8 @@ describe Stepable do Class.new do include Stepable + attr_writer :return_non_success + steps :method1, :method2, :method3 def execute @@ -15,18 +17,18 @@ describe Stepable do private - def method1 + def method1(_result) { status: :success } end - def method2 - return { status: :error } unless @pass + def method2(result) + return { status: :not_a_success } if @return_non_success - { status: :success, variable1: 'var1' } + result.merge({ status: :success, variable1: 'var1', excluded_variable: 'a' }) end - def method3 - { status: :success, variable2: 'var2' } + def method3(result) + result.except(:excluded_variable).merge({ status: :success, variable2: 'var2' }) end end end @@ -41,8 +43,8 @@ describe Stepable do private - def appended_method1 - { status: :success } + def appended_method1(previous_result) + previous_result.merge({ status: :success }) end end end @@ -51,21 +53,19 @@ describe Stepable do described_class.prepend(prepended_module) end - it 'stops after the first error' do + it 'stops after the first non success status' do + subject.return_non_success = true + expect(subject).not_to receive(:method3) expect(subject).not_to receive(:appended_method1) expect(subject.execute).to eq( - status: :error, - failed_step: :method2 + status: :not_a_success, + last_step: :method2 ) end context 'when all methods return success' do - before do - subject.instance_variable_set(:@pass, true) - end - it 'calls all methods in order' do expect(subject).to receive(:method1).and_call_original.ordered expect(subject).to receive(:method2).and_call_original.ordered @@ -82,6 +82,10 @@ describe Stepable do variable2: 'var2' ) end + + it 'can modify results of previous steps' do + expect(subject.execute).not_to include(excluded_variable: 'a') + end end context 'with multiple stepable classes' do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 15dcccb37d9..f4dcbfbc190 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -183,12 +183,12 @@ describe Snippet do end end - describe '.only_global_snippets' do + describe '.only_personal_snippets' do it 'returns snippets not associated with any projects' do create(:project_snippet) snippet = create(:snippet) - snippets = described_class.only_global_snippets + snippets = described_class.only_personal_snippets expect(snippets).to eq([snippet]) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 240c917e7cf..8eb2f9b5bc0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3735,6 +3735,80 @@ describe User do end end + describe '#notification_settings_for' do + let(:user) { create(:user) } + let(:source) { nil } + + subject { user.notification_settings_for(source) } + + context 'when source is nil' do + it 'returns a blank global notification settings object' do + expect(subject.source).to eq(nil) + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + + context 'when source is a Group' do + let(:group) { create(:group) } + + subject { user.notification_settings_for(group, inherit: true) } + + context 'when group has no existing notification settings' do + context 'when group has no ancestors' do + it 'will be a default Global notification setting' do + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + + context 'when group has ancestors' do + context 'when an ancestor has a level other than Global' do + let(:ancestor) { create(:group) } + let(:group) { create(:group, parent: ancestor) } + + before do + create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com') + end + + it 'has the same level set' do + expect(subject.level).to eq('participating') + end + + it 'has the same email set' do + expect(subject.notification_email).to eq('ancestor@example.com') + end + + context 'when inherit is false' do + subject { user.notification_settings_for(group) } + + it 'does not inherit settings' do + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + end + + context 'when an ancestor has a Global level but has an email set' do + let(:grand_ancestor) { create(:group) } + let(:ancestor) { create(:group, parent: grand_ancestor) } + let(:group) { create(:group, parent: ancestor) } + + before do + create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com') + create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com') + end + + it 'has the same email set' do + expect(subject.level).to eq('global') + expect(subject.notification_email).to eq('ancestor@example.com') + end + end + end + end + end + end + describe '#notification_email_for' do let(:user) { create(:user) } let(:group) { create(:group) } diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index ea7ea02cee3..6e1fdb7aad0 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -147,35 +147,44 @@ describe ApplicationSettings::UpdateService do using RSpec::Parameterized::TableSyntax where(:params_performance_bar_enabled, - :params_performance_bar_allowed_group_path, - :previous_performance_bar_allowed_group_id, - :expected_performance_bar_allowed_group_id) do - true | '' | nil | nil - true | '' | 42_000_000 | nil - true | nil | nil | nil - true | nil | 42_000_000 | nil - true | 'foo' | nil | nil - true | 'foo' | 42_000_000 | nil - true | 'group_a' | nil | 42_000_000 - true | 'group_b' | 42_000_000 | 43_000_000 - true | 'group_a' | 42_000_000 | 42_000_000 - false | '' | nil | nil - false | '' | 42_000_000 | nil - false | nil | nil | nil - false | nil | 42_000_000 | nil - false | 'foo' | nil | nil - false | 'foo' | 42_000_000 | nil - false | 'group_a' | nil | nil - false | 'group_b' | 42_000_000 | nil - false | 'group_a' | 42_000_000 | nil + :params_performance_bar_allowed_group_path, + :previous_performance_bar_allowed_group_id, + :expected_performance_bar_allowed_group_id, + :expected_valid) do + true | '' | nil | nil | true + true | '' | 42_000_000 | nil | true + true | nil | nil | nil | true + true | nil | 42_000_000 | nil | true + true | 'foo' | nil | nil | false + true | 'foo' | 42_000_000 | 42_000_000 | false + true | 'group_a' | nil | 42_000_000 | true + true | 'group_b' | 42_000_000 | 43_000_000 | true + true | 'group_b/' | 42_000_000 | 43_000_000 | true + true | 'group_a' | 42_000_000 | 42_000_000 | true + false | '' | nil | nil | true + false | '' | 42_000_000 | nil | true + false | nil | nil | nil | true + false | nil | 42_000_000 | nil | true + false | 'foo' | nil | nil | true + false | 'foo' | 42_000_000 | nil | true + false | 'group_a' | nil | nil | true + false | 'group_b' | 42_000_000 | nil | true + false | 'group_a' | 42_000_000 | nil | true + nil | '' | nil | nil | true + nil | 'foo' | nil | nil | false + nil | 'group_a' | nil | 42_000_000 | true end with_them do let(:params) do { - performance_bar_enabled: params_performance_bar_enabled, performance_bar_allowed_group_path: params_performance_bar_allowed_group_path - } + }.tap do |params_hash| + # Treat nil in the table as missing + unless params_performance_bar_enabled.nil? + params_hash[:performance_bar_enabled] = params_performance_bar_enabled + end + end end before do @@ -202,6 +211,14 @@ describe ApplicationSettings::UpdateService do .not_to change(application_settings, :performance_bar_allowed_group_id) end end + + it 'adds errors to the model for invalid params' do + expect(subject.execute).to eq(expected_valid) + + unless expected_valid + expect(application_settings.errors[:performance_bar_allowed_group_id]).to be_present + end + end end context 'when :performance_bar_allowed_group_path is not present' do @@ -221,7 +238,7 @@ describe ApplicationSettings::UpdateService do let(:group) { create(:group) } let(:params) { { performance_bar_allowed_group_path: group.full_path } } - it 'implicitely defaults to true' do + it 'implicitly defaults to true' do expect { subject.execute } .to change(application_settings, :performance_bar_allowed_group_id) .from(nil).to(group.id) |
