diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 5984efd1cf8..a669e004d3a 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -249,7 +249,7 @@ } .filtered-search-input-dropdown-menu { - max-height: $dropdown-max-height; + max-height: $dropdown-max-height-lg; max-width: 280px; overflow: auto; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 8d388151dbc..0bb5933327d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -23,6 +23,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo before_action do push_frontend_feature_flag(:vue_issuable_sidebar, @project.group) + push_frontend_feature_flag(:release_search_filter, @project) end around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] diff --git a/app/models/user.rb b/app/models/user.rb index c0d73cb435c..07cd8431d1a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1454,7 +1454,7 @@ class User < ApplicationRecord # Does the user have access to all private groups & projects? # Overridden in EE to also check auditor? def full_private_access? - admin? + can?(:read_all_resources) end def update_two_factor_requirement diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 18c23cbd13a..8f5c6957a20 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -21,10 +21,6 @@ class BasePolicy < DeclarativePolicy::Base with_options scope: :user, score: 0 condition(:deactivated) { @user&.deactivated? } - desc "User has access to all private groups & projects" - with_options scope: :user, score: 0 - condition(:full_private_access) { @user&.full_private_access? } - with_options scope: :user, score: 0 condition(:external_user) { @user.nil? || @user.external? } @@ -40,10 +36,12 @@ class BasePolicy < DeclarativePolicy::Base ::Gitlab::ExternalAuthorization.perform_check? end - rule { external_authorization_enabled & ~full_private_access }.policy do + rule { external_authorization_enabled & ~can?(:read_all_resources) }.policy do prevent :read_cross_project end + rule { admin }.enable :read_all_resources + rule { default }.enable :read_cross_project end diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb index 40dd49b4afd..67c66e42d79 100644 --- a/app/policies/personal_snippet_policy.rb +++ b/app/policies/personal_snippet_policy.rb @@ -30,5 +30,5 @@ class PersonalSnippetPolicy < BasePolicy rule { can?(:create_note) }.enable :award_emoji - rule { full_private_access }.enable :read_personal_snippet + rule { can?(:read_all_resources) }.enable :read_personal_snippet end diff --git a/app/policies/project_snippet_policy.rb b/app/policies/project_snippet_policy.rb index 2a3e4ca174b..d9d09eb04cd 100644 --- a/app/policies/project_snippet_policy.rb +++ b/app/policies/project_snippet_policy.rb @@ -28,7 +28,7 @@ class ProjectSnippetPolicy < BasePolicy all?(private_snippet | (internal_snippet & external_user), ~project.guest, ~is_author, - ~full_private_access) + ~can?(:read_all_resources)) end.prevent :read_project_snippet rule { internal_snippet & ~is_author & ~admin }.policy do diff --git a/changelogs/unreleased/hide-projects-when-admin-mode-is-disabled.yml b/changelogs/unreleased/hide-projects-when-admin-mode-is-disabled.yml new file mode 100644 index 00000000000..4957b104e2e --- /dev/null +++ b/changelogs/unreleased/hide-projects-when-admin-mode-is-disabled.yml @@ -0,0 +1,5 @@ +--- +title: Hide projects without access to admin user when admin mode is disabled +merge_request: 18530 +author: Diego Louzán +type: changed diff --git a/changelogs/unreleased/nfriend-add-releases-filter-for-merge-requests.yml b/changelogs/unreleased/nfriend-add-releases-filter-for-merge-requests.yml new file mode 100644 index 00000000000..87e987a73b4 --- /dev/null +++ b/changelogs/unreleased/nfriend-add-releases-filter-for-merge-requests.yml @@ -0,0 +1,5 @@ +--- +title: Add "release" filter to merge request search page +merge_request: 19315 +author: +type: added diff --git a/db/migrate/20191101092917_replace_index_on_metrics_merged_at.rb b/db/migrate/20191101092917_replace_index_on_metrics_merged_at.rb new file mode 100644 index 00000000000..b2baaee2b76 --- /dev/null +++ b/db/migrate/20191101092917_replace_index_on_metrics_merged_at.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class ReplaceIndexOnMetricsMergedAt < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_request_metrics, :merged_at + remove_concurrent_index :merge_request_metrics, [:merged_at, :id] + end + + def down + add_concurrent_index :merge_request_metrics, [:merged_at, :id] + remove_concurrent_index :merge_request_metrics, :merged_at + end +end diff --git a/db/post_migrate/20190918104222_schedule_productivity_analytics_backfill.rb b/db/post_migrate/20190918104222_schedule_productivity_analytics_backfill.rb index 23d3bbbc395..cd759735f00 100644 --- a/db/post_migrate/20190918104222_schedule_productivity_analytics_backfill.rb +++ b/db/post_migrate/20190918104222_schedule_productivity_analytics_backfill.rb @@ -6,7 +6,7 @@ class ScheduleProductivityAnalyticsBackfill < ActiveRecord::Migration[5.2] DOWNTIME = false def up - # no-op since the scheduling times out on GitLab.com + # no-op since the migration was removed end def down diff --git a/db/schema.rb b/db/schema.rb index 220e57527a5..6fbfd45a11a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2290,7 +2290,7 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do t.index ["latest_closed_by_id"], name: "index_merge_request_metrics_on_latest_closed_by_id" t.index ["merge_request_id", "merged_at"], name: "index_merge_request_metrics_on_merge_request_id_and_merged_at", where: "(merged_at IS NOT NULL)" t.index ["merge_request_id"], name: "index_merge_request_metrics" - t.index ["merged_at", "id"], name: "index_merge_request_metrics_on_merged_at_and_id" + t.index ["merged_at"], name: "index_merge_request_metrics_on_merged_at" t.index ["merged_by_id"], name: "index_merge_request_metrics_on_merged_by_id" t.index ["pipeline_id"], name: "index_merge_request_metrics_on_pipeline_id" end diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index a2da37dac97..d7cfa0b6c16 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -89,6 +89,10 @@ your GitLab installation has three repository storages: `default`, `storage1` and `storage2`. You can use as little as just one server with one repository storage if desired. +Note: **Note:** The token referred to throughout the Gitaly documentation is +just an arbitrary password selected by the administrator. It is unrelated to +tokens created for the GitLab API or other similar web API tokens. + ### 1. Installation First install Gitaly on each Gitaly server using either diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index cc9df479492..d716325f332 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -245,47 +245,29 @@ end #### Use self-descriptive wrapper methods -When it's not possible/logical to modify the implementation of a -method. Wrap it in a self-descriptive method and use that method. +When it's not possible/logical to modify the implementation of a method, then +wrap it in a self-descriptive method and use that method. -For example, in CE only an `admin` is allowed to access all private -projects/groups, but in EE also an `auditor` has full private -access. It would be incorrect to override the implementation of -`User#admin?`, so instead add a method `full_private_access?` to -`app/models/users.rb`. The implementation in CE will be: +For example, in GitLab-FOSS, the only user created by the system is `User.ghost` +but in EE there are several types of bot-users that aren't really users. It would +be incorrect to override the implementation of `User#ghost?`, so instead we add +a method `#internal?` to `app/models/user.rb`. The implementation will be: ```ruby -def full_private_access? - admin? +def internal? + ghost? end ``` In EE, the implementation `ee/app/models/ee/users.rb` would be: ```ruby -override :full_private_access? -def full_private_access? - super || auditor? +override :internal? +def internal? + super || bot? end ``` -In `lib/gitlab/visibility_level.rb` this method is used to return the -allowed visibility levels: - -```ruby -def levels_for_user(user = nil) - if user.full_private_access? - [PRIVATE, INTERNAL, PUBLIC] - elsif # ... -end -``` - -See [CE MR][ce-mr-full-private] and [EE MR][ee-mr-full-private] for -full implementation details. - -[ce-mr-full-private]: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/12373 -[ee-mr-full-private]: https://gitlab.com/gitlab-org/gitlab/merge_requests/2199 - ### Code in `config/routes` When we add `draw :admin` in `config/routes.rb`, the application will try to diff --git a/doc/development/utilities.md b/doc/development/utilities.md index a5f6fec4cd8..25869a0d2b5 100644 --- a/doc/development/utilities.md +++ b/doc/development/utilities.md @@ -184,3 +184,83 @@ class Commit request_cache(:author) { author_email } end ``` + +## `ReactiveCaching` + +The `ReactiveCaching` concern is used to fetch some data in the background and +store it in the Rails cache, keeping it up-to-date for as long as it is being +requested. If the data hasn't been requested for `reactive_cache_lifetime`, +it will stop being refreshed, and then be removed. + +Example of use: + +```ruby +class Foo < ApplicationRecord + include ReactiveCaching + + after_save :clear_reactive_cache! + + def calculate_reactive_cache + # Expensive operation here. The return value of this method is cached + end + + def result + with_reactive_cache do |data| + # ... + end + end +end +``` + +In this example, the first time `#result` is called, it will return `nil`. +However, it will enqueue a background worker to call `#calculate_reactive_cache` +and set an initial cache lifetime of ten minutes. + +The background worker needs to find or generate the object on which +`with_reactive_cache` was called. +The default behaviour can be overridden by defining a custom +`reactive_cache_worker_finder`. +Otherwise, the background worker will use the class name and primary key to get +the object using the ActiveRecord `find_by` method. + +```ruby +class Bar + include ReactiveCaching + + self.reactive_cache_key = ->() { ["bar", "thing"] } + self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } + + def self.from_cache(var1, var2) + # This method will be called by the background worker with "bar1" and + # "bar2" as arguments. + new(var1, var2) + end + + def initialize(var1, var2) + # ... + end + + def calculate_reactive_cache + # Expensive operation here. The return value of this method is cached + end + + def result + with_reactive_cache("bar1", "bar2") do |data| + # ... + end + end +end +``` + +Each time the background job completes, it stores the return value of +`#calculate_reactive_cache`. It is also re-enqueued to run again after +`reactive_cache_refresh_interval`, therefore, it will keep the stored value up to date. +Calculations are never run concurrently. + +Calling `#result` while a value is cached will call the block given to +`#with_reactive_cache`, yielding the cached value. It will also extend the +lifetime by the `reactive_cache_lifetime` value. + +Once the lifetime has expired, no more background jobs will be enqueued and +calling `#result` will again return `nil` - starting the process all over +again. diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index a3606fadb89..52b7035389a 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -176,7 +176,7 @@ Here's a list of what you can't do with subgroups: - [GitLab Pages](../../project/pages/index.md) supports projects hosted under a subgroup, but not subgroup websites. That means that only the highest-level group supports - [group websites](../../project/pages/getting_started_part_one.md#gitlab-pages-domain-names), + [group websites](../../project/pages/getting_started_part_one.md#gitlab-pages-default-domain-names), although you can have project websites under a subgroup. - It is not possible to share a project with a group that's an ancestor of the group the project is in. That means you can only share as you walk down diff --git a/doc/user/project/integrations/generic_alerts.md b/doc/user/project/integrations/generic_alerts.md index b5f86c00eb3..62310dd9177 100644 --- a/doc/user/project/integrations/generic_alerts.md +++ b/doc/user/project/integrations/generic_alerts.md @@ -37,12 +37,12 @@ Example request: ```sh curl --request POST \ --data '{"title": "Incident title"}' \ - --header "Authorization: Bearer " \ + --header "Authorization: Bearer " \ --header "Content-Type: application/json" \ ``` -The `` and `` values can be found when [setting up generic alerts](#setting-up-generic-alerts). +The `` and `` values can be found when [setting up generic alerts](#setting-up-generic-alerts). Example payload: diff --git a/doc/user/project/pages/getting_started/new_or_existing_website.md b/doc/user/project/pages/getting_started/new_or_existing_website.md index efdd4e6a158..62b5fa33117 100644 --- a/doc/user/project/pages/getting_started/new_or_existing_website.md +++ b/doc/user/project/pages/getting_started/new_or_existing_website.md @@ -14,7 +14,7 @@ To do so, follow the steps below. 1. From your **Project**'s **[Dashboard](https://gitlab.com/dashboard/projects)**, click **New project**, and name it according to the - [Pages domain names](../getting_started_part_one.md#gitlab-pages-domain-names). + [Pages domain names](../getting_started_part_one.md#gitlab-pages-default-domain-names). 1. Clone it to your local computer, add your website files to your project, add, commit and push to GitLab. Alternativelly, you can run `git init` in your local directory, diff --git a/doc/user/project/pages/getting_started_part_one.md b/doc/user/project/pages/getting_started_part_one.md index f9169fba220..b876e547ba5 100644 --- a/doc/user/project/pages/getting_started_part_one.md +++ b/doc/user/project/pages/getting_started_part_one.md @@ -8,7 +8,7 @@ type: concepts, reference On this document, learn how to name your project for GitLab Pages according to your intended website's URL. -## GitLab Pages domain names +## GitLab Pages default domain names >**Note:** If you use your own GitLab instance to deploy your @@ -80,7 +80,7 @@ To understand Pages domains clearly, read the examples below. - On your GitLab instance, replace `gitlab.io` above with your Pages server domain. Ask your sysadmin for this information. -## URLs and Baseurls +## URLs and baseurls Every Static Site Generator (SSG) default configuration expects to find your website under a (sub)domain (`example.com`), not @@ -108,7 +108,7 @@ example we've just mentioned, you'd have to change Jekyll's `_config.yml` to: baseurl: "" ``` -## Custom Domains +## Custom domains GitLab Pages supports custom domains and subdomains, served under HTTP or HTTPS. See [GitLab Pages custom domains and SSL/TLS Certificates](custom_domains_ssl_tls_certification/index.md) for more information. diff --git a/doc/user/project/pages/index.md b/doc/user/project/pages/index.md index d59c2e4fdf3..d1698cd54a1 100644 --- a/doc/user/project/pages/index.md +++ b/doc/user/project/pages/index.md @@ -83,7 +83,7 @@ to build your site and publish it to the GitLab Pages server. The sequence of scripts that GitLab CI/CD runs to accomplish this task is created from a file named `.gitlab-ci.yml`, which you can [create and modify](getting_started_part_four.md) at will. A specific `job` called `pages` in the configuration file will make GitLab aware that you are deploying a GitLab Pages website. -You can either use GitLab's [default domain for GitLab Pages websites](getting_started_part_one.md#gitlab-pages-domain-names), +You can either use GitLab's [default domain for GitLab Pages websites](getting_started_part_one.md#gitlab-pages-default-domain-names), `*.gitlab.io`, or your own domain (`example.com`). In that case, you'll need admin access to your domain's registrar (or control panel) to set it up with Pages. diff --git a/lib/gitlab/graphql/connections/keyset/connection.rb b/lib/gitlab/graphql/connections/keyset/connection.rb index 0daf726c005..c75ea206edb 100644 --- a/lib/gitlab/graphql/connections/keyset/connection.rb +++ b/lib/gitlab/graphql/connections/keyset/connection.rb @@ -8,10 +8,15 @@ # https://coderwall.com/p/lkcaag/pagination-you-re-probably-doing-it-wrong # # It currently supports sorting on two columns, but the last column must -# be the primary key. For example +# be the primary key. If it's not already included, an order on the +# primary key will be added automatically, like `order(id: :desc)` # # Issue.order(created_at: :asc).order(:id) -# Issue.order(due_date: :asc).order(:id) +# Issue.order(due_date: :asc) +# +# You can also use `Gitlab::Database.nulls_last_order`: +# +# Issue.reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) # # It will tolerate non-attribute ordering, but only attributes determine the cursor. # For example, this is legitimate: diff --git a/lib/gitlab/graphql/connections/keyset/order_info.rb b/lib/gitlab/graphql/connections/keyset/order_info.rb index 6c4be93bfee..4d85e8f79b7 100644 --- a/lib/gitlab/graphql/connections/keyset/order_info.rb +++ b/lib/gitlab/graphql/connections/keyset/order_info.rb @@ -5,12 +5,15 @@ module Gitlab module Connections module Keyset class OrderInfo - def initialize(order_value) - @order_value = order_value - end + attr_reader :attribute_name, :sort_direction - def attribute_name - order_value.expr.name + def initialize(order_value) + if order_value.is_a?(String) + @attribute_name, @sort_direction = extract_nulls_last_order(order_value) + else + @attribute_name = order_value.expr.name + @sort_direction = order_value.direction + end end def operator_for(before_or_after) @@ -22,10 +25,10 @@ module Gitlab end end - # Only allow specific node types. For example ignore String nodes + # Only allow specific node types def self.build_order_list(relation) order_list = relation.order_values.select do |value| - value.is_a?(Arel::Nodes::Ascending) || value.is_a?(Arel::Nodes::Descending) + supported_order_value?(value) end order_list.map { |info| OrderInfo.new(info) } @@ -52,12 +55,21 @@ module Gitlab end end + def self.supported_order_value?(order_value) + return true if order_value.is_a?(Arel::Nodes::Ascending) || order_value.is_a?(Arel::Nodes::Descending) + return false unless order_value.is_a?(String) + + tokens = order_value.downcase.split + + tokens.last(2) == %w(nulls last) && tokens.count == 4 + end + private - attr_reader :order_value + def extract_nulls_last_order(order_value) + tokens = order_value.downcase.split - def sort_direction - order_value.direction + [tokens.first, (tokens[1] == 'asc' ? :asc : :desc)] end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bc5744db4af..942aeabc050 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6271,6 +6271,12 @@ msgstr "" msgid "Environment:" msgstr "" +msgid "EnvironmentDashboard|API" +msgstr "" + +msgid "EnvironmentDashboard|Created through the Deployment API" +msgstr "" + msgid "Environments" msgstr "" diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 4ec12b5a7f7..e97a94f6fa5 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -describe ProjectsFinder do +describe ProjectsFinder, :do_not_mock_admin_mode do + include AdminModeHelper + describe '#execute' do let(:user) { create(:user) } let(:group) { create(:group, :public) } @@ -188,5 +190,21 @@ describe ProjectsFinder do it { is_expected.to eq([internal_project, public_project]) } end + + describe 'with admin user' do + let(:user) { create(:admin) } + + context 'admin mode enabled' do + before do + enable_admin_mode!(current_user) + end + + it { is_expected.to match_array([public_project, internal_project, private_project, shared_project]) } + end + + context 'admin mode disabled' do + it { is_expected.to match_array([public_project, internal_project]) } + end + end end end diff --git a/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb b/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb index 608a9ed1d85..17ddcaefeeb 100644 --- a/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb +++ b/spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb @@ -17,6 +17,26 @@ describe Gitlab::Graphql::Connections::Keyset::OrderInfo do expect(order_list.last.operator_for(:after)).to eq '>' end end + + context 'when order contains NULLS LAST' do + let(:relation) { Project.order(Arel.sql('projects.updated_at Asc Nulls Last')).order(:id) } + + it 'does not ignore the SQL order' do + expect(order_list.count).to eq 2 + expect(order_list.first.attribute_name).to eq 'projects.updated_at' + expect(order_list.first.operator_for(:after)).to eq '>' + expect(order_list.last.attribute_name).to eq 'id' + expect(order_list.last.operator_for(:after)).to eq '>' + end + end + + context 'when order contains invalid formatted NULLS LAST ' do + let(:relation) { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')).order(:id) } + + it 'ignores the SQL order' do + expect(order_list.count).to eq 1 + end + end end describe '#validate_ordering' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8eb2f9b5bc0..ee7edb1516c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe User do +describe User, :do_not_mock_admin_mode do include ProjectForksHelper include TermsHelper @@ -2797,10 +2797,26 @@ describe User do expect(user.full_private_access?).to be_falsy end - it 'returns true for admin user' do - user = build(:user, :admin) + context 'for admin user' do + include_context 'custom session' - expect(user.full_private_access?).to be_truthy + let(:user) { build(:user, :admin) } + + context 'when admin mode is disabled' do + it 'returns false' do + expect(user.full_private_access?).to be_falsy + end + end + + context 'when admin mode is enabled' do + before do + Gitlab::Auth::CurrentUserMode.new(user).enable_admin_mode!(password: user.password) + end + + it 'returns true' do + expect(user.full_private_access?).to be_truthy + end + end end end diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb index 9e59d35b1bb..81aee4cfcac 100644 --- a/spec/policies/base_policy_spec.rb +++ b/spec/policies/base_policy_spec.rb @@ -2,8 +2,9 @@ require 'spec_helper' -describe BasePolicy do +describe BasePolicy, :do_not_mock_admin_mode do include ExternalAuthorizationServiceHelpers + include AdminModeHelper describe '.class_for' do it 'detects policy class based on the subject ancestors' do @@ -36,8 +37,42 @@ describe BasePolicy do it { is_expected.not_to be_allowed(:read_cross_project) } - it 'allows admins' do - expect(described_class.new(build(:admin), nil)).to be_allowed(:read_cross_project) + context 'for admins' do + let(:current_user) { build(:admin) } + + subject { described_class.new(current_user, nil) } + + it 'allowed when in admin mode' do + enable_admin_mode!(current_user) + + is_expected.to be_allowed(:read_cross_project) + end + + it 'prevented when not in admin mode' do + is_expected.not_to be_allowed(:read_cross_project) + end + end + end + end + + describe 'full private access' do + let(:current_user) { create(:user) } + + subject { described_class.new(current_user, nil) } + + it { is_expected.not_to be_allowed(:read_all_resources) } + + context 'for admins' do + let(:current_user) { build(:admin) } + + it 'allowed when in admin mode' do + enable_admin_mode!(current_user) + + is_expected.to be_allowed(:read_all_resources) + end + + it 'prevented when not in admin mode' do + is_expected.not_to be_allowed(:read_all_resources) end end end