diff options
22 files changed, 108 insertions, 80 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index f04d527d2fc..df95e933bb6 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -ac315ef254b265af5323387a1df1abb5d57daa95 +3e9cc60933881ce8c33c379c72dad3907755aac2 diff --git a/app/assets/stylesheets/components/batch_comments/review_bar.scss b/app/assets/stylesheets/components/batch_comments/review_bar.scss index 6f5c5c5a080..5e1128dc4ce 100644 --- a/app/assets/stylesheets/components/batch_comments/review_bar.scss +++ b/app/assets/stylesheets/components/batch_comments/review_bar.scss @@ -11,7 +11,7 @@ padding-right: $gutter_collapsed_width; background: $white; border-top: 1px solid $border-color; - transition: padding $sidebar-transition-duration; + transition: padding $gl-transition-duration-medium; .page-with-icon-sidebar & { padding-left: $contextual-sidebar-collapsed-width; diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss index a5c5ca85eab..ad0036df607 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar.scss @@ -204,7 +204,7 @@ // .page-with-contextual-sidebar { - transition: padding-left $sidebar-transition-duration; + transition: padding-left $gl-transition-duration-medium; @include media-breakpoint-up(md) { padding-left: $contextual-sidebar-collapsed-width; @@ -233,7 +233,7 @@ @include gl-fixed; @include gl-bottom-0; @include gl-left-0; - transition: width $sidebar-transition-duration, left $sidebar-transition-duration; + transition: width $gl-transition-duration-medium, left $gl-transition-duration-medium; z-index: 600; width: $contextual-sidebar-width; top: $header-height; diff --git a/app/assets/stylesheets/framework/contextual_sidebar_header.scss b/app/assets/stylesheets/framework/contextual_sidebar_header.scss index 57fe80ff3e1..a3d752dcc3d 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar_header.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar_header.scss @@ -5,7 +5,7 @@ > a, > button { - transition: padding $sidebar-transition-duration; + transition: padding $gl-transition-duration-medium; font-weight: $gl-font-weight-bold; display: flex; width: 100%; diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index 2cea3b96ff7..47856f1a0d3 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -478,7 +478,7 @@ } @mixin side-panel-toggle { - transition: width $sidebar-transition-duration; + transition: width $gl-transition-duration-medium; height: $toggle-sidebar-height; padding: 0 $gl-padding; background-color: $gray-light; diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 13201d43fd0..ae0f18753ad 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -13,7 +13,7 @@ } .page-initialised .content-wrapper { - transition: padding $sidebar-transition-duration; + transition: padding $gl-transition-duration-medium; } .right-sidebar-collapsed { @@ -109,7 +109,7 @@ @include maintain-sidebar-dimensions; width: 0; padding: 0; - transition: width $sidebar-transition-duration; + transition: width $gl-transition-duration-medium; &.right-sidebar-expanded { @include maintain-sidebar-dimensions; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 68fdfae6713..e9ad930ef2b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -5,7 +5,6 @@ $grid-size: 8px; $gutter-collapsed-width: 62px; $gutter-width: 290px; $gutter-inner-width: 250px; -$sidebar-transition-duration: 0.3s; $sidebar-breakpoint: 1024px; $default-transition-duration: 0.15s; $contextual-sidebar-width: 256px; diff --git a/app/assets/stylesheets/framework/vue_transitions.scss b/app/assets/stylesheets/framework/vue_transitions.scss index 1a536b97142..e3ac615234c 100644 --- a/app/assets/stylesheets/framework/vue_transitions.scss +++ b/app/assets/stylesheets/framework/vue_transitions.scss @@ -2,7 +2,7 @@ .fade-leave-active, .fade-in-enter-active, .fade-out-leave-active { - transition: opacity $sidebar-transition-duration $general-hover-transition-curve; + transition: opacity $gl-transition-duration-medium $general-hover-transition-curve; } .fade-enter, diff --git a/app/assets/stylesheets/page_bundles/boards.scss b/app/assets/stylesheets/page_bundles/boards.scss index 81d35b8bc7b..197073412e8 100644 --- a/app/assets/stylesheets/page_bundles/boards.scss +++ b/app/assets/stylesheets/page_bundles/boards.scss @@ -35,7 +35,7 @@ .boards-app { @include media-breakpoint-up(sm) { - transition: width $sidebar-transition-duration; + transition: width $gl-transition-duration-medium; width: 100%; &.is-compact { @@ -349,7 +349,7 @@ .right-sidebar.right-sidebar-expanded { &.boards-sidebar-slide-enter-active, &.boards-sidebar-slide-leave-active { - transition: width $sidebar-transition-duration, padding $sidebar-transition-duration; + transition: width $gl-transition-duration-medium, padding $gl-transition-duration-medium; } &.boards-sidebar-slide-enter, diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 51f964a4b70..69797c6b303 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -132,7 +132,7 @@ // stylelint-disable-next-line length-zero-no-unit bottom: var(--review-bar-height, 0px); right: 0; - transition: width $sidebar-transition-duration; + transition: width $gl-transition-duration-medium; background-color: $white; z-index: 200; overflow: hidden; diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 45decccfc36..bc261a95e03 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -25,7 +25,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController # the number of failed sign in attempts def failure if params[:username].present? && AuthHelper.form_based_provider?(failed_strategy.name) - user = User.by_login(params[:username]) + user = User.find_by_login(params[:username]) user&.increment_failed_attempts! log_failed_login(params[:username], failed_strategy.name) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6195d152f00..fe3b8d9b8b4 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -215,11 +215,11 @@ class SessionsController < Devise::SessionsController def find_user strong_memoize(:find_user) do if session[:otp_user_id] && user_params[:login] - User.by_id_and_login(session[:otp_user_id], user_params[:login]).first + User.by_login(user_params[:login]).find_by_id(session[:otp_user_id]) elsif session[:otp_user_id] User.find(session[:otp_user_id]) elsif user_params[:login] - User.by_login(user_params[:login]) + User.find_by_login(user_params[:login]) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 5503f28a4fe..4a89f414660 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -448,6 +448,11 @@ class User < ApplicationRecord scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) } scope :by_username, -> (usernames) { iwhere(username: Array(usernames).map(&:to_s)) } scope :by_name, -> (names) { iwhere(name: Array(names)) } + scope :by_login, -> (login) do + return none if login.blank? + + login.include?('@') ? iwhere(email: login) : iwhere(username: login) + end scope :by_user_email, -> (emails) { iwhere(email: Array(emails)) } scope :by_emails, -> (emails) { joins(:emails).where(emails: { email: Array(emails).map(&:downcase) }) } scope :for_todos, -> (todos) { where(id: todos.select(:user_id).distinct) } @@ -482,7 +487,6 @@ class User < ApplicationRecord scope :order_oldest_sign_in, -> { reorder(arel_table[:current_sign_in_at].asc.nulls_last) } scope :order_recent_last_activity, -> { reorder(arel_table[:last_activity_on].desc.nulls_last, arel_table[:id].asc) } scope :order_oldest_last_activity, -> { reorder(arel_table[:last_activity_on].asc.nulls_first, arel_table[:id].desc) } - scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) } scope :dormant, -> { with_state(:active).human_or_service_user.where('last_activity_on <= ?', MINIMUM_INACTIVE_DAYS.day.ago.to_date) } scope :with_no_activity, -> { with_state(:active).human_or_service_user.where(last_activity_on: nil).where('created_at <= ?', MINIMUM_DAYS_CREATED.day.ago.to_date) } scope :by_provider_and_extern_uid, ->(provider, extern_uid) { joins(:identities).merge(Identity.with_extern_uid(provider, extern_uid)) } @@ -765,14 +769,8 @@ class User < ApplicationRecord true end - def by_login(login) - return unless login - - if login.include?('@') - unscoped.iwhere(email: login).take - else - unscoped.iwhere(username: login).take - end + def find_by_login(login) + by_login(login).take end def find_by_username(username) diff --git a/doc/administration/geo/replication/datatypes.md b/doc/administration/geo/replication/datatypes.md index 1481d79b81a..a616760401e 100644 --- a/doc/administration/geo/replication/datatypes.md +++ b/doc/administration/geo/replication/datatypes.md @@ -57,6 +57,8 @@ verification methods: | Blobs | Pipeline artifacts _(object storage)_ | Geo with API/Managed (*2*) | _Not implemented_ | | Blobs | Pages _(file system)_ | Geo with API | SHA256 checksum | | Blobs | Pages _(object storage)_ | Geo with API/Managed (*2*) | _Not implemented_ | +| Blobs | CI Secure Files _(file system)_ | Geo with API | SHA256 checksum | +| Blobs | CI Secure Files _(object storage)_ | Geo with API/Managed (*2*) | _Not implemented_ | - (*1*): Redis replication can be used as part of HA with Redis sentinel. It's not used between Geo sites. - (*2*): Object storage replication can be performed by Geo or by your object storage provider/appliance @@ -194,6 +196,7 @@ successfully, you must replicate their data using some other means. |[Project snippets](../../../user/snippets.md) | **Yes** (10.2) | **Yes** (10.2) | N/A | N/A | | |[CI job artifacts](../../../ci/pipelines/job_artifacts.md) | **Yes** (10.4) | **Yes** (14.10) | [**Yes** (15.1)](https://gitlab.com/groups/gitlab-org/-/epics/5551) | [No](object_storage.md#verification-of-files-in-object-storage) | Verification is behind the feature flag `geo_job_artifact_replication`, enabled by default in 14.10. | |[CI Pipeline Artifacts](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/ci/pipeline_artifact.rb) | [**Yes** (13.11)](https://gitlab.com/gitlab-org/gitlab/-/issues/238464) | [**Yes** (13.11)](https://gitlab.com/gitlab-org/gitlab/-/issues/238464) | [**Yes** (15.1)](https://gitlab.com/groups/gitlab-org/-/epics/5551) | [No](object_storage.md#verification-of-files-in-object-storage) | Persists additional artifacts after a pipeline completes. | +|[CI Secure Files](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/ci/secure_file.rb) | [**Yes** (15.3)](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91430) | [**Yes** (15.3)](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91430) | [**Yes** (15.3)](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91430) | [No](object_storage.md#verification-of-files-in-object-storage) | Verification is behind the feature flag `geo_ci_secure_file_replication`, enabled by default in 15.3. | |[Container Registry](../../packages/container_registry.md) | **Yes** (12.3) | No | No | No | Disabled by default. See [instructions](container_registry.md) to enable. | |[Infrastructure Registry](../../../user/packages/infrastructure_registry/index.md) | **Yes** (14.0) | **Yes** (14.0) | [**Yes** (15.1)](https://gitlab.com/groups/gitlab-org/-/epics/5551) | [No](object_storage.md#verification-of-files-in-object-storage) | Behind feature flag `geo_package_file_replication`, enabled by default. | |[Project designs repository](../../../user/project/issues/design_management.md) | **Yes** (12.7) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/32467) | N/A | N/A | Designs also require replication of LFS objects and Uploads. | diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4e021da238d..570f35839e1 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11891,7 +11891,7 @@ Represents an external issue. ##### `GeoNode.ciSecureFileRegistries` -Find Ci Secure File registries on this Geo node Available only when feature flag `geo_ci_secure_file_replication` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice. +Find Ci Secure File registries on this Geo node. Returns [`CiSecureFileRegistryConnection`](#cisecurefileregistryconnection). diff --git a/doc/ci/environments/protected_environments.md b/doc/ci/environments/protected_environments.md index 0f1d1deb1dc..ef762979900 100644 --- a/doc/ci/environments/protected_environments.md +++ b/doc/ci/environments/protected_environments.md @@ -189,11 +189,14 @@ and are protected at the same time. ### Configure group-level memberships +> - Operators are required to have Owner+ role from the original Maintainer+ role and this role change is introduced from GitLab 15.3 [with a flag](https://gitlab.com/gitlab-org/gitlab/-/issues/369873) named `group_level_protected_environment_settings_permission`. Disabled by default. +> - Original behavior where Operators are required to have Maintainer+ role can be achieved by enabling [flag](https://gitlab.com/gitlab-org/gitlab/-/issues/369875) named `override_group_level_protected_environment_settings_permission`. Disabled by default. + To maximize the effectiveness of group-level protected environments, [group-level memberships](../../user/group/index.md) must be correctly configured: -- Operators should be given at least the Maintainer role +- Operators should be given at least the Owner role for the top-level group. They can maintain CI/CD configurations for the higher environments (such as production) in the group-level settings page, which includes group-level protected environments, @@ -203,7 +206,7 @@ configured: This ensures that only operators can configure the organization-wide deployment ruleset. - Developers should be given no more than the Developer role - for the top-level group, or explicitly given the Maintainer role for a child project + for the top-level group, or explicitly given the Owner role for a child project They do *not* have access to the CI/CD configurations in the top-level group, so operators can ensure that the critical configuration won't be accidentally changed by the developers. diff --git a/doc/development/code_review.md b/doc/development/code_review.md index de20bcdc059..8aa3987bb53 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -62,6 +62,9 @@ Team members' domain expertise can be viewed on the [engineering projects](https ### Reviewer roulette +NOTE: +Reviewer roulette is an internal tool for use on GitLab.com, and not available for use on customer installations. + The [Danger bot](dangerbot.md) randomly picks a reviewer and a maintainer for each area of the codebase that your merge request seems to touch. It only makes **recommendations** and you should override it if you think someone else is a better @@ -487,7 +490,7 @@ Before taking the decision to merge: - If the MR contains both Quality and non-Quality-related changes, the MR should be merged by the relevant maintainer for user-facing changes (backend, frontend, or database) after the Quality related changes are approved by a Software Engineer in Test. If a merge request is fundamentally ready, but needs only trivial fixes (such as -typos), consider demonstrating a [bias for action](https://about.gitlab.com/handbook/values/#bias-for-action) +typos), consider demonstrating a [bias for action](https://about.gitlab.com/handbook/values/#bias-for-action) by making those changes directly without going back to the author. You can do this by using the [suggest changes](../user/project/merge_requests/reviews/suggestions.md) feature to apply your own suggestions to the merge request. Note that: diff --git a/doc/user/application_security/iac_scanning/index.md b/doc/user/application_security/iac_scanning/index.md index a14bc34550d..16f08de738b 100644 --- a/doc/user/application_security/iac_scanning/index.md +++ b/doc/user/application_security/iac_scanning/index.md @@ -64,7 +64,7 @@ variables: SAST_IMAGE_SUFFIX: '-fips' include: - - template: Security/SAST-IaC.latest.gitlab-ci.yml + - template: Jobs/SAST-IaC.gitlab-ci.yml ``` ### Making IaC analyzers available to all GitLab tiers @@ -98,11 +98,11 @@ To configure IaC Scanning for a project you can: ### Configure IaC Scanning manually To enable IaC Scanning you must [include](../../../ci/yaml/index.md#includetemplate) the -[`SAST-IaC.latest.gitlab-ci.yml template`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/SAST-IaC.latest.gitlab-ci.yml) provided as part of your GitLab installation. Here is an example of how to include it: +[`SAST-IaC.gitlab-ci.yml template`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/SAST-IaC.gitlab-ci.yml) provided as part of your GitLab installation. Here is an example of how to include it: ```yaml include: - - template: Security/SAST-IaC.latest.gitlab-ci.yml + - template: Jobs/SAST-IaC.gitlab-ci.yml ``` The included template creates IaC scanning jobs in your CI/CD pipeline and scans diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 6c3487c28ea..6213dd203c4 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -92,7 +92,7 @@ module Gitlab return unless authenticate_using_internal_or_ldap_password? Gitlab::Auth::UniqueIpsLimiter.limit_user! do - user = User.by_login(login) + user = User.find_by_login(login) break if user && !user.can_log_in_with_non_expired_password? @@ -279,7 +279,7 @@ module Gitlab if deploy_key_matches DeployKey.find(deploy_key_matches[1]) else - User.by_login(login) + User.find_by_login(login) end return unless actor diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index 7adaaef86e4..c994f179b66 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -103,7 +103,7 @@ module Gitlab return unless has_basic_credentials?(current_request) login, token = user_name_and_password(current_request) - user = User.by_login(login) + user = User.find_by_login(login) user if user && Gitlab::LfsToken.new(user).token_valid?(token) end diff --git a/spec/frontend/clusters/clusters_bundle_spec.js b/spec/frontend/clusters/clusters_bundle_spec.js index f7aee02d7ff..ad2aa4acbaf 100644 --- a/spec/frontend/clusters/clusters_bundle_spec.js +++ b/spec/frontend/clusters/clusters_bundle_spec.js @@ -57,9 +57,9 @@ describe('Clusters', () => { it('should show the creating container', () => { cluster.updateContainer(null, 'creating'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeFalsy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(false); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(true); expect(window.location.reload).not.toHaveBeenCalled(); }); @@ -67,9 +67,9 @@ describe('Clusters', () => { cluster.updateContainer(null, 'creating'); cluster.updateContainer('creating', 'creating'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeFalsy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(false); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(true); expect(window.location.reload).not.toHaveBeenCalled(); }); }); @@ -80,9 +80,9 @@ describe('Clusters', () => { cluster.updateContainer(null, 'creating'); cluster.updateContainer('creating', 'created'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(true); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(true); expect(window.location.reload).toHaveBeenCalled(); expect(cluster.setClusterNewlyCreated).toHaveBeenCalledWith(true); }); @@ -94,9 +94,9 @@ describe('Clusters', () => { cluster.updateContainer(null, 'created'); cluster.updateContainer('created', 'created'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeFalsy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(true); + expect(cluster.successContainer.classList.contains('hidden')).toBe(false); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(true); expect(window.location.reload).not.toHaveBeenCalled(); expect(cluster.setClusterNewlyCreated).toHaveBeenCalledWith(false); }); @@ -108,9 +108,9 @@ describe('Clusters', () => { cluster.updateContainer(null, 'created'); cluster.updateContainer('created', 'created'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(true); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(true); expect(window.location.reload).not.toHaveBeenCalled(); expect(cluster.setClusterNewlyCreated).not.toHaveBeenCalled(); }); @@ -120,11 +120,11 @@ describe('Clusters', () => { it('should show the error container', () => { cluster.updateContainer(null, 'errored', 'this is an error'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(true); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); - expect(cluster.errorContainer.classList.contains('hidden')).toBeFalsy(); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(false); expect(cluster.errorReasonContainer.textContent).toContain('this is an error'); }); @@ -132,11 +132,11 @@ describe('Clusters', () => { it('should show `error` banner when previously `creating`', () => { cluster.updateContainer('creating', 'errored'); - expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.creatingContainer.classList.contains('hidden')).toBe(true); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.successContainer.classList.contains('hidden')).toBe(true); - expect(cluster.errorContainer.classList.contains('hidden')).toBeFalsy(); + expect(cluster.errorContainer.classList.contains('hidden')).toBe(false); }); }); diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 01b6b36db77..e107cc15b9d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1083,20 +1083,6 @@ RSpec.describe User do end end - describe '.by_id_and_login' do - let_it_be(:user) { create(:user) } - - it 'finds a user regardless of case' do - expect(described_class.by_id_and_login(user.id, user.username.upcase)) - .to contain_exactly(user) - end - - it 'finds a user when login is an email address regardless of case' do - expect(described_class.by_id_and_login(user.id, user.email.upcase)) - .to contain_exactly(user) - end - end - describe '.for_todos' do let_it_be(:user1) { create(:user) } let_it_be(:user2) { create(:user) } @@ -3003,17 +2989,53 @@ RSpec.describe User do end end + shared_examples "find user by login" do + let_it_be(:user) { create(:user) } + let_it_be(:invalid_login) { "#{user.username}-NOT-EXISTS" } + + context 'when login is nil or empty' do + it 'returns nil' do + expect(login_method(nil)).to be_nil + expect(login_method('')).to be_nil + end + end + + context 'when login is invalid' do + it 'returns nil' do + expect(login_method(invalid_login)).to be_nil + end + end + + context 'when login is username' do + it 'returns user' do + expect(login_method(user.username)).to eq(user) + expect(login_method(user.username.downcase)).to eq(user) + expect(login_method(user.username.upcase)).to eq(user) + end + end + + context 'when login is email' do + it 'returns user' do + expect(login_method(user.email)).to eq(user) + expect(login_method(user.email.downcase)).to eq(user) + expect(login_method(user.email.upcase)).to eq(user) + end + end + end + describe '.by_login' do - let(:username) { 'John' } - let!(:user) { create(:user, username: username) } - - it 'gets the correct user' do - expect(described_class.by_login(user.email.upcase)).to eq user - expect(described_class.by_login(user.email)).to eq user - expect(described_class.by_login(username.downcase)).to eq user - expect(described_class.by_login(username)).to eq user - expect(described_class.by_login(nil)).to be_nil - expect(described_class.by_login('')).to be_nil + it_behaves_like "find user by login" do + def login_method(login) + described_class.by_login(login).take + end + end + end + + describe '.find_by_login' do + it_behaves_like "find user by login" do + def login_method(login) + described_class.find_by_login(login) + end end end |