diff --git a/app/assets/javascripts/integrations/edit/components/jira_trigger_fields.vue b/app/assets/javascripts/integrations/edit/components/jira_trigger_fields.vue index af4e9acf4ba..03b1e8586ae 100644 --- a/app/assets/javascripts/integrations/edit/components/jira_trigger_fields.vue +++ b/app/assets/javascripts/integrations/edit/components/jira_trigger_fields.vue @@ -1,7 +1,16 @@ @@ -89,7 +164,7 @@ export default { + + + + + + + {{ s__('JiraService|Enable Jira transitions') }} + + + + + + {{ issueTransitionOption.label }} + + + + + + diff --git a/app/assets/javascripts/integrations/edit/index.js b/app/assets/javascripts/integrations/edit/index.js index 1909f584591..792e7d8e85e 100644 --- a/app/assets/javascripts/integrations/edit/index.js +++ b/app/assets/javascripts/integrations/edit/index.js @@ -29,6 +29,8 @@ function parseDatasetToProps(data) { testPath, resetPath, vulnerabilitiesIssuetype, + jiraIssueTransitionAutomatic, + jiraIssueTransitionId, ...booleanAttributes } = data; const { @@ -60,6 +62,8 @@ function parseDatasetToProps(data) { initialTriggerMergeRequest: mergeRequestEvents, initialEnableComments: enableComments, initialCommentDetail: commentDetail, + initialJiraIssueTransitionAutomatic: jiraIssueTransitionAutomatic, + initialJiraIssueTransitionId: jiraIssueTransitionId, }, jiraIssuesProps: { showJiraIssuesIntegration, diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index 3cab198c1f9..6262d29a734 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -44,6 +44,7 @@ module ServiceParams # make those event names plural as special case. :issues_events, :issues_url, + :jira_issue_transition_automatic, :jira_issue_transition_id, :manual_configuration, :merge_requests_events, diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 277b5157ebf..cbf877ca324 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -67,7 +67,7 @@ module GroupsHelper def group_open_issues_count(group) if Feature.enabled?(:cached_sidebar_open_issues_count, group, default_enabled: :yaml) - cached_open_group_issues_count(group) + cached_issuables_count(group, type: :issues) else number_with_delimiter(group_issues_count(state: 'opened')) end @@ -80,18 +80,11 @@ module GroupsHelper .count end - def cached_open_group_issues_count(group) - count_service = Groups::OpenIssuesCountService - issues_count = count_service.new(group, current_user).count - - if issues_count > count_service::CACHED_COUNT_THRESHOLD - ActiveSupport::NumberHelper - .number_to_human( - issues_count, - units: { thousand: 'k', million: 'm' }, precision: 1, significant: false, format: '%n%u' - ) + def group_open_merge_requests_count(group) + if Feature.enabled?(:cached_sidebar_merge_requests_count, group, default_enabled: :yaml) + cached_issuables_count(@group, type: :merge_requests) else - number_with_delimiter(issues_count) + number_with_delimiter(group_merge_requests_count(state: 'opened')) end end @@ -102,6 +95,14 @@ module GroupsHelper .count end + def cached_issuables_count(group, type: nil) + count_service = issuables_count_service_class(type) + return unless count_service.present? + + issuables_count = count_service.new(group, current_user).count + format_issuables_count(count_service, issuables_count) + end + def group_dependency_proxy_url(group) # The namespace path can include uppercase letters, which # Docker doesn't allow. The proxy expects it to be downcased. @@ -120,7 +121,9 @@ module GroupsHelper @has_group_title = true full_title = [] - group.ancestors.reverse.each_with_index do |parent, index| + ancestors = group.ancestors.with_route + + ancestors.reverse_each.with_index do |parent, index| if index > 0 add_to_breadcrumb_dropdown(group_title_link(parent, hidable: false, show_avatar: true, for_dropdown: true), location: :before) else @@ -306,6 +309,26 @@ module GroupsHelper def ancestor_locked_and_has_been_overridden(group) s_("GroupSettings|This setting is applied on %{ancestor_group} and has been overridden on this subgroup.").html_safe % { ancestor_group: ancestor_group(group) } end + + def issuables_count_service_class(type) + if type == :issues + Groups::OpenIssuesCountService + elsif type == :merge_requests + Groups::MergeRequestsCountService + end + end + + def format_issuables_count(count_service, count) + if count > count_service::CACHED_COUNT_THRESHOLD + ActiveSupport::NumberHelper + .number_to_human( + count, + units: { thousand: 'k', million: 'm' }, precision: 1, significant: false, format: '%n%u' + ) + else + number_with_delimiter(count) + end + end end GroupsHelper.prepend_if_ee('EE::GroupsHelper') diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index 14d20e7c622..1bbfdb431de 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -86,7 +86,7 @@ module ServicesHelper end def integration_form_data(integration, group: nil) - { + form_data = { id: integration.id, show_active: integration.show_active_box?.to_s, activated: (integration.active || integration.new_record?).to_s, @@ -106,6 +106,13 @@ module ServicesHelper test_path: scoped_test_integration_path(integration), reset_path: scoped_reset_integration_path(integration, group: group) } + + if integration.is_a?(JiraService) + form_data[:jira_issue_transition_automatic] = integration.jira_issue_transition_automatic + form_data[:jira_issue_transition_id] = integration.jira_issue_transition_id + end + + form_data end def trigger_events_for_service(integration) diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 7236b582f3e..b4ef0be8076 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -31,7 +31,7 @@ class JiraService < IssueTrackerService # TODO: we can probably just delegate as part of # https://gitlab.com/gitlab-org/gitlab/issues/29404 - data_field :username, :password, :url, :api_url, :jira_issue_transition_id, :project_key, :issues_enabled, + data_field :username, :password, :url, :api_url, :jira_issue_transition_automatic, :jira_issue_transition_id, :project_key, :issues_enabled, :vulnerabilities_enabled, :vulnerabilities_issuetype before_update :reset_password @@ -124,9 +124,6 @@ class JiraService < IssueTrackerService end def fields - transition_id_help_path = help_page_path('user/project/integrations/jira', anchor: 'obtaining-a-transition-id') - transition_id_help_link_start = ''.html_safe % { transition_id_help_path: transition_id_help_path } - [ { type: 'text', @@ -156,13 +153,6 @@ class JiraService < IssueTrackerService non_empty_password_help: s_('JiraService|Leave blank to use your current password or API token'), placeholder: s_('JiraService|Use a password for server version and an API token for cloud version'), required: true - }, - { - type: 'text', - name: 'jira_issue_transition_id', - title: s_('JiraService|Jira workflow transition IDs'), - placeholder: s_('JiraService|For example, 12, 24'), - help: s_('JiraService|Set transition IDs for Jira workflow transitions. %{link_start}Learn more%{link_end}'.html_safe % { link_start: transition_id_help_link_start, link_end: ''.html_safe }) } ] end @@ -190,17 +180,19 @@ class JiraService < IssueTrackerService # support any events. end - def find_issue(issue_key, rendered_fields: false) - options = {} - options = options.merge(expand: 'renderedFields') if rendered_fields + def find_issue(issue_key, rendered_fields: false, transitions: false) + expands = [] + expands << 'renderedFields' if rendered_fields + expands << 'transitions' if transitions + options = { expand: expands.join(',') } if expands.any? - jira_request { client.Issue.find(issue_key, options) } + jira_request { client.Issue.find(issue_key, options || {}) } end def close_issue(entity, external_issue, current_user) - issue = find_issue(external_issue.iid) + issue = find_issue(external_issue.iid, transitions: jira_issue_transition_automatic) - return if issue.nil? || has_resolution?(issue) || !jira_issue_transition_id.present? + return if issue.nil? || has_resolution?(issue) || !issue_transition_enabled? commit_id = case entity when Commit then entity.id @@ -275,6 +267,10 @@ class JiraService < IssueTrackerService true end + def issue_transition_enabled? + jira_issue_transition_automatic || jira_issue_transition_id.present? + end + private def server_info @@ -295,22 +291,46 @@ class JiraService < IssueTrackerService # the issue is transitioned at the order given by the user # if any transition fails it will log the error message and stop the transition sequence def transition_issue(issue) - jira_issue_transition_id.scan(Gitlab::Regex.jira_transition_id_regex).each do |transition_id| - issue.transitions.build.save!(transition: { id: transition_id }) - rescue => error - log_error( - "Issue transition failed", - error: { - exception_class: error.class.name, - exception_message: error.message, - exception_backtrace: Gitlab::BacktraceCleaner.clean_backtrace(error.backtrace) - }, - client_url: client_url - ) - return false + return transition_issue_to_done(issue) if jira_issue_transition_automatic + + jira_issue_transition_id.scan(Gitlab::Regex.jira_transition_id_regex).all? do |transition_id| + transition_issue_to_id(issue, transition_id) end end + def transition_issue_to_id(issue, transition_id) + issue.transitions.build.save!( + transition: { id: transition_id } + ) + + true + rescue => error + log_error( + "Issue transition failed", + error: { + exception_class: error.class.name, + exception_message: error.message, + exception_backtrace: Gitlab::BacktraceCleaner.clean_backtrace(error.backtrace) + }, + client_url: client_url + ) + + false + end + + def transition_issue_to_done(issue) + transitions = issue.transitions rescue [] + + transition = transitions.find do |transition| + status = transition&.to&.statusCategory + status && status['key'] == 'done' + end + + return false unless transition + + transition_issue_to_id(issue, transition.id) + end + def log_usage(action, user) key = "i_ecosystem_jira_service_#{action}" diff --git a/app/services/groups/count_service.rb b/app/services/groups/count_service.rb new file mode 100644 index 00000000000..2a15ae3bc57 --- /dev/null +++ b/app/services/groups/count_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Groups + class CountService < BaseCountService + include Gitlab::Utils::StrongMemoize + + VERSION = 1 + CACHED_COUNT_THRESHOLD = 1000 + EXPIRATION_TIME = 24.hours + + attr_reader :group, :user + + def initialize(group, user = nil) + @group = group + @user = user + end + + def count + cached_count = Rails.cache.read(cache_key) + return cached_count unless cached_count.blank? + + refreshed_count = uncached_count + update_cache_for_key(cache_key) { refreshed_count } if refreshed_count > CACHED_COUNT_THRESHOLD + refreshed_count + end + + def cache_key + ['groups', "#{issuable_key}_count_service", VERSION, group.id, cache_key_name] + end + + private + + def relation_for_count + raise NotImplementedError + end + + def cache_options + super.merge({ expires_in: EXPIRATION_TIME }) + end + + def cache_key_name + raise NotImplementedError, 'cache_key_name must be implemented and return a String' + end + + def issuable_key + raise NotImplementedError, 'issuable_key must be implemented and return a String' + end + end +end diff --git a/app/services/groups/merge_requests_count_service.rb b/app/services/groups/merge_requests_count_service.rb new file mode 100644 index 00000000000..bb49efe571a --- /dev/null +++ b/app/services/groups/merge_requests_count_service.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Groups + # Service class for counting and caching the number of open merge requests of a group. + class MergeRequestsCountService < Groups::CountService + private + + def cache_key_name + 'open_merge_requests_count' + end + + def relation_for_count + MergeRequestsFinder + .new(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true) + .execute + end + + def issuable_key + 'open_merge_requests' + end + end +end diff --git a/app/services/groups/open_issues_count_service.rb b/app/services/groups/open_issues_count_service.rb index db1ca09212a..ef787a04315 100644 --- a/app/services/groups/open_issues_count_service.rb +++ b/app/services/groups/open_issues_count_service.rb @@ -2,47 +2,12 @@ module Groups # Service class for counting and caching the number of open issues of a group. - class OpenIssuesCountService < BaseCountService - include Gitlab::Utils::StrongMemoize - - VERSION = 1 + class OpenIssuesCountService < Groups::CountService PUBLIC_COUNT_KEY = 'group_public_open_issues_count' TOTAL_COUNT_KEY = 'group_total_open_issues_count' - CACHED_COUNT_THRESHOLD = 1000 - EXPIRATION_TIME = 24.hours - - attr_reader :group, :user - - def initialize(group, user = nil) - @group = group - @user = user - end - - # Reads count value from cache and return it if present. - # If empty or expired, #uncached_count will calculate the issues count for the group and - # compare it with the threshold. If it is greater, it will be written to the cache and returned. - # If below, it will be returned without being cached. - # This results in only caching large counts and calculating the rest with every call to maintain - # accuracy. - def count - cached_count = Rails.cache.read(cache_key) - return cached_count unless cached_count.blank? - - refreshed_count = uncached_count - update_cache_for_key(cache_key) { refreshed_count } if refreshed_count > CACHED_COUNT_THRESHOLD - refreshed_count - end - - def cache_key(key = nil) - ['groups', 'open_issues_count_service', VERSION, group.id, cache_key_name] - end private - def cache_options - super.merge({ expires_in: EXPIRATION_TIME }) - end - def cache_key_name public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY end @@ -60,5 +25,9 @@ module Groups def relation_for_count IssuesFinder.new(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: public_only?).execute end + + def issuable_key + 'open_issues' + end end end diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml index 15e777f5c36..f0f85e0618d 100644 --- a/app/views/groups/merge_requests.html.haml +++ b/app/views/groups/merge_requests.html.haml @@ -2,7 +2,7 @@ - page_title _("Merge Requests") -- if group_merge_requests_count(state: 'all') == 0 +- if @merge_requests&.size == 0 = render 'shared/empty_states/merge_requests', project_select_button: true - else .top-area diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index e9437ab8a3f..48187f661a7 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -1,5 +1,5 @@ - issues_count = group_open_issues_count(@group) -- merge_requests_count = group_merge_requests_count(state: 'opened') +- merge_requests_count = group_open_merge_requests_count(@group) - aside_title = @group.subgroup? ? _('Subgroup navigation') : _('Group navigation') - overview_title = @group.subgroup? ? _('Subgroup overview') : _('Group overview') @@ -92,13 +92,13 @@ = sprite_icon('git-merge') %span.nav-item-name = _('Merge Requests') - %span.badge.badge-pill.count= number_with_delimiter(merge_requests_count) + %span.badge.badge-pill.count= merge_requests_count %ul.sidebar-sub-level-items.is-fly-out-only = nav_link(path: 'groups#merge_requests', html_options: { class: "fly-out-top-item" } ) do = link_to merge_requests_group_path(@group) do %strong.fly-out-top-item-name = _('Merge Requests') - %span.badge.badge-pill.count.merge_counter.js-merge-counter.fly-out-badge= number_with_delimiter(merge_requests_count) + %span.badge.badge-pill.count.merge_counter.js-merge-counter.fly-out-badge= merge_requests_count = render_if_exists "layouts/nav/ee/security_link" # EE-specific diff --git a/changelogs/unreleased/16119-jira-issue-transition-automatic.yml b/changelogs/unreleased/16119-jira-issue-transition-automatic.yml new file mode 100644 index 00000000000..0e7c31f129d --- /dev/null +++ b/changelogs/unreleased/16119-jira-issue-transition-automatic.yml @@ -0,0 +1,5 @@ +--- +title: Support automatic transitions of Jira issues +merge_request: 55773 +author: +type: added diff --git a/changelogs/unreleased/299178-follow-up-from-cached-sidebar-issues-count-in-group-sidebar.yml b/changelogs/unreleased/299178-follow-up-from-cached-sidebar-issues-count-in-group-sidebar.yml new file mode 100644 index 00000000000..d0acbc372d6 --- /dev/null +++ b/changelogs/unreleased/299178-follow-up-from-cached-sidebar-issues-count-in-group-sidebar.yml @@ -0,0 +1,5 @@ +--- +title: Cache open merge requests count in group sidebar +merge_request: 55971 +author: +type: performance diff --git a/changelogs/unreleased/326212-controller-projects-hookscontroller-index-executes-more-than-100-s.yml b/changelogs/unreleased/326212-controller-projects-hookscontroller-index-executes-more-than-100-s.yml new file mode 100644 index 00000000000..67d8dc5df43 --- /dev/null +++ b/changelogs/unreleased/326212-controller-projects-hookscontroller-index-executes-more-than-100-s.yml @@ -0,0 +1,5 @@ +--- +title: Avoid N+1 queries in breadcrumbs +merge_request: 57725 +author: +type: performance diff --git a/config/feature_flags/development/cached_sidebar_merge_requests_count.yml b/config/feature_flags/development/cached_sidebar_merge_requests_count.yml new file mode 100644 index 00000000000..f542ba6323c --- /dev/null +++ b/config/feature_flags/development/cached_sidebar_merge_requests_count.yml @@ -0,0 +1,8 @@ +--- +name: cached_sidebar_merge_requests_count +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55971 +rollout_issue_url: +milestone: '13.11' +type: development +group: group::product planning +default_enabled: true diff --git a/db/migrate/20210224161552_add_jira_issue_transition_automatic_to_jira_tracker_data.rb b/db/migrate/20210224161552_add_jira_issue_transition_automatic_to_jira_tracker_data.rb new file mode 100644 index 00000000000..6c788b9d554 --- /dev/null +++ b/db/migrate/20210224161552_add_jira_issue_transition_automatic_to_jira_tracker_data.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddJiraIssueTransitionAutomaticToJiraTrackerData < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :jira_tracker_data, :jira_issue_transition_automatic, :boolean, null: false, default: false + end +end diff --git a/db/schema_migrations/20210224161552 b/db/schema_migrations/20210224161552 new file mode 100644 index 00000000000..1fd37a69dd3 --- /dev/null +++ b/db/schema_migrations/20210224161552 @@ -0,0 +1 @@ +328e095123eb0b8822342b0d4a338d42265ca8eafbcadcc7e15628e9d02c863d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index be155891f23..2bdf4873acf 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13977,6 +13977,7 @@ CREATE TABLE jira_tracker_data ( encrypted_proxy_username_iv text, encrypted_proxy_password text, encrypted_proxy_password_iv text, + jira_issue_transition_automatic boolean DEFAULT false NOT NULL, CONSTRAINT check_0bf84b76e9 CHECK ((char_length(vulnerabilities_issuetype) <= 255)), CONSTRAINT check_214cf6a48b CHECK ((char_length(project_key) <= 255)) ); diff --git a/doc/api/services.md b/doc/api/services.md index a5fd465f2ac..189dfc48cd5 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -814,7 +814,8 @@ Parameters: | `username` | string | yes | The username of the user created to be used with GitLab/Jira. | | `password` | string | yes | The password of the user created to be used with GitLab/Jira. | | `active` | boolean | no | Activates or deactivates the service. Defaults to false (deactivated). | -| `jira_issue_transition_id` | string | no | The ID of a transition that moves issues to a closed state. You can find this number under the Jira workflow administration (**Administration > Issues > Workflows**) by selecting **View** under **Operations** of the desired workflow of your project. The ID of each state can be found inside the parenthesis of each transition name under the transitions ID column. By default, this ID is set to `2`. | +| `jira_issue_transition_automatic` | boolean | no | Enable [automatic issue transitions](../user/project/integrations/jira.md#automatic-issue-transitions). Takes precedence over `jira_issue_transition_id` if enabled. Defaults to `false` | +| `jira_issue_transition_id` | string | no | The ID of one or more transitions for [custom issue transitions](../user/project/integrations/jira.md#custom-issue-transitions). Ignored if `jira_issue_transition_automatic` is enabled. Defaults to a blank string, which disables custom transitions. | | `commit_events` | boolean | false | Enable notifications for commit events | | `merge_requests_events` | boolean | false | Enable notifications for merge request events | | `comment_on_event_enabled` | boolean | false | Enable comments inside Jira issues on each GitLab event (commit / merge request) | diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index 4757e353dd5..a997f2fbb08 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -251,9 +251,6 @@ Prerequisites: - [Authenticate](#authenticate-to-the-package-registry) to the Package Registry. - Set a [project-level npm endpoint](#use-the-gitlab-endpoint-for-npm-packages). -- Your npm package name must be in the format of [@scope/package-name](#package-naming-convention). - It must match exactly, including the case. This is different than the - npm naming convention, but it is required to work with the GitLab Package Registry. To upload an npm package to your project, run this command: diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md index 2f6aa06007f..b5f37a4069f 100644 --- a/doc/user/project/integrations/jira.md +++ b/doc/user/project/integrations/jira.md @@ -106,7 +106,8 @@ To enable the Jira integration in a project: 1. To include a comment on the Jira issue when the above reference is made in GitLab, select **Enable comments**. - 1. Select the **Comment detail**: **Standard** or **All details**. +1. To transition Jira issues when a [closing reference](../issues/managing_issues.md#closing-issues-automatically) is made in GitLab, + select **Enable Jira transitions**. 1. Enter the further details on the page as described in the following table. @@ -116,7 +117,6 @@ To enable the Jira integration in a project: | `Jira API URL` | The base URL to the Jira instance API. Web URL value is used if not set. For example, `https://jira-api.example.com`. Leave this field blank (or use the same value of `Web URL`) if using **Jira on Atlassian cloud**. | | `Username or Email` | Created in [configure Jira](#configure-jira) step. Use `username` for **Jira Server** or `email` for **Jira on Atlassian cloud**. | | `Password/API token` | Created in [configure Jira](#configure-jira) step. Use `password` for **Jira Server** or `API token` for **Jira on Atlassian cloud**. | - | `Jira workflow transition IDs` | Required for closing Jira issues via commits or merge requests. These are the IDs of transitions in Jira that move issues to a particular state. (See [Obtaining a transition ID](#obtaining-a-transition-id).) If you insert multiple transition IDs separated by `,` or `;`, the issue is moved to each state, one after another, using the given order. In GitLab 13.6 and earlier, field was called `Transition ID`. | 1. To enable users to view Jira issues inside the GitLab project, select **Enable Jira issues** and enter a Jira project key. **(PREMIUM)** @@ -138,10 +138,26 @@ To enable the Jira integration in a project: Your GitLab project can now interact with all Jira projects in your instance and the project now displays a Jira link that opens the Jira project. -#### Obtaining a transition ID +#### Automatic issue transitions -In the most recent Jira user interface, you can no longer see transition IDs in the workflow -administration UI. You can get the ID you need in either of the following ways: +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/...) in GitLab 13.10. + +In this mode the referenced Jira issue is transitioned to the next available status with a category of "Done". + +See the [Configure GitLab](#configure-gitlab) section, check the **Enable Jira transitions** setting and select the **Move to Done** option. + +#### Custom issue transitions + +For advanced workflows you can specify custom Jira transition IDs. + +See the [Configure GitLab](#configure-gitlab) section, check the **Enable Jira transitions** setting, select the **Custom transitions** option, and enter your transition IDs in the text field. + +If you insert multiple transition IDs separated by `,` or `;`, the issue is moved to each state, one after another, using the given order. If a transition fails the sequence is aborted. + +To see the transition IDs on Jira Cloud, edit a workflow in the **Text** view. +The transition IDs display in the **Transitions** column. + +On Jira Server you can get the transition IDs in either of the following ways: 1. By using the API, with a request like `https://yourcompany.atlassian.net/rest/api/2/issue/ISSUE-123/transitions` using an issue that is in the appropriate "open" state diff --git a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md index 406a79217d0..89b4c999b06 100644 --- a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md +++ b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md @@ -28,6 +28,39 @@ You can [search and filter the results](../../search/index.md#filtering-issue-an ![Group Issues list view](img/group_merge_requests_list_view.png) +## Cached merge request count + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/299542) in GitLab 13.11. +> - It's [deployed behind a feature flag](../../feature_flags.md), enabled by default. +> - It's enabled on GitLab.com. +> - It's recommended for production use. +> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-cached-merge-request-count). + +WARNING: +This feature might not be available to you. Check the **version history** note above for details. + +In a group, the sidebar displays the total count of open merge requests and this value is cached if higher +than 1000. The cached value is rounded to thousands (or millions) and updated every 24 hours. + +### Enable or disable cached merge request count **(FREE SELF)** + +Cached merge request count in the left sidebar is under development but ready for production use. It is +deployed behind a feature flag that is **enabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) +can disable it. + +To disable it: + +```ruby +Feature.disable(:cached_sidebar_merge_requests_count) +``` + +To enable it: + +```ruby +Feature.enable(:cached_sidebar_merge_requests_count) +``` + ## Semi-linear history merge requests A merge commit is created for every merge, but the branch is only merged if diff --git a/lib/api/helpers/services_helpers.rb b/lib/api/helpers/services_helpers.rb index b2e50ee8352..2f2ad88c942 100644 --- a/lib/api/helpers/services_helpers.rb +++ b/lib/api/helpers/services_helpers.rb @@ -541,11 +541,17 @@ module API type: String, desc: 'The password of the user created to be used with GitLab/Jira' }, + { + required: false, + name: :jira_issue_transition_automatic, + type: Boolean, + desc: 'Enable automatic issue transitions' + }, { required: false, name: :jira_issue_transition_id, type: String, - desc: 'The ID of a transition that moves issues to a closed state. You can find this number under the Jira workflow administration (**Administration > Issues > Workflows**) by selecting **View** under **Operations** of the desired workflow of your project. The ID of each state can be found inside the parenthesis of each transition name under the **Transitions (id)** column ([see screenshot][trans]). By default, this ID is set to `2`' + desc: 'The ID of one or more transitions for custom issue transitions' }, { required: false, diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 6332e5e9a17..96fcf1a8f3f 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -128,6 +128,11 @@ module API yield end end + + # Overridden in EE + def geo_proxy + {} + end end namespace 'internal' do @@ -322,6 +327,12 @@ module API two_factor_otp_check end + + # Workhorse calls this to determine if it is a Geo secondary site + # that should proxy requests. FOSS can quickly return empty data. + get '/geo_proxy', feature_category: :geo_replication do + geo_proxy + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 63eb1869460..84142147d00 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17396,6 +17396,9 @@ msgstr "" msgid "JiraService|An error occurred while fetching issue list" msgstr "" +msgid "JiraService|Automatically transitions Jira issues to the \"Done\" category. %{linkStart}Learn more%{linkEnd}" +msgstr "" + msgid "JiraService|Define the type of Jira issue to create from a vulnerability." msgstr "" @@ -17408,6 +17411,9 @@ msgstr "" msgid "JiraService|Enable Jira issues creation from vulnerabilities" msgstr "" +msgid "JiraService|Enable Jira transitions" +msgstr "" + msgid "JiraService|Enter new password or API token" msgstr "" @@ -17456,10 +17462,10 @@ msgstr "" msgid "JiraService|Jira project key" msgstr "" -msgid "JiraService|Jira workflow transition IDs" +msgid "JiraService|Leave blank to use your current password or API token" msgstr "" -msgid "JiraService|Leave blank to use your current password or API token" +msgid "JiraService|Move to Done" msgstr "" msgid "JiraService|Not all data may be displayed here. To view more details or make changes to this issue, go to %{linkStart}Jira%{linkEnd}." @@ -17480,7 +17486,7 @@ msgstr "" msgid "JiraService|Select issue type" msgstr "" -msgid "JiraService|Set transition IDs for Jira workflow transitions. %{link_start}Learn more%{link_end}" +msgid "JiraService|Set a custom final state by using transition IDs. %{linkStart}Learn about transition IDs%{linkEnd}" msgstr "" msgid "JiraService|Sign in to GitLab.com to get started." @@ -17492,12 +17498,18 @@ msgstr "" msgid "JiraService|This issue is synchronized with Jira" msgstr "" +msgid "JiraService|Transition Jira issues to their final state:" +msgstr "" + msgid "JiraService|Use a password for server version and an API token for cloud version" msgstr "" msgid "JiraService|Use a username for server version and an email for cloud version" msgstr "" +msgid "JiraService|Use custom transitions" +msgstr "" + msgid "JiraService|Username or Email" msgstr "" diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index d1b556b58fb..a0b394562de 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -171,8 +171,8 @@ module QA # This is a helpful workaround when there is a transparent element overlapping # the target element and so, normal `click_element` on target would raise # Selenium::WebDriver::Error::ElementClickInterceptedError - def click_element_coordinates(name) - page.driver.browser.action.move_to(find_element(name).native).click.perform + def click_element_coordinates(name, **kwargs) + page.driver.browser.action.move_to(find_element(name, **kwargs).native).click.perform end # replace with (..., page = self.class) diff --git a/qa/qa/page/project/settings/services/jira.rb b/qa/qa/page/project/settings/services/jira.rb index eaa3e90db78..bbf1ef14dba 100644 --- a/qa/qa/page/project/settings/services/jira.rb +++ b/qa/qa/page/project/settings/services/jira.rb @@ -10,7 +10,13 @@ module QA element :service_url_field, ':data-qa-selector="`${fieldId}_field`"' # rubocop:disable QA/ElementWithPattern element :service_username_field, ':data-qa-selector="`${fieldId}_field`"' # rubocop:disable QA/ElementWithPattern element :service_password_field, ':data-qa-selector="`${fieldId}_field`"' # rubocop:disable QA/ElementWithPattern - element :service_jira_issue_transition_id_field, ':data-qa-selector="`${fieldId}_field`"' # rubocop:disable QA/ElementWithPattern + end + + view 'app/assets/javascripts/integrations/edit/components/jira_trigger_fields.vue' do + element :service_jira_issue_transition_enabled + element :service_jira_issue_transition_automatic_true, ':data-qa-selector="`service_jira_issue_transition_automatic_${issueTransitionOption.value}`"' # rubocop:disable QA/ElementWithPattern + element :service_jira_issue_transition_automatic_false, ':data-qa-selector="`service_jira_issue_transition_automatic_${issueTransitionOption.value}`"' # rubocop:disable QA/ElementWithPattern + element :service_jira_issue_transition_id_field end view 'app/assets/javascripts/integrations/edit/components/integration_form.vue' do @@ -23,7 +29,10 @@ module QA set_jira_server_url(url) set_username(Runtime::Env.jira_admin_username) set_password(Runtime::Env.jira_admin_password) - set_transaction_ids('11,21,31,41') + + enable_transitions + use_custom_transitions + set_transition_ids('11,21,31,41') click_save_changes_button wait_until(reload: false) do @@ -45,12 +54,24 @@ module QA fill_element(:service_password_field, password) end - def set_transaction_ids(transaction_ids) - fill_element(:service_jira_issue_transition_id_field, transaction_ids) + def enable_transitions + click_element_coordinates(:service_jira_issue_transition_enabled, visible: false) + end + + def use_automatic_transitions + click_element_coordinates(:service_jira_issue_transition_automatic_true, visible: false) + end + + def use_custom_transitions + click_element_coordinates(:service_jira_issue_transition_automatic_false, visible: false) + end + + def set_transition_ids(transition_ids) + fill_element(:service_jira_issue_transition_id_field, transition_ids) end def click_save_changes_button - click_element :save_changes_button + click_element(:save_changes_button) end end end diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index e183d711b30..b27a8192a64 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -64,7 +64,7 @@ module QA super end - def click_element_coordinates(name) + def click_element_coordinates(name, **kwargs) log(%Q(clicking the coordinates of :#{name})) super diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 18d3b2d99b7..7b9d7bfb3e0 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -56,6 +56,7 @@ FactoryBot.define do api_url { nil } username { 'jira_username' } password { 'jira_password' } + jira_issue_transition_automatic { false } jira_issue_transition_id { '56-1' } issues_enabled { false } project_key { nil } @@ -66,7 +67,9 @@ FactoryBot.define do before(:create) do |service, evaluator| if evaluator.create_data create(:jira_tracker_data, service: service, - url: evaluator.url, api_url: evaluator.api_url, jira_issue_transition_id: evaluator.jira_issue_transition_id, + url: evaluator.url, api_url: evaluator.api_url, + jira_issue_transition_automatic: evaluator.jira_issue_transition_automatic, + jira_issue_transition_id: evaluator.jira_issue_transition_id, username: evaluator.username, password: evaluator.password, issues_enabled: evaluator.issues_enabled, project_key: evaluator.project_key, vulnerabilities_enabled: evaluator.vulnerabilities_enabled, vulnerabilities_issuetype: evaluator.vulnerabilities_issuetype diff --git a/spec/features/projects/services/user_activates_jira_spec.rb b/spec/features/projects/services/user_activates_jira_spec.rb index 85afc54be48..10f84aae93f 100644 --- a/spec/features/projects/services/user_activates_jira_spec.rb +++ b/spec/features/projects/services/user_activates_jira_spec.rb @@ -6,12 +6,13 @@ RSpec.describe 'User activates Jira', :js do include_context 'project service activation' include_context 'project service Jira context' + before do + stub_request(:get, test_url).to_return(body: { key: 'value' }.to_json) + end + describe 'user tests Jira Service' do context 'when Jira connection test succeeds' do before do - server_info = { key: 'value' }.to_json - stub_request(:get, test_url).with(basic_auth: %w(username password)).to_return(body: server_info) - visit_project_integration('Jira') fill_form click_test_then_save_integration(expect_test_to_fail: false) @@ -81,4 +82,68 @@ RSpec.describe 'User activates Jira', :js do end end end + + describe 'issue transition settings' do + it 'using custom transitions' do + visit_project_integration('Jira') + + expect(page).to have_field('Enable Jira transitions', checked: false) + + check 'Enable Jira transitions' + + expect(page).to have_field('Move to Done', checked: true) + + fill_form + choose 'Use custom transitions' + click_save_integration + + within '[data-testid="issue-transition-mode"]' do + expect(page).to have_content('This field is required.') + end + + fill_in 'service[jira_issue_transition_id]', with: '1, 2, 3' + click_save_integration + + expect(page).to have_content('Jira settings saved and active.') + expect(project.reload.jira_service.data_fields).to have_attributes( + jira_issue_transition_automatic: false, + jira_issue_transition_id: '1, 2, 3' + ) + end + + it 'using automatic transitions' do + create(:jira_service, project: project, jira_issue_transition_automatic: false, jira_issue_transition_id: '1, 2, 3') + visit_project_integration('Jira') + + expect(page).to have_field('Enable Jira transitions', checked: true) + expect(page).to have_field('Use custom transitions', checked: true) + expect(page).to have_field('service[jira_issue_transition_id]', with: '1, 2, 3') + + choose 'Move to Done' + click_save_integration + + expect(page).to have_content('Jira settings saved and active.') + expect(project.reload.jira_service.data_fields).to have_attributes( + jira_issue_transition_automatic: true, + jira_issue_transition_id: '' + ) + end + + it 'disabling issue transitions' do + create(:jira_service, project: project, jira_issue_transition_automatic: true, jira_issue_transition_id: '1, 2, 3') + visit_project_integration('Jira') + + expect(page).to have_field('Enable Jira transitions', checked: true) + expect(page).to have_field('Move to Done', checked: true) + + uncheck 'Enable Jira transitions' + click_save_integration + + expect(page).to have_content('Jira settings saved and active.') + expect(project.reload.jira_service.data_fields).to have_attributes( + jira_issue_transition_automatic: false, + jira_issue_transition_id: '' + ) + end + end end diff --git a/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js b/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js index c6e7ee44355..5c04add61a1 100644 --- a/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js +++ b/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js @@ -30,14 +30,23 @@ describe('JiraTriggerFields', () => { const findCommentSettings = () => wrapper.find('[data-testid="comment-settings"]'); const findCommentDetail = () => wrapper.find('[data-testid="comment-detail"]'); const findCommentSettingsCheckbox = () => findCommentSettings().find(GlFormCheckbox); + const findIssueTransitionEnabled = () => + wrapper.find('[data-testid="issue-transition-enabled"] input[type="checkbox"]'); + const findIssueTransitionMode = () => wrapper.find('[data-testid="issue-transition-mode"]'); + const findIssueTransitionModeRadios = () => + findIssueTransitionMode().findAll('input[type="radio"]'); + const findIssueTransitionIdsField = () => + wrapper.find('input[type="text"][name="service[jira_issue_transition_id]"]'); describe('template', () => { describe('initialTriggerCommit and initialTriggerMergeRequest are false', () => { - it('does not show comment settings', () => { + it('does not show trigger settings', () => { createComponent(); expect(findCommentSettings().isVisible()).toBe(false); expect(findCommentDetail().isVisible()).toBe(false); + expect(findIssueTransitionEnabled().exists()).toBe(false); + expect(findIssueTransitionMode().exists()).toBe(false); }); }); @@ -48,9 +57,11 @@ describe('JiraTriggerFields', () => { }); }); - it('shows comment settings', () => { + it('shows trigger settings', () => { expect(findCommentSettings().isVisible()).toBe(true); expect(findCommentDetail().isVisible()).toBe(false); + expect(findIssueTransitionEnabled().isVisible()).toBe(true); + expect(findIssueTransitionMode().exists()).toBe(false); }); // As per https://vuejs.org/v2/guide/forms.html#Checkbox-1, @@ -73,13 +84,15 @@ describe('JiraTriggerFields', () => { }); describe('initialTriggerMergeRequest is true', () => { - it('shows comment settings', () => { + it('shows trigger settings', () => { createComponent({ initialTriggerMergeRequest: true, }); expect(findCommentSettings().isVisible()).toBe(true); expect(findCommentDetail().isVisible()).toBe(false); + expect(findIssueTransitionEnabled().isVisible()).toBe(true); + expect(findIssueTransitionMode().exists()).toBe(false); }); }); @@ -95,21 +108,94 @@ describe('JiraTriggerFields', () => { }); }); - it('disables checkboxes and radios if inheriting', () => { + describe('initialJiraIssueTransitionAutomatic is false, initialJiraIssueTransitionId is not set', () => { + it('selects automatic transitions when enabling transitions', () => { + createComponent({ + initialTriggerCommit: true, + initialEnableComments: true, + }); + + const checkbox = findIssueTransitionEnabled(); + expect(checkbox.element.checked).toBe(false); + checkbox.trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + const [radio1, radio2] = findIssueTransitionModeRadios().wrappers; + expect(radio1.element.checked).toBe(true); + expect(radio2.element.checked).toBe(false); + }); + }); + }); + + describe('initialJiraIssueTransitionAutomatic is true', () => { + it('uses automatic transitions', () => { + createComponent({ + initialTriggerCommit: true, + initialJiraIssueTransitionAutomatic: true, + }); + + expect(findIssueTransitionEnabled().element.checked).toBe(true); + + const [radio1, radio2] = findIssueTransitionModeRadios().wrappers; + expect(radio1.element.checked).toBe(true); + expect(radio2.element.checked).toBe(false); + + expect(findIssueTransitionIdsField().exists()).toBe(false); + }); + }); + + describe('initialJiraIssueTransitionId is set', () => { + it('uses custom transitions', () => { + createComponent({ + initialTriggerCommit: true, + initialJiraIssueTransitionId: '1, 2, 3', + }); + + expect(findIssueTransitionEnabled().element.checked).toBe(true); + + const [radio1, radio2] = findIssueTransitionModeRadios().wrappers; + expect(radio1.element.checked).toBe(false); + expect(radio2.element.checked).toBe(true); + + const field = findIssueTransitionIdsField(); + expect(field.isVisible()).toBe(true); + expect(field.element).toMatchObject({ + type: 'text', + value: '1, 2, 3', + }); + }); + }); + + describe('initialJiraIssueTransitionAutomatic is true, initialJiraIssueTransitionId is set', () => { + it('uses automatic transitions', () => { + createComponent({ + initialTriggerCommit: true, + initialJiraIssueTransitionAutomatic: true, + initialJiraIssueTransitionId: '1, 2, 3', + }); + + expect(findIssueTransitionEnabled().element.checked).toBe(true); + + const [radio1, radio2] = findIssueTransitionModeRadios().wrappers; + expect(radio1.element.checked).toBe(true); + expect(radio2.element.checked).toBe(false); + + expect(findIssueTransitionIdsField().exists()).toBe(false); + }); + }); + + it('disables input fields if inheriting', () => { createComponent( { initialTriggerCommit: true, initialEnableComments: true, + initialJiraIssueTransitionId: '1, 2, 3', }, true, ); - wrapper.findAll('[type=checkbox]').wrappers.forEach((checkbox) => { - expect(checkbox.attributes('disabled')).toBe('disabled'); - }); - - wrapper.findAll('[type=radio]').wrappers.forEach((radio) => { - expect(radio.attributes('disabled')).toBe('disabled'); + wrapper.findAll('[type=text], [type=checkbox], [type=radio]').wrappers.forEach((input) => { + expect(input.attributes('disabled')).toBe('disabled'); }); }); }); diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 0dcfa30bc1b..678c789b070 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -109,6 +109,16 @@ RSpec.describe GroupsHelper do subject end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + helper.group_title(nested_group) + end + + expect do + helper.group_title(very_deep_nested_group) + end.not_to exceed_query_limit(control_count) + end end describe '#share_with_group_lock_help_text' do @@ -481,45 +491,68 @@ RSpec.describe GroupsHelper do end end - describe '#cached_open_group_issues_count' do + describe '#cached_issuables_count' do let_it_be(:current_user) { create(:user) } let_it_be(:group) { create(:group, name: 'group') } - let_it_be(:count_service) { Groups::OpenIssuesCountService } + + subject { helper.cached_issuables_count(group, type: type) } before do allow(helper).to receive(:current_user) { current_user } + allow(count_service).to receive(:new).and_call_original end - it 'returns all digits for count value under 1000' do - allow_next_instance_of(count_service) do |service| - allow(service).to receive(:count).and_return(999) + shared_examples 'caching issuables count' do + it 'calls the correct service class' do + subject + expect(count_service).to have_received(:new).with(group, current_user) end - expect(helper.cached_open_group_issues_count(group)).to eq('999') + it 'returns all digits for count value under 1000' do + allow_next_instance_of(count_service) do |service| + allow(service).to receive(:count).and_return(999) + end + + expect(subject).to eq('999') + end + + it 'returns truncated digits for count value over 1000' do + allow_next_instance_of(count_service) do |service| + allow(service).to receive(:count).and_return(2300) + end + + expect(subject).to eq('2.3k') + end + + it 'returns truncated digits for count value over 10000' do + allow_next_instance_of(count_service) do |service| + allow(service).to receive(:count).and_return(12560) + end + + expect(subject).to eq('12.6k') + end + + it 'returns truncated digits for count value over 100000' do + allow_next_instance_of(count_service) do |service| + allow(service).to receive(:count).and_return(112560) + end + + expect(subject).to eq('112.6k') + end end - it 'returns truncated digits for count value over 1000' do - allow_next_instance_of(count_service) do |service| - allow(service).to receive(:count).and_return(2300) - end + context 'with issue type' do + let(:type) { :issues } + let(:count_service) { Groups::OpenIssuesCountService } - expect(helper.cached_open_group_issues_count(group)).to eq('2.3k') + it_behaves_like 'caching issuables count' end - it 'returns truncated digits for count value over 10000' do - allow_next_instance_of(count_service) do |service| - allow(service).to receive(:count).and_return(12560) - end + context 'with merge request type' do + let(:type) { :merge_requests } + let(:count_service) { Groups::MergeRequestsCountService } - expect(helper.cached_open_group_issues_count(group)).to eq('12.6k') - end - - it 'returns truncated digits for count value over 100000' do - allow_next_instance_of(count_service) do |service| - allow(service).to receive(:count).and_return(112560) - end - - expect(helper.cached_open_group_issues_count(group)).to eq('112.6k') + it_behaves_like 'caching issuables count' end end end diff --git a/spec/helpers/services_helper_spec.rb b/spec/helpers/services_helper_spec.rb index 1726a8362a7..6dd872225ba 100644 --- a/spec/helpers/services_helper_spec.rb +++ b/spec/helpers/services_helper_spec.rb @@ -27,17 +27,31 @@ RSpec.describe ServicesHelper do ] end + let(:jira_fields) do + [ + :jira_issue_transition_automatic, + :jira_issue_transition_id + ] + end + subject { helper.integration_form_data(integration) } context 'Slack service' do let(:integration) { build(:slack_service) } it { is_expected.to include(*fields) } + it { is_expected.not_to include(*jira_fields) } specify do expect(subject[:reset_path]).to eq(helper.scoped_reset_integration_path(integration)) end end + + context 'Jira service' do + let(:integration) { build(:jira_service) } + + it { is_expected.to include(*fields, *jira_fields) } + end end describe '#scoped_reset_integration_path' do diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 3fc39fd3266..77b52c719c5 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -82,11 +82,8 @@ RSpec.describe JiraService do subject(:fields) { service.fields } - it 'includes transition help link' do - transition_id_field = fields.find { |field| field[:name] == 'jira_issue_transition_id' } - - expect(transition_id_field[:title]).to eq('Jira workflow transition IDs') - expect(transition_id_field[:help]).to include('/help/user/project/integrations/jira') + it 'returns custom fields' do + expect(fields.pluck(:name)).to eq(%w[url api_url username password]) end end @@ -460,10 +457,10 @@ RSpec.describe JiraService do end context 'with options' do - let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}?expand=renderedFields" } + let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}?expand=renderedFields,transitions" } it 'calls the Jira API with the options to get the issue' do - jira_service.find_issue(issue_key, rendered_fields: true) + jira_service.find_issue(issue_key, rendered_fields: true, transitions: true) expect(WebMock).to have_requested(:get, issue_url) end @@ -494,7 +491,7 @@ RSpec.describe JiraService do end before do - allow(jira_service).to receive_messages(jira_issue_transition_id: '999') + jira_service.jira_issue_transition_id = '999' # These stubs are needed to test JiraService#close_issue. # We close the issue then do another request to API to check if it got closed. @@ -505,7 +502,7 @@ RSpec.describe JiraService do allow(closed_issue).to receive(:resolution).and_return(true) allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) - allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return('JIRA-123') + allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return(issue_key) allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) WebMock.stub_request(:get, issue_url).with(basic_auth: %w(jira-username jira-password)) @@ -664,6 +661,61 @@ RSpec.describe JiraService do ).once end + context 'when custom transition IDs are blank' do + before do + jira_service.jira_issue_transition_id = '' + end + + it 'does not transition the issue' do + close_issue + + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + + context 'when using automatic issue transitions' do + let(:transitions) do + [ + { id: '1' }, + { id: '2', to: { statusCategory: { key: 'new' } } }, + { id: '3', to: { statusCategory: { key: 'done' } } }, + { id: '4', to: { statusCategory: { key: 'done' } } } + ] + end + + before do + jira_service.jira_issue_transition_automatic = true + + close_issue + end + + it 'uses the next transition with a status category of done' do + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"3"/ + ).once + end + + context 'when no done transition is available' do + let(:transitions) do + [ + { id: '1', to: { statusCategory: { key: 'new' } } } + ] + end + + it 'does not attempt to transition' do + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + + context 'when no valid transitions are returned' do + let(:transitions) { 'foo' } + + it 'does not attempt to transition' do + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + end + context 'when using multiple transition ids' do before do allow(jira_service).to receive_messages(jira_issue_transition_id: '1,2,3') @@ -902,4 +954,22 @@ RSpec.describe JiraService do end end end + + describe '#issue_transition_enabled?' do + it 'returns true if automatic transitions are enabled' do + jira_service.jira_issue_transition_automatic = true + + expect(jira_service.issue_transition_enabled?).to be(true) + end + + it 'returns true if custom transitions are set' do + jira_service.jira_issue_transition_id = '1, 2, 3' + + expect(jira_service.issue_transition_enabled?).to be(true) + end + + it 'returns false if automatic and custom transitions are disabled' do + expect(jira_service.issue_transition_enabled?).to be(false) + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index c4420c332c7..8b060fa2217 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1413,6 +1413,29 @@ RSpec.describe API::Internal::Base do end end + describe 'GET /internal/geo_proxy' do + subject { get api('/internal/geo_proxy'), params: { secret_token: secret_token } } + + context 'with valid auth' do + it 'returns empty data' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end + end + + context 'with invalid auth' do + let(:secret_token) { 'invalid_token' } + + it 'returns unauthorized' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + def lfs_auth_project(project) post( api("/internal/lfs_authenticate"), diff --git a/spec/services/groups/merge_requests_count_service_spec.rb b/spec/services/groups/merge_requests_count_service_spec.rb new file mode 100644 index 00000000000..10c7ba5fca4 --- /dev/null +++ b/spec/services/groups/merge_requests_count_service_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::MergeRequestsCountService, :use_clean_rails_memory_store_caching do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :public)} + let_it_be(:project) { create(:project, :repository, namespace: group) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + subject { described_class.new(group, user) } + + describe '#relation_for_count' do + before do + group.add_reporter(user) + allow(MergeRequestsFinder).to receive(:new).and_call_original + end + + it 'uses the MergeRequestsFinder to scope merge requests' do + expect(MergeRequestsFinder) + .to receive(:new) + .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true) + + subject.count + end + end + + it_behaves_like 'a counter caching service with threshold' +end diff --git a/spec/services/groups/open_issues_count_service_spec.rb b/spec/services/groups/open_issues_count_service_spec.rb index 8bbb1c90c6b..740e9846119 100644 --- a/spec/services/groups/open_issues_count_service_spec.rb +++ b/spec/services/groups/open_issues_count_service_spec.rb @@ -54,53 +54,7 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac end end - context 'with different cache values' do - let(:public_count_key) { subject.cache_key(described_class::PUBLIC_COUNT_KEY) } - let(:under_threshold) { described_class::CACHED_COUNT_THRESHOLD - 1 } - let(:over_threshold) { described_class::CACHED_COUNT_THRESHOLD + 1 } - - context 'when cache is empty' do - before do - Rails.cache.delete(public_count_key) - end - - it 'refreshes cache if value over threshold' do - allow(subject).to receive(:uncached_count).and_return(over_threshold) - - expect(subject.count).to eq(over_threshold) - expect(Rails.cache.read(public_count_key)).to eq(over_threshold) - end - - it 'does not refresh cache if value under threshold' do - allow(subject).to receive(:uncached_count).and_return(under_threshold) - - expect(subject.count).to eq(under_threshold) - expect(Rails.cache.read(public_count_key)).to be_nil - end - end - - context 'when cached count is under the threshold value' do - before do - Rails.cache.write(public_count_key, under_threshold) - end - - it 'does not refresh cache' do - expect(Rails.cache).not_to receive(:write) - expect(subject.count).to eq(under_threshold) - end - end - - context 'when cached count is over the threshold value' do - before do - Rails.cache.write(public_count_key, over_threshold) - end - - it 'does not refresh cache' do - expect(Rails.cache).not_to receive(:write) - expect(subject.count).to eq(over_threshold) - end - end - end + it_behaves_like 'a counter caching service with threshold' end end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 87e5750ce6e..561fb524897 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -166,20 +166,6 @@ RSpec.describe MergeRequests::MergeService do service.execute(merge_request) end - context 'when jira_issue_transition_id is not present' do - before do - allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(nil) - end - - it 'does not close issue' do - jira_tracker.update!(jira_issue_transition_id: nil) - - expect_any_instance_of(JiraService).not_to receive(:transition_issue) - - service.execute(merge_request) - end - end - context 'wrong issue markdown' do it 'does not close issues on Jira issue tracker' do jira_issue = ExternalIssue.new('#JIRA-123', project) diff --git a/spec/support/shared_contexts/project_service_jira_context.rb b/spec/support/shared_contexts/project_service_jira_context.rb index 8e01de70846..54bb9fd108e 100644 --- a/spec/support/shared_contexts/project_service_jira_context.rb +++ b/spec/support/shared_contexts/project_service_jira_context.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true RSpec.shared_context 'project service Jira context' do - let(:url) { 'http://jira.example.com' } - let(:test_url) { 'http://jira.example.com/rest/api/2/serverInfo' } + let(:url) { 'https://jira.example.com' } + let(:test_url) { 'https://jira.example.com/rest/api/2/serverInfo' } def fill_form(disable: false) click_active_checkbox if disable @@ -10,6 +10,5 @@ RSpec.shared_context 'project service Jira context' do fill_in 'service_url', with: url fill_in 'service_username', with: 'username' fill_in 'service_password', with: 'password' - fill_in 'service_jira_issue_transition_id', with: '25' end end diff --git a/spec/support/shared_contexts/project_service_shared_context.rb b/spec/support/shared_contexts/project_service_shared_context.rb index b4b9ab456e0..a8e75c624e8 100644 --- a/spec/support/shared_contexts/project_service_shared_context.rb +++ b/spec/support/shared_contexts/project_service_shared_context.rb @@ -15,7 +15,10 @@ RSpec.shared_context 'project service activation' do def visit_project_integration(name) visit_project_integrations - click_link(name) + + within('#content-body') do + click_link(name) + end end def click_active_checkbox diff --git a/spec/support/shared_examples/services/groups_count_service_shared_examples.rb b/spec/support/shared_examples/services/groups_count_service_shared_examples.rb new file mode 100644 index 00000000000..84937c3d4d7 --- /dev/null +++ b/spec/support/shared_examples/services/groups_count_service_shared_examples.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +# The calling spec should use `:use_clean_rails_memory_store_caching` +# when including this shared example. E.g.: +# +# describe MyCountService, :use_clean_rails_memory_store_caching do +# it_behaves_like 'a counter caching service with threshold' +# end +RSpec.shared_examples 'a counter caching service with threshold' do + let(:cache_key) { subject.cache_key } + let(:under_threshold) { described_class::CACHED_COUNT_THRESHOLD - 1 } + let(:over_threshold) { described_class::CACHED_COUNT_THRESHOLD + 1 } + + context 'when cache is empty' do + before do + Rails.cache.delete(cache_key) + end + + it 'refreshes cache if value over threshold' do + allow(subject).to receive(:uncached_count).and_return(over_threshold) + + expect(subject.count).to eq(over_threshold) + expect(Rails.cache.read(cache_key)).to eq(over_threshold) + end + + it 'does not refresh cache if value under threshold' do + allow(subject).to receive(:uncached_count).and_return(under_threshold) + + expect(subject.count).to eq(under_threshold) + expect(Rails.cache.read(cache_key)).to be_nil + end + end + + context 'when cached count is under the threshold value' do + before do + Rails.cache.write(cache_key, under_threshold) + end + + it 'does not refresh cache' do + expect(Rails.cache).not_to receive(:write) + expect(subject.count).to eq(under_threshold) + end + end + + context 'when cached count is over the threshold value' do + before do + Rails.cache.write(cache_key, over_threshold) + end + + it 'does not refresh cache' do + expect(Rails.cache).not_to receive(:write) + expect(subject.count).to eq(over_threshold) + end + end +end