diff options
28 files changed, 637 insertions, 119 deletions
diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 9cf42b34974..a057913fd5a 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -141,7 +141,7 @@ export default { data-qa-selector="file_name_link" > <i - v-if="!loadingPath" + v-if="path !== loadingPath" :aria-label="type" role="img" :class="iconName" diff --git a/app/graphql/mutations/concerns/mutations/resolves_issuable.rb b/app/graphql/mutations/concerns/mutations/resolves_issuable.rb index 3a4db5ae18d..d63cc27a450 100644 --- a/app/graphql/mutations/concerns/mutations/resolves_issuable.rb +++ b/app/graphql/mutations/concerns/mutations/resolves_issuable.rb @@ -6,19 +6,25 @@ module Mutations include Mutations::ResolvesProject def resolve_issuable(type:, parent_path:, iid:) - parent = resolve_issuable_parent(parent_path) + parent = resolve_issuable_parent(type, parent_path) issuable_resolver(type, parent, context).resolve(iid: iid.to_s) end + private + def issuable_resolver(type, parent, context) resolver_class = "Resolvers::#{type.to_s.classify.pluralize}Resolver".constantize resolver_class.single.new(object: parent, context: context, field: nil) end - def resolve_issuable_parent(parent_path) + def resolve_issuable_parent(type, parent_path) + return unless type == :issue || type == :merge_request + resolve_project(full_path: parent_path) end end end + +Mutations::ResolvesIssuable.prepend_if_ee('::EE::Mutations::ResolvesIssuable') diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 4886f2debb1..7d67e258991 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Runner < ApplicationRecord - VERSION = '0.14.0' + VERSION = '0.15.0' self.table_name = 'clusters_applications_runners' diff --git a/app/models/member.rb b/app/models/member.rb index a87efdf63ff..5aa5655fc4a 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -468,6 +468,7 @@ class Member < ApplicationRecord # for a Member to be commited before attempting to update the highest role. # rubocop: disable CodeReuse/ServiceClass def update_highest_role + return unless Feature.enabled?(:highest_role_callback) return unless user_id.present? return unless previous_changes[:access_level].present? diff --git a/app/models/user.rb b/app/models/user.rb index 65ff4c98b15..8709bfb18fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -217,6 +217,7 @@ class User < ApplicationRecord before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? } before_validation :ensure_namespace_correct before_save :ensure_namespace_correct # in case validation is skipped + before_save :ensure_bio_is_assigned_to_user_details, if: :bio_changed? after_validation :set_username_errors after_update :username_changed_hook, if: :saved_change_to_username? after_destroy :post_destroy_hook @@ -1262,6 +1263,13 @@ class User < ApplicationRecord end end + # Temporary, will be removed when bio is fully migrated + def ensure_bio_is_assigned_to_user_details + return if Feature.disabled?(:migrate_bio_to_user_details, default_enabled: true) + + user_detail.bio = bio.to_s[0...255] # bio can be NULL in users, but cannot be NULL in user_details + end + def set_username_errors namespace_path_errors = self.errors.delete(:"namespace.path") self.errors[:username].concat(namespace_path_errors) if namespace_path_errors diff --git a/changelogs/unreleased/206913-migrate-users-bio.yml b/changelogs/unreleased/206913-migrate-users-bio.yml new file mode 100644 index 00000000000..92d1a8da4a3 --- /dev/null +++ b/changelogs/unreleased/206913-migrate-users-bio.yml @@ -0,0 +1,5 @@ +--- +title: Add user_details.bio column and migrate data from users.bio +merge_request: 27773 +author: +type: changed diff --git a/changelogs/unreleased/georgekoltsov-log-added-team-members-on-import.yml b/changelogs/unreleased/georgekoltsov-log-added-team-members-on-import.yml new file mode 100644 index 00000000000..4450f5b3551 --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-log-added-team-members-on-import.yml @@ -0,0 +1,5 @@ +--- +title: Log member additions when importing Project/Group +merge_request: 27930 +author: +type: other diff --git a/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-15-0.yml b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-15-0.yml new file mode 100644 index 00000000000..319c8cc81d1 --- /dev/null +++ b/changelogs/unreleased/update-gitlab-runner-helm-chart-to-0-15-0.yml @@ -0,0 +1,5 @@ +--- +title: Update GitLab Runner Helm Chart to 0.15.0 +merge_request: 27670 +author: +type: other diff --git a/db/migrate/20200323071918_add_bio_to_user_details.rb b/db/migrate/20200323071918_add_bio_to_user_details.rb new file mode 100644 index 00000000000..90e4b964695 --- /dev/null +++ b/db/migrate/20200323071918_add_bio_to_user_details.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddBioToUserDetails < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:user_details, :bio, :string, default: '', allow_null: false, limit: 255, update_column_in_batches_args: { batch_column_name: :user_id }) + end + + def down + remove_column(:user_details, :bio) + end +end diff --git a/db/migrate/20200323074147_add_temp_index_on_users_bio.rb b/db/migrate/20200323074147_add_temp_index_on_users_bio.rb new file mode 100644 index 00000000000..4b26bb971f5 --- /dev/null +++ b/db/migrate/20200323074147_add_temp_index_on_users_bio.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddTempIndexOnUsersBio < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'tmp_idx_on_user_id_where_bio_is_filled' + + disable_ddl_transaction! + + def up + add_concurrent_index :users, :id, where: "(COALESCE(bio, '') IS DISTINCT FROM '')", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :users, INDEX_NAME + end +end diff --git a/db/post_migrate/20200323080714_trigger_background_migration_for_users_bio.rb b/db/post_migrate/20200323080714_trigger_background_migration_for_users_bio.rb new file mode 100644 index 00000000000..31ab41a6b88 --- /dev/null +++ b/db/post_migrate/20200323080714_trigger_background_migration_for_users_bio.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class TriggerBackgroundMigrationForUsersBio < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INTERVAL = 2.minutes.to_i + BATCH_SIZE = 500 + MIGRATION = 'MigrateUsersBioToUserDetails' + + disable_ddl_transaction! + + class User < ActiveRecord::Base + self.table_name = 'users' + + include ::EachBatch + end + + def up + relation = User.where("(COALESCE(bio, '') IS DISTINCT FROM '')") + + queue_background_migration_jobs_by_range_at_intervals(relation, + MIGRATION, + INTERVAL, + batch_size: BATCH_SIZE) + end + + def down + # no-op + end +end diff --git a/db/structure.sql b/db/structure.sql index adaa0a594e6..8bd83517bb8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -6152,7 +6152,8 @@ ALTER SEQUENCE public.user_custom_attributes_id_seq OWNED BY public.user_custom_ CREATE TABLE public.user_details ( user_id bigint NOT NULL, - job_title character varying(200) DEFAULT ''::character varying NOT NULL + job_title character varying(200) DEFAULT ''::character varying NOT NULL, + bio character varying(255) DEFAULT ''::character varying NOT NULL ); CREATE SEQUENCE public.user_details_user_id_seq @@ -10243,6 +10244,8 @@ CREATE UNIQUE INDEX term_agreements_unique_index ON public.term_agreements USING CREATE INDEX tmp_build_stage_position_index ON public.ci_builds USING btree (stage_id, stage_idx) WHERE (stage_idx IS NOT NULL); +CREATE INDEX tmp_idx_on_user_id_where_bio_is_filled ON public.users USING btree (id) WHERE ((COALESCE(bio, ''::character varying))::text IS DISTINCT FROM ''::text); + CREATE INDEX undefined_vulnerabilities ON public.vulnerability_occurrences USING btree (id) WHERE (severity = 0); CREATE INDEX undefined_vulnerability ON public.vulnerabilities USING btree (id) WHERE (severity = 0); @@ -12797,7 +12800,10 @@ COPY "schema_migrations" (version) FROM STDIN; 20200319203901 20200320112455 20200320123839 +20200323071918 +20200323074147 20200323075043 +20200323080714 20200323122201 20200323134519 20200324115359 diff --git a/doc/install/README.md b/doc/install/README.md index 6b497763d93..06066bc0c7d 100644 --- a/doc/install/README.md +++ b/doc/install/README.md @@ -28,9 +28,7 @@ with Kubernetes. ## Requirements -Before installing GitLab, make sure to check the [requirements documentation](requirements.md) -which includes useful information on the supported Operating Systems as well as -the hardware requirements. +Before installing GitLab, it is of critical importance to review the system [requirements](requirements.md). The system requirements include details on the minimum hardware, software, database, and additional requirements to support GitLab. ## Installing GitLab using the Omnibus GitLab package (recommended) diff --git a/doc/integration/sourcegraph.md b/doc/integration/sourcegraph.md index da384fa9528..c0ce3c30ca6 100644 --- a/doc/integration/sourcegraph.md +++ b/doc/integration/sourcegraph.md @@ -4,7 +4,8 @@ type: reference, how-to # Sourcegraph integration -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/16556) in GitLab 12.5. Please note that this integration is in BETA and [behind a feature flag](#enable-the-sourcegraph-feature-flag). +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/16556) in GitLab 12.5. +> - Note that this integration is in BETA and deployed [behind a feature flag](#enable-the-sourcegraph-feature-flag) disabled by default. Self-managed instances can opt to enable it. [Sourcegraph](https://sourcegraph.com) provides code intelligence features, natively integrated into the GitLab UI. @@ -106,18 +107,17 @@ When visiting one of these views, you can now hover over a code reference to see ## Sourcegraph for GitLab.com -Sourcegraph powered code intelligence will be incrementally rolled out on GitLab.com. -It will eventually become available for all public projects, but for now, it is only -available for some specific projects within the [`gitlab-org`](https://gitlab.com/gitlab-org/) -group, e.g., <https://gitlab.com/gitlab-org/gitlab>. This means that you can see -it working and use it to dig into the code of these projects, but you cannot use -it on your own project on GitLab.com yet. +Sourcegraph powered code intelligence is avaialable for all public projects on GitLab.com. -If you would like to use it in your own projects as of GitLab 12.5, you can do so by -setting up a self-managed GitLab instance. +Support for private projects is currently not available for GitLab.com; +follow the epic [&2201](https://gitlab.com/groups/gitlab-org/-/epics/2201) +for updates. -Follow the epic [&2201](https://gitlab.com/groups/gitlab-org/-/epics/2201) for -updates. +## Troubleshooting + +### Sourcegraph isn't working + +If you enabled Sourcegraph for your project but still it doesn't looklike it's working, it might be because Sourcegraph has not indexed theproject yet. You can check for Sourcegraph's availability of your project by visiting `https://sourcegraph.com/gitlab.com/<project-path>`replacing `<project-path>` with the path to your GitLab project. ## Sourcegraph and Privacy diff --git a/doc/policy/maintenance.md b/doc/policy/maintenance.md index e9311285ba5..4c27bc5c4fd 100644 --- a/doc/policy/maintenance.md +++ b/doc/policy/maintenance.md @@ -168,6 +168,9 @@ Please see the table below for some examples: | 12.5.8 | 11.3.4 | `11.3.4` -> `11.11.8` -> `12.0.12` -> `12.5.8` | `11.11.8` is the last version in version `11`. `12.0.x` [is a required step](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23211#note_272842444). | | 12.8.5 | 9.2.6 | `9.2.6` -> `9.5.10` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.8.5` | Four intermediate versions are required: the final 9.5, 10.8, 11.11 releases, plus 12.0. | +NOTE: **Note:** +Instructions for installing a specific version of GitLab or downloading the package locally for installation can be found at [GitLab Repositories](https://packages.gitlab.com/gitlab). + ## More information More information about the release procedures can be found in our diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index a99444c013f..92466dab033 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -96,7 +96,9 @@ GitLab.com is using the IP range `34.74.90.64/28` for traffic from its Web/API fleet. You can expect connections from webhooks or repository mirroring to come from those IPs and whitelist them. -For connections from CI/CD runners we are not providing static IP addresses. +GitLab.com is fronted by Cloudflare. For incoming connections to GitLab.com you might need to whitelist CIDR blocks of Cloudflare ([IPv4](https://www.cloudflare.com/ips-v4) and [IPv6](https://www.cloudflare.com/ips-v6)) + +For outgoing connections from CI/CD runners we are not providing static IP addresses. All our runners are deployed into Google Cloud Platform (GCP) - any IP based firewall can be configured by looking up all [IP address ranges or CIDR blocks for GCP](https://cloud.google.com/compute/docs/faq#where_can_i_find_product_name_short_ip_ranges). diff --git a/doc/user/project/merge_requests/creating_merge_requests.md b/doc/user/project/merge_requests/creating_merge_requests.md index 268789e092e..e8c691a3d7f 100644 --- a/doc/user/project/merge_requests/creating_merge_requests.md +++ b/doc/user/project/merge_requests/creating_merge_requests.md @@ -155,6 +155,26 @@ and the target project and branch where you want to merge the changes into. Click on **Compare branches and continue** to go to the [**New Merge Request** page](#new-merge-request-page) and fill in the details. +## New merge request from a fork + +After forking a project and applying your local changes, complete the following steps to +create a merge request from your fork to contribute back to the main project: + +1. Go to **Projects > Your Projects** and select your fork of the repository. +1. In the left menu, go to **Merge Requests**, and click **New Merge Request**. +1. In the **Source branch** drop-down list box, select your branch in your forked repository as the source branch. +1. In the **Target branch** drop-down list box, select the branch from the upstream repository as the target branch. +1. After entering the credentials, click **Compare branches and continue** to compare your local changes to the upstream repository. +1. Assign a user to review your changes, and click **Submit merge request**. + +When the changes are merged, your changes are added to the upstream repository and +the branch as per specification. After your work is merged, if you don't intend to +make any other contributions to the upstream project, you can unlink your +fork from its upstream project in the **Settings > Advanced Settings** section by +[removing the forking relashionship](../settings/index.md#removing-a-fork-relationship). + +For further details, [see the forking workflow documentation](../repository/forking_workflow.md). + ## New merge request by email **(CORE ONLY)** _This feature needs [incoming email](../../../administration/incoming_email.md) diff --git a/lib/gitlab/background_migration/migrate_users_bio_to_user_details.rb b/lib/gitlab/background_migration/migrate_users_bio_to_user_details.rb new file mode 100644 index 00000000000..ca64d13b118 --- /dev/null +++ b/lib/gitlab/background_migration/migrate_users_bio_to_user_details.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class MigrateUsersBioToUserDetails + class User < ActiveRecord::Base + self.table_name = 'users' + end + + class UserDetails < ActiveRecord::Base + self.table_name = 'user_details' + end + + def perform(start_id, stop_id) + return if Feature.disabled?(:migrate_bio_to_user_details, default_enabled: true) + + relation = User + .select("id AS user_id", "substring(COALESCE(bio, '') from 1 for 255) AS bio") + .where("(COALESCE(bio, '') IS DISTINCT FROM '')") + .where(id: (start_id..stop_id)) + + ActiveRecord::Base.connection.execute <<-EOF.strip_heredoc + INSERT INTO user_details + (user_id, bio) + #{relation.to_sql} + ON CONFLICT (user_id) + DO UPDATE SET + "bio" = EXCLUDED."bio"; + EOF + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 64eef530663..dc4de9b1906 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -375,8 +375,11 @@ module Gitlab # less "complex" without introducing extra methods (which actually will # make things _more_ complex). # + # `batch_column_name` option is for tables without primary key, in this + # case an other unique integer column can be used. Example: :user_id + # # rubocop: disable Metrics/AbcSize - def update_column_in_batches(table, column, value, batch_size: nil) + def update_column_in_batches(table, column, value, batch_size: nil, batch_column_name: :id) if transaction_open? raise 'update_column_in_batches can not be run inside a transaction, ' \ 'you can disable transactions by calling disable_ddl_transaction! ' \ @@ -403,14 +406,14 @@ module Gitlab batch_size = max_size if batch_size > max_size end - start_arel = table.project(table[:id]).order(table[:id].asc).take(1) + start_arel = table.project(table[batch_column_name]).order(table[batch_column_name].asc).take(1) start_arel = yield table, start_arel if block_given? - start_id = exec_query(start_arel.to_sql).to_a.first['id'].to_i + start_id = exec_query(start_arel.to_sql).to_a.first[batch_column_name.to_s].to_i loop do - stop_arel = table.project(table[:id]) - .where(table[:id].gteq(start_id)) - .order(table[:id].asc) + stop_arel = table.project(table[batch_column_name]) + .where(table[batch_column_name].gteq(start_id)) + .order(table[batch_column_name].asc) .take(1) .skip(batch_size) @@ -420,12 +423,12 @@ module Gitlab update_arel = Arel::UpdateManager.new .table(table) .set([[table[column], value]]) - .where(table[:id].gteq(start_id)) + .where(table[batch_column_name].gteq(start_id)) if stop_row - stop_id = stop_row['id'].to_i + stop_id = stop_row[batch_column_name.to_s].to_i start_id = stop_id - update_arel = update_arel.where(table[:id].lt(stop_id)) + update_arel = update_arel.where(table[batch_column_name].lt(stop_id)) end update_arel = yield table, update_arel if block_given? @@ -461,7 +464,7 @@ module Gitlab # # This method can also take a block which is passed directly to the # `update_column_in_batches` method. - def add_column_with_default(table, column, type, default:, limit: nil, allow_null: false, &block) + def add_column_with_default(table, column, type, default:, limit: nil, allow_null: false, update_column_in_batches_args: {}, &block) if transaction_open? raise 'add_column_with_default can not be run inside a transaction, ' \ 'you can disable transactions by calling disable_ddl_transaction! ' \ @@ -483,7 +486,12 @@ module Gitlab begin default_after_type_cast = connection.type_cast(default, column_for(table, column)) - update_column_in_batches(table, column, default_after_type_cast, &block) + + if update_column_in_batches_args.any? + update_column_in_batches(table, column, default_after_type_cast, **update_column_in_batches_args, &block) + else + update_column_in_batches(table, column, default_after_type_cast, &block) + end change_column_null(table, column, false) unless allow_null # We want to rescue _all_ exceptions here, even those that don't inherit diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index fd76252eb36..5832b72ab46 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -64,8 +64,19 @@ module Gitlab def add_team_member(member, existing_user = nil) member['user'] = existing_user + member_hash = member_hash(member) - relation_class.create(member_hash(member)).persisted? + member = relation_class.create(member_hash) + + if member.persisted? + log_member_addition(member_hash) + + true + else + log_member_addition_failure(member_hash, member.errors.full_messages) + + false + end end def member_hash(member) @@ -103,6 +114,34 @@ module Gitlab relation_class::MAINTAINER end + + def log_member_addition(member_hash) + log_params = base_log_params(member_hash) + log_params[:message] = '[Project/Group Import] Added new member' + + logger.info(log_params) + end + + def log_member_addition_failure(member_hash, errors) + log_params = base_log_params(member_hash) + log_params[:message] = "[Project/Group Import] Member addition failed: #{errors&.join(', ')}" + + logger.info(log_params) + end + + def base_log_params(member_hash) + { + user_id: member_hash['user']&.id, + access_level: member_hash['access_level'], + importable_type: @importable.class.to_s, + importable_id: @importable.id, + root_namespace_id: @importable.try(:root_ancestor)&.id + } + end + + def logger + @logger ||= Gitlab::Import::Logger.build + end end end end diff --git a/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb b/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb index 75b5d31d134..145e42e2a51 100644 --- a/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb +++ b/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb @@ -3,76 +3,27 @@ require 'spec_helper' describe Mutations::ResolvesIssuable do - let(:mutation_class) do + let_it_be(:mutation_class) do Class.new(Mutations::BaseMutation) do include Mutations::ResolvesIssuable end end - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:context) { { current_user: user } } - let(:mutation) { mutation_class.new(object: nil, context: context, field: nil) } - - shared_examples 'resolving an issuable' do |type| - context 'when user has access' do - let(:source) { type == :merge_request ? 'source_project' : 'project' } - let(:issuable) { create(type, author: user, "#{source}" => project) } - - subject { mutation.resolve_issuable(type: type, parent_path: project.full_path, iid: issuable.iid) } - - before do - project.add_developer(user) - end - - it 'resolves issuable by iid' do - result = type == :merge_request ? subject.sync : subject - expect(result).to eq(issuable) - end - - it 'uses the correct Resolver to resolve issuable' do - resolver_class = "Resolvers::#{type.to_s.classify.pluralize}Resolver".constantize - resolved_project = mutation.resolve_project(full_path: project.full_path) - - allow(mutation).to receive(:resolve_project) - .with(full_path: project.full_path) - .and_return(resolved_project) - - expect(resolver_class).to receive(:new) - .with(object: resolved_project, context: context, field: nil) - .and_call_original - - subject - end - - it 'uses the ResolvesProject to resolve project' do - expect(Resolvers::ProjectResolver).to receive(:new) - .with(object: nil, context: context, field: nil) - .and_call_original - - subject - end - - it 'returns nil if issuable is not found' do - result = mutation.resolve_issuable(type: type, parent_path: project.full_path, iid: "100") - result = type == :merge_request ? result.sync : result - - expect(result).to be_nil - end - - it 'returns nil if parent path is empty' do - result = mutation.resolve_issuable(type: type, parent_path: "", iid: issuable.iid) - - expect(result).to be_nil - end - end - end + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:context) { { current_user: user } } + let_it_be(:mutation) { mutation_class.new(object: nil, context: context, field: nil) } + let(:parent) { issuable.project } context 'with issues' do - it_behaves_like 'resolving an issuable', :issue + let(:issuable) { create(:issue, project: project) } + + it_behaves_like 'resolving an issuable in GraphQL', :issue end context 'with merge requests' do - it_behaves_like 'resolving an issuable', :merge_request + let(:issuable) { create(:merge_request, source_project: project) } + + it_behaves_like 'resolving an issuable in GraphQL', :merge_request end end diff --git a/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb new file mode 100644 index 00000000000..8603eb73bd5 --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigrateUsersBioToUserDetails, :migration, schema: 20200323074147 do + let(:users) { table(:users) } + + let(:user_details) do + klass = table(:user_details) + klass.primary_key = :user_id + klass + end + + let!(:user_needs_migration) { users.create(name: 'user1', email: 'test1@test.com', projects_limit: 1, bio: 'bio') } + let!(:user_needs_no_migration) { users.create(name: 'user2', email: 'test2@test.com', projects_limit: 1) } + let!(:user_also_needs_no_migration) { users.create(name: 'user3', email: 'test3@test.com', projects_limit: 1, bio: '') } + let!(:user_with_long_bio) { users.create(name: 'user4', email: 'test4@test.com', projects_limit: 1, bio: 'a' * 256) } # 255 is the max + + let!(:user_already_has_details) { users.create(name: 'user5', email: 'test5@test.com', projects_limit: 1, bio: 'my bio') } + let!(:existing_user_details) { user_details.find_or_create_by(user_id: user_already_has_details.id).update(bio: 'my bio') } + + # unlikely scenario since we have triggers + let!(:user_has_different_details) { users.create(name: 'user6', email: 'test6@test.com', projects_limit: 1, bio: 'different') } + let!(:different_existing_user_details) { user_details.find_or_create_by(user_id: user_has_different_details.id).update(bio: 'bio') } + + let(:user_ids) do + [ + user_needs_migration, + user_needs_no_migration, + user_also_needs_no_migration, + user_with_long_bio, + user_already_has_details, + user_has_different_details + ].map(&:id) + end + + subject { described_class.new.perform(user_ids.min, user_ids.max) } + + it 'migrates all relevant records' do + subject + + all_user_details = user_details.all + expect(all_user_details.size).to eq(4) + end + + it 'migrates `bio`' do + subject + + user_detail = user_details.find_by!(user_id: user_needs_migration.id) + + expect(user_detail.bio).to eq('bio') + end + + it 'migrates long `bio`' do + subject + + user_detail = user_details.find_by!(user_id: user_with_long_bio.id) + + expect(user_detail.bio).to eq('a' * 255) + end + + it 'does not change existing user detail' do + expect { subject }.not_to change { user_details.find_by!(user_id: user_already_has_details.id).attributes } + end + + it 'changes existing user detail when the columns are different' do + expect { subject }.to change { user_details.find_by!(user_id: user_has_different_details.id).bio }.from('bio').to('different') + end + + it 'does not migrate record' do + subject + + user_detail = user_details.find_by(user_id: user_needs_no_migration.id) + + expect(user_detail).to be_nil + end + + it 'does not migrate empty bio' do + subject + + user_detail = user_details.find_by(user_id: user_also_needs_no_migration.id) + + expect(user_detail).to be_nil + end + + context 'when `migrate_bio_to_user_details` feature flag is off' do + before do + stub_feature_flags(migrate_bio_to_user_details: false) + end + + it 'does nothing' do + already_existing_user_details = user_details.where(user_id: [ + user_has_different_details.id, + user_already_has_details.id + ]) + + subject + + expect(user_details.all).to match_array(already_existing_user_details) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 9ac2660908c..8b765ce122d 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -657,6 +657,30 @@ describe Gitlab::Database::MigrationHelpers do end end + context 'when `update_column_in_batches_args` is given' do + let(:column) { UserDetail.columns.find { |c| c.name == "user_id" } } + + it 'uses `user_id` for `update_column_in_batches`' do + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + allow(model).to receive(:column_for).with(:user_details, :foo).and_return(column) + allow(model).to receive(:update_column_in_batches).with(:user_details, :foo, 10, batch_column_name: :user_id) + allow(model).to receive(:change_column_null).with(:user_details, :foo, false) + allow(model).to receive(:change_column_default).with(:user_details, :foo, 10) + + expect(model).to receive(:add_column) + .with(:user_details, :foo, :integer, default: nil) + + model.add_column_with_default( + :user_details, + :foo, + :integer, + default: 10, + update_column_in_batches_args: { batch_column_name: :user_id } + ) + end + end + context 'when a column limit is set' do it 'adds the column with a limit' do allow(model).to receive(:transaction_open?).and_return(false) diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 7e2b5ed534f..61e893bfb3c 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -78,6 +78,66 @@ describe Gitlab::ImportExport::MembersMapper do members_mapper.map end + context 'logging' do + let(:logger) { Gitlab::Import::Logger.build } + + before do + allow(logger).to receive(:info) + allow(members_mapper).to receive(:logger).and_return(logger) + end + + it 'logs member addition' do + expected_log_params = ->(user_id) do + { + user_id: user_id, + root_namespace_id: importable.root_ancestor.id, + importable_type: importable.class.to_s, + importable_id: importable.id, + access_level: exported_members.first['access_level'], + message: '[Project/Group Import] Added new member' + } + end + + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once + + members_mapper.map + end + + context 'when exporter member is invalid' do + let(:exported_members) do + [ + { + "id" => 2, + "access_level" => -5, # invalid access level + "source_id" => 14, + "source_type" => source_type, + "notification_level" => 3, + "created_at" => "2016-03-11T10:21:44.822Z", + "updated_at" => "2016-03-11T10:21:44.822Z", + "created_by_id" => nil, + "invite_email" => nil, + "invite_token" => nil, + "invite_accepted_at" => nil, + "user" => + { + "id" => exported_user_id, + "email" => user2.email, + "username" => 'test' + }, + "user_id" => 19 + } + ] + end + + it 'logs member addition failure' do + expect(logger).to receive(:info).with(hash_including(message: a_string_including('Access level is not included in the list'))).once + + members_mapper.map + end + end + end + context 'user is not an admin' do let(:user) { create(:user) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index ce3ee3fcfb0..13a0426624b 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -586,49 +586,97 @@ describe Member do end context 'when after_commit :update_highest_role' do - where(:member_type, :source_type) do - :project_member | :project - :group_member | :group - end + context 'with feature flag enabled' do + where(:member_type, :source_type) do + :project_member | :project + :group_member | :group + end - with_them do - describe 'create member' do - it 'initializes a new Members::UpdateHighestRoleService object' do - source = create(source_type) # source owner initializes a new service object too - user = create(:user) + with_them do + describe 'create member' do + it 'initializes a new Members::UpdateHighestRoleService object' do + source = create(source_type) # source owner initializes a new service object too + user = create(:user) - expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original + expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original - create(member_type, :guest, user: user, source_type => source) + create(member_type, :guest, user: user, source_type => source) + end end - end - context 'when member exists' do - let!(:member) { create(member_type) } + context 'when member exists' do + let!(:member) { create(member_type) } + + describe 'update member' do + context 'when access level was changed' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original + + member.update(access_level: Gitlab::Access::GUEST) + end + end + + context 'when access level was not changed' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + + member.update(notification_level: NotificationSetting.levels[:disabled]) + end + end + end - describe 'update member' do - context 'when access level was changed' do + describe 'destroy member' do it 'initializes a new Members::UpdateHighestRoleService object' do expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original - member.update(access_level: Gitlab::Access::GUEST) + member.destroy end end + end + end + end - context 'when access level was not changed' do - it 'does not initialize a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + context 'with feature flag disabled' do + before do + stub_feature_flags(highest_role_callback: false) + end - member.update(notification_level: NotificationSetting.levels[:disabled]) - end + where(:member_type, :source_type) do + :project_member | :project + :group_member | :group + end + + with_them do + describe 'create member' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + source = create(source_type) + user = create(:user) + + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(user.id) + + create(member_type, :guest, user: user, source_type => source) end end - describe 'destroy member' do - it 'initializes a new Members::UpdateHighestRoleService object' do - expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original + context 'when member exists' do + let!(:member) { create(member_type) } - member.destroy + describe 'update member' do + context 'when access level was changed' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + + member.update(access_level: Gitlab::Access::GUEST) + end + end + end + + describe 'destroy member' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + + member.destroy + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bbd45afadd7..b7abcb31321 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -55,6 +55,67 @@ describe User, :do_not_mock_admin_mode do it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } it { is_expected.to have_many(:releases).dependent(:nullify) } + describe "#bio" do + it 'syncs bio with `user_details.bio` on create' do + user = create(:user, bio: 'my bio') + + expect(user.bio).to eq(user.user_detail.bio) + end + + context 'when `migrate_bio_to_user_details` feature flag is off' do + before do + stub_feature_flags(migrate_bio_to_user_details: false) + end + + it 'does not sync bio with `user_details.bio`' do + user = create(:user, bio: 'my bio') + + expect(user.bio).to eq('my bio') + expect(user.user_detail.bio).to eq('') + end + end + + it 'syncs bio with `user_details.bio` on update' do + user = create(:user) + + user.update!(bio: 'my bio') + + expect(user.bio).to eq(user.user_detail.bio) + end + + context 'when `user_details` association already exists' do + let(:user) { create(:user) } + + before do + create(:user_detail, user: user) + end + + it 'syncs bio with `user_details.bio`' do + user.update!(bio: 'my bio') + + expect(user.bio).to eq(user.user_detail.bio) + end + + it 'falls back to "" when nil is given' do + user.update!(bio: nil) + + expect(user.bio).to eq(nil) + expect(user.user_detail.bio).to eq('') + end + + # very unlikely scenario + it 'truncates long bio when syncing to user_details' do + invalid_bio = 'a' * 256 + truncated_bio = 'a' * 255 + + user.bio = invalid_bio + user.save(validate: false) + + expect(user.user_detail.bio).to eq(truncated_bio) + end + end + end + describe "#abuse_report" do let(:current_user) { create(:user) } let(:other_user) { create(:user) } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 6d1b76a9aea..92cc6dc887e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -739,6 +739,17 @@ describe API::Users, :do_not_mock_admin_mode do expect(user.reload.bio).to eq('new test bio') end + it "updates user with empty bio" do + user.bio = 'previous bio' + user.save! + + put api("/users/#{user.id}", admin), params: { bio: '' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['bio']).to eq('') + expect(user.reload.bio).to eq('') + end + it "updates user with new password and forces reset on next login" do put api("/users/#{user.id}", admin), params: { password: '12345678' } diff --git a/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb b/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb new file mode 100644 index 00000000000..b56181371c3 --- /dev/null +++ b/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'resolving an issuable in GraphQL' do |type| + subject { mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: issuable.iid) } + + context 'when user has access' do + before do + parent.add_developer(user) + end + + it 'resolves issuable by iid' do + result = type == :merge_request ? subject.sync : subject + expect(result).to eq(issuable) + end + + it 'uses the correct Resolver to resolve issuable' do + resolver_class = "Resolvers::#{type.to_s.classify.pluralize}Resolver".constantize + resolve_method = type == :epic ? :resolve_group : :resolve_project + resolved_parent = mutation.send(resolve_method, full_path: parent.full_path) + + allow(mutation).to receive(resolve_method) + .with(full_path: parent.full_path) + .and_return(resolved_parent) + + expect(resolver_class).to receive(:new) + .with(object: resolved_parent, context: context, field: nil) + .and_call_original + + subject + end + + it 'uses correct Resolver to resolve issuable parent' do + resolver_class = type == :epic ? 'Resolvers::GroupResolver' : 'Resolvers::ProjectResolver' + + expect(resolver_class.constantize).to receive(:new) + .with(object: nil, context: context, field: nil) + .and_call_original + + subject + end + + it 'returns nil if issuable is not found' do + result = mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: "100") + result = type == :merge_request ? result.sync : result + + expect(result).to be_nil + end + + it 'returns nil if parent path is not present' do + result = mutation.resolve_issuable(type: type, parent_path: "", iid: issuable.iid) + + expect(result).to be_nil + end + end +end |