diff --git a/.gitlab/ci/review-apps/qa.gitlab-ci.yml b/.gitlab/ci/review-apps/qa.gitlab-ci.yml index 07f8bd4e5fd..6cb7a81d638 100644 --- a/.gitlab/ci/review-apps/qa.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/qa.gitlab-ci.yml @@ -16,10 +16,10 @@ GITLAB_ADMIN_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}" GITLAB_QA_ADMIN_ACCESS_TOKEN: "${REVIEW_APPS_ROOT_TOKEN}" GITHUB_ACCESS_TOKEN: "${REVIEW_APPS_QA_GITHUB_ACCESS_TOKEN}" - EE_LICENSE: "${REVIEW_APPS_EE_LICENSE}" SIGNUP_DISABLED: "true" before_script: # Use $CI_MERGE_REQUEST_SOURCE_BRANCH_SHA so that GitLab image built in omnibus-gitlab-mirror and QA image are in sync. + - export EE_LICENSE="$(cat $REVIEW_APPS_EE_LICENSE_FILE)" - if [ -n "$CI_MERGE_REQUEST_SOURCE_BRANCH_SHA" ]; then git checkout -f ${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}; fi diff --git a/app/assets/javascripts/environments/components/new_environments_app.vue b/app/assets/javascripts/environments/components/new_environments_app.vue index a5526f9cd71..bfb5689d623 100644 --- a/app/assets/javascripts/environments/components/new_environments_app.vue +++ b/app/assets/javascripts/environments/components/new_environments_app.vue @@ -1,6 +1,7 @@ + diff --git a/app/assets/javascripts/jira_connect/subscriptions/index.js b/app/assets/javascripts/jira_connect/subscriptions/index.js index 8a7a80d885d..cd1fc1d4455 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/index.js +++ b/app/assets/javascripts/jira_connect/subscriptions/index.js @@ -7,25 +7,11 @@ import Translate from '~/vue_shared/translate'; import JiraConnectApp from './components/app.vue'; import createStore from './store'; -import { getGitlabSignInURL, sizeToParent } from './utils'; +import { sizeToParent } from './utils'; const store = createStore(); -/** - * Add `return_to` query param to all HAML-defined GitLab sign in links. - */ -const updateSignInLinks = async () => { - await Promise.all( - Array.from(document.querySelectorAll('.js-jira-connect-sign-in')).map(async (el) => { - const updatedLink = await getGitlabSignInURL(el.getAttribute('href')); - el.setAttribute('href', updatedLink); - }), - ); -}; - -export async function initJiraConnect() { - await updateSignInLinks(); - +export function initJiraConnect() { const el = document.querySelector('.js-jira-connect-app'); if (!el) { return null; @@ -35,7 +21,7 @@ export async function initJiraConnect() { Vue.use(Translate); Vue.use(GlFeatureFlagsPlugin); - const { groupsPath, subscriptions, subscriptionsPath, usersPath } = el.dataset; + const { groupsPath, subscriptions, subscriptionsPath, usersPath, gitlabUserPath } = el.dataset; sizeToParent(); return new Vue({ @@ -46,6 +32,7 @@ export async function initJiraConnect() { subscriptions: JSON.parse(subscriptions), subscriptionsPath, usersPath, + gitlabUserPath, }, render(createElement) { return createElement(JiraConnectApp); diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3bec7928058..89949b82ae5 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -282,7 +282,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo if merge_request.errors.present? render json: @merge_request.errors, status: :bad_request else - render json: serializer.represent(@merge_request, serializer: 'basic') + render json: serializer.represent(@merge_request, serializer: params[:serializer] || 'basic') end end end diff --git a/app/helpers/jira_connect_helper.rb b/app/helpers/jira_connect_helper.rb index 475469a6df9..9a0f0944fd1 100644 --- a/app/helpers/jira_connect_helper.rb +++ b/app/helpers/jira_connect_helper.rb @@ -8,7 +8,8 @@ module JiraConnectHelper groups_path: api_v4_groups_path(params: { min_access_level: Gitlab::Access::MAINTAINER, skip_groups: skip_groups }), subscriptions: subscriptions.map { |s| serialize_subscription(s) }.to_json, subscriptions_path: jira_connect_subscriptions_path, - users_path: current_user ? nil : jira_connect_users_path + users_path: current_user ? nil : jira_connect_users_path, # users_path is used to determine if user is signed in + gitlab_user_path: current_user ? user_path(current_user) : nil } end diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 1d8b657025c..f2e1d158c2d 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -40,7 +40,9 @@ module SystemNoteHelper 'new_alert_added' => 'warning', 'severity' => 'information-o', 'cloned' => 'documents', - 'issue_type' => 'pencil-square' + 'issue_type' => 'pencil-square', + 'attention_requested' => 'user', + 'attention_request_removed' => 'user' }.freeze def system_note_icon_name(note) diff --git a/app/models/concerns/merge_request_reviewer_state.rb b/app/models/concerns/merge_request_reviewer_state.rb index 216a3a0bd64..5859f43a70c 100644 --- a/app/models/concerns/merge_request_reviewer_state.rb +++ b/app/models/concerns/merge_request_reviewer_state.rb @@ -15,11 +15,5 @@ module MergeRequestReviewerState inclusion: { in: self.states.keys } after_initialize :set_state, unless: :persisted? - - def set_state - if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml) - self.state = :attention_requested - end - end end end diff --git a/app/models/merge_request_assignee.rb b/app/models/merge_request_assignee.rb index fd8e5860040..77b46fa50f4 100644 --- a/app/models/merge_request_assignee.rb +++ b/app/models/merge_request_assignee.rb @@ -10,6 +10,12 @@ class MergeRequestAssignee < ApplicationRecord scope :in_projects, ->(project_ids) { joins(:merge_request).where(merge_requests: { target_project_id: project_ids }) } + def set_state + if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml) + self.state = MergeRequestReviewer.find_by(user_id: self.user_id, merge_request_id: self.merge_request_id)&.state || :attention_requested + end + end + def cache_key [model_name.cache_key, id, state, assignee.cache_key] end diff --git a/app/models/merge_request_reviewer.rb b/app/models/merge_request_reviewer.rb index 4abf0fa09f0..8c75fb2e4e6 100644 --- a/app/models/merge_request_reviewer.rb +++ b/app/models/merge_request_reviewer.rb @@ -6,6 +6,12 @@ class MergeRequestReviewer < ApplicationRecord belongs_to :merge_request belongs_to :reviewer, class_name: 'User', foreign_key: :user_id, inverse_of: :merge_request_reviewers + def set_state + if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml) + self.state = MergeRequestAssignee.find_by(user_id: self.user_id, merge_request_id: self.merge_request_id)&.state || :attention_requested + end + end + def cache_key [model_name.cache_key, id, state, reviewer.cache_key] end diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 749b9dce97c..7b13109dbc4 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -24,6 +24,7 @@ class SystemNoteMetadata < ApplicationRecord opened closed merged duplicate locked unlocked outdated reviewer tag due_date pinned_embed cherry_pick health_status approved unapproved status alert_issue_added relate unrelate new_alert_added severity + attention_requested attention_request_removed ].freeze validates :note, presence: true, unless: :importing? diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb index 4e36ae065bb..fd24e87454c 100644 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ b/app/services/merge_requests/toggle_attention_requested_service.rb @@ -19,7 +19,10 @@ module MergeRequests update_state(assignee) if reviewer&.attention_requested? || assignee&.attention_requested? + create_attention_request_note notity_user + else + create_remove_attention_request_note end success @@ -35,6 +38,14 @@ module MergeRequests todo_service.create_attention_requested_todo(merge_request, current_user, user) end + def create_attention_request_note + SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) + end + + def create_remove_attention_request_note + SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) + end + def assignee merge_request.find_assignee(user) end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e98dfb872fe..0d13c73d49d 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -115,6 +115,14 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).change_status(status, source) end + def request_attention(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).request_attention(user) + end + + def remove_attention_request(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).remove_attention_request(user) + end + # Called when 'merge when pipeline succeeds' is executed def merge_when_pipeline_succeeds(noteable, project, author, sha) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).merge_when_pipeline_succeeds(sha) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 92540f957c8..d33dcd65589 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -323,6 +323,52 @@ module SystemNotes existing_mentions_for(mentioned_in, noteable, notes).exists? end + # Called when a user's attention has been requested for a Notable + # + # user - User's whos attention has been requested + # + # Example Note text: + # + # "requested attention from @eli.wisoky" + # + # Returns the created Note object + def request_attention(user) + body = "requested attention from #{user.to_reference}" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_requested')) + end + + # Called when a user's attention request has been removed for a Notable + # + # user - User's whos attention request has been removed + # + # Example Note text: + # + # "removed attention request from @eli.wisoky" + # + # Returns the created Note object + def remove_attention_request(user) + body = "removed attention request from #{user.to_reference}" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_request_removed')) + end + + # Called when a Noteable has been marked as the canonical Issue of a duplicate + # + # duplicate_issue - Issue that was a duplicate of this + # + # Example Note text: + # + # "marked #1234 as a duplicate of this issue" + # + # "marked other_project#5678 as a duplicate of this issue" + # + # Returns the created Note object + def mark_canonical_issue_of_duplicate(duplicate_issue) + body = "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue" + create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + end + # Called when a Noteable has been marked as a duplicate of another Issue # # canonical_issue - Issue that this is a duplicate of @@ -342,22 +388,6 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end - # Called when a Noteable has been marked as the canonical Issue of a duplicate - # - # duplicate_issue - Issue that was a duplicate of this - # - # Example Note text: - # - # "marked #1234 as a duplicate of this issue" - # - # "marked other_project#5678 as a duplicate of this issue" - # - # Returns the created Note object - def mark_canonical_issue_of_duplicate(duplicate_issue) - body = "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue" - create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) - end - def add_email_participants(body) create_note(NoteSummary.new(noteable, project, author, body)) end diff --git a/app/views/jira_connect/subscriptions/index.html.haml b/app/views/jira_connect/subscriptions/index.html.haml index be2be7288f8..d92c30c8840 100644 --- a/app/views/jira_connect/subscriptions/index.html.haml +++ b/app/views/jira_connect/subscriptions/index.html.haml @@ -1,13 +1,6 @@ %header.jira-connect-header.gl-display-flex.gl-align-items-center.gl-justify-content-center.gl-px-5.gl-border-b-solid.gl-border-b-gray-100.gl-border-b-1.gl-bg-white = link_to brand_header_logo, Gitlab.config.gitlab.url, target: '_blank', rel: 'noopener noreferrer' -.jira-connect-user.gl-font-base - - if current_user - - user_link = link_to(current_user.to_reference, jira_connect_users_path, target: '_blank', rel: 'noopener noreferrer', class: 'js-jira-connect-sign-in') - = _('Signed in to GitLab as %{user_link}').html_safe % { user_link: user_link } - - elsif @subscriptions.present? - = link_to _('Sign in to GitLab'), jira_connect_users_path, target: '_blank', rel: 'noopener noreferrer', class: 'js-jira-connect-sign-in' - %main.jira-connect-app.gl-px-5.gl-pt-7.gl-mx-auto .js-jira-connect-app{ data: jira_connect_app_data(@subscriptions) } diff --git a/doc/development/database/multiple_databases.md b/doc/development/database/multiple_databases.md index 6dce3217b24..1338e83070f 100644 --- a/doc/development/database/multiple_databases.md +++ b/doc/development/database/multiple_databases.md @@ -630,9 +630,13 @@ keys"](loose_foreign_keys.md). ## `config/database.yml` -GitLab will support running multiple databases in the future, for example to [separate tables for the continuous integration features](https://gitlab.com/groups/gitlab-org/-/epics/6167) from the main database. In order to prepare for this change, we [validate the structure of the configuration](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67877) in `database.yml` to ensure that only known databases are used. +GitLab is adding support to run multiple databases, for example to +[separate tables for the continuous integration features](https://gitlab.com/groups/gitlab-org/-/epics/6167) +from the main database. In order to prepare for this change, we +[validate the structure of the configuration](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67877) +in `database.yml` to ensure that only known databases are used. -Previously, the `config/database.yml` would look like this: +Previously, the `config/database.yml` looked like this: ```yaml production: @@ -642,15 +646,16 @@ production: ... ``` -With the support for many databases the support for this -syntax is deprecated and will be removed in [15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/338182). +With the support for many databases this +syntax is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/338182) +and will be removed in [15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/338182). The new `config/database.yml` needs to include a database name to define a database configuration. Only `main:` and `ci:` database -names are supported today. The `main:` needs to always be a first +names are supported. The `main:` database must always be a first entry in a hash. This change applies to decomposed and non-decomposed -change. If an invalidate or deprecated syntax is used the error -or warning will be printed during application start. +change. If an invalid or deprecated syntax is used the error +or warning is printed during application start. ```yaml # Non-decomposed database diff --git a/doc/integration/elasticsearch.md b/doc/integration/elasticsearch.md index 20f8fdc55f2..2f9ec7a41c2 100644 --- a/doc/integration/elasticsearch.md +++ b/doc/integration/elasticsearch.md @@ -375,7 +375,7 @@ Sometimes, you might want to abandon the unfinished reindex job and resume the i 1. On the top bar, select **Menu > Admin**. 1. On the left sidebar, select **Settings > Advanced Search**. -1. Expand **Elasticsearch zero-downtime reindexing**. +1. Expand **Advanced Search**. 1. Clear the **Pause Elasticsearch indexing** checkbox. ## Advanced Search migrations diff --git a/doc/integration/mattermost/index.md b/doc/integration/mattermost/index.md index dcd4d509cd8..531903f7194 100644 --- a/doc/integration/mattermost/index.md +++ b/doc/integration/mattermost/index.md @@ -18,7 +18,7 @@ Each release of GitLab Mattermost is compiled and manually tested on an AMD 64 c ## Getting started -GitLab Mattermost expects to run on its own virtual host. In your DNS settings, you will need +GitLab Mattermost expects to run on its own virtual host. In your DNS settings, you need two entries pointing to the same machine. For example, `gitlab.example.com` and `mattermost.example.com`. @@ -41,7 +41,7 @@ GitLab Mattermost is disabled by default. To enable it: The Omnibus GitLab package attempts to automatically authorize GitLab Mattermost with GitLab if the applications are running on the same server. Automatic authorization requires access to the GitLab database. If the GitLab database is not available -you will need to manually authorize GitLab Mattermost for access to GitLab using the process described in the [Authorize GitLab Mattermost section](#authorize-gitlab-mattermost). +you need to manually authorize GitLab Mattermost for access to GitLab using the process described in the [Authorize GitLab Mattermost section](#authorize-gitlab-mattermost). ## Configuring Mattermost @@ -51,7 +51,7 @@ Mattermost settings and where they can be set is available [in the Mattermost do While using the System Console is recommended, you can also configure Mattermost using one of the following options: 1. Edit the Mattermost configuration directly through `/var/opt/gitlab/mattermost/config.json`. -1. Specify environment variables used to run Mattermost by changing the `mattermost['env']` setting in `gitlab.rb`. Any settings configured in this way will be disabled from the System Console and cannot be changed without restarting Mattermost. +1. Specify environment variables used to run Mattermost by changing the `mattermost['env']` setting in `gitlab.rb`. Any settings configured in this way are disabled from the System Console and cannot be changed without restarting Mattermost. ## Running GitLab Mattermost with HTTPS @@ -71,7 +71,7 @@ mattermost_nginx['redirect_http_to_https'] = true ``` If you haven't named your certificate and key `mattermost.gitlab.example.crt` -and `mattermost.gitlab.example.key` then you'll need to also add the full paths +and `mattermost.gitlab.example.key` then you need to also add the full paths as shown below. ```ruby @@ -85,7 +85,7 @@ Once the configuration is set, run `sudo gitlab-ctl reconfigure` to apply the ch ## Running GitLab Mattermost on its own server -If you want to run GitLab and GitLab Mattermost on two separate servers the GitLab services will still be set up on your GitLab Mattermost server, but they will not accept user requests or +If you want to run GitLab and GitLab Mattermost on two separate servers the GitLab services are still set up on your GitLab Mattermost server, but they do not accept user requests or consume system resources. You can use the following settings and configuration details on the GitLab Mattermost server to effectively disable the GitLab service bundled into the Omnibus package. ```ruby @@ -124,7 +124,7 @@ http://mattermost.example.com/login/gitlab/complete Note that you do not need to select any options under **Scopes**. Choose **Save application**. -Once the application is created you will be provided with an `Application ID` and `Secret`. One other piece of information needed is the URL of GitLab instance. +Once the application is created you are provided with an `Application ID` and `Secret`. One other piece of information needed is the URL of GitLab instance. Return to the server running GitLab Mattermost and edit the `/etc/gitlab/gitlab.rb` configuration file as follows using the values you received above: ```ruby @@ -190,7 +190,7 @@ sudo -i -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/po #### Back up the `data` directory and `config.json` -Mattermost has a `data` directory and `config.json` file that will need to be backed up as well: +Mattermost has a `data` directory and `config.json` file that need to be backed up as well: ```shell sudo tar -zcvf mattermost_data_$(date --rfc-3339=date).gz -C /var/opt/gitlab/mattermost data config.json diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index fc159e1b70c..f4a5044c049 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -367,7 +367,8 @@ RSpec.describe Projects::MergeRequestsController do namespace_id: project.namespace, project_id: project, id: merge_request.iid, - merge_request: mr_params + merge_request: mr_params, + serializer: 'basic' }.merge(additional_params) put :update, params: params diff --git a/spec/frontend/environments/graphql/resolvers_spec.js b/spec/frontend/environments/graphql/resolvers_spec.js index 4d2a0818996..320e4794de0 100644 --- a/spec/frontend/environments/graphql/resolvers_spec.js +++ b/spec/frontend/environments/graphql/resolvers_spec.js @@ -1,6 +1,7 @@ import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import { resolvers } from '~/environments/graphql/resolvers'; +import pollIntervalQuery from '~/environments/graphql/queries/poll_interval.query.graphql'; import { TEST_HOST } from 'helpers/test_constants'; import { environmentsApp, resolvedEnvironmentsApp, folder, resolvedFolder } from './mock_data'; @@ -21,10 +22,27 @@ describe('~/frontend/environments/graphql/resolvers', () => { describe('environmentApp', () => { it('should fetch environments and map them to frontend data', async () => { - mock.onGet(ENDPOINT, { params: { nested: true } }).reply(200, environmentsApp); + const cache = { writeQuery: jest.fn() }; + mock.onGet(ENDPOINT, { params: { nested: true } }).reply(200, environmentsApp, {}); - const app = await mockResolvers.Query.environmentApp(); + const app = await mockResolvers.Query.environmentApp(null, null, { cache }); expect(app).toEqual(resolvedEnvironmentsApp); + expect(cache.writeQuery).toHaveBeenCalledWith({ + query: pollIntervalQuery, + data: { interval: undefined }, + }); + }); + it('should set the poll interval when there is one', async () => { + const cache = { writeQuery: jest.fn() }; + mock + .onGet(ENDPOINT, { params: { nested: true } }) + .reply(200, environmentsApp, { 'poll-interval': 3000 }); + + await mockResolvers.Query.environmentApp(null, null, { cache }); + expect(cache.writeQuery).toHaveBeenCalledWith({ + query: pollIntervalQuery, + data: { interval: 3000 }, + }); }); }); describe('folder', () => { @@ -42,7 +60,7 @@ describe('~/frontend/environments/graphql/resolvers', () => { it('should post to the stop environment path', async () => { mock.onPost(ENDPOINT).reply(200); - await mockResolvers.Mutations.stopEnvironment(null, { environment: { stopPath: ENDPOINT } }); + await mockResolvers.Mutation.stopEnvironment(null, { environment: { stopPath: ENDPOINT } }); expect(mock.history.post).toContainEqual( expect.objectContaining({ url: ENDPOINT, method: 'post' }), @@ -53,7 +71,7 @@ describe('~/frontend/environments/graphql/resolvers', () => { it('should post to the retry environment path', async () => { mock.onPost(ENDPOINT).reply(200); - await mockResolvers.Mutations.rollbackEnvironment(null, { + await mockResolvers.Mutation.rollbackEnvironment(null, { environment: { retryUrl: ENDPOINT }, }); @@ -66,7 +84,7 @@ describe('~/frontend/environments/graphql/resolvers', () => { it('should DELETE to the delete environment path', async () => { mock.onDelete(ENDPOINT).reply(200); - await mockResolvers.Mutations.deleteEnvironment(null, { + await mockResolvers.Mutation.deleteEnvironment(null, { environment: { deletePath: ENDPOINT }, }); @@ -79,7 +97,7 @@ describe('~/frontend/environments/graphql/resolvers', () => { it('should post to the auto stop path', async () => { mock.onPost(ENDPOINT).reply(200); - await mockResolvers.Mutations.cancelAutoStop(null, { + await mockResolvers.Mutation.cancelAutoStop(null, { environment: { autoStopPath: ENDPOINT }, }); diff --git a/spec/frontend/jira_connect/subscriptions/components/app_spec.js b/spec/frontend/jira_connect/subscriptions/components/app_spec.js index 8e464968453..47fe96262ee 100644 --- a/spec/frontend/jira_connect/subscriptions/components/app_spec.js +++ b/spec/frontend/jira_connect/subscriptions/components/app_spec.js @@ -5,6 +5,7 @@ import JiraConnectApp from '~/jira_connect/subscriptions/components/app.vue'; import AddNamespaceButton from '~/jira_connect/subscriptions/components/add_namespace_button.vue'; import SignInButton from '~/jira_connect/subscriptions/components/sign_in_button.vue'; import SubscriptionsList from '~/jira_connect/subscriptions/components/subscriptions_list.vue'; +import UserLink from '~/jira_connect/subscriptions/components/user_link.vue'; import createStore from '~/jira_connect/subscriptions/store'; import { SET_ALERT } from '~/jira_connect/subscriptions/store/mutation_types'; import { __ } from '~/locale'; @@ -12,6 +13,7 @@ import { mockSubscription } from '../mock_data'; jest.mock('~/jira_connect/subscriptions/utils', () => ({ retrieveAlert: jest.fn().mockReturnValue({ message: 'error message' }), + getGitlabSignInURL: jest.fn(), })); describe('JiraConnectApp', () => { @@ -83,6 +85,22 @@ describe('JiraConnectApp', () => { }); }, ); + + it('renders UserLink component', () => { + createComponent({ + provide: { + usersPath: '/user', + subscriptions: [], + }, + }); + + const userLink = wrapper.findComponent(UserLink); + expect(userLink.exists()).toBe(true); + expect(userLink.props()).toEqual({ + hasSubscriptions: false, + userSignedIn: false, + }); + }); }); describe('alert', () => { diff --git a/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js b/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js new file mode 100644 index 00000000000..b98a36269a3 --- /dev/null +++ b/spec/frontend/jira_connect/subscriptions/components/user_link_spec.js @@ -0,0 +1,91 @@ +import { GlSprintf } from '@gitlab/ui'; +import UserLink from '~/jira_connect/subscriptions/components/user_link.vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; + +jest.mock('~/jira_connect/subscriptions/utils', () => ({ + getGitlabSignInURL: jest.fn().mockImplementation((path) => Promise.resolve(path)), +})); + +describe('SubscriptionsList', () => { + let wrapper; + + const createComponent = (propsData = {}, { provide } = {}) => { + wrapper = shallowMountExtended(UserLink, { + propsData, + provide, + stubs: { + GlSprintf, + }, + }); + }; + + const findSignInLink = () => wrapper.findByTestId('sign-in-link'); + const findGitlabUserLink = () => wrapper.findByTestId('gitlab-user-link'); + const findSprintf = () => wrapper.findComponent(GlSprintf); + + afterEach(() => { + wrapper.destroy(); + }); + + describe.each` + userSignedIn | hasSubscriptions | expectGlSprintf | expectGlLink + ${true} | ${false} | ${true} | ${false} + ${false} | ${true} | ${false} | ${true} + ${true} | ${true} | ${true} | ${false} + ${false} | ${false} | ${false} | ${false} + `( + 'when `userSignedIn` is $userSignedIn and `hasSubscriptions` is $hasSubscriptions', + ({ userSignedIn, hasSubscriptions, expectGlSprintf, expectGlLink }) => { + it('renders template correctly', () => { + createComponent({ + userSignedIn, + hasSubscriptions, + }); + + expect(findSprintf().exists()).toBe(expectGlSprintf); + expect(findSignInLink().exists()).toBe(expectGlLink); + }); + }, + ); + + describe('sign in link', () => { + it('renders with correct href', async () => { + const mockUsersPath = '/user'; + createComponent( + { + userSignedIn: false, + hasSubscriptions: true, + }, + { provide: { usersPath: mockUsersPath } }, + ); + + await waitForPromises(); + + expect(findSignInLink().exists()).toBe(true); + expect(findSignInLink().attributes('href')).toBe(mockUsersPath); + }); + }); + + describe('gitlab user link', () => { + window.gon = { current_username: 'root' }; + + beforeEach(() => { + createComponent( + { + userSignedIn: true, + hasSubscriptions: true, + }, + { provide: { gitlabUserPath: '/root' } }, + ); + }); + + it('renders with correct href', () => { + expect(findGitlabUserLink().attributes('href')).toBe('/root'); + }); + + it('contains GitLab user handle', () => { + expect(findGitlabUserLink().text()).toBe('@root'); + }); + }); +}); diff --git a/spec/frontend/jira_connect/subscriptions/index_spec.js b/spec/frontend/jira_connect/subscriptions/index_spec.js deleted file mode 100644 index b97918a198e..00000000000 --- a/spec/frontend/jira_connect/subscriptions/index_spec.js +++ /dev/null @@ -1,36 +0,0 @@ -import { initJiraConnect } from '~/jira_connect/subscriptions'; -import { getGitlabSignInURL } from '~/jira_connect/subscriptions/utils'; - -jest.mock('~/jira_connect/subscriptions/utils'); - -describe('initJiraConnect', () => { - const mockInitialHref = 'https://gitlab.com'; - - beforeEach(() => { - setFixtures(` - Sign In - Another Sign In - `); - }); - - const assertSignInLinks = (expectedLink) => { - Array.from(document.querySelectorAll('.js-jira-connect-sign-in')).forEach((el) => { - expect(el.getAttribute('href')).toBe(expectedLink); - }); - }; - - describe('Sign in links', () => { - it('are updated on initialization', async () => { - const mockSignInLink = `https://gitlab.com?return_to=${encodeURIComponent('/test/location')}`; - getGitlabSignInURL.mockResolvedValue(mockSignInLink); - - // assert the initial state - assertSignInLinks(mockInitialHref); - - await initJiraConnect(); - - // assert the update has occurred - assertSignInLinks(mockSignInLink); - }); - }); -}); diff --git a/spec/helpers/jira_connect_helper_spec.rb b/spec/helpers/jira_connect_helper_spec.rb index 55a5c724665..0f78185dc7d 100644 --- a/spec/helpers/jira_connect_helper_spec.rb +++ b/spec/helpers/jira_connect_helper_spec.rb @@ -19,7 +19,9 @@ RSpec.describe JiraConnectHelper do is_expected.to include( :groups_path, :subscriptions_path, - :users_path + :users_path, + :subscriptions, + :gitlab_user_path ) end @@ -32,6 +34,10 @@ RSpec.describe JiraConnectHelper do expect(subject[:groups_path]).to include("#{skip_groups_param}=#{subscription.namespace.id}") end + + it 'assigns gitlab_user_path to nil' do + expect(subject[:gitlab_user_path]).to be_nil + end end context 'user is logged in' do @@ -42,6 +48,10 @@ RSpec.describe JiraConnectHelper do it 'assigns users_path to nil' do expect(subject[:users_path]).to be_nil end + + it 'assigns gitlab_user_path correctly' do + expect(subject[:gitlab_user_path]).to eq(user_path(user)) + end end end end diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 5bb8e7184a3..58b802de8e0 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' RSpec.describe MergeRequestAssignee do + let(:assignee) { create(:user) } let(:merge_request) { create(:merge_request) } - subject { merge_request.merge_request_assignees.build(assignee: create(:user)) } + subject { merge_request.merge_request_assignees.build(assignee: assignee) } describe 'associations' do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } @@ -41,4 +42,13 @@ RSpec.describe MergeRequestAssignee do it_behaves_like 'having unique enum values' it_behaves_like 'having reviewer state' + + describe 'syncs to reviewer state' do + before do + reviewer = merge_request.merge_request_reviewers.build(reviewer: assignee) + reviewer.update!(state: :reviewed) + end + + it { is_expected.to have_attributes(state: 'reviewed') } + end end diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index d69d60c94f0..d99fd4afb0f 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -3,14 +3,24 @@ require 'spec_helper' RSpec.describe MergeRequestReviewer do + let(:reviewer) { create(:user) } let(:merge_request) { create(:merge_request) } - subject { merge_request.merge_request_reviewers.build(reviewer: create(:user)) } + subject { merge_request.merge_request_reviewers.build(reviewer: reviewer) } it_behaves_like 'having unique enum values' it_behaves_like 'having reviewer state' + describe 'syncs to assignee state' do + before do + assignee = merge_request.merge_request_assignees.build(assignee: reviewer) + assignee.update!(state: :reviewed) + end + + it { is_expected.to have_attributes(state: 'reviewed') } + end + describe 'associations' do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } diff --git a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb index e2455a71eef..e5ba7bcefae 100644 --- a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb +++ b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb @@ -19,6 +19,8 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do allow(NotificationService).to receive(:new) { notification_service } allow(service).to receive(:todo_service).and_return(todo_service) allow(service).to receive(:notification_service).and_return(notification_service) + allow(SystemNoteService).to receive(:request_attention) + allow(SystemNoteService).to receive(:remove_attention_request) project.add_developer(current_user) project.add_developer(user) @@ -93,6 +95,12 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do service.execute end + + it 'creates a request attention system note' do + expect(SystemNoteService).to receive(:request_attention).with(merge_request, merge_request.project, current_user, assignee_user) + + service.execute + end end context 'assignee is the same as reviewer' do @@ -132,6 +140,12 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do service.execute end + + it 'creates a remove attention request system note' do + expect(SystemNoteService).to receive(:remove_attention_request).with(merge_request, merge_request.project, current_user, user) + + service.execute + end end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 11c6dbe92e7..3ec2c71b20c 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -146,6 +146,30 @@ RSpec.describe SystemNoteService do end end + describe '.request_attention' do + let(:user) { double } + + it 'calls IssuableService' do + expect_next_instance_of(::SystemNotes::IssuablesService) do |service| + expect(service).to receive(:request_attention).with(user) + end + + described_class.request_attention(noteable, project, author, user) + end + end + + describe '.remove_attention_request' do + let(:user) { double } + + it 'calls IssuableService' do + expect_next_instance_of(::SystemNotes::IssuablesService) do |service| + expect(service).to receive(:remove_attention_request).with(user) + end + + described_class.remove_attention_request(noteable, project, author, user) + end + end + describe '.merge_when_pipeline_succeeds' do it 'calls MergeRequestsService' do sha = double diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index 43760e296bc..7e53e66303b 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -199,6 +199,42 @@ RSpec.describe ::SystemNotes::IssuablesService do end end + describe '#request_attention' do + subject { service.request_attention(user) } + + let(:user) { create(:user) } + + it_behaves_like 'a system note' do + let(:action) { 'attention_requested' } + end + + context 'when attention requested' do + it_behaves_like 'a note with overridable created_at' + + it 'sets the note text' do + expect(subject.note).to eq "requested attention from @#{user.username}" + end + end + end + + describe '#remove_attention_request' do + subject { service.remove_attention_request(user) } + + let(:user) { create(:user) } + + it_behaves_like 'a system note' do + let(:action) { 'attention_request_removed' } + end + + context 'when attention request is removed' do + it_behaves_like 'a note with overridable created_at' + + it 'sets the note text' do + expect(subject.note).to eq "removed attention request from @#{user.username}" + end + end + end + describe '#change_title' do let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') } diff --git a/spec/views/jira_connect/subscriptions/index.html.haml_spec.rb b/spec/views/jira_connect/subscriptions/index.html.haml_spec.rb deleted file mode 100644 index 0a4d283a983..00000000000 --- a/spec/views/jira_connect/subscriptions/index.html.haml_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'jira_connect/subscriptions/index.html.haml' do - let(:user) { build_stubbed(:user) } - - before do - allow(view).to receive(:current_user).and_return(user) - assign(:subscriptions, create_list(:jira_connect_subscription, 1)) - end - - context 'when the user is signed in' do - it 'shows link to user profile' do - render - - expect(rendered).to have_link(user.to_reference) - end - end - - context 'when the user is not signed in' do - let(:user) { nil } - - it 'shows "Sign in" link' do - render - - expect(rendered).to have_link('Sign in to GitLab') - end - end -end