diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 10800cff591..a8a3d86d5a3 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -67a362bf7aaab3aae021d19fda728c24b7723d7a +c5786b09543e40acc6e05bd4d29f6d89106b8e8a diff --git a/app/assets/javascripts/batch_comments/mixins/resolved_status.js b/app/assets/javascripts/batch_comments/mixins/resolved_status.js index 0b085da1ff9..85bdf6b7a36 100644 --- a/app/assets/javascripts/batch_comments/mixins/resolved_status.js +++ b/app/assets/javascripts/batch_comments/mixins/resolved_status.js @@ -1,9 +1,7 @@ import { mapGetters } from 'vuex'; import { sprintf, s__, __ } from '~/locale'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { - mixins: [glFeatureFlagsMixin()], props: { discussionId: { type: String, @@ -54,11 +52,7 @@ export default { resolveButtonTitle() { if (this.isDraft || this.discussionId) return this.resolvedStatusMessage; - let title = __('Mark as resolved'); - - if (this.glFeatures.removeResolveNote) { - title = __('Resolve thread'); - } + let title = __('Resolve thread'); if (this.resolvedBy) { title = sprintf(__('Resolved by %{name}'), { name: this.resolvedBy.name }); diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 185f4a70367..0b5abacc963 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -86,7 +86,7 @@ export default { isRequesting: false, isResolving: false, commentLineStart: {}, - resolveAsThread: this.glFeatures.removeResolveNote, + resolveAsThread: true, }; }, computed: { @@ -139,14 +139,9 @@ export default { return this.note.isDraft; }, canResolve() { - if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false; + if (!this.discussionRoot) return false; - if (this.glFeatures.removeResolveNote) return this.note.current_user.can_resolve_discussion; - - return ( - this.note.current_user.can_resolve || - (this.note.isDraft && this.note.discussion_id !== null) - ); + return this.note.current_user.can_resolve_discussion; }, lineRange() { return this.note.position?.line_range; diff --git a/app/assets/javascripts/notes/mixins/resolvable.js b/app/assets/javascripts/notes/mixins/resolvable.js index baada4c5ce8..27ed8e203b0 100644 --- a/app/assets/javascripts/notes/mixins/resolvable.js +++ b/app/assets/javascripts/notes/mixins/resolvable.js @@ -1,24 +1,11 @@ import { deprecatedCreateFlash as Flash } from '~/flash'; import { __ } from '~/locale'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { - mixins: [glFeatureFlagsMixin()], computed: { discussionResolved() { if (this.discussion) { - const { notes, resolved } = this.discussion; - - if (this.glFeatures.removeResolveNote) { - return Boolean(resolved); - } - - if (notes) { - // Decide resolved state using store. Only valid for discussions. - return notes.filter((note) => !note.system).every((note) => note.resolved); - } - - return resolved; + return Boolean(this.discussion.resolved); } return this.note.resolved; @@ -47,7 +34,7 @@ export default { let endpoint = discussion && this.discussion ? this.discussion.resolve_path : `${this.note.path}/resolve`; - if (this.glFeatures.removeResolveNote && this.discussionResolvePath) { + if (this.discussionResolvePath) { endpoint = this.discussionResolvePath; } diff --git a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue index d84f967bbb1..6afc33ec8a5 100644 --- a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue +++ b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue @@ -40,7 +40,7 @@ export default { helpPath() { return setUrlFragment( this.pageInfo.helpPath, - this.pageInfo.persisted ? 'moving-a-wiki-page' : 'creating-a-new-wiki-page', + this.pageInfo.persisted ? 'move-a-wiki-page' : 'create-a-new-wiki-page', ); }, commitMessageI18n() { diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e1f2c0ec7a9..d7c7d79f2dc 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -36,7 +36,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:unified_diff_components, @project, default_enabled: true) push_frontend_feature_flag(:default_merge_ref_for_diffs, @project, default_enabled: :yaml) push_frontend_feature_flag(:core_security_mr_widget_counts, @project) - push_frontend_feature_flag(:remove_resolve_note, @project, default_enabled: true) push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true) push_frontend_feature_flag(:codequality_backend_comparison, @project, default_enabled: :yaml) push_frontend_feature_flag(:local_file_reviews, default_enabled: :yaml) diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index f13ba9caee0..0734f178640 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -74,6 +74,18 @@ module Emails end end + def ssh_key_expired_email(user, fingerprints) + return unless user && user.active? + + @user = user + @fingerprints = fingerprints + @target_url = profile_keys_url + + Gitlab::I18n.with_locale(@user.preferred_language) do + mail(to: @user.notification_email, subject: subject(_("Your SSH key has expired"))) + end + end + def unknown_sign_in_email(user, ip, time) @user = user @ip = ip diff --git a/app/models/key.rb b/app/models/key.rb index 18fa8aaaa16..d9da700376a 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -43,6 +43,7 @@ class Key < ApplicationRecord scope :preload_users, -> { preload(:user) } scope :for_user, -> (user) { where(user: user) } scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) } + scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) } def self.regular_keys where(type: ['Key', nil]) diff --git a/app/models/user.rb b/app/models/user.rb index 9ed12ac72af..decd1117c6c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -103,6 +103,7 @@ class User < ApplicationRecord # Profile has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key' has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent has_many :group_deploy_keys has_many :gpg_keys @@ -398,6 +399,14 @@ class User < ApplicationRecord .without_impersonation .expired_today_and_not_notified) end + scope :with_ssh_key_expired_today, -> do + includes(:expired_today_and_unnotified_keys) + .where('EXISTS (?)', + ::Key + .select(1) + .where('keys.user_id = users.id') + .expired_today_and_not_notified) + end scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } diff --git a/app/services/keys/expiry_notification_service.rb b/app/services/keys/expiry_notification_service.rb new file mode 100644 index 00000000000..4ba896477d2 --- /dev/null +++ b/app/services/keys/expiry_notification_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Keys + class ExpiryNotificationService < ::Keys::BaseService + attr_accessor :keys + + def initialize(user, params) + @keys = params[:keys] + + super + end + + def execute + return unless user.can?(:receive_notifications) + + notification_service.ssh_key_expired(user, keys.map(&:fingerprint)) + + keys.update_all(expiry_notification_delivered_at: Time.current.utc) + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 26d7bd162df..6e014c0a670 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -79,6 +79,13 @@ class NotificationService mailer.access_token_expired_email(user).deliver_later end + # Notify the user when at least one of their ssh key has expired today + def ssh_key_expired(user, fingerprints) + return unless user.can?(:receive_notifications) + + mailer.ssh_key_expired_email(user, fingerprints).deliver_later + end + # Notify a user when a previously unknown IP or device is used to # sign in to their account def unknown_sign_in(user, ip, time) diff --git a/app/views/notify/ssh_key_expired_email.html.haml b/app/views/notify/ssh_key_expired_email.html.haml new file mode 100644 index 00000000000..21138bb0113 --- /dev/null +++ b/app/views/notify/ssh_key_expired_email.html.haml @@ -0,0 +1,13 @@ +%p + = _('Hi %{username}!') % { username: sanitize_name(@user.name) } +%p + = _('Your SSH keys with the following fingerprints has expired:') +%table + %tbody + - @fingerprints.each do |fingerprint| + %tr + %td= fingerprint + +%p + - ssh_key_link_start = ''.html_safe % { url: @target_url } + = html_escape(_('You can create a new one or check them in your %{ssh_key_link_start}SSH keys%{ssh_key_link_end} settings.')) % { ssh_key_link_start: ssh_key_link_start, ssh_key_link_end: ''.html_safe } diff --git a/app/views/notify/ssh_key_expired_email.text.erb b/app/views/notify/ssh_key_expired_email.text.erb new file mode 100644 index 00000000000..77b76084606 --- /dev/null +++ b/app/views/notify/ssh_key_expired_email.text.erb @@ -0,0 +1,9 @@ +<%= _('Hi %{username}!') % { username: sanitize_name(@user.name) } %> + +<%= _('Your SSH keys with the following fingerprints has expired:') %> + +<% @fingerprints.each do |fingerprint| %> + - <%= fingerprint %> +<% end %> + +<%= _('You can create a new one or check them in your SSH keys settings %{ssh_key_link}.') % { ssh_key_link: @target_url } %> diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fe5fb189d7e..921def1eef8 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -443,6 +443,14 @@ :weight: 1 :idempotent: :tags: [] +- :name: cronjob:ssh_keys_expired_notification + :feature_category: :compliance_management + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: cronjob:stuck_ci_jobs :feature_category: :continuous_integration :has_external_dependencies: diff --git a/app/workers/ssh_keys/expired_notification_worker.rb b/app/workers/ssh_keys/expired_notification_worker.rb new file mode 100644 index 00000000000..57a9060b573 --- /dev/null +++ b/app/workers/ssh_keys/expired_notification_worker.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module SshKeys + class ExpiredNotificationWorker + include ApplicationWorker + include CronjobQueue + + feature_category :compliance_management + idempotent! + + def perform + return unless ::Feature.enabled?(:ssh_key_expiration_email_notification, default_enabled: :yaml) + + User.with_ssh_key_expired_today.find_each do |user| + with_context(user: user) do + Gitlab::AppLogger.info "#{self.class}: Notifying User #{user.id} about expired ssh key(s)" + + keys = user.expired_today_and_unnotified_keys + + Keys::ExpiryNotificationService.new(user, { keys: keys }).execute + end + end + end + end +end diff --git a/changelogs/unreleased/320756-remove-remove_resolve_note-feature-flag.yml b/changelogs/unreleased/320756-remove-remove_resolve_note-feature-flag.yml new file mode 100644 index 00000000000..32e87c2f845 --- /dev/null +++ b/changelogs/unreleased/320756-remove-remove_resolve_note-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Remove remove_resolve_note feature flag +merge_request: 57757 +author: +type: other diff --git a/changelogs/unreleased/ssh_key_expiration.yml b/changelogs/unreleased/ssh_key_expiration.yml new file mode 100644 index 00000000000..aa190ee5208 --- /dev/null +++ b/changelogs/unreleased/ssh_key_expiration.yml @@ -0,0 +1,5 @@ +--- +title: Send email notification on SSH key expiration +merge_request: 56888 +author: +type: added diff --git a/config/feature_flags/development/remove_resolve_note.yml b/config/feature_flags/development/ssh_key_expiration_email_notification.yml similarity index 52% rename from config/feature_flags/development/remove_resolve_note.yml rename to config/feature_flags/development/ssh_key_expiration_email_notification.yml index 008a469e16d..ee051a4648b 100644 --- a/config/feature_flags/development/remove_resolve_note.yml +++ b/config/feature_flags/development/ssh_key_expiration_email_notification.yml @@ -1,8 +1,8 @@ --- -name: remove_resolve_note -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45549 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320756 -milestone: '13.6' +name: ssh_key_expiration_email_notification +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56888 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326386 +milestone: '13.11' type: development -group: group::code review -default_enabled: true +group: group::compliance +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index a7ea11b9674..21e1461c361 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -560,6 +560,9 @@ Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvi Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *' Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker' +Settings.cron_jobs['ssh_keys_expired_notification_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2 * * *' +Settings.cron_jobs['ssh_keys_expired_notification_worker']['job_class'] = 'SshKeys::ExpiredNotificationWorker' Gitlab.com do Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({}) diff --git a/db/migrate/20210317192943_add_expiry_notification_delivered_to_keys.rb b/db/migrate/20210317192943_add_expiry_notification_delivered_to_keys.rb new file mode 100644 index 00000000000..15f319b3965 --- /dev/null +++ b/db/migrate/20210317192943_add_expiry_notification_delivered_to_keys.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddExpiryNotificationDeliveredToKeys < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :keys, :expiry_notification_delivered_at, :datetime_with_timezone + end +end diff --git a/db/migrate/20210322182751_add_index_to_keys_on_expires_at_and_expiry_notification_undelivered.rb b/db/migrate/20210322182751_add_index_to_keys_on_expires_at_and_expiry_notification_undelivered.rb new file mode 100644 index 00000000000..6387d8a6a43 --- /dev/null +++ b/db/migrate/20210322182751_add_index_to_keys_on_expires_at_and_expiry_notification_undelivered.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_keys_on_expires_at_and_expiry_notification_undelivered' + disable_ddl_transaction! + + def up + add_concurrent_index :keys, + "date(timezone('UTC', expires_at)), expiry_notification_delivered_at", + where: 'expiry_notification_delivered_at IS NULL', name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name(:keys, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20210317192943 b/db/schema_migrations/20210317192943 new file mode 100644 index 00000000000..d03b325fa77 --- /dev/null +++ b/db/schema_migrations/20210317192943 @@ -0,0 +1 @@ +dfb88ea7a213da1e56bef532255f01a284d7b9be9ec8a6b9dd0e95ec04d0f524 \ No newline at end of file diff --git a/db/schema_migrations/20210322182751 b/db/schema_migrations/20210322182751 new file mode 100644 index 00000000000..615f3c7a5de --- /dev/null +++ b/db/schema_migrations/20210322182751 @@ -0,0 +1 @@ +79ad2de15faef8edb8752c2a9c89f1739a805af999c86db6e73482a613c4f9d1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 01bbd00bd44..6e38971a445 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14031,7 +14031,8 @@ CREATE TABLE keys ( public boolean DEFAULT false NOT NULL, last_used_at timestamp without time zone, fingerprint_sha256 bytea, - expires_at timestamp with time zone + expires_at timestamp with time zone, + expiry_notification_delivered_at timestamp with time zone ); CREATE SEQUENCE keys_id_seq @@ -22991,6 +22992,8 @@ CREATE INDEX index_jira_imports_on_user_id ON jira_imports USING btree (user_id) CREATE INDEX index_jira_tracker_data_on_service_id ON jira_tracker_data USING btree (service_id); +CREATE INDEX index_keys_on_expires_at_and_expiry_notification_undelivered ON keys USING btree (date(timezone('UTC'::text, expires_at)), expiry_notification_delivered_at) WHERE (expiry_notification_delivered_at IS NULL); + CREATE UNIQUE INDEX index_keys_on_fingerprint ON keys USING btree (fingerprint); CREATE INDEX index_keys_on_fingerprint_sha256 ON keys USING btree (fingerprint_sha256); diff --git a/doc/administration/geo/replication/datatypes.md b/doc/administration/geo/replication/datatypes.md index 467e5ec5fcc..4c0e223f3af 100644 --- a/doc/administration/geo/replication/datatypes.md +++ b/doc/administration/geo/replication/datatypes.md @@ -175,7 +175,7 @@ successfully, you must replicate their data using some other means. | [Application data in PostgreSQL](../../postgresql/index.md) | **Yes** (10.2) | **Yes** (10.2) | No | | | [Project repository](../../../user/project/repository/) | **Yes** (10.2) | **Yes** (10.7) | No | | | [Project wiki repository](../../../user/project/wiki/) | **Yes** (10.2) | **Yes** (10.7) | No | -| [Group wiki repository](../../../user/group/index.md#group-wikis) | [**Yes** (13.10)](https://gitlab.com/gitlab-org/gitlab/-/issues/208147) | No | No | Behind feature flag `geo_group_wiki_repository_replication`, enabled by default | +| [Group wiki repository](../../../user/project/wiki/index.md#group-wikis) | [**Yes** (13.10)](https://gitlab.com/gitlab-org/gitlab/-/issues/208147) | No | No | Behind feature flag `geo_group_wiki_repository_replication`, enabled by default | | [Uploads](../../uploads.md) | **Yes** (10.2) | [No](https://gitlab.com/groups/gitlab-org/-/epics/1817) | No | Verified only on transfer or manually using [Integrity Check Rake Task](../../raketasks/check.md) on both sites and comparing the output between them. | | [LFS objects](../../lfs/index.md) | **Yes** (10.2) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/8922) | Via Object Storage provider if supported. Native Geo support (Beta). | Verified only on transfer or manually using [Integrity Check Rake Task](../../raketasks/check.md) on both sites and comparing the output between them. GitLab versions 11.11.x and 12.0.x are affected by [a bug that prevents any new LFS objects from replicating](https://gitlab.com/gitlab-org/gitlab/-/issues/32696). | | [Personal snippets](../../../user/snippets.md) | **Yes** (10.2) | **Yes** (10.2) | No | | diff --git a/doc/administration/wikis/index.md b/doc/administration/wikis/index.md index 57bbe913216..bf6ff457ad3 100644 --- a/doc/administration/wikis/index.md +++ b/doc/administration/wikis/index.md @@ -73,3 +73,9 @@ You can also use the API to [retrieve the current value](../../api/settings.md#g ```shell curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/application/settings" ``` + +## Related topics + +- [User documentation for wikis](../../user/project/wiki/index.md) +- [Project wikis API](../../api/wikis.md) +- [Group wikis API](../../api/group_wikis.md) diff --git a/doc/api/group_wikis.md b/doc/api/group_wikis.md index 6c5e2b77f93..f0c38d4d4b9 100644 --- a/doc/api/group_wikis.md +++ b/doc/api/group_wikis.md @@ -9,7 +9,8 @@ type: reference, api > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/212199) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5. -Available only in APIv4. +The [group wikis](../user/project/wiki/index.md#group-wikis) API is available only in APIv4. +An API for [project wikis](wikis.md) is also available. ## List wiki pages diff --git a/doc/api/wikis.md b/doc/api/wikis.md index 1b8d091e3fd..569708cdfcc 100644 --- a/doc/api/wikis.md +++ b/doc/api/wikis.md @@ -9,7 +9,8 @@ type: reference, api > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13372) in GitLab 10.0. -Available only in APIv4. +The project [wikis](../user/project/wiki/index.md) API is available only in APIv4. +An API for [group wikis](group_wikis.md) is also available. ## List wiki pages diff --git a/doc/development/changelog.md b/doc/development/changelog.md index 5d2833902a4..8936f8c05ee 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -46,6 +46,7 @@ the `author` field. GitLab team members **should not**. and with `type` set to `security`. - Any user-facing change **must** have a changelog entry. This includes both visual changes (regardless of how minor), and changes to the rendered DOM which impact how a screen reader may announce the content. - Any client-facing change to our REST and GraphQL APIs **must** have a changelog entry. See the [complete list what comprises a GraphQL breaking change](api_graphql_styleguide.md#breaking-changes). +- Any change that introduces an [Advanced Search migration](elasticsearch.md#creating-a-new-advanced-search-migration) **must** have a changelog entry. - Performance improvements **should** have a changelog entry. - Changes that need to be documented in the Product Intelligence [Event Dictionary](https://about.gitlab.com/handbook/product/product-intelligence-guide/#event-dictionary) also require a changelog entry. diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 9051b621bcd..ed586b4b68f 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -249,6 +249,7 @@ requirements. 1. [Black-box tests/end-to-end tests](../testing_guide/testing_levels.md#black-box-tests-at-the-system-level-aka-end-to-end-tests) added if required. Please contact [the quality team](https://about.gitlab.com/handbook/engineering/quality/#teams) with any questions. +1. The User Experience (UX) for people not using the features isn't negatively affected. ## Dependencies diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md index 5c98dc2e473..2dde15145b4 100644 --- a/doc/development/feature_flags/index.md +++ b/doc/development/feature_flags/index.md @@ -16,7 +16,7 @@ in the GitLab codebase to conditionally enable features and test them. Features that are developed and merged behind a feature flag -should not include a changelog entry. The entry should be added either in the merge +should not include a changelog entry. A changelog entry with `type: added` should be included in the merge request removing the feature flag or the merge request where the default value of the feature flag is set to enabled. If the feature contains any database migrations, it *should* include a changelog entry for the database changes. diff --git a/doc/ssh/README.md b/doc/ssh/README.md index 11ab991a7f3..38356878822 100644 --- a/doc/ssh/README.md +++ b/doc/ssh/README.md @@ -218,6 +218,7 @@ To use SSH with GitLab, copy your public key to your GitLab account. The expiration date is informational only, and does not prevent you from using the key. However, administrators can view expiration dates and use them for guidance when [deleting keys](../user/admin_area/credentials_inventory.md#delete-a-users-ssh-key). + GitLab checks all SSH keys at 02:00 AM UTC every day. It emails an expiration notice for all SSH keys that expire on the current date. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322637) in GitLab 13.11.) 1. Select **Add key**. ## Verify that you can connect diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 180bd589ac5..d880560eb51 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -20,6 +20,7 @@ Then you can: [merge requests](../project/merge_requests/reviewing_and_managing_merge_requests.md#view-merge-requests-for-all-projects-in-a-group) for all projects in the group, together in a single list view. - [Bulk edit](../group/bulk_editing/index.md) issues, epics, and merge requests. +- [Create a wiki](../project/wiki/index.md) for the group. You can also create [subgroups](subgroups/index.md). @@ -322,25 +323,6 @@ LDAP user permissions can be manually overridden by an administrator. To overrid Now you can edit the user's permissions from the **Members** page. -## Group wikis **(PREMIUM)** - -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13195) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5. - -Group wikis work the same way as [project wikis](../project/wiki/index.md). - -Group wikis can be edited by members with [Developer permissions](../../user/permissions.md#group-members-permissions) -and above. - -You can move group wiki repositories by using the [Group repository storage moves API](../../api/group_repository_storage_moves.md). - -There are a few limitations compared to project wikis: - -- Git LFS is not supported. -- Group wikis are not included in global search. -- Changes to group wikis don't show up in the group's activity feed. - -For updates, follow [the epic that tracks feature parity with project wikis](https://gitlab.com/groups/gitlab-org/-/epics/2782). - ## Transfer a group You can transfer groups in the following ways: @@ -629,6 +611,7 @@ The group's new subgroups have push rules set for them based on either: ## Related topics +- [Group wikis](../project/wiki/index.md) - [Maximum artifacts size](../admin_area/settings/continuous_integration.md#maximum-artifacts-size). **(FREE SELF)** - [Repositories analytics](repositories_analytics/index.md): View overall activity of all projects with code coverage. **(PREMIUM)** - [Contribution analytics](contribution_analytics/index.md): View the contributions (pushes, merge requests, diff --git a/doc/user/group/value_stream_analytics/index.md b/doc/user/group/value_stream_analytics/index.md index 96eecfb2759..18b627bc4ba 100644 --- a/doc/user/group/value_stream_analytics/index.md +++ b/doc/user/group/value_stream_analytics/index.md @@ -202,7 +202,12 @@ GitLab allows users to create multiple value streams, hide default stages and cr Stages are visually depicted as a horizontal process flow. Selecting a stage updates the content below the value stream. -This is enabled by default. If you have a self-managed instance, an +Hovering over a stage item displays a popover with the following information: + +- Start event description for the given stage +- End event description + +Horizontal path navigation is enabled by default. If you have a self-managed instance, an administrator can [open a Rails console](../../../administration/troubleshooting/navigating_gitlab_via_rails_console.md) and disable it with the following command: diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 2fe9df9f29f..e858324b889 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -39,12 +39,13 @@ usernames. A GitLab administrator can configure the GitLab instance to NOTE: In GitLab 11.0, the Master role was renamed to Maintainer. -While Maintainer is the highest project-level role, some actions can only be performed by a personal namespace or group owner, -or an instance administrator, who receives all permissions. For more information, see [projects members documentation](project/members/index.md). +The Owner permission is only available at the group or personal namespace level (and for instance administrators) and is inherited by its projects. +While Maintainer is the highest project-level role, some actions can only be performed by a personal namespace or group owner, or an instance administrator, who receives all permissions. +For more information, see [projects members documentation](project/members/index.md). The following table depicts the various user permission levels in a project. -| Action | Guest | Reporter | Developer |Maintainer| Owner (*10*) | +| Action | Guest | Reporter | Developer |Maintainer| Owner | |---------------------------------------------------|---------|------------|-------------|----------|--------| | Download project | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | @@ -170,10 +171,10 @@ The following table depicts the various user permission levels in a project. | Manage Terraform state | | | | ✓ | ✓ | | Manage license policy **(ULTIMATE)** | | | | ✓ | ✓ | | Edit comments (posted by any user) | | | | ✓ | ✓ | -| Reposition comments on images (posted by any user)|✓ (*11*) | ✓ (*11*) | ✓ (*11*) | ✓ | ✓ | +| Reposition comments on images (posted by any user)|✓ (*10*) | ✓ (*10*) | ✓ (*10*) | ✓ | ✓ | | Manage Error Tracking | | | | ✓ | ✓ | | Delete wiki pages | | | | ✓ | ✓ | -| View project Audit Events | | | ✓ (*12*) | ✓ | ✓ | +| View project Audit Events | | | ✓ (*11*) | ✓ | ✓ | | Manage [push rules](../push_rules/push_rules.md) | | | | ✓ | ✓ | | Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** | | | | ✓ | ✓ | | View 2FA status of members | | | | ✓ | ✓ | @@ -201,7 +202,6 @@ The following table depicts the various user permission levels in a project. 1. When [Share Group Lock](group/index.md#prevent-a-project-from-being-shared-with-groups) is enabled the project can't be shared with other groups. It does not affect group with group sharing. 1. For information on eligible approvers for merge requests, see [Eligible approvers](project/merge_requests/merge_request_approvals.md#eligible-approvers). -1. Owner permission is only available at the group or personal namespace level (and for instance admins) and is inherited by its projects. 1. Applies only to comments on [Design Management](project/issues/design_management.md) designs. 1. Users can only view events based on their individual actions. @@ -209,7 +209,7 @@ The following table depicts the various user permission levels in a project. ### Wiki and issues -Project features like wiki and issues can be hidden from users depending on +Project features like [wikis](project/wiki/index.md) and issues can be hidden from users depending on which visibility level you select on project settings. - Disabled: disabled for everyone diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md index 057c2930706..d9c21c8e3c4 100644 --- a/doc/user/project/members/index.md +++ b/doc/user/project/members/index.md @@ -78,7 +78,8 @@ want to add. ![Search for people](img/add_user_search_people_v13_8.png) Select the user and the [permission level](../../permissions.md) -that you'd like to give the user. Note that you can select more than one user. +that you'd like to give the user. You can add more than one user at a time. +The Owner role can only be assigned at the group level. ![Give user permissions](img/add_user_give_permissions_v13_8.png) diff --git a/doc/user/project/wiki/img/wiki_create_home_page.png b/doc/user/project/wiki/img/wiki_create_home_page.png deleted file mode 100644 index 658af33d76e..00000000000 Binary files a/doc/user/project/wiki/img/wiki_create_home_page.png and /dev/null differ diff --git a/doc/user/project/wiki/img/wiki_create_new_page.png b/doc/user/project/wiki/img/wiki_create_new_page.png deleted file mode 100644 index 8954ec0d3a8..00000000000 Binary files a/doc/user/project/wiki/img/wiki_create_new_page.png and /dev/null differ diff --git a/doc/user/project/wiki/index.md b/doc/user/project/wiki/index.md index 224042406cb..2550543b9d4 100644 --- a/doc/user/project/wiki/index.md +++ b/doc/user/project/wiki/index.md @@ -7,68 +7,61 @@ type: reference, how-to # Wiki **(FREE)** -A separate system for documentation called Wiki, is built right into each -GitLab project. It is enabled by default on all new projects and you can find -it under **Wiki** in your project. +If you don't want to keep your documentation in your repository, but you do want +to keep it in the same project as your code, you can use the wiki GitLab provides +in each GitLab project. Every wiki is a separate Git repository, so you can create +wiki pages in the web interface, or [locally using Git](#create-or-edit-wiki-pages-locally). -Wikis are very convenient if you don't want to keep your documentation in your -repository, but you do want to keep it in the same project where your code -resides. - -You can create Wiki pages in the web interface or -[locally using Git](#create-or-edit-wiki-pages-locally) since every Wiki is -a separate Git repository. - -[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13195) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5, -**group wikis** became available. Their usage is similar to project wikis, with a few [limitations](../../group/index.md#group-wikis). +To access the wiki for a project or group, go to the page for your project or group +and, in the left sidebar, select **Wiki**. If **Wiki** is not listed in the +left sidebar, a project administrator has [disabled it](#enable-or-disable-a-project-wiki). ## Create the wiki home page -The first time you visit a Wiki, you are directed to create the Home page. -The Home page is necessary to be created because it serves as the landing page -when viewing a Wiki. Complete the **Content** section, and then select -**Create page**. You can always edit it later, so go ahead and write a welcome -message. +When a wiki is created, it is empty. On your first visit, GitLab instructs you +to create a page to serve as the landing page when a user views the wiki: -![New home page](img/wiki_create_home_page.png) +1. Select a **Format** for [styling your text](#style-your-wiki-content). +1. Add a welcome message in the **Content** section. You can always edit it later. +1. Add a **Commit message**. Git requires a commit message, so GitLab creates one + if you don't enter one yourself. +1. Select **Create page**. ## Create a new wiki page -NOTE: -Requires Developer [permissions](../../permissions.md). +Users with Developer [permissions](../../permissions.md) can create new wiki pages: -Create a new page by selecting the **New page** button that can be found -in all wiki pages. +1. Go to the page for your project or group. +1. In the left sidebar, select **Wiki**. +1. Select **New page** on this page, or any other wiki page. +1. Select a [content format](#style-your-wiki-content). +1. Add a title for your new page. You can specify a full path for the wiki page + by using `/` in the title to indicate subdirectories. GitLab creates any missing + subdirectories in the path. For example, a title of `docs/my-page` creates a wiki + page with a path `/wikis/docs/my-page`. +1. Add content to your wiki page. +1. Add a **Commit message**. Git requires a commit message, so GitLab creates one + if you don't enter one yourself. +1. Select **Create page**. -Enter a title for your new wiki page. +## Style your wiki content -You can specify a full path for the wiki page by using '/' in the -title to indicate subdirectories. Any missing directories are created -automatically. For example, a title of `docs/my-page` creates a wiki -page with a path `/wikis/docs/my-page`. +GitLab wikis support Markdown, RDoc, AsciiDoc, and Org for content. -After you enter the page name, it's time to fill in its content. GitLab wikis -support Markdown, RDoc, AsciiDoc, and Org. For Markdown based pages, all the -[Markdown features](../../markdown.md) are supported and for links there is -some [wiki specific](../../markdown.md#wiki-specific-markdown) behavior. - -In the web interface the commit message is optional, but the GitLab Wiki is -based on Git and needs a commit message, so one is created for you if you -don't enter one. - -When you're ready, select **Create page** and the new page is created. - -![New page](img/wiki_create_new_page.png) +Wiki pages written in Markdown support all [Markdown features](../../markdown.md), +and also provide some [wiki-specific behavior](../../markdown.md#wiki-specific-markdown) +for links. ### Store attachments for wiki pages > - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/33475) in GitLab 11.3. -Any file uploaded to the wiki with the GitLab -interface is stored in the wiki Git repository, and is available -if you clone the wiki repository locally. All uploaded files prior to GitLab -11.3 are stored in GitLab itself. If you want them to be part of the wiki's Git -repository, you must upload them again. +When you upload a file to the wiki through the GitLab interface, the file is stored +in the wiki's Git repository. The file is available to you if you clone the +wiki repository locally. + +Files uploaded to a wiki in GitLab 11.3 and earlier are stored in GitLab itself. +You must re-upload the files to add them to the wiki's Git repository. ### Special characters in page titles @@ -226,6 +219,36 @@ Example for `_sidebar` (using Markdown format): Support for displaying a generated table of contents with a custom side navigation is planned. +## Group wikis **(PREMIUM)** + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13195) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5. + +Group wikis work the same way as project wikis. Their usage is similar to project wikis, with a few limitations. + +Group wikis can be edited by members with [Developer permissions](../../permissions.md#group-members-permissions) +and above. + +You can move group wiki repositories by using the [Group repository storage moves API](../../../api/group_repository_storage_moves.md). + +There are a few limitations compared to project wikis: + +- Git LFS is not supported. +- Group wikis are not included in global search. +- Changes to group wikis don't show up in the group's activity feed. + +For updates, follow [the epic that tracks feature parity with project wikis](https://gitlab.com/groups/gitlab-org/-/epics/2782). + +## Enable or disable a project wiki + +Wikis are enabled by default in GitLab. Project [administrators](../../permissions.md) +can enable or disable the project wiki by following the instructions in +[Sharing and permissions](../settings/index.md#sharing-and-permissions). + +Administrators for self-managed GitLab installs can +[configure additional wiki settings](../../../administration/wikis/index.md). + ## Resources -- [Group wikis](../../group/index.md#group-wikis) +- [Wiki settings for administrators](../../../administration/wikis/index.md) +- [Project wikis API](../../../api/wikis.md) +- [Group wikis API](../../../api/group_wikis.md) diff --git a/doc/user/snippets.md b/doc/user/snippets.md index f0ea3173c82..e0ed5321bd5 100644 --- a/doc/user/snippets.md +++ b/doc/user/snippets.md @@ -127,7 +127,7 @@ A single snippet can support up to 10 files, which helps keep related files toge If you need more than 10 files for your snippet, we recommend you a create a [wiki](project/wiki/index.md) instead. Wikis are available for projects at all -subscription levels, and [groups](group/index.md#group-wikis) for +subscription levels, and [groups](project/wiki/index.md#group-wikis) for [GitLab Premium](https://about.gitlab.com/pricing). Snippets with multiple files display a file count in the [snippet list](http://snippets.gitlab.com/): diff --git a/lib/api/users.rb b/lib/api/users.rb index 7bc0f3c2dab..e2d4eb69f1e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -571,8 +571,6 @@ module API end # rubocop: disable CodeReuse/ActiveRecord delete ":id", feature_category: :users do - Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/issues/20757') - authenticated_as_admin! user = User.find_by(id: params[:id]) diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake index bf18332a8eb..3572fd0d080 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -5,21 +5,21 @@ return if Rails.env.production? namespace :spec do desc 'GitLab | RSpec | Run unit tests' RSpec::Core::RakeTask.new(:unit, :rspec_opts) do |t, args| - require_dependency 'quality/test_level' + require_test_level t.pattern = Quality::TestLevel.new.pattern(:unit) t.rspec_opts = args[:rspec_opts] end desc 'GitLab | RSpec | Run integration tests' RSpec::Core::RakeTask.new(:integration, :rspec_opts) do |t, args| - require_dependency 'quality/test_level' + require_test_level t.pattern = Quality::TestLevel.new.pattern(:integration) t.rspec_opts = args[:rspec_opts] end desc 'GitLab | RSpec | Run system tests' RSpec::Core::RakeTask.new(:system, :rspec_opts) do |t, args| - require_dependency 'quality/test_level' + require_test_level t.pattern = Quality::TestLevel.new.pattern(:system) t.rspec_opts = args[:rspec_opts] end @@ -28,4 +28,8 @@ namespace :spec do RSpec::Core::RakeTask.new(:api) do |t| t.pattern = 'spec/requests/api/**/*_spec.rb' end + + def require_test_level + require_relative '../../tooling/quality/test_level' + end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a197e17b164..6744bceb66d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18804,9 +18804,6 @@ msgstr "" msgid "Mark as ready" msgstr "" -msgid "Mark as resolved" -msgstr "" - msgid "Mark this issue as a duplicate of another issue" msgstr "" @@ -23300,6 +23297,9 @@ msgstr "" msgid "Prevent users from modifying MR approval rules in projects and merge requests." msgstr "" +msgid "Prevent users from modifying MR approval rules." +msgstr "" + msgid "Prevent users from performing write operations on GitLab while performing maintenance." msgstr "" @@ -30900,6 +30900,9 @@ msgstr "" msgid "There was an error when subscribing to this label." msgstr "" +msgid "There was an error when trying to sync your license. Please verify that your instance is using an active license key." +msgstr "" + msgid "There was an error when unsubscribing from this label." msgstr "" @@ -34815,6 +34818,12 @@ msgstr "" msgid "You can create a new one or check them in your %{pat_link_start}personal access tokens%{pat_link_end} settings" msgstr "" +msgid "You can create a new one or check them in your %{ssh_key_link_start}SSH keys%{ssh_key_link_end} settings." +msgstr "" + +msgid "You can create a new one or check them in your SSH keys settings %{ssh_key_link}." +msgstr "" + msgid "You can create a new one or check them in your personal access tokens settings %{pat_link}" msgstr "" @@ -35310,12 +35319,18 @@ msgstr "" msgid "Your Public Email will be displayed on your public profile." msgstr "" +msgid "Your SSH key has expired" +msgstr "" + msgid "Your SSH key was deleted" msgstr "" msgid "Your SSH keys (%{count})" msgstr "" +msgid "Your SSH keys with the following fingerprints has expired:" +msgstr "" + msgid "Your To-Do List" msgstr "" @@ -35436,6 +35451,9 @@ msgstr "" msgid "Your license is valid from" msgstr "" +msgid "Your license was successfully synced." +msgstr "" + msgid "Your license will be included in your GitLab backup and will survive upgrades, so in normal usage you should never need to re-upload your %{code_open}.gitlab-license%{code_close} file." msgstr "" diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index d9957fb6ced..6ec19327942 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -109,7 +109,7 @@ function rspec_paralellized_job() { cp "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" "${KNAPSACK_REPORT_PATH}" if [[ -z "${KNAPSACK_TEST_FILE_PATTERN}" ]]; then - pattern=$(ruby -r./lib/quality/test_level.rb -e "puts Quality::TestLevel.new(%(${spec_folder_prefix})).pattern(:${test_level})") + pattern=$(ruby -r./tooling/quality/test_level.rb -e "puts Quality::TestLevel.new(%(${spec_folder_prefix})).pattern(:${test_level})") export KNAPSACK_TEST_FILE_PATTERN="${pattern}" fi diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb index f60d7da6a30..a90ff3721d3 100644 --- a/spec/features/discussion_comments/merge_request_spec.rb +++ b/spec/features/discussion_comments/merge_request_spec.rb @@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do let(:merge_request) { create(:merge_request, source_project: project) } before do - stub_feature_flags(remove_resolve_note: false) - project.add_maintainer(user) sign_in(user) diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index a4e9df604a9..34d78880991 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -18,10 +18,6 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j end end - before do - stub_feature_flags(remove_resolve_note: false) - end - describe 'as a user with access to the project' do before do project.add_maintainer(user) @@ -37,7 +33,7 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j context 'resolving the thread' do before do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end it 'hides the link for creating a new issue' do diff --git a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb index 99dc71f0559..ac3471e8401 100644 --- a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb @@ -14,10 +14,6 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue', "a[title=\"#{title}\"][href=\"#{url}\"]" end - before do - stub_feature_flags(remove_resolve_note: false) - end - describe 'As a user with access to the project' do before do project.add_maintainer(user) @@ -39,7 +35,7 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue', context 'resolving the thread' do before do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end it 'hides the link for creating a new issue' do diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index caa04059469..9a3f97a0943 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -15,10 +15,6 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do diff_refs: merge_request.diff_refs) end - before do - stub_feature_flags(remove_resolve_note: false) - end - context 'no threads' do before do project.add_maintainer(user) @@ -67,7 +63,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark thread as resolved' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end expect(page).to have_selector('.discussion-body', visible: false) @@ -84,7 +80,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to unresolve thread' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click click_button 'Unresolve thread' end @@ -96,7 +92,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do describe 'resolved thread' do before do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end visit_merge_request @@ -197,7 +193,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to resolve from reply form without a comment' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end page.within '.line-resolve-all-container' do @@ -234,7 +230,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'hides jump to next button when all resolved' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end expect(page).to have_selector('.discussion-next-btn', visible: false) @@ -264,7 +260,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do visit_merge_request end - it 'does not mark thread as resolved when resolving single note' do + it 'marks thread as resolved when resolving single note' do page.within("#note_#{note.id}") do first('.line-resolve-btn').click @@ -273,15 +269,13 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do expect(first('.line-resolve-btn')['aria-label']).to eq("Resolved by #{user.name}") end - expect(page).to have_content('Last updated') - page.within '.line-resolve-all-container' do - expect(page).to have_content('1 unresolved thread') + expect(page).to have_content('All threads resolved') end end it 'resolves thread' do - resolve_buttons = page.all('.note .line-resolve-btn', count: 2) + resolve_buttons = page.all('.note .line-resolve-btn', count: 1) resolve_buttons.each do |button| button.click end @@ -332,7 +326,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark all threads as resolved' do page.all('.discussion-reply-holder', count: 2).each do |reply_holder| page.within reply_holder do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end end @@ -344,7 +338,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to quickly scroll to next unresolved thread' do page.within('.discussion-reply-holder', match: :first) do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end page.within '.line-resolve-all-container' do @@ -416,7 +410,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark thread as resolved' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click end page.within '.diff-content .note' do @@ -431,7 +425,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to unresolve thread' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click click_button 'Unresolve thread' end @@ -459,7 +453,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & unresolve thread' do page.within '.diff-content' do - click_button 'Resolve thread' + find('button[data-qa-selector="resolve_discussion_button"]').click find_field('Reply…').click diff --git a/spec/frontend/notes/components/noteable_discussion_spec.js b/spec/frontend/notes/components/noteable_discussion_spec.js index dd65351ef88..735bc2b70dd 100644 --- a/spec/frontend/notes/components/noteable_discussion_spec.js +++ b/spec/frontend/notes/components/noteable_discussion_spec.js @@ -124,14 +124,7 @@ describe('noteable_discussion component', () => { ...getJSONFixture(discussionWithTwoUnresolvedNotes)[0], expanded: true, }; - discussion.notes = discussion.notes.map((note) => ({ - ...note, - resolved: false, - current_user: { - ...note.current_user, - can_resolve: true, - }, - })); + discussion.resolved = false; wrapper.setProps({ discussion }); diff --git a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js index b18bad1ad9b..8ab0b87d2ee 100644 --- a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js +++ b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js @@ -120,8 +120,8 @@ describe('WikiForm', () => { it.each` persisted | titleHelpText | titleHelpLink - ${true} | ${'You can move this page by adding the path to the beginning of the title.'} | ${'/help/user/project/wiki/index#moving-a-wiki-page'} - ${false} | ${'You can specify the full path for the new file. We will automatically create any missing directories.'} | ${'/help/user/project/wiki/index#creating-a-new-wiki-page'} + ${true} | ${'You can move this page by adding the path to the beginning of the title.'} | ${'/help/user/project/wiki/index#move-a-wiki-page'} + ${false} | ${'You can specify the full path for the new file. We will automatically create any missing directories.'} | ${'/help/user/project/wiki/index#create-a-new-wiki-page'} `( 'shows appropriate title help text and help link for when persisted=$persisted', async ({ persisted, titleHelpLink, titleHelpText }) => { diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index a32e566fc90..764186dc226 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -212,6 +212,57 @@ RSpec.describe Emails::Profile do end end + describe 'notification email for expired ssh key' do + let_it_be(:user) { create(:user) } + let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] } + + context 'when valid' do + subject { Notify.ssh_key_expired_email(user, fingerprints) } + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + + it 'is sent to the user' do + is_expected.to deliver_to user.email + end + + it 'has the correct subject' do + is_expected.to have_subject /Your SSH key has expired/ + end + + it 'mentions the ssh keu has expired' do + is_expected.to have_body_text /Your SSH keys with the following fingerprints has expired/ + end + + it 'includes a link to ssh key page' do + is_expected.to have_body_text /#{profile_keys_url}/ + end + + it 'includes the email reason' do + is_expected.to have_body_text /You're receiving this email because of your account on localhost/ + end + end + + context 'when invalid' do + context 'when user does not exist' do + it do + expect { Notify.ssh_key_expired_email(nil) }.not_to change { ActionMailer::Base.deliveries.count } + end + end + + context 'when user is not active' do + before do + user.block! + end + + it do + expect { Notify.ssh_key_expired_email(user) }.not_to change { ActionMailer::Base.deliveries.count } + end + end + end + end + describe 'user unknown sign in email' do let_it_be(:user) { create(:user) } let_it_be(:ip) { '169.0.0.1' } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 3d33a39d353..195ec1fa83b 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -75,6 +75,18 @@ RSpec.describe Key, :mailer do .to eq([key_3, key_1, key_2]) end end + + describe '.expired_today_and_not_notified' do + let_it_be(:user) { create(:user) } + let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user) } + let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user, expiry_notification_delivered_at: Time.current) } + let_it_be(:expired_yesterday) { create(:key, expires_at: 1.day.ago, user: user) } + let_it_be(:future_expiry) { create(:key, expires_at: 1.day.from_now, user: user) } + + it 'returns tokens that have expired today' do + expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified) + end + end end context "validation of uniqueness (based on fingerprint uniqueness)" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index abd32a88db4..9498aa75289 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -85,6 +85,7 @@ RSpec.describe User do it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } + it { is_expected.to have_many(:expired_today_and_unnotified_keys) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:events).dependent(:delete_all) } @@ -1000,6 +1001,18 @@ RSpec.describe User do end end + describe '.with_ssh_key_expired_today' do + let_it_be(:user1) { create(:user) } + let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user1) } + + let_it_be(:user2) { create(:user) } + let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user2, expiry_notification_delivered_at: Time.current) } + + it 'returns users whose token has expired today' do + expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1) + end + end + describe '.active_without_ghosts' do let_it_be(:user1) { create(:user, :external) } let_it_be(:user2) { create(:user, state: 'blocked') } diff --git a/spec/services/keys/expiry_notification_service_spec.rb b/spec/services/keys/expiry_notification_service_spec.rb new file mode 100644 index 00000000000..4cfd576a15f --- /dev/null +++ b/spec/services/keys/expiry_notification_service_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Keys::ExpiryNotificationService do + let_it_be_with_reload(:user) { create(:user) } + let_it_be_with_reload(:expired_key) { create(:key, expires_at: Time.current, user: user) } + + let(:params) { { keys: keys } } + + subject { described_class.new(user, params) } + + context 'with expired key', :mailer do + let(:keys) { user.keys } + + it 'sends a notification' do + perform_enqueued_jobs do + subject.execute + end + should_email(user) + end + + it 'uses notification service to send email to the user' do + expect_next_instance_of(NotificationService) do |notification_service| + expect(notification_service).to receive(:ssh_key_expired).with(expired_key.user, [expired_key.fingerprint]) + end + + subject.execute + end + + it 'updates notified column' do + expect { subject.execute }.to change { expired_key.reload.expiry_notification_delivered_at } + end + + context 'when user does not have permission to receive notification' do + before do + user.block! + end + + it 'does not send notification' do + perform_enqueued_jobs do + subject.execute + end + should_not_email(user) + end + + it 'does not update notified column' do + expect { subject.execute }.not_to change { expired_key.reload.expiry_notification_delivered_at } + end + end + end +end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index ad833bb7c71..63bfc090a0d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -288,6 +288,27 @@ RSpec.describe NotificationService, :mailer do end end end + + describe '#ssh_key_expired' do + let_it_be(:user) { create(:user) } + let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] } + + subject { notification.ssh_key_expired(user, fingerprints) } + + it 'sends email to the token owner' do + expect { subject }.to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email") + end + + context 'when user is not allowed to receive notifications' do + before do + user.block! + end + + it 'does not send email to the token owner' do + expect { subject }.not_to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email") + end + end + end end describe '#unknown_sign_in' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d12b960d4fc..84c38ca0ce2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,7 +25,7 @@ ENV["RAILS_ENV"] = 'test' ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true' -require File.expand_path('../config/environment', __dir__) +require_relative '../config/environment' require 'rspec/mocks' require 'rspec/rails' @@ -72,6 +72,8 @@ Dir[Rails.root.join("spec/support/shared_contexts/*.rb")].sort.each { |f| requir Dir[Rails.root.join("spec/support/shared_examples/*.rb")].sort.each { |f| require f } Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f } +require_relative '../tooling/quality/test_level' + quality_level = Quality::TestLevel.new RSpec.configure do |config| diff --git a/spec/support/shared_examples/features/discussion_comments_shared_example.rb b/spec/support/shared_examples/features/discussion_comments_shared_example.rb index 86ba2821c78..808e0be6be2 100644 --- a/spec/support/shared_examples/features/discussion_comments_shared_example.rb +++ b/spec/support/shared_examples/features/discussion_comments_shared_example.rb @@ -304,7 +304,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re let(:reply_id) { find("#{comments_selector} .note:last-of-type", match: :first)['data-note-id'] } it 'can be replied to after resolving' do - click_button "Resolve thread" + find('button[data-qa-selector="resolve_discussion_button"]').click wait_for_requests refresh @@ -316,7 +316,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re it 'shows resolved thread when toggled' do submit_reply('a') - click_button "Resolve thread" + find('button[data-qa-selector="resolve_discussion_button"]').click wait_for_requests expect(page).to have_selector(".note-row-#{note_id}", visible: true) diff --git a/spec/lib/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb similarity index 98% rename from spec/lib/quality/test_level_spec.rb rename to spec/tooling/quality/test_level_spec.rb index 32960cd571b..89abe337347 100644 --- a/spec/lib/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative '../../../tooling/quality/test_level' RSpec.describe Quality::TestLevel do describe '#pattern' do @@ -197,7 +197,7 @@ RSpec.describe Quality::TestLevel do it 'raises an error for an unknown level' do expect { subject.level_for('spec/unknown/foo_spec.rb') } .to raise_error(described_class::UnknownTestLevelError, - %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/lib/quality/test_level.rb.}) + %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/tooling/quality/test_level.rb.}) end end diff --git a/spec/workers/ssh_keys/expired_notification_worker_spec.rb b/spec/workers/ssh_keys/expired_notification_worker_spec.rb new file mode 100644 index 00000000000..249ee404870 --- /dev/null +++ b/spec/workers/ssh_keys/expired_notification_worker_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do + subject(:worker) { described_class.new } + + it 'uses a cronjob queue' do + expect(worker.sidekiq_options_hash).to include( + 'queue' => 'cronjob:ssh_keys_expired_notification', + 'queue_namespace' => :cronjob + ) + end + + describe '#perform' do + let_it_be(:user) { create(:user) } + + context 'with expiring key today' do + let_it_be_with_reload(:expired_today) { create(:key, expires_at: Time.current, user: user) } + + it 'invoke the notification service' do + expect_next_instance_of(Keys::ExpiryNotificationService) do |expiry_service| + expect(expiry_service).to receive(:execute) + end + + worker.perform + end + + it 'updates notified column' do + expect { worker.perform }.to change { expired_today.reload.expiry_notification_delivered_at } + end + + include_examples 'an idempotent worker' do + subject do + perform_multiple(worker: worker) + end + end + + context 'when feature is not enabled' do + before do + stub_feature_flags(ssh_key_expiration_email_notification: false) + end + + it 'does not update notified column' do + expect { worker.perform }.not_to change { expired_today.reload.expiry_notification_delivered_at } + end + end + end + + context 'when key has expired in the past' do + let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) } + + it 'does not update notified column' do + expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at } + end + end + end +end diff --git a/lib/quality/test_level.rb b/tooling/quality/test_level.rb similarity index 100% rename from lib/quality/test_level.rb rename to tooling/quality/test_level.rb