diff options
45 files changed, 624 insertions, 517 deletions
diff --git a/.gitlab/merge_request_templates/Documentation.md b/.gitlab/merge_request_templates/Documentation.md index 5db1c719aea..b7c21ed3b67 100644 --- a/.gitlab/merge_request_templates/Documentation.md +++ b/.gitlab/merge_request_templates/Documentation.md @@ -33,6 +33,7 @@ Documentation-related MRs should be reviewed by a Technical Writer for a non-blo - [ ] The headings should be something you'd do a Google search for. Instead of `Default behavior`, say something like `Default behavior when you close an issue`. - [ ] The headings (other than the page title) should be active. Instead of `Configuring GDK`, say something like `Configure GDK`. - [ ] Any task steps should be written as a numbered list. + - If the content still needs to be edited for topic types, you can create a follow-up issue with the ~"docs-technical-debt" label. - [ ] Review by assigned maintainer, who can always request/require the above reviews. Maintainer's review can occur before or after a technical writer review. - [ ] Ensure a release milestone is set. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c59167abffb..cd169869e27 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -b6648dea882b755e747d7d6be7a2355a9f28b6ae +28e6eebf54f40a2ff820c996698c720b1a898266 @@ -124,7 +124,7 @@ gem 'fog-aws', '~> 3.9' # Locked until fog-google resolves https://github.com/fog/fog-google/issues/421. # Also see config/initializers/fog_core_patch.rb. gem 'fog-core', '= 2.1.0' -gem 'gitlab-fog-google', '~> 1.13', require: 'fog/google' +gem 'fog-google', '~> 1.15', require: 'fog/google' gem 'fog-local', '~> 0.6' gem 'fog-openstack', '~> 1.0' gem 'fog-rackspace', '~> 0.1.1' diff --git a/Gemfile.lock b/Gemfile.lock index ac113252f85..d7c3651a455 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -412,6 +412,12 @@ GEM excon (~> 0.58) formatador (~> 0.2) mime-types + fog-google (1.15.0) + fog-core (<= 2.1.0) + fog-json (~> 1.2) + fog-xml (~> 0.1.0) + google-api-client (>= 0.44.2, < 0.51) + google-cloud-env (~> 1.2) fog-json (1.2.0) fog-core multi_json (~> 1.10) @@ -475,13 +481,6 @@ GEM fog-json (~> 1.2.0) mime-types ms_rest_azure (~> 0.12.0) - gitlab-fog-google (1.13.0) - addressable (>= 2.7.0) - fog-core (<= 2.1.0) - fog-json (~> 1.2) - fog-xml (~> 0.1.0) - google-api-client (>= 0.44.2, < 0.51) - google-cloud-env (~> 1.2) gitlab-labkit (0.18.0) actionpack (>= 5.0.0, < 7.0.0) activesupport (>= 5.0.0, < 7.0.0) @@ -531,7 +530,7 @@ GEM retriable (>= 2.0, < 4.0) rexml signet (~> 0.12) - google-cloud-env (1.4.0) + google-cloud-env (1.5.0) faraday (>= 0.17.3, < 2.0) google-protobuf (3.17.1) googleapis-common-protos-types (1.0.6) @@ -1474,6 +1473,7 @@ DEPENDENCIES fog-aliyun (~> 0.3) fog-aws (~> 3.9) fog-core (= 2.1.0) + fog-google (~> 1.15) fog-local (~> 0.6) fog-openstack (~> 1.0) fog-rackspace (~> 0.1.1) @@ -1489,7 +1489,6 @@ DEPENDENCIES gitlab-dangerfiles (~> 2.1.2) gitlab-experiment (~> 0.5.4) gitlab-fog-azure-rm (~> 1.1.1) - gitlab-fog-google (~> 1.13) gitlab-labkit (~> 0.18.0) gitlab-license (~> 1.5) gitlab-mail_room (~> 0.0.9) diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue index 0b0b4fa97ae..87af062c535 100644 --- a/app/assets/javascripts/diffs/components/diff_row.vue +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -155,6 +155,7 @@ export default { }; </script> +<!-- eslint-disable-next-line vue/no-deprecated-functional-template --> <template functional> <div :class="$options.classNameMap(props)" class="diff-grid-row diff-tr line_holder"> <div diff --git a/app/models/group.rb b/app/models/group.rb index e4127b2b2d4..8e6beacf7c3 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -296,7 +296,7 @@ class Group < Namespace end def add_users(users, access_level, current_user: nil, expires_at: nil) - GroupMember.add_users( + Members::Groups::CreatorService.add_users( # rubocop:todo CodeReuse/ServiceClass self, users, access_level, @@ -306,14 +306,13 @@ class Group < Namespace end def add_user(user, access_level, current_user: nil, expires_at: nil, ldap: false) - GroupMember.add_user( - self, - user, - access_level, - current_user: current_user, - expires_at: expires_at, - ldap: ldap - ) + Members::Groups::CreatorService.new(self, # rubocop:todo CodeReuse/ServiceClass + user, + access_level, + current_user: current_user, + expires_at: expires_at, + ldap: ldap) + .execute end def add_guest(user, current_user = nil) diff --git a/app/models/member.rb b/app/models/member.rb index 70041578681..d1225b9fcc5 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -232,140 +232,9 @@ class Member < ApplicationRecord find_by(invite_token: invite_token) end - def add_user(source, user, access_level, existing_members: nil, current_user: nil, expires_at: nil, ldap: false) - # rubocop: disable CodeReuse/ServiceClass - # `user` can be either a User object, User ID or an email to be invited - member = retrieve_member(source, user, existing_members) - access_level = retrieve_access_level(access_level) - - return member unless can_update_member?(current_user, member) - - set_member_attributes( - member, - access_level, - current_user: current_user, - expires_at: expires_at, - ldap: ldap - ) - - if member.request? - ::Members::ApproveAccessRequestService.new( - current_user, - access_level: access_level - ).execute( - member, - skip_authorization: ldap, - skip_log_audit_event: ldap - ) - else - member.save - end - - member - # rubocop: enable CodeReuse/ServiceClass - end - - # Populates the attributes of a member. - # - # This logic resides in a separate method so that EE can extend this logic, - # without having to patch the `add_user` method directly. - def set_member_attributes(member, access_level, current_user: nil, expires_at: nil, ldap: false) - member.attributes = { - created_by: member.created_by || current_user, - access_level: access_level, - expires_at: expires_at - } - end - - def add_users(source, users, access_level, current_user: nil, expires_at: nil) - return [] unless users.present? - - emails, users, existing_members = parse_users_list(source, users) - - self.transaction do - (emails + users).map! do |user| - add_user( - source, - user, - access_level, - existing_members: existing_members, - current_user: current_user, - expires_at: expires_at - ) - end - end - end - - def access_levels - Gitlab::Access.sym_options - end - def valid_email?(email) Devise.email_regexp.match?(email) end - - private - - def parse_users_list(source, list) - emails = [] - user_ids = [] - users = [] - existing_members = {} - - list.each do |item| - case item - when User - users << item - when Integer - user_ids << item - when /\A\d+\Z/ - user_ids << item.to_i - when Devise.email_regexp - emails << item - end - end - - if user_ids.present? - users.concat(User.where(id: user_ids)) - # the below will automatically discard invalid user_ids - existing_members = source.members_and_requesters.where(user_id: user_ids).index_by(&:user_id) - end - - [emails, users, existing_members] - end - - # This method is used to find users that have been entered into the "Add members" field. - # These can be the User objects directly, their IDs, their emails, or new emails to be invited. - def retrieve_user(user) - return user if user.is_a?(User) - - return User.find_by(id: user) if user.is_a?(Integer) - - User.find_by_any_email(user) || user - end - - def retrieve_member(source, user, existing_members) - user = retrieve_user(user) - - if user.is_a?(User) - if existing_members - existing_members[user.id] || source.members.build(user_id: user.id) - else - source.members_and_requesters.find_or_initialize_by(user_id: user.id) - end - else - source.members.build(invite_email: user) - end - end - - def retrieve_access_level(access_level) - access_levels.fetch(access_level) { access_level.to_i } - end - - def can_update_member?(current_user, member) - # There is no current user for bulk actions, in which case anything is allowed - !current_user || current_user.can?(:"update_#{member.type.underscore}", member) - end end def real_source_type diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index c7bc31cde5d..ab044b80133 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -32,10 +32,6 @@ class GroupMember < Member Gitlab::Access.options_with_owner end - def self.access_levels - Gitlab::Access.sym_options_with_owner - end - def self.pluck_user_ids pluck(:user_id) end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 41ecc4cbf01..5040879e177 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -48,7 +48,7 @@ class ProjectMember < Member project_ids.each do |project_id| project = Project.find(project_id) - add_users( + Members::Projects::CreatorService.add_users( # rubocop:todo CodeReuse/ServiceClass project, users, access_level, @@ -80,12 +80,6 @@ class ProjectMember < Member def access_level_roles Gitlab::Access.options end - - private - - def can_update_member?(current_user, member) - super || (member.owner? && member.new_record?) - end end def project diff --git a/app/models/project_team.rb b/app/models/project_team.rb index a85afada901..4586aa2b4b4 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -42,7 +42,7 @@ class ProjectTeam end def add_users(users, access_level, current_user: nil, expires_at: nil) - ProjectMember.add_users( + Members::Projects::CreatorService.add_users( # rubocop:todo CodeReuse/ServiceClass project, users, access_level, @@ -52,13 +52,12 @@ class ProjectTeam end def add_user(user, access_level, current_user: nil, expires_at: nil) - ProjectMember.add_user( - project, - user, - access_level, - current_user: current_user, - expires_at: expires_at - ) + Members::Projects::CreatorService.new(project, # rubocop:todo CodeReuse/ServiceClass + user, + access_level, + current_user: current_user, + expires_at: expires_at) + .execute end # Remove all users from project team diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb new file mode 100644 index 00000000000..f6972f81162 --- /dev/null +++ b/app/services/members/creator_service.rb @@ -0,0 +1,172 @@ +# frozen_string_literal: true + +module Members + # This class serves as more of an app-wide way we add/create members + # All roads to add members should take this path. + class CreatorService + class << self + def parsed_access_level(access_level) + access_levels.fetch(access_level) { access_level.to_i } + end + + def access_levels + raise NotImplementedError + end + + def add_users(source, users, access_level, current_user: nil, expires_at: nil) + return [] unless users.present? + + emails, users, existing_members = parse_users_list(source, users) + + Member.transaction do + (emails + users).map! do |user| + new(source, + user, + access_level, + existing_members: existing_members, + current_user: current_user, + expires_at: expires_at) + .execute + end + end + end + + private + + def parse_users_list(source, list) + emails = [] + user_ids = [] + users = [] + existing_members = {} + + list.each do |item| + case item + when User + users << item + when Integer + user_ids << item + when /\A\d+\Z/ + user_ids << item.to_i + when Devise.email_regexp + emails << item + end + end + + if user_ids.present? + users.concat(User.id_in(user_ids)) + # the below will automatically discard invalid user_ids + existing_members = source.members_and_requesters.where(user_id: user_ids).index_by(&:user_id) # rubocop:todo CodeReuse/ActiveRecord + end + + [emails, users, existing_members] + end + end + + def initialize(source, user, access_level, **args) + @source = source + @user = user + @access_level = self.class.parsed_access_level(access_level) + @args = args + end + + def execute + find_or_build_member + update_member + + member + end + + private + + attr_reader :source, :user, :access_level, :member, :args + + def update_member + return unless can_update_member? + + member.attributes = member_attributes + + if member.request? + approve_request + else + member.save + end + end + + def can_update_member? + # There is no current user for bulk actions, in which case anything is allowed + !current_user # inheriting classes will add more logic + end + + # Populates the attributes of a member. + # + # This logic resides in a separate method so that EE can extend this logic, + # without having to patch the `add_user` method directly. + def member_attributes + { + created_by: member.created_by || current_user, + access_level: access_level, + expires_at: args[:expires_at] + } + end + + def approve_request + ::Members::ApproveAccessRequestService.new(current_user, + access_level: access_level) + .execute( + member, + skip_authorization: ldap, + skip_log_audit_event: ldap + ) + end + + def current_user + args[:current_user] + end + + def find_or_build_member + @user = parse_user_param + + @member = if user.is_a?(User) + find_or_initialize_member_by_user + else + source.members.build(invite_email: user) + end + end + + # This method is used to find users that have been entered into the "Add members" field. + # These can be the User objects directly, their IDs, their emails, or new emails to be invited. + def parse_user_param + case user + when User + user + when Integer + # might not return anything - this needs enhancement + User.find_by(id: user) # rubocop:todo CodeReuse/ActiveRecord + else + # must be an email or at least we'll consider it one + User.find_by_any_email(user) || user + end + end + + def find_or_initialize_member_by_user + if existing_members + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/334062 + # i'm not so sure this is needed as the parse_users_list looks at members_and_requesters... + # so it is like we could just do a find or initialize by here and be fine + existing_members[user.id] || source.members.build(user_id: user.id) + else + source.members_and_requesters.find_or_initialize_by(user_id: user.id) # rubocop:todo CodeReuse/ActiveRecord + end + end + + def existing_members + args[:existing_members] + end + + def ldap + args[:ldap] || false + end + end +end + +Members::CreatorService.prepend_mod_with('Members::CreatorService') diff --git a/app/services/members/groups/creator_service.rb b/app/services/members/groups/creator_service.rb new file mode 100644 index 00000000000..df4d3f59d3b --- /dev/null +++ b/app/services/members/groups/creator_service.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Members + module Groups + class CreatorService < Members::CreatorService + def self.access_levels + Gitlab::Access.sym_options_with_owner + end + + private + + def can_update_member? + super || current_user.can?(:update_group_member, member) + end + end + end +end diff --git a/app/services/members/invite_service.rb b/app/services/members/invite_service.rb index 48010f9c8e7..6298943977b 100644 --- a/app/services/members/invite_service.rb +++ b/app/services/members/invite_service.rb @@ -21,7 +21,7 @@ module Members def validate_invites! super - # we need the below due to add_users hitting Member#parse_users_list and ignoring invalid emails + # we need the below due to add_users hitting Members::CreatorService.parse_users_list and ignoring invalid emails # ideally we wouldn't need this, but we can't really change the add_users method valid, invalid = invites.partition { |email| Member.valid_email?(email) } @invites = valid diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb new file mode 100644 index 00000000000..2e974177075 --- /dev/null +++ b/app/services/members/projects/creator_service.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Members + module Projects + class CreatorService < Members::CreatorService + def self.access_levels + Gitlab::Access.sym_options + end + + private + + def can_update_member? + super || current_user.can?(:update_project_member, member) || adding_a_new_owner? + end + + def adding_a_new_owner? + # this condition is reached during testing setup a lot due to use of `.add_user` + member.owner? && member.new_record? + end + end + end +end diff --git a/doc/administration/auth/oidc.md b/doc/administration/auth/oidc.md index 30ca7d94a1e..1d6fb2aaff5 100644 --- a/doc/administration/auth/oidc.md +++ b/doc/administration/auth/oidc.md @@ -231,7 +231,7 @@ standard Azure B2C user flows [do not send the OpenID `email` claim](https://git other words, they do not work with the [`allow_single_sign_on` or `auto_link_user` parameters](../../integration/omniauth.md#initial-omniauth-configuration). With a standard Azure B2C policy, GitLab cannot create a new account or -link to an existing one with an e-mail address. +link to an existing one with an email address. Carefully follow the instructions for [creating a custom policy](https://docs.microsoft.com/en-us/azure/active-directory-b2c/tutorial-create-user-flows?pivots=b2c-custom-policy). @@ -330,7 +330,7 @@ gitlab_rails['omniauth_providers'] = [ the respective client IDs in the XML policy files. - Add `https://jwt.ms` as a redirect URI to the app, and use the [custom policy tester](https://docs.microsoft.com/en-us/azure/active-directory-b2c/tutorial-create-user-flows?pivots=b2c-custom-policy#test-the-custom-policy). -Make sure the payload includes `email` that matches the user's e-mail access. +Make sure the payload includes `email` that matches the user's email access. - After you enable the custom policy, users might see "Invalid username or password" after they try to sign in. This might be a configuration issue with the `IdentityExperienceFramework` app. See [this Microsoft comment](https://docs.microsoft.com/en-us/answers/questions/50355/unable-to-sign-on-using-custom-policy.html?childToView=122370#comment-122370) diff --git a/doc/administration/incoming_email.md b/doc/administration/incoming_email.md index efa9f49a1da..dbbf36916dc 100644 --- a/doc/administration/incoming_email.md +++ b/doc/administration/incoming_email.md @@ -16,7 +16,7 @@ GitLab has several features based on receiving incoming email messages: - [New merge request by email](../user/project/merge_requests/creating_merge_requests.md#new-merge-request-by-email): allow GitLab users to create a new merge request by sending an email to a user-specific email address. -- [Service Desk](../user/project/service_desk.md): provide e-mail support to +- [Service Desk](../user/project/service_desk.md): provide email support to your customers through GitLab. ## Requirements diff --git a/doc/administration/troubleshooting/debug.md b/doc/administration/troubleshooting/debug.md index 6861cdcde4e..748bc95aa6c 100644 --- a/doc/administration/troubleshooting/debug.md +++ b/doc/administration/troubleshooting/debug.md @@ -119,7 +119,7 @@ an SMTP server, but you're not seeing mail delivered. Here's how to check the se irb(main):003:0> Notify.test_email('youremail@email.com', 'Hello World', 'This is a test message').deliver_now ``` - If you do not receive an e-mail and/or see an error message, then check + If you do not receive an email and/or see an error message, then check your mail server settings. ## Advanced Issues diff --git a/doc/api/import.md b/doc/api/import.md index e1585d02ae3..d556c8af971 100644 --- a/doc/api/import.md +++ b/doc/api/import.md @@ -20,7 +20,7 @@ POST /import/github | `repo_id` | integer | yes | GitHub repository ID | | `new_name` | string | no | New repository name | | `target_namespace` | string | yes | Namespace to import repository into. Supports subgroups like `/namespace/subgroup`. | -| `github_hostname` | string | no | Custom GitHub enterprise hostname. Defaults to GitHub.com if `github_hostname` is not set. | +| `github_hostname` | string | no | Custom GitHub Enterprise hostname. Do not set for GitHub.com. | ```shell curl --request POST \ @@ -31,7 +31,8 @@ curl --request POST \ "personal_access_token": "aBc123abC12aBc123abC12abC123+_A/c123", "repo_id": "12345", "target_namespace": "group/subgroup", - "new_name": "NEW-NAME" + "new_name": "NEW-NAME", + "github_hostname": "https://github.example.com" }' ``` diff --git a/doc/api/members.md b/doc/api/members.md index 19a6a2d9e38..560fc8262c0 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -262,16 +262,18 @@ GET /groups/:id/billable_members The supported values for the `sort` attribute are: -| Value | Description | -| ------------------- | ------------------------ | -| `access_level_asc` | Access level, ascending | -| `access_level_desc` | Access level, descending | -| `last_joined` | Last joined | -| `name_asc` | Name, ascending | -| `name_desc` | Name, descending | -| `oldest_joined` | Oldest joined | -| `oldest_sign_in` | Oldest sign in | -| `recent_sign_in` | Recent sign in | +| Value | Description | +| ----------------------- | ---------------------------- | +| `access_level_asc` | Access level, ascending | +| `access_level_desc` | Access level, descending | +| `last_joined` | Last joined | +| `name_asc` | Name, ascending | +| `name_desc` | Name, descending | +| `oldest_joined` | Oldest joined | +| `oldest_sign_in` | Oldest sign in | +| `recent_sign_in` | Recent sign in | +| `last_activity_on_asc` | Last active date, ascending | +| `last_activity_on_desc` | Last active date, descending | ```shell curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/:id/billable_members" diff --git a/doc/api/settings.md b/doc/api/settings.md index 8225713fc00..ce62cd193f7 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -258,7 +258,7 @@ listed in the descriptions of the relevant settings. | `disabled_oauth_sign_in_sources` | array of strings | no | Disabled OAuth sign-in sources. | | `dns_rebinding_protection_enabled` | boolean | no | Enforce DNS rebinding attack protection. | | `domain_denylist_enabled` | boolean | no | (**If enabled, requires:** `domain_denylist`) Allows blocking sign-ups from emails from specific domains. | -| `domain_denylist` | array of strings | no | Users with e-mail addresses that match these domain(s) **cannot** sign up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. | +| `domain_denylist` | array of strings | no | Users with email addresses that match these domain(s) **cannot** sign up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. | | `domain_allowlist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. | | `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys. | | `ecdsa_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `0` (no restriction). `-1` disables ECDSA keys. | @@ -413,7 +413,7 @@ listed in the descriptions of the relevant settings. | `unique_ips_limit_time_window` | integer | required by: `unique_ips_limit_enabled` | How many seconds an IP is counted towards the limit. | | `usage_ping_enabled` | boolean | no | Every week GitLab reports license usage back to GitLab, Inc. | | `user_default_external` | boolean | no | Newly registered users are external by default. | -| `user_default_internal_regex` | string | no | Specify an e-mail address regex pattern to identify default internal users. | +| `user_default_internal_regex` | string | no | Specify an email address regex pattern to identify default internal users. | | `user_oauth_applications` | boolean | no | Allow users to register any application to use GitLab as an OAuth provider. | | `user_show_add_ssh_key_message` | boolean | no | When set to `false` disable the `You won't be able to pull or push project code via SSH` warning shown to users with no uploaded SSH key. | | `version_check_enabled` | boolean | no | Let GitLab inform you when an update is available. | diff --git a/doc/api/users.md b/doc/api/users.md index 0e7b197b106..0d922487cf9 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -1352,7 +1352,7 @@ Parameters: - `id` (required) - ID of specified user - `email` (required) - email address -- `skip_confirmation` (optional) - Skip confirmation and assume e-mail is verified - true or false (default) +- `skip_confirmation` (optional) - Skip confirmation and assume email is verified - true or false (default) ## Delete email for current user diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 20e47b501e6..20f24b0fafc 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -91,8 +91,32 @@ To skip some checks based on tags when pushing, you can set the `LEFTHOOK_EXCLUD LEFTHOOK_EXCLUDE=frontend,documentation git push ... ``` +As an alternative, you can create `lefthook-local.yml` with this structure: + +```yaml +pre-push: + exclude_tags: + - frontend + - documentation +``` + For more information, check out [Lefthook documentation](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#skip-some-tags-on-the-fly). +### Skip or enable a specific Lefthook check + +To skip or enable a check based on its name when pushing, you can add `skip: true` +or `skip: false` to the `lefthook-local.yml` section for that hook. For instance, +you might want to enable the gettext check to detect issues with `locale/gitlab.pot`: + +```yaml +pre-push: + commands: + gettext: + skip: false +``` + +For more information, check out [this Lefthook documentation section](https://github.com/evilmartians/lefthook/blob/master/docs/full_guide.md#skipping-commands). + ## Ruby, Rails, RSpec Our codebase style is defined and enforced by [RuboCop](https://github.com/rubocop-hq/rubocop). diff --git a/doc/development/documentation/styleguide/word_list.md b/doc/development/documentation/styleguide/word_list.md index cd2ee5b83dd..a3b877a18d9 100644 --- a/doc/development/documentation/styleguide/word_list.md +++ b/doc/development/documentation/styleguide/word_list.md @@ -60,6 +60,10 @@ Do not use. If the user doesn't find the process to be these things, we lose the Do not use Latin abbreviations. Use **for example**, **such as**, **for instance**, or **like** instead. ([Vale](../testing.md#vale) rule: [`LatinTerms.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/LatinTerms.yml)) +## email + +Do not use **e-mail** with a hyphen. When plural, use **emails** or **email messages**. + ## enable See [the Microsoft style guide](https://docs.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/e/enable-enables) for guidance. diff --git a/doc/development/scalability.md b/doc/development/scalability.md index b260618c220..675f0968ec5 100644 --- a/doc/development/scalability.md +++ b/doc/development/scalability.md @@ -243,7 +243,7 @@ Ruby on Rails applications. In GitLab, Sidekiq performs the heavy lifting of many activities, including: - Updating merge requests after a push. -- Sending e-mails. +- Sending email messages. - Updating user authorizations. - Processing CI builds and pipelines. @@ -276,7 +276,7 @@ in a timely manner: this to `ProcessCommitWorker`. - Redistribute/gerrymander Sidekiq processes by queue types. Long-running jobs (for example, relating to project import) can often - squeeze out jobs that run fast (for example, delivering e-mail). [This technique + squeeze out jobs that run fast (for example, delivering email). [This technique was used in to optimize our existing Sidekiq deployment](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/7219#note_218019483). - Optimize jobs. Eliminating unnecessary work, reducing network calls (including SQL and Gitaly), and optimizing processor time can yield significant diff --git a/doc/integration/shibboleth.md b/doc/integration/shibboleth.md index c4cf19747be..c52535c297d 100644 --- a/doc/integration/shibboleth.md +++ b/doc/integration/shibboleth.md @@ -67,7 +67,7 @@ The following changes are needed to enable Shibboleth: your users appear to be authenticated by Shibboleth and Apache, but GitLab rejects their account with a URI that contains "e-mail is invalid" then your Shibboleth Identity Provider or Attribute Authority may be asserting multiple - e-mail addresses. In this instance, you might consider setting the + email addresses. In this instance, you might consider setting the `multi_values` argument to `first`. The file should look like this: diff --git a/doc/operations/feature_flags.md b/doc/operations/feature_flags.md index 5df787142fd..326d026f23f 100644 --- a/doc/operations/feature_flags.md +++ b/doc/operations/feature_flags.md @@ -392,10 +392,10 @@ end > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36617) in GitLab 13.2. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/251234) in GitLab 13.5. -> - [Updated](https://gitlab.com/gitlab-org/gitlab/-/issues/220333) in GitLab 14.1 +> - Showing related feature flags in issues [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/220333) in GitLab 14.1. You can link related issues to a feature flag. In the **Linked issues** section, click the `+` button and input the issue reference number or the full URL of the issue. -The issue(s) will then appear in the related feautre flag and vice versa. +The issues then appear in the related feature flag and the other way round. This feature is similar to the [linked issues](../user/project/issues/related_issues.md) feature. diff --git a/doc/user/admin_area/review_abuse_reports.md b/doc/user/admin_area/review_abuse_reports.md index a179eb70453..7816d0648b2 100644 --- a/doc/user/admin_area/review_abuse_reports.md +++ b/doc/user/admin_area/review_abuse_reports.md @@ -14,7 +14,7 @@ reports in the Admin Area. ## Receiving notifications of abuse reports -To receive notifications of new abuse reports by e-mail, follow these steps: +To receive notifications of new abuse reports by email, follow these steps: 1. On the top bar, select **Menu >** **{admin}** **Admin**. 1. On the left sidebar, select **Settings > Reporting**. diff --git a/doc/user/group/settings/import_export.md b/doc/user/group/settings/import_export.md index 1d5db6757bc..94a79ec6e74 100644 --- a/doc/user/group/settings/import_export.md +++ b/doc/user/group/settings/import_export.md @@ -70,7 +70,7 @@ For more details on the specific data persisted in a group export, see the ![Export group panel](img/export_panel_v13_0.png) -1. After the export is generated, you should receive an e-mail with a link to the [exported contents](#exported-contents) +1. After the export is generated, you should receive an email with a link to the [exported contents](#exported-contents) in a compressed tar archive, with contents in NDJSON format. 1. Alternatively, you can come back to the project settings and download the diff --git a/doc/user/project/import/bitbucket_server.md b/doc/user/project/import/bitbucket_server.md index 963b9f524ff..3a28b3c8afa 100644 --- a/doc/user/project/import/bitbucket_server.md +++ b/doc/user/project/import/bitbucket_server.md @@ -51,7 +51,7 @@ The Bitbucket Server importer works as follows: ### User assignment When issues/pull requests are being imported, the Bitbucket importer tries to -find the author's e-mail address with a confirmed e-mail address in the GitLab +find the author's email address with a confirmed email address in the GitLab user database. If no such user is available, the project creator is set as the author. The importer appends a note in the comment to mark the original creator. diff --git a/doc/user/project/merge_requests/creating_merge_requests.md b/doc/user/project/merge_requests/creating_merge_requests.md index 430c6488b26..c1bff994522 100644 --- a/doc/user/project/merge_requests/creating_merge_requests.md +++ b/doc/user/project/merge_requests/creating_merge_requests.md @@ -231,7 +231,7 @@ _In GitLab 11.7, we updated the format of the generated email address. However the older format is still supported, allowing existing aliases or contacts to continue working._ -### Adding patches when creating a merge request via e-mail +### Adding patches when creating a merge request via email > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/22723) in GitLab 11.5. diff --git a/doc/user/project/pages/lets_encrypt_for_gitlab_pages.md b/doc/user/project/pages/lets_encrypt_for_gitlab_pages.md index ce49699785e..978e35b3a9f 100644 --- a/doc/user/project/pages/lets_encrypt_for_gitlab_pages.md +++ b/doc/user/project/pages/lets_encrypt_for_gitlab_pages.md @@ -60,7 +60,7 @@ might be slightly different. Follow the sudo certbot certonly -a manual -d example.com --email your@email.com ``` - Alternatively, you can register without adding an e-mail account, + Alternatively, you can register without adding an email account, but you aren't notified about the certificate expiration's date: ```shell diff --git a/doc/user/project/service_desk.md b/doc/user/project/service_desk.md index 2021137c8a5..dc8bfdfef42 100644 --- a/doc/user/project/service_desk.md +++ b/doc/user/project/service_desk.md @@ -234,7 +234,7 @@ The configuration options are the same as for configuring > Introduced in [GitLab 13.11](https://gitlab.com/gitlab-org/gitlab/-/issues/214900) Service Desk can be configured to read Microsoft Exchange Online mailboxes with the Microsoft -Graph API instead of IMAP. Follow the [documentation in the incoming e-mail section for setting up an OAuth2 application for Microsoft Graph](../../administration/incoming_email.md#microsoft-graph). +Graph API instead of IMAP. Follow the [documentation in the incoming email section for setting up an OAuth2 application for Microsoft Graph](../../administration/incoming_email.md#microsoft-graph). - Example for Omnibus GitLab installations: diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index 08d00149037..b9f048e6536 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -156,7 +156,7 @@ To export a project and its data, follow these steps: ![Export button](img/import_export_export_button.png) -1. Once the export is generated, you should receive an e-mail with a link to +1. Once the export is generated, you should receive an email with a link to download the file: ![Email download link](img/import_export_mail_link.png) diff --git a/lefthook.yml b/lefthook.yml index daab6e7f4a0..81ff2ecdada 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -38,3 +38,9 @@ pre-push: files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD glob: 'doc/*.md' run: if command -v vale 2> /dev/null; then vale --config .vale.ini --minAlertLevel error {files}; else echo "Vale not found. Install Vale"; fi + gettext: + skip: true # This is disabled by default. You can enable this check by adding skip: false in lefhook-local.yml https://github.com/evilmartians/lefthook/blob/master/docs/full_guide.md#skipping-commands + tags: backend frontend view haml + files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD | while read file;do git diff --unified=1 $(git merge-base origin/master HEAD)..HEAD $file | grep -Fqe '_(' && echo $file;done; true + glob: "*.{haml,rb,js,vue}" + run: bin/rake gettext:updated_check diff --git a/lib/gitlab/backup_logger.rb b/lib/gitlab/backup_logger.rb new file mode 100644 index 00000000000..fad36b860ae --- /dev/null +++ b/lib/gitlab/backup_logger.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Gitlab + class BackupLogger < Gitlab::JsonLogger + def self.file_name_noext + 'backup_json' + end + end +end diff --git a/lib/gitlab/email/handler/service_desk_handler.rb b/lib/gitlab/email/handler/service_desk_handler.rb index 2187c01acfb..3cd369f9a2c 100644 --- a/lib/gitlab/email/handler/service_desk_handler.rb +++ b/lib/gitlab/email/handler/service_desk_handler.rb @@ -42,10 +42,6 @@ module Gitlab end end - def metrics_params - super.merge(project: project&.full_path) - end - def metrics_event :receive_email_service_desk end diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake index 5b17a8c185a..fff4927e6b1 100644 --- a/lib/tasks/gitlab/backup.rake +++ b/lib/tasks/gitlab/backup.rake @@ -282,6 +282,7 @@ namespace :gitlab do def puts_time(msg) progress.puts "#{Time.now} -- #{msg}" + Gitlab::BackupLogger.info(message: "#{Rainbow.uncolor(msg)}") end def progress diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb index f6b618163f0..12a2dda2855 100644 --- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do it 'adds metric events for incoming and reply emails' do metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) - expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, anything) + expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, { handler: 'Gitlab::Email::Handler::ServiceDeskHandler' }) expect(metric_transaction).to receive(:add_event).with(:service_desk_thank_you_email) receiver.execute diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 600fe96c3f9..1727a9f89f3 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -508,282 +508,6 @@ RSpec.describe Member do end end - describe '.add_user' do - %w[project group].each do |source_type| - context "when source is a #{source_type}" do - let_it_be(:source, reload: true) { create(source_type, :public) } - let_it_be(:user) { create(:user) } - let_it_be(:admin) { create(:admin) } - - it 'returns a <Source>Member object' do - member = described_class.add_user(source, user, :maintainer) - - expect(member).to be_a "#{source_type.classify}Member".constantize - expect(member).to be_persisted - end - - context 'when admin mode is enabled', :enable_admin_mode do - it 'sets members.created_by to the given admin current_user' do - member = described_class.add_user(source, user, :maintainer, current_user: admin) - - expect(member.created_by).to eq(admin) - end - end - - context 'when admin mode is disabled' do - it 'rejects setting members.created_by to the given admin current_user' do - member = described_class.add_user(source, user, :maintainer, current_user: admin) - - expect(member.created_by).to be_nil - end - end - - it 'sets members.expires_at to the given expires_at' do - member = described_class.add_user(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) - - expect(member.expires_at).to eq(Date.new(2016, 9, 22)) - end - - described_class.access_levels.each do |sym_key, int_access_level| - it "accepts the :#{sym_key} symbol as access level" do - expect(source.users).not_to include(user) - - member = described_class.add_user(source, user.id, sym_key) - - expect(member.access_level).to eq(int_access_level) - expect(source.users.reload).to include(user) - end - - it "accepts the #{int_access_level} integer as access level" do - expect(source.users).not_to include(user) - - member = described_class.add_user(source, user.id, int_access_level) - - expect(member.access_level).to eq(int_access_level) - expect(source.users.reload).to include(user) - end - end - - context 'with no current_user' do - context 'when called with a known user id' do - it 'adds the user as a member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, user.id, :maintainer) - - expect(source.users.reload).to include(user) - end - end - - context 'when called with an unknown user id' do - it 'adds the user as a member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, non_existing_record_id, :maintainer) - - expect(source.users.reload).not_to include(user) - end - end - - context 'when called with a user object' do - it 'adds the user as a member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, user, :maintainer) - - expect(source.users.reload).to include(user) - end - end - - context 'when called with a requester user object' do - before do - source.request_access(user) - end - - it 'adds the requester as a member' do - expect(source.users).not_to include(user) - expect(source.requesters.exists?(user_id: user)).to be_truthy - - expect { described_class.add_user(source, user, :maintainer) } - .to raise_error(Gitlab::Access::AccessDeniedError) - - expect(source.users.reload).not_to include(user) - expect(source.requesters.reload.exists?(user_id: user)).to be_truthy - end - end - - context 'when called with a known user email' do - it 'adds the user as a member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, user.email, :maintainer) - - expect(source.users.reload).to include(user) - end - end - - context 'when called with a known user secondary email' do - let(:secondary_email) { create(:email, email: 'secondary@example.com', user: user) } - - it 'adds the user as a member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, secondary_email.email, :maintainer) - - expect(source.users.reload).to include(user) - end - end - - context 'when called with an unknown user email' do - it 'creates an invited member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, 'user@example.com', :maintainer) - - expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') - end - end - - context 'when called with an unknown user email starting with a number' do - it 'creates an invited member', :aggregate_failures do - email_starting_with_number = "#{user.id}_email@example.com" - - described_class.add_user(source, email_starting_with_number, :maintainer) - - expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number) - expect(source.users.reload).not_to include(user) - end - end - end - - context 'when current_user can update member', :enable_admin_mode do - it 'creates the member' do - expect(source.users).not_to include(user) - - described_class.add_user(source, user, :maintainer, current_user: admin) - - expect(source.users.reload).to include(user) - end - - context 'when called with a requester user object' do - before do - source.request_access(user) - end - - it 'adds the requester as a member' do - expect(source.users).not_to include(user) - expect(source.requesters.exists?(user_id: user)).to be_truthy - - described_class.add_user(source, user, :maintainer, current_user: admin) - - expect(source.users.reload).to include(user) - expect(source.requesters.reload.exists?(user_id: user)).to be_falsy - end - end - end - - context 'when current_user cannot update member' do - it 'does not create the member' do - expect(source.users).not_to include(user) - - member = described_class.add_user(source, user, :maintainer, current_user: user) - - expect(source.users.reload).not_to include(user) - expect(member).not_to be_persisted - end - - context 'when called with a requester user object' do - before do - source.request_access(user) - end - - it 'does not destroy the requester' do - expect(source.users).not_to include(user) - expect(source.requesters.exists?(user_id: user)).to be_truthy - - described_class.add_user(source, user, :maintainer, current_user: user) - - expect(source.users.reload).not_to include(user) - expect(source.requesters.exists?(user_id: user)).to be_truthy - end - end - end - - context 'when member already exists' do - before do - source.add_user(user, :developer) - end - - context 'with no current_user' do - it 'updates the member' do - expect(source.users).to include(user) - - described_class.add_user(source, user, :maintainer) - - expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - - context 'when current_user can update member', :enable_admin_mode do - it 'updates the member' do - expect(source.users).to include(user) - - described_class.add_user(source, user, :maintainer, current_user: admin) - - expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - - context 'when current_user cannot update member' do - it 'does not update the member' do - expect(source.users).to include(user) - - described_class.add_user(source, user, :maintainer, current_user: user) - - expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) - end - end - end - end - end - end - - describe '.add_users' do - %w[project group].each do |source_type| - context "when source is a #{source_type}" do - let_it_be(:source) { create(source_type, :public) } - let_it_be(:admin) { create(:admin) } - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - it 'returns a <Source>Member objects' do - members = described_class.add_users(source, [user1, user2], :maintainer) - - expect(members).to be_a Array - expect(members.size).to eq(2) - expect(members.first).to be_a "#{source_type.classify}Member".constantize - expect(members.first).to be_persisted - end - - it 'returns an empty array' do - members = described_class.add_users(source, [], :maintainer) - - expect(members).to be_a Array - expect(members).to be_empty - end - - it 'supports differents formats' do - list = ['joe@local.test', admin, user1.id, user2.id.to_s] - - members = described_class.add_users(source, list, :maintainer) - - expect(members.size).to eq(4) - expect(members.first).to be_invite - end - end - end - end - describe '#accept_request' do let(:member) { create(:project_member, requested_at: Time.current.utc) } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 8c942228059..472f4280d26 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -47,27 +47,6 @@ RSpec.describe GroupMember do end end - describe '.access_levels' do - it 'returns Gitlab::Access.options_with_owner' do - expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) - end - end - - describe '.add_users' do - it 'adds the given users to the given group' do - group = create(:group) - users = create_list(:user, 2) - - described_class.add_users( - group, - [users.first.id, users.second], - described_class::MAINTAINER - ) - - expect(group.users).to include(users.first, users.second) - end - end - it_behaves_like 'members notifications', :group describe '#namespace_id' do diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index b84b408cb4b..4c59bda856f 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -23,19 +23,6 @@ RSpec.describe ProjectMember do end end - describe '.add_user' do - it 'adds the user as a member' do - user = create(:user) - project = create(:project) - - expect(project.users).not_to include(user) - - described_class.add_user(project, user, :maintainer, current_user: project.owner) - - expect(project.users.reload).to include(user) - end - end - describe '#real_source_type' do subject { create(:project_member).real_source_type } diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb new file mode 100644 index 00000000000..4427c4e7d9f --- /dev/null +++ b/spec/services/members/groups/creator_service_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::Groups::CreatorService do + it_behaves_like 'member creation' do + let_it_be(:source, reload: true) { create(:group, :public) } + let_it_be(:member_type) { GroupMember } + end + + describe '.access_levels' do + it 'returns Gitlab::Access.options_with_owner' do + expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) + end + end +end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb new file mode 100644 index 00000000000..c6917a21bcd --- /dev/null +++ b/spec/services/members/projects/creator_service_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::Projects::CreatorService do + it_behaves_like 'member creation' do + let_it_be(:source, reload: true) { create(:project, :public) } + let_it_be(:member_type) { ProjectMember } + end + + describe '.access_levels' do + it 'returns Gitlab::Access.sym_options' do + expect(described_class.access_levels).to eq(Gitlab::Access.sym_options) + end + end +end diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 7ede6f0d8d4..c111d250d34 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -75,3 +75,259 @@ RSpec.shared_examples '#valid_level_roles' do |entity_name| expect(presenter.valid_level_roles).to eq(expected_roles) end end + +RSpec.shared_examples_for "member creation" do + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + + describe '#execute' do + it 'returns a Member object', :aggregate_failures do + member = described_class.new(source, user, :maintainer).execute + + expect(member).to be_a member_type + expect(member).to be_persisted + end + + context 'when admin mode is enabled', :enable_admin_mode do + it 'sets members.created_by to the given admin current_user' do + member = described_class.new(source, user, :maintainer, current_user: admin).execute + + expect(member.created_by).to eq(admin) + end + end + + context 'when admin mode is disabled' do + it 'rejects setting members.created_by to the given admin current_user' do + member = described_class.new(source, user, :maintainer, current_user: admin).execute + + expect(member.created_by).to be_nil + end + end + + it 'sets members.expires_at to the given expires_at' do + member = described_class.new(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)).execute + + expect(member.expires_at).to eq(Date.new(2016, 9, 22)) + end + + described_class.access_levels.each do |sym_key, int_access_level| + it "accepts the :#{sym_key} symbol as access level", :aggregate_failures do + expect(source.users).not_to include(user) + + member = described_class.new(source, user.id, sym_key).execute + + expect(member.access_level).to eq(int_access_level) + expect(source.users.reload).to include(user) + end + + it "accepts the #{int_access_level} integer as access level", :aggregate_failures do + expect(source.users).not_to include(user) + + member = described_class.new(source, user.id, int_access_level).execute + + expect(member.access_level).to eq(int_access_level) + expect(source.users.reload).to include(user) + end + end + + context 'with no current_user' do + context 'when called with a known user id' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.new(source, user.id, :maintainer).execute + + expect(source.users.reload).to include(user) + end + end + + context 'when called with an unknown user id' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.new(source, non_existing_record_id, :maintainer).execute + + expect(source.users.reload).not_to include(user) + end + end + + context 'when called with a user object' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.new(source, user, :maintainer).execute + + expect(source.users.reload).to include(user) + end + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'adds the requester as a member', :aggregate_failures do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + expect do + described_class.new(source, user, :maintainer).execute + end.to raise_error(Gitlab::Access::AccessDeniedError) + + expect(source.users.reload).not_to include(user) + expect(source.requesters.reload.exists?(user_id: user)).to be_truthy + end + end + + context 'when called with a known user email' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.new(source, user.email, :maintainer).execute + + expect(source.users.reload).to include(user) + end + end + + context 'when called with an unknown user email' do + it 'creates an invited member' do + expect(source.users).not_to include(user) + + described_class.new(source, 'user@example.com', :maintainer).execute + + expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') + end + end + + context 'when called with an unknown user email starting with a number' do + it 'creates an invited member', :aggregate_failures do + email_starting_with_number = "#{user.id}_email@example.com" + + described_class.new(source, email_starting_with_number, :maintainer).execute + + expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number) + expect(source.users.reload).not_to include(user) + end + end + end + + context 'when current_user can update member', :enable_admin_mode do + it 'creates the member' do + expect(source.users).not_to include(user) + + described_class.new(source, user, :maintainer, current_user: admin).execute + + expect(source.users.reload).to include(user) + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'adds the requester as a member', :aggregate_failures do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + described_class.new(source, user, :maintainer, current_user: admin).execute + + expect(source.users.reload).to include(user) + expect(source.requesters.reload.exists?(user_id: user)).to be_falsy + end + end + end + + context 'when current_user cannot update member' do + it 'does not create the member', :aggregate_failures do + expect(source.users).not_to include(user) + + member = described_class.new(source, user, :maintainer, current_user: user).execute + + expect(source.users.reload).not_to include(user) + expect(member).not_to be_persisted + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'does not destroy the requester', :aggregate_failures do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + described_class.new(source, user, :maintainer, current_user: user).execute + + expect(source.users.reload).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + end + end + end + + context 'when member already exists' do + before do + source.add_user(user, :developer) + end + + context 'with no current_user' do + it 'updates the member' do + expect(source.users).to include(user) + + described_class.new(source, user, :maintainer).execute + + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + + context 'when current_user can update member', :enable_admin_mode do + it 'updates the member' do + expect(source.users).to include(user) + + described_class.new(source, user, :maintainer, current_user: admin).execute + + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + + context 'when current_user cannot update member' do + it 'does not update the member' do + expect(source.users).to include(user) + + described_class.new(source, user, :maintainer, current_user: user).execute + + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) + end + end + end + end + + describe '.add_users' do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + + it 'returns a Member objects' do + members = described_class.add_users(source, [user1, user2], :maintainer) + + expect(members).to be_a Array + expect(members.size).to eq(2) + expect(members.first).to be_a member_type + expect(members.first).to be_persisted + end + + it 'returns an empty array' do + members = described_class.add_users(source, [], :maintainer) + + expect(members).to be_a Array + expect(members).to be_empty + end + + it 'supports different formats' do + list = ['joe@local.test', admin, user1.id, user2.id.to_s] + + members = described_class.add_users(source, list, :maintainer) + + expect(members.size).to eq(4) + expect(members.first).to be_invite + end + end +end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index e5a210bb344..7c708689acb 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -209,6 +209,23 @@ RSpec.describe 'gitlab:app namespace rake task', :delete do expect { run_rake_task("gitlab:backup:#{task}:create") }.to output(/Dumping /).to_stdout_from_any_process end end + + it 'logs the progress to log file' do + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping database ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "[SKIPPED]") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping repositories ...") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping uploads ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping builds ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping artifacts ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping pages ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping lfs objects ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "Dumping container registry images ... ") + expect(Gitlab::BackupLogger).to receive(:info).with(message: "done").exactly(7).times + + task_list.each do |task| + run_rake_task("gitlab:backup:#{task}:create") + end + end end end |