From e49bd57279b72cf517853aec369e341fa3442d60 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 7 Jul 2020 21:09:13 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../Security developer workflow.md | 9 +- .../Security Release.md | 22 +- Gemfile | 2 +- Gemfile.lock | 4 +- app/models/merge_request_diff.rb | 20 +- ...e-tempfile-from-external-diff-creation.yml | 5 - ...move-manage-stage-usage-activity-to-ce.yml | 5 + ...rop-old-non-unique-index-on-mr-metrics.yml | 5 + changelogs/unreleased/update-rack-timeout.yml | 5 - danger/roulette/Dangerfile | 20 +- ...drop_old_non_unique_index_on_mr_metrics.rb | 18 ++ db/structure.sql | 3 +- .../monitoring/prometheus/gitlab_metrics.md | 1 + doc/api/import.md | 2 +- doc/development/dangerbot.md | 14 +- doc/development/telemetry/usage_ping.md | 12 ++ .../project/integrations/generic_alerts.md | 15 +- lib/carrier_wave_string_file.rb | 8 +- .../Security/Coverage-Fuzzing.gitlab-ci.yml | 33 +++ lib/gitlab/danger/roulette.rb | 30 ++- lib/gitlab/danger/teammate.rb | 44 +++- lib/gitlab/usage_data.rb | 26 ++- .../jira/jira_basic_integration_spec.rb | 21 +- .../renderers/build_uneditable_token_spec.js | 2 +- .../services/renderers/mock_data.js | 45 ++++ .../renderers/render_embedded_ruby_spec.js | 4 +- .../renderers/render_kramdown_list_spec.js | 4 +- .../renderers/render_kramdown_text_spec.js | 4 +- spec/lib/gitlab/danger/roulette_spec.rb | 197 ++++++++++++------ spec/lib/gitlab/danger/teammate_spec.rb | 82 +++++++- spec/lib/gitlab/usage_data_spec.rb | 36 ++++ 31 files changed, 548 insertions(+), 150 deletions(-) delete mode 100644 changelogs/unreleased/216458-remove-tempfile-from-external-diff-creation.yml create mode 100644 changelogs/unreleased/217362-move-manage-stage-usage-activity-to-ce.yml create mode 100644 changelogs/unreleased/drop-old-non-unique-index-on-mr-metrics.yml delete mode 100644 changelogs/unreleased/update-rack-timeout.yml create mode 100644 db/migrate/20200707071941_drop_old_non_unique_index_on_mr_metrics.rb create mode 100644 lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml create mode 100644 spec/frontend/vue_shared/components/rich_content_editor/services/renderers/mock_data.js diff --git a/.gitlab/issue_templates/Security developer workflow.md b/.gitlab/issue_templates/Security developer workflow.md index 64d5285f73d..7de137bd2e2 100644 --- a/.gitlab/issue_templates/Security developer workflow.md +++ b/.gitlab/issue_templates/Security developer workflow.md @@ -9,19 +9,17 @@ Set the title to: `Description of the original issue` ## Prior to starting the security release work - [ ] Read the [security process for developers] if you are not familiar with it. -- [ ] Mark this [issue as related] to the Security Release tracking issue. You can find it on the topic of the `#releases` Slack channel. -- [ ] Run `scripts/security-harness` in your local repository to prevent accidentally pushing to any remote besides `gitlab.com/gitlab-org/security`. +- [ ] Mark this [issue as related] to the Security Release Tracking Issue. You can find it on the topic of the `#releases` Slack channel. - Fill out the [Links section](#links): - [ ] Next to **Issue on GitLab**, add a link to the `gitlab-org/gitlab` issue that describes the security vulnerability. - - [ ] Next to **Security Release tracking issue**, add a link to the security release issue that will include this security issue. ## Development +- [ ] Run `scripts/security-harness` in your local repository to prevent accidentally pushing to any remote besides `gitlab.com/gitlab-org/security`. - [ ] Create a new branch prefixing it with `security-`. - [ ] Create a merge request targeting `master` on `gitlab.com/gitlab-org/security` and use the [Security Release merge request template]. -- [ ] Follow the same [code review process]: Assign to a reviewer, then to a maintainer. -After your merge request has been approved according to our [approval guidelines], you're ready to prepare the backports +After your merge request has been approved according to our [approval guidelines] and by a team member of the AppSec team, you're ready to prepare the backports ## Backports @@ -49,7 +47,6 @@ After your merge request has been approved according to our [approval guidelines | Description | Link | | -------- | -------- | | Issue on [GitLab](https://gitlab.com/gitlab-org/gitlab/issues) | #TODO | -| Security Release tracking issue | #TODO | ### Details diff --git a/.gitlab/merge_request_templates/Security Release.md b/.gitlab/merge_request_templates/Security Release.md index f852bebae95..bdf26041e62 100644 --- a/.gitlab/merge_request_templates/Security Release.md +++ b/.gitlab/merge_request_templates/Security Release.md @@ -13,25 +13,33 @@ See [the general developer security release guidelines](https://gitlab.com/gitla ## Developer checklist - [ ] **On "Related issues" section, write down the [GitLab Security] issue it belongs to (i.e. `Related to `).** -- [ ] Merge request targets `master`, or `X-Y-stable` for backports. +- [ ] Merge request targets `master`, or a versioned stable branch (`X-Y-stable-ee`). - [ ] Milestone is set for the version this merge request applies to. A closed milestone can be assigned via [quick actions]. - [ ] Title of this merge request is the same as for all backports. -- [ ] A [CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html) is added without a `merge_request` value, with `type` set to `security` -- [ ] Assign to a reviewer and maintainer, per our [Code Review process]. +- [ ] A [CHANGELOG entry] is added without a `merge_request` value, with `type` set to `security` - [ ] For the MR targeting `master`: - - [ ] Ask for a non-blocking review from the AppSec team member associated to the issue in the [Canonical repository](https://gitlab.com/gitlab-org/gitlab). If you're unsure who to ping, ask on `#sec-appsec` Slack channel. + - [ ] Assign to a reviewer and maintainer, per our [Code Review process]. - [ ] Ensure it's approved according to our [Approval Guidelines]. -- [ ] Merge request _must not_ close the corresponding security issue, _unless_ it targets `master`. + - [ ] Ensure it's approved by an AppSec engineer. + - If you're unsure who should approve, find the AppSec engineer associated to the issue in the [Canonical repository], or ask #sec-appsec on Slack. + - Trigger the [`package-and-qa` build]. The docker image generated will be used by the AppSec engineer to validate the security vulnerability has been remediated. + - [ ] Merge request _must_ close the corresponding security issue. +- [ ] For a backport MR targeting a versioned stable branch (`X-Y-stable-ee`) + - [ ] Ensure it's approved by a maintainer. **Note:** Reviewer/maintainer should not be a Release Manager ## Maintainer checklist + - [ ] Correct milestone is applied and the title is matching across all backports - [ ] Assigned to `@gitlab-release-tools-bot` with passing CI pipelines and **when all backports including the MR targeting master are ready.** /label ~security [GitLab Security]: https://gitlab.com/gitlab-org/security/gitlab -[approval guidelines]: https://docs.gitlab.com/ee/development/code_review.html#approval-guidelines -[Code Review process]: https://docs.gitlab.com/ee/development/code_review.html [quick actions]: https://docs.gitlab.com/ee/user/project/quick_actions.html#quick-actions-for-issues-merge-requests-and-epics +[CHANGELOG entry]: https://docs.gitlab.com/ee/development/changelog.html +[Code Review process]: https://docs.gitlab.com/ee/development/code_review.html +[Approval Guidelines]: https://docs.gitlab.com/ee/development/code_review.html#approval-guidelines +[Canonical repository]: https://gitlab.com/gitlab-org/gitlab +[`package-and-qa` build]: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/#using-the-package-and-qa-job diff --git a/Gemfile b/Gemfile index 006872eaefd..c86874201fc 100644 --- a/Gemfile +++ b/Gemfile @@ -164,7 +164,6 @@ gem 'diff_match_patch', '~> 0.1.0' # Application server gem 'rack', '~> 2.0.9' -gem 'rack-timeout', '~> 0.5.1' group :unicorn do gem 'unicorn', '~> 5.5' @@ -174,6 +173,7 @@ end group :puma do gem 'gitlab-puma', '~> 4.3.3.gitlab.2', require: false gem 'gitlab-puma_worker_killer', '~> 0.1.1.gitlab.1', require: false + gem 'rack-timeout', require: false end # State machine diff --git a/Gemfile.lock b/Gemfile.lock index f68fd455352..70f2dfafe3a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -817,7 +817,7 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) - rack-timeout (0.5.2) + rack-timeout (0.5.1) rails (6.0.3.1) actioncable (= 6.0.3.1) actionmailbox (= 6.0.3.1) @@ -1350,7 +1350,7 @@ DEPENDENCIES rack-cors (~> 1.0.6) rack-oauth2 (~> 1.9.3) rack-proxy (~> 0.6.0) - rack-timeout (~> 0.5.1) + rack-timeout rails (~> 6.0.3.1) rails-controller-testing rails-i18n (~> 6.0) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index ba51f2bd822..eb5250d5cf6 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -493,6 +493,8 @@ class MergeRequestDiff < ApplicationRecord self.stored_externally = true rows + ensure + tempfile&.unlink end def create_merge_request_diff_files(rows) @@ -503,19 +505,17 @@ class MergeRequestDiff < ApplicationRecord end def build_external_diff_tempfile(rows) - pos = 0 + Tempfile.open(external_diff.filename) do |file| + rows.each do |row| + data = row.delete(:diff) + row[:external_diff_offset] = file.pos + row[:external_diff_size] = data.bytesize - segments = rows.map do |row| - segment = row.delete(:diff) + file.write(data) + end - row[:external_diff_offset] = pos - row[:external_diff_size] = segment.bytesize - pos += segment.bytesize - - segment + file end - - CarrierWaveStringFile.new(segments.join(''), external_diff.filename) end def build_merge_request_diff_files(diffs) diff --git a/changelogs/unreleased/216458-remove-tempfile-from-external-diff-creation.yml b/changelogs/unreleased/216458-remove-tempfile-from-external-diff-creation.yml deleted file mode 100644 index a0768fcd695..00000000000 --- a/changelogs/unreleased/216458-remove-tempfile-from-external-diff-creation.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Remove tempfile from external diff creation -merge_request: 35750 -author: -type: performance diff --git a/changelogs/unreleased/217362-move-manage-stage-usage-activity-to-ce.yml b/changelogs/unreleased/217362-move-manage-stage-usage-activity-to-ce.yml new file mode 100644 index 00000000000..b31db0b7ccc --- /dev/null +++ b/changelogs/unreleased/217362-move-manage-stage-usage-activity-to-ce.yml @@ -0,0 +1,5 @@ +--- +title: Move manage stage usage activity to CE +merge_request: 36089 +author: +type: changed diff --git a/changelogs/unreleased/drop-old-non-unique-index-on-mr-metrics.yml b/changelogs/unreleased/drop-old-non-unique-index-on-mr-metrics.yml new file mode 100644 index 00000000000..369ff3100bd --- /dev/null +++ b/changelogs/unreleased/drop-old-non-unique-index-on-mr-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Remove non-unique index on `merge_request_metrics.merge_request_id` column +merge_request: 36170 +author: +type: changed diff --git a/changelogs/unreleased/update-rack-timeout.yml b/changelogs/unreleased/update-rack-timeout.yml deleted file mode 100644 index c6a43afbf49..00000000000 --- a/changelogs/unreleased/update-rack-timeout.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Update `rack-timeout` to `0.5.2` -merge_request: 36071 -author: -type: changed diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 64faae1d35b..0ada46f8463 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -12,17 +12,19 @@ MARKDOWN CATEGORY_TABLE_HEADER = < 1 && !mr_author.reviewer?(project, spin.category, []) @@ -85,9 +90,8 @@ if changes.any? markdown_row_for_spin(spin) end - unknown = changes.fetch(:unknown, []) - - markdown(MESSAGE) markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty? + + unknown = changes.fetch(:unknown, []) markdown(UNKNOWN_FILES_MESSAGE + helper.markdown_list(unknown)) unless unknown.empty? end diff --git a/db/migrate/20200707071941_drop_old_non_unique_index_on_mr_metrics.rb b/db/migrate/20200707071941_drop_old_non_unique_index_on_mr_metrics.rb new file mode 100644 index 00000000000..aa90a0c5915 --- /dev/null +++ b/db/migrate/20200707071941_drop_old_non_unique_index_on_mr_metrics.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class DropOldNonUniqueIndexOnMrMetrics < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_merge_request_metrics' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name(:merge_request_metrics, INDEX_NAME) + end + + def down + add_concurrent_index :merge_request_metrics, :merge_request_id, name: INDEX_NAME + end +end diff --git a/db/structure.sql b/db/structure.sql index d55d42d4a60..b3cbb733313 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19443,8 +19443,6 @@ CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id ON public.merg CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id_partial ON public.merge_request_diffs USING btree (merge_request_id, id) WHERE ((NOT stored_externally) OR (stored_externally IS NULL)); -CREATE INDEX index_merge_request_metrics ON public.merge_request_metrics USING btree (merge_request_id); - CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON public.merge_request_metrics USING btree (first_deployed_to_production_at); CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON public.merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL); @@ -23588,5 +23586,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200704143633 20200706005325 20200706170536 +20200707071941 \. diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index ee9f12f85b2..c07e874c40e 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -173,6 +173,7 @@ configuration option in `gitlab.yml`. These metrics are served from the | `geo_repositories_retrying_verification_count` | Gauge | 11.2 | Number of repositories verification failures that Geo is actively trying to correct on secondary | `url` | | `geo_wikis_retrying_verification_count` | Gauge | 11.2 | Number of wikis verification failures that Geo is actively trying to correct on secondary | `url` | | `global_search_bulk_cron_queue_size` | Gauge | 12.10 | Number of database records waiting to be synchronized to Elasticsearch | | +| `global_search_awaiting_indexing_queue_size` | Gauge | 13.2 | Number of database updates waiting to be synchronized to Elasticsearch while indexing is paused | | | `package_files_count` | Gauge | 13.0 | Number of package files on primary | `url` | | `package_files_checksummed_count` | Gauge | 13.0 | Number of package files checksummed on primary | `url` | | `package_files_checksum_failed_count` | Gauge | 13.0 | Number of package files failed to calculate the checksum on primary diff --git a/doc/api/import.md b/doc/api/import.md index 3daf5e7be53..54e7eb12ed1 100644 --- a/doc/api/import.md +++ b/doc/api/import.md @@ -55,7 +55,7 @@ POST /import/bitbucket_server ```shell curl --request POST \ - --url https://gitlab.example.com/api/v4/import/bitbucket/server \ + --url https://gitlab.example.com/api/v4/import/bitbucket_server \ --header "content-type: application/json" \ --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" \ --data '{ diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index eda9e1e21cc..93434479846 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -128,10 +128,18 @@ Here is a (non-exhaustive) list of the kinds of things Danger has been used for at GitLab so far: - Coding style -- Database review workflow -- Documentation review workflow +- Database review +- Documentation review - Merge request metrics -- Reviewer roulette workflow +- Reviewer roulette. Reviewers and maintainers are chosen based on: + - Their roles (backend, frontend, database, etc). + - Their availability: + - No "OOO"/"PTO"/"Parental Leave" in their GitLab or Slack status. + - No `:red_circle:`/`:palm_tree:`/`:beach:`/`:beach_umbrella:`/`:beach_with_umbrella:` emojis in GitLab or Slack status. + - [Experimental] Their timezone: people for which the local hour is between + 6 AM and 2 PM are eligible to be picked. This is to ensure they have a good + chance to get to perform a review during their current work day. The experimentation is tracked in + [this issue](https://gitlab.com/gitlab-org/quality/team-tasks/-/issues/563) - Single codebase effort ## Limitations diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index a269e9fcc05..bb226d91923 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -655,6 +655,18 @@ appear to be associated to any of the services running, since they all appear to | `projects_prometheus_active` | `usage_activity_by_stage` | `monitor` | | EE | | | `projects_with_error_tracking_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | | `projects_with_tracing_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | +| `events` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `groups` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `users_created_at` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `omniauth_providers` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `ldap_keys` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_users` | `usage_activity_by_stage` | `manage` | | EE | | +| `value_stream_management_customized_group_stages` | `usage_activity_by_stage` | `manage` | | EE | | +| `projects_with_compliance_framework` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_servers` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_group_sync_enabled` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_admin_sync_enabled` | `usage_activity_by_stage` | `manage` | | EE | | +| `group_saml_enabled` | `usage_activity_by_stage` | `manage` | | EE | | | `keys` | `usage_activity_by_stage` | `create` | | | | | `projects_jira_dvcs_server_active` | `usage_activity_by_stage` | `plan` | | | | | `service_desk_enabled_projects` | `usage_activity_by_stage` | `plan` | | | | diff --git a/doc/user/project/integrations/generic_alerts.md b/doc/user/project/integrations/generic_alerts.md index 30524195ce6..f7c2da5f8ce 100644 --- a/doc/user/project/integrations/generic_alerts.md +++ b/doc/user/project/integrations/generic_alerts.md @@ -47,6 +47,14 @@ You can customize the payload by sending the following parameters. All fields ot | `severity` | String | The severity of the alert. Must be one of `critical`, `high`, `medium`, `low`, `info`, `unknown`. Default is `critical`. | | `fingerprint` | String or Array | The unique identifier of the alert. This can be used to group occurrences of the same alert. | +You can also add custom fields to the alert's payload. The values of extra parameters +are not limited to primitive types, such as strings or numbers, but can be a nested +JSON object. For example: + +```json +{ "foo": { "bar": { "baz": 42 } } } +``` + TIP: **Payload size:** Ensure your requests are smaller than the [payload application limits](../../../administration/instance_limits.md#generic-alert-json-payloads). @@ -73,6 +81,11 @@ Example payload: "monitoring_tool": "value", "hosts": "value", "severity": "high", - "fingerprint": "d19381d4e8ebca87b55cda6e8eee7385" + "fingerprint": "d19381d4e8ebca87b55cda6e8eee7385", + "foo": { + "bar": { + "baz": 42 + } + } } ``` diff --git a/lib/carrier_wave_string_file.rb b/lib/carrier_wave_string_file.rb index fca247dc4c6..c9a64d9e631 100644 --- a/lib/carrier_wave_string_file.rb +++ b/lib/carrier_wave_string_file.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true class CarrierWaveStringFile < StringIO - attr_reader :original_filename - - def initialize(data, original_filename = "") - super(data) - - @original_filename = original_filename + def original_filename + "" end end diff --git a/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml new file mode 100644 index 00000000000..aa30fe0c58d --- /dev/null +++ b/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml @@ -0,0 +1,33 @@ +# Read more about this feature https://docs.gitlab.com/ee/user/application_security/coverage_fuzzing + +variables: + # Which branch we want to run full fledged long running fuzzing jobs. + # All others will run fuzzing regression + COVERAGE_FUZZING_BRANCH: "$CI_DEFAULT_BRANCH" + # This is using semantic version and will always download latest v1 gitlab-cov-fuzz release + COVERAGE_FUZZING_VERSION: v1 + # This is for users who have an offline environment and will have to replicate gitlab-cov-fuzz release binaries + # to their own servers + COVERAGE_FUZZING_URL_PREFIX: "https://gitlab.com/gitlab-org/security-products/analyzers/gitlab-cov-fuzz/-/raw" + +.fuzz_base: + stage: fuzz + allow_failure: true + before_script: + - if [ -x "$(command -v apt-get)" ] ; then apt-get update && apt-get install -y wget; fi + - wget -O gitlab-cov-fuzz "${COVERAGE_FUZZING_URL_PREFIX}"/"${COVERAGE_FUZZING_VERSION}"/binaries/gitlab-cov-fuzz_Linux_x86_64 + - chmod a+x gitlab-cov-fuzz + - export REGRESSION=true + - if [[ $CI_COMMIT_BRANCH = $COVERAGE_FUZZING_BRANCH ]]; then REGRESSION=false; fi; + artifacts: + paths: + - corpus + - crashes + reports: + coverage_fuzzing: gl-coverage-fuzzing-report.json + when: always + rules: + - if: $COVERAGE_FUZZING_DISABLED + when: never + - if: $GITLAB_FEATURES =~ /\bcoverage_fuzzing\b/ + - if: $CI_RUNNER_EXECUTABLE_ARCH == "linux" diff --git a/lib/gitlab/danger/roulette.rb b/lib/gitlab/danger/roulette.rb index 380b035293c..236d4195576 100644 --- a/lib/gitlab/danger/roulette.rb +++ b/lib/gitlab/danger/roulette.rb @@ -6,6 +6,7 @@ module Gitlab module Danger module Roulette ROULETTE_DATA_URL = 'https://gitlab-org.gitlab.io/gitlab-roulette/roulette.json' + HOURS_WHEN_PERSON_CAN_BE_PICKED = (6..14).freeze Spin = Struct.new(:category, :reviewer, :maintainer, :optional_role) @@ -13,7 +14,7 @@ module Gitlab # for each change category that a Merge Request contains. # # @return [Array] - def spin(project, categories, branch_name) + def spin(project, categories, branch_name, timezone_experiment: false) team = begin project_team(project) @@ -25,7 +26,7 @@ module Gitlab canonical_branch_name = canonical_branch_name(branch_name) spin_per_category = categories.each_with_object({}) do |category, memo| - memo[category] = spin_for_category(team, project, category, canonical_branch_name) + memo[category] = spin_for_category(team, project, category, canonical_branch_name, timezone_experiment: timezone_experiment) end spin_per_category.map do |category, spin| @@ -79,9 +80,14 @@ module Gitlab # Known issue: If someone is rejected due to OOO, and then becomes not OOO, the # selection will change on next spin # @param [Array] people - def spin_for_person(people, random:) - people.shuffle(random: random) - .find(&method(:valid_person?)) + def spin_for_person(people, random:, timezone_experiment: false) + shuffled_people = people.shuffle(random: random) + + if timezone_experiment + shuffled_people.find(&method(:valid_person_with_timezone?)) + else + shuffled_people.find(&method(:valid_person?)) + end end private @@ -89,7 +95,13 @@ module Gitlab # @param [Teammate] person # @return [Boolean] def valid_person?(person) - !mr_author?(person) && person.available && person.has_capacity + !mr_author?(person) && person.available + end + + # @param [Teammate] person + # @return [Boolean] + def valid_person_with_timezone?(person) + valid_person?(person) && HOURS_WHEN_PERSON_CAN_BE_PICKED.cover?(person.local_hour) end # @param [Teammate] person @@ -104,7 +116,7 @@ module Gitlab end end - def spin_for_category(team, project, category, branch_name) + def spin_for_category(team, project, category, branch_name, timezone_experiment: false) reviewers, traintainers, maintainers = %i[reviewer traintainer maintainer].map do |role| spin_role_for_category(team, role, project, category) @@ -115,8 +127,8 @@ module Gitlab # Make traintainers have triple the chance to be picked as a reviewer random = new_random(branch_name) - reviewer = spin_for_person(reviewers + traintainers + traintainers, random: random) - maintainer = spin_for_person(maintainers, random: random) + reviewer = spin_for_person(reviewers + traintainers + traintainers, random: random, timezone_experiment: timezone_experiment) + maintainer = spin_for_person(maintainers, random: random, timezone_experiment: timezone_experiment) Spin.new(category, reviewer, maintainer) end diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb index c732b19a535..3a4d4b1ba46 100644 --- a/lib/gitlab/danger/teammate.rb +++ b/lib/gitlab/danger/teammate.rb @@ -3,7 +3,7 @@ module Gitlab module Danger class Teammate - attr_reader :username, :name, :markdown_name, :role, :projects, :available, :has_capacity + attr_reader :username, :name, :role, :projects, :available, :tz_offset_hours # The options data are produced by https://gitlab.com/gitlab-org/gitlab-roulette/-/blob/master/lib/team_member.rb def initialize(options = {}) @@ -13,7 +13,7 @@ module Gitlab @role = options['role'] @projects = options['projects'] @available = options['available'] - @has_capacity = options['has_capacity'] + @tz_offset_hours = options['tz_offset_hours'] end def in_project?(name) @@ -34,8 +34,48 @@ module Gitlab has_capability?(project, category, :maintainer, labels) end + def markdown_name(timezone_experiment: false, author: nil) + return @markdown_name unless timezone_experiment + + "#{@markdown_name} (#{utc_offset_text(author)})" + end + + def local_hour + (Time.now.utc + tz_offset_hours * 3600).hour + end + + protected + + def floored_offset_hours + floored_offset = tz_offset_hours.floor(0) + + floored_offset == tz_offset_hours ? floored_offset : tz_offset_hours + end + private + def utc_offset_text(author = nil) + offset_text = + if floored_offset_hours >= 0 + "UTC+#{floored_offset_hours}" + else + "UTC#{floored_offset_hours}" + end + + return offset_text unless author + + "#{offset_text}, #{offset_diff_compared_to_author(author)}" + end + + def offset_diff_compared_to_author(author) + diff = floored_offset_hours - author.floored_offset_hours + return "same timezone as `@#{author.username}`" if diff.zero? + + ahead_or_behind = diff < 0 ? 'behind' : 'ahead' + + "#{diff.abs} hours #{ahead_or_behind} `@#{author.username}`" + end + def has_capability?(project, category, kind, labels) case category when :test diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 7c51f0d422e..ba5eafc2b88 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -420,9 +420,8 @@ module Gitlab distinct_count( query, :author_id, - batch_size: 5_000, # Based on query performance, this is the optimal batch size. - start: User.minimum(:id), - finish: User.maximum(:id) + start: user_minimum_id, + finish: user_maximum_id ) end # rubocop: enable CodeReuse/ActiveRecord @@ -486,9 +485,16 @@ module Gitlab end # Omitted because no user, creator or author associated: `campaigns_imported_from_github`, `ldap_group_links` + # rubocop: disable CodeReuse/ActiveRecord def usage_activity_by_stage_manage(time_period) - {} + { + events: distinct_count(::Event.where(time_period), :author_id), + groups: distinct_count(::GroupMember.where(time_period), :user_id), + users_created: count(::User.where(time_period), start: user_minimum_id, finish: user_maximum_id), + omniauth_providers: filtered_omniauth_provider_names.reject { |name| name == 'group_saml' } + } end + # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord def usage_activity_by_stage_monitor(time_period) @@ -596,6 +602,18 @@ module Gitlab distinct_count(clusters.where(time_period), :user_id) end # rubocop: enable CodeReuse/ActiveRecord + + def omniauth_provider_names + ::Gitlab.config.omniauth.providers.map(&:name) + end + + # LDAP provider names are set by customers and could include + # sensitive info (server names, etc). LDAP providers normally + # don't appear in omniauth providers but filter to ensure + # no internal details leak via usage ping. + def filtered_omniauth_provider_names + omniauth_provider_names.reject { |name| name.starts_with?('ldap') } + end end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb b/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb index 05a932fd53e..1dfb3150901 100644 --- a/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/jira/jira_basic_integration_spec.rb @@ -6,18 +6,19 @@ module QA describe 'Jira integration', :jira, :orchestrated, :requires_admin do let(:jira_project_key) { 'JITP' } + let(:project) do + Resource::Project.fabricate_via_api! do |project| + project.name = "project_with_jira_integration" + end + end - before(:all) do + before do page.visit Vendor::Jira::JiraAPI.perform(&:base_url) QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, raise_on_failure: true) do page.has_text? 'Welcome to Jira' end - @project = Resource::Project.fabricate_via_api! do |project| - project.name = "project_with_jira_integration" - end - # Retry is required because allow_local_requests_from_web_hooks_and_services # takes some time to get enabled. # Bug issue: https://gitlab.com/gitlab-org/gitlab/-/issues/217010 @@ -27,7 +28,7 @@ module QA page.visit Runtime::Scenario.gitlab_address Flow::Login.sign_in_unless_signed_in - @project.visit! + project.visit! Page::Project::Menu.perform(&:go_to_integrations_settings) QA::Page::Project::Settings::Integrations.perform(&:click_jira_link) @@ -67,9 +68,11 @@ module QA expect_issue_done(issue_key) end + private + def create_mr_with_description(description) Resource::MergeRequest.fabricate! do |merge_request| - merge_request.project = @project + merge_request.project = project merge_request.target_new_branch = !master_branch_exists? merge_request.description = description end @@ -80,7 +83,7 @@ module QA push.branch_name = 'master' push.commit_message = commit_message push.file_content = commit_message - push.project = @project + push.project = project push.new_branch = !master_branch_exists? end end @@ -98,7 +101,7 @@ module QA end def master_branch_exists? - @project.repository_branches.map { |item| item[:name] }.include?("master") + project.repository_branches.map { |item| item[:name] }.include?("master") end end end diff --git a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token_spec.js b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token_spec.js index 51a42e0ff26..244e37f18c6 100644 --- a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token_spec.js +++ b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token_spec.js @@ -11,7 +11,7 @@ import { uneditableCloseToken, uneditableCloseTokens, uneditableTokens, -} from '../../mock_data'; +} from './mock_data'; describe('Build Uneditable Token renderer helper', () => { describe('buildUneditableOpenTokens', () => { diff --git a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/mock_data.js b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/mock_data.js new file mode 100644 index 00000000000..e021a4543d5 --- /dev/null +++ b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/mock_data.js @@ -0,0 +1,45 @@ +// Node spec helpers + +export const buildMockTextNode = literal => { + return { + firstChild: null, + literal, + type: 'text', + }; +}; + +export const buildMockListNode = literal => { + return { + firstChild: { + firstChild: { + firstChild: buildMockTextNode(literal), + type: 'paragraph', + }, + type: 'item', + }, + type: 'list', + }; +}; + +export const normalTextNode = buildMockTextNode('This is just normal text.'); +export const normalListNode = buildMockListNode('Just another bullet point'); + +// Token spec helpers + +const uneditableOpenToken = { + type: 'openTag', + tagName: 'div', + attributes: { contenteditable: false }, + classNames: [ + 'gl-px-4 gl-py-2 gl-opacity-5 gl-bg-gray-100 gl-user-select-none gl-cursor-not-allowed', + ], +}; + +export const uneditableCloseToken = { type: 'closeTag', tagName: 'div' }; +export const originToken = { + type: 'text', + content: '{:.no_toc .hidden-md .hidden-lg}', +}; +export const uneditableOpenTokens = [uneditableOpenToken, originToken]; +export const uneditableCloseTokens = [originToken, uneditableCloseToken]; +export const uneditableTokens = [...uneditableOpenTokens, uneditableCloseToken]; diff --git a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_embedded_ruby_spec.js b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_embedded_ruby_spec.js index 34a4efb6a69..b723ee8c8a0 100644 --- a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_embedded_ruby_spec.js +++ b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_embedded_ruby_spec.js @@ -1,7 +1,9 @@ import renderer from '~/vue_shared/components/rich_content_editor/services/renderers/render_embedded_ruby_text'; import { buildUneditableTokens } from '~/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token'; -import { embeddedRubyTextNode, normalTextNode } from '../../mock_data'; +import { buildMockTextNode, normalTextNode } from './mock_data'; + +const embeddedRubyTextNode = buildMockTextNode('<%= partial("some/path") %>'); describe('Render Embedded Ruby Text renderer', () => { describe('canRender', () => { diff --git a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_list_spec.js b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_list_spec.js index 8199cb9ef7b..a8b28d7ac99 100644 --- a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_list_spec.js +++ b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_list_spec.js @@ -4,7 +4,9 @@ import { buildUneditableCloseToken, } from '~/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token'; -import { kramdownListNode, normalListNode } from '../../mock_data'; +import { buildMockListNode, normalListNode } from './mock_data'; + +const kramdownListNode = buildMockListNode('TOC'); describe('Render Kramdown List renderer', () => { describe('canRender', () => { diff --git a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_text_spec.js b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_text_spec.js index 65015c4e548..97ff9794e69 100644 --- a/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_text_spec.js +++ b/spec/frontend/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_text_spec.js @@ -1,7 +1,9 @@ import renderer from '~/vue_shared/components/rich_content_editor/services/renderers/render_kramdown_text'; import { buildUneditableTokens } from '~/vue_shared/components/rich_content_editor/services/renderers/build_uneditable_token'; -import { kramdownTextNode, normalTextNode } from '../../mock_data'; +import { buildMockTextNode, normalTextNode } from './mock_data'; + +const kramdownTextNode = buildMockTextNode('{:toc}'); describe('Render Kramdown Text renderer', () => { describe('canRender', () => { diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb index 24f547a2e92..341a50a2921 100644 --- a/spec/lib/gitlab/danger/roulette_spec.rb +++ b/spec/lib/gitlab/danger/roulette_spec.rb @@ -2,10 +2,15 @@ require 'fast_spec_helper' require 'webmock/rspec' +require 'timecop' require 'gitlab/danger/roulette' RSpec.describe Gitlab::Danger::Roulette do + around do |example| + Timecop.freeze(Time.utc(2020, 06, 22, 10)) { example.run } + end + let(:backend_maintainer) do { username: 'backend-maintainer', @@ -13,7 +18,7 @@ RSpec.describe Gitlab::Danger::Roulette do role: 'Backend engineer', projects: { 'gitlab' => 'maintainer backend' }, available: true, - has_capacity: true + tz_offset_hours: 2.0 } end let(:frontend_reviewer) do @@ -23,7 +28,7 @@ RSpec.describe Gitlab::Danger::Roulette do role: 'Frontend engineer', projects: { 'gitlab' => 'reviewer frontend' }, available: true, - has_capacity: true + tz_offset_hours: 2.0 } end let(:frontend_maintainer) do @@ -33,7 +38,7 @@ RSpec.describe Gitlab::Danger::Roulette do role: 'Frontend engineer', projects: { 'gitlab' => "maintainer frontend" }, available: true, - has_capacity: true + tz_offset_hours: 2.0 } end let(:software_engineer_in_test) do @@ -46,7 +51,7 @@ RSpec.describe Gitlab::Danger::Roulette do 'gitlab-qa' => 'maintainer' }, available: true, - has_capacity: true + tz_offset_hours: 2.0 } end let(:engineering_productivity_reviewer) do @@ -56,7 +61,7 @@ RSpec.describe Gitlab::Danger::Roulette do role: 'Engineering Productivity', projects: { 'gitlab' => 'reviewer backend' }, available: true, - has_capacity: true + tz_offset_hours: 2.0 } end @@ -102,13 +107,14 @@ RSpec.describe Gitlab::Danger::Roulette do let!(:branch_name) { 'a-branch' } let!(:mr_labels) { ['backend', 'devops::create'] } let!(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa') } + let(:timezone_experiment) { false } let(:spins) do - # Stub the request at the latest time so that we can modify the raw data, e.g. available and has_capacity fields. + # Stub the request at the latest time so that we can modify the raw data, e.g. available fields. WebMock .stub_request(:get, described_class::ROULETTE_DATA_URL) .to_return(body: teammate_json) - subject.spin(project, categories, branch_name) + subject.spin(project, categories, branch_name, timezone_experiment: timezone_experiment) end before do @@ -116,63 +122,77 @@ RSpec.describe Gitlab::Danger::Roulette do allow(subject).to receive_message_chain(:gitlab, :mr_labels).and_return(mr_labels) end - context 'when change contains backend category' do - let(:categories) { [:backend] } + context 'when timezone_experiment == false' do + context 'when change contains backend category' do + let(:categories) { [:backend] } - it 'assigns backend reviewer and maintainer' do - expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer)) - end - - context 'when teammate is not available' do - before do - backend_maintainer[:available] = false + it 'assigns backend reviewer and maintainer' do + expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer)) end - it 'assigns backend reviewer and no maintainer' do - expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil)) + context 'when teammate is not available' do + before do + backend_maintainer[:available] = false + end + + it 'assigns backend reviewer and no maintainer' do + expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil)) + end end end - context 'when teammate has no capacity' do - before do - backend_maintainer[:has_capacity] = false - end + context 'when change contains frontend category' do + let(:categories) { [:frontend] } - it 'assigns backend reviewer and no maintainer' do - expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil)) + it 'assigns frontend reviewer and maintainer' do + expect(spins).to contain_exactly(matching_spin(:frontend, reviewer: frontend_reviewer, maintainer: frontend_maintainer)) + end + end + + context 'when change contains QA category' do + let(:categories) { [:qa] } + + it 'assigns QA reviewer' do + expect(spins).to contain_exactly(matching_spin(:qa, reviewer: software_engineer_in_test)) + end + end + + context 'when change contains Engineering Productivity category' do + let(:categories) { [:engineering_productivity] } + + it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do + expect(spins).to contain_exactly(matching_spin(:engineering_productivity, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer)) + end + end + + context 'when change contains test category' do + let(:categories) { [:test] } + + it 'assigns corresponding SET' do + expect(spins).to contain_exactly(matching_spin(:test, reviewer: software_engineer_in_test)) end end end - context 'when change contains frontend category' do - let(:categories) { [:frontend] } + context 'when timezone_experiment == true' do + let(:timezone_experiment) { true } - it 'assigns frontend reviewer and maintainer' do - expect(spins).to contain_exactly(matching_spin(:frontend, reviewer: frontend_reviewer, maintainer: frontend_maintainer)) - end - end + context 'when change contains backend category' do + let(:categories) { [:backend] } - context 'when change contains QA category' do - let(:categories) { [:qa] } + it 'assigns backend reviewer and maintainer' do + expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer)) + end - it 'assigns QA reviewer' do - expect(spins).to contain_exactly(matching_spin(:qa, reviewer: software_engineer_in_test)) - end - end + context 'when teammate is not in a good timezone' do + before do + backend_maintainer[:tz_offset_hours] = 5.0 + end - context 'when change contains Engineering Productivity category' do - let(:categories) { [:engineering_productivity] } - - it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do - expect(spins).to contain_exactly(matching_spin(:engineering_productivity, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer)) - end - end - - context 'when change contains test category' do - let(:categories) { [:test] } - - it 'assigns corresponding SET' do - expect(spins).to contain_exactly(matching_spin(:test, reviewer: software_engineer_in_test)) + it 'assigns backend reviewer and no maintainer' do + expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil)) + end + end end end end @@ -244,34 +264,83 @@ RSpec.describe Gitlab::Danger::Roulette do end describe '#spin_for_person' do - let(:person1) { Gitlab::Danger::Teammate.new('username' => 'rymai', 'available' => true, 'has_capacity' => true) } - let(:person2) { Gitlab::Danger::Teammate.new('username' => 'godfat', 'available' => true, 'has_capacity' => true) } - let(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa', 'available' => true, 'has_capacity' => true) } - let(:ooo) { Gitlab::Danger::Teammate.new('username' => 'jacopo-beschi', 'available' => false, 'has_capacity' => true) } - let(:no_capacity) { Gitlab::Danger::Teammate.new('username' => 'uncharged', 'available' => true, 'has_capacity' => false) } + let(:person_tz_offset_hours) { 0.0 } + let(:person1) do + Gitlab::Danger::Teammate.new( + 'username' => 'rymai', + 'available' => true, + 'tz_offset_hours' => person_tz_offset_hours + ) + end + let(:person2) do + Gitlab::Danger::Teammate.new( + 'username' => 'godfat', + 'available' => true, + 'tz_offset_hours' => person_tz_offset_hours) + end + let(:author) do + Gitlab::Danger::Teammate.new( + 'username' => 'filipa', + 'available' => true, + 'tz_offset_hours' => 0.0) + end + let(:unavailable) do + Gitlab::Danger::Teammate.new( + 'username' => 'jacopo-beschi', + 'available' => false, + 'tz_offset_hours' => 0.0) + end before do allow(subject).to receive_message_chain(:gitlab, :mr_author).and_return(author.username) end - it 'returns a random person' do - persons = [person1, person2] + (-4..4).each do |utc_offset| + context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do + let(:person_tz_offset_hours) { utc_offset } - selected = subject.spin_for_person(persons, random: Random.new) + [false, true].each do |timezone_experiment| + context "with timezone_experiment == #{timezone_experiment}" do + it 'returns a random person' do + persons = [person1, person2] - expect(selected.username).to be_in(persons.map(&:username)) + selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment) + + expect(selected.username).to be_in(persons.map(&:username)) + end + end + end + end end - it 'excludes OOO persons' do - expect(subject.spin_for_person([ooo], random: Random.new)).to be_nil + ((-12..-5).to_a + (5..12).to_a).each do |utc_offset| + context "when local hour for person is #{10 + utc_offset} (offset: #{utc_offset})" do + let(:person_tz_offset_hours) { utc_offset } + + [false, true].each do |timezone_experiment| + context "with timezone_experiment == #{timezone_experiment}" do + it 'returns a random person or nil' do + persons = [person1, person2] + + selected = subject.spin_for_person(persons, random: Random.new, timezone_experiment: timezone_experiment) + + if timezone_experiment + expect(selected).to be_nil + else + expect(selected.username).to be_in(persons.map(&:username)) + end + end + end + end + end + end + + it 'excludes unavailable persons' do + expect(subject.spin_for_person([unavailable], random: Random.new)).to be_nil end it 'excludes mr.author' do expect(subject.spin_for_person([author], random: Random.new)).to be_nil end - - it 'excludes person with no capacity' do - expect(subject.spin_for_person([no_capacity], random: Random.new)).to be_nil - end end end diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index cdc4c5feb4f..78fd6145154 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -2,14 +2,27 @@ require 'fast_spec_helper' +require 'timecop' require 'rspec-parameterized' require 'gitlab/danger/teammate' RSpec.describe Gitlab::Danger::Teammate do + using RSpec::Parameterized::TableSyntax + subject { described_class.new(options.stringify_keys) } - let(:options) { { username: 'luigi', projects: projects, role: role } } + let(:tz_offset_hours) { 2.0 } + let(:options) do + { + username: 'luigi', + projects: projects, + role: role, + markdown_name: '[Luigi](https://gitlab.com/luigi) (`@luigi`)', + tz_offset_hours: tz_offset_hours + } + end + let(:capabilities) { ['reviewer backend'] } let(:projects) { { project => capabilities } } let(:role) { 'Engineer, Manage' } let(:labels) { [] } @@ -114,4 +127,71 @@ RSpec.describe Gitlab::Danger::Teammate do expect(subject.maintainer?(project, :frontend, labels)).to be_falsey end end + + describe '#local_hour' do + around do |example| + Timecop.freeze(Time.utc(2020, 6, 23, 10)) { example.run } + end + + context 'when author is given' do + where(:tz_offset_hours, :expected_local_hour) do + -12 | 22 + -10 | 0 + 2 | 12 + 4 | 14 + 12 | 22 + end + + with_them do + it 'returns the correct local_hour' do + expect(subject.local_hour).to eq(expected_local_hour) + end + end + end + end + + describe '#markdown_name' do + context 'when timezone_experiment == false' do + it 'returns markdown name as-is' do + expect(subject.markdown_name).to eq(options[:markdown_name]) + expect(subject.markdown_name(timezone_experiment: false)).to eq(options[:markdown_name]) + end + end + + context 'when timezone_experiment == true' do + it 'returns markdown name with timezone info' do + expect(subject.markdown_name(timezone_experiment: true)).to eq("#{options[:markdown_name]} (UTC+2)") + end + + context 'when offset is 1.5' do + let(:tz_offset_hours) { 1.5 } + + it 'returns markdown name with timezone info, not truncated' do + expect(subject.markdown_name(timezone_experiment: true)).to eq("#{options[:markdown_name]} (UTC+1.5)") + end + end + + context 'when author is given' do + where(:tz_offset_hours, :author_offset, :diff_text) do + -12 | -10 | "2 hours behind `@mario`" + -10 | -12 | "2 hours ahead `@mario`" + -10 | 2 | "12 hours behind `@mario`" + 2 | 4 | "2 hours behind `@mario`" + 4 | 2 | "2 hours ahead `@mario`" + 2 | 2 | "same timezone as `@mario`" + end + + with_them do + it 'returns markdown name with timezone info' do + author = described_class.new(options.merge(username: 'mario', tz_offset_hours: author_offset).stringify_keys) + + floored_offset_hours = subject.__send__(:floored_offset_hours) + utc_offset = floored_offset_hours >= 0 ? "+#{floored_offset_hours}" : floored_offset_hours + + expect(subject.markdown_name(timezone_experiment: true, author: author)).to eq("#{options[:markdown_name]} (UTC#{utc_offset}, #{diff_text})") + end + end + end + end + end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index cad99beaf20..ab59a79b3e5 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -75,6 +75,42 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ) end end + + context 'for manage' do + it 'includes accurate usage_activity_by_stage data' do + stub_config( + omniauth: + { providers: omniauth_providers } + ) + + for_defined_days_back do + user = create(:user) + create(:event, author: user) + create(:group_member, user: user) + end + + expect(described_class.uncached_data[:usage_activity_by_stage][:manage]).to include( + events: 2, + groups: 2, + users_created: Gitlab.ee? ? 6 : 5, + omniauth_providers: ['google_oauth2'] + ) + expect(described_class.uncached_data[:usage_activity_by_stage_monthly][:manage]).to include( + events: 1, + groups: 1, + users_created: Gitlab.ee? ? 4 : 3, + omniauth_providers: ['google_oauth2'] + ) + end + + def omniauth_providers + [ + OpenStruct.new(name: 'google_oauth2'), + OpenStruct.new(name: 'ldapmain'), + OpenStruct.new(name: 'group_saml') + ] + end + end end context 'for create' do