diff options
22 files changed, 551 insertions, 199 deletions
diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js index 085df1ff9eb..229612f5e9d 100644 --- a/app/assets/javascripts/create_merge_request_dropdown.js +++ b/app/assets/javascripts/create_merge_request_dropdown.js @@ -42,7 +42,7 @@ export default class CreateMergeRequestDropdown { this.refInput = this.wrapperEl.querySelector('.js-ref'); this.refMessage = this.wrapperEl.querySelector('.js-ref-message'); this.unavailableButton = this.wrapperEl.querySelector('.unavailable'); - this.unavailableButtonArrow = this.unavailableButton.querySelector('.spinner'); + this.unavailableButtonSpinner = this.unavailableButton.querySelector('.spinner'); this.unavailableButtonText = this.unavailableButton.querySelector('.text'); this.branchCreated = false; @@ -417,12 +417,10 @@ export default class CreateMergeRequestDropdown { setUnavailableButtonState(isLoading = true) { if (isLoading) { - this.unavailableButtonArrow.classList.remove('hide'); - this.unavailableButtonArrow.classList.remove('fa-exclamation-triangle'); + this.unavailableButtonSpinner.classList.remove('hide'); this.unavailableButtonText.textContent = __('Checking branch availability...'); } else { - this.unavailableButtonArrow.classList.add('hide'); - this.unavailableButtonArrow.classList.add('fa-exclamation-triangle'); + this.unavailableButtonSpinner.classList.add('hide'); this.unavailableButtonText.textContent = __('New branch unavailable'); } } diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index 552e8cac3a7..80a88489545 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -16,7 +16,6 @@ import { GlButtonGroup, } from '@gitlab/ui'; import AccessorUtils from '~/lib/utils/accessor'; -import Icon from '~/vue_shared/components/icon.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import { __ } from '~/locale'; import { isEmpty } from 'lodash'; @@ -59,7 +58,7 @@ export default { { key: 'status', label: '', - tdClass: `${tableDataClass} text-right`, + tdClass: `${tableDataClass} text-center`, }, { key: 'details', @@ -67,6 +66,11 @@ export default { thClass: 'invisible w-0', }, ], + statusFilters: { + unresolved: __('Unresolved'), + ignored: __('Ignored'), + resolved: __('Resolved'), + }, sortFields: { last_seen: __('Last Seen'), first_seen: __('First Seen'), @@ -83,7 +87,6 @@ export default { GlLoadingIcon, GlTable, GlFormInput, - Icon, GlPagination, TimeAgo, GlButtonGroup, @@ -136,6 +139,7 @@ export default { 'sortField', 'recentSearches', 'pagination', + 'statusFilter', 'cursor', ]), paginationRequired() { @@ -169,6 +173,7 @@ export default { 'fetchPaginatedResults', 'updateStatus', 'removeIgnoredResolvedErrors', + 'filterByStatus', ]), setSearchText(text) { this.errorSearchQuery = text; @@ -191,9 +196,16 @@ export default { isCurrentSortField(field) { return field === this.sortField; }, + isCurrentStatusFilter(filter) { + return filter === this.statusFilter; + }, getIssueUpdatePath(errorId) { return `/${this.projectPath}/-/error_tracking/${errorId}.json`; }, + filterErrors(status, label) { + this.filterValue = label; + return this.filterByStatus(status); + }, updateIssueStatus(errorId, status) { this.updateStatus({ endpoint: this.getIssueUpdatePath(errorId), @@ -260,11 +272,32 @@ export default { </div> <gl-dropdown - class="sort-control" + :text="$options.statusFilters[statusFilter]" + class="status-dropdown mr-2" + menu-class="dropdown" + :disabled="loading" + > + <gl-dropdown-item + v-for="(label, status) in $options.statusFilters" + :key="status" + @click="filterErrors(status, label)" + > + <span class="d-flex"> + <gl-icon + class="flex-shrink-0 append-right-4" + :class="{ invisible: !isCurrentStatusFilter(status) }" + name="mobile-issue-close" + /> + {{ label }} + </span> + </gl-dropdown-item> + </gl-dropdown> + + <gl-dropdown :text="$options.sortFields[sortField]" left :disabled="loading" - menu-class="sort-dropdown" + menu-class="dropdown" > <gl-dropdown-item v-for="(label, field) in $options.sortFields" @@ -272,7 +305,7 @@ export default { @click="sortByField(field)" > <span class="d-flex"> - <icon + <gl-icon class="flex-shrink-0 append-right-4" :class="{ invisible: !isCurrentSortField(field) }" name="mobile-issue-close" diff --git a/app/assets/javascripts/error_tracking/store/list/actions.js b/app/assets/javascripts/error_tracking/store/list/actions.js index 6f8573c0f4d..4170c1bf759 100644 --- a/app/assets/javascripts/error_tracking/store/list/actions.js +++ b/app/assets/javascripts/error_tracking/store/list/actions.js @@ -18,6 +18,7 @@ export function startPolling({ state, commit, dispatch }) { search_term: state.searchQuery, sort: state.sortField, cursor: state.cursor, + issue_status: state.statusFilter, }, }, successCallback: ({ data }) => { @@ -83,6 +84,12 @@ export const searchByQuery = ({ commit, dispatch }, query) => { dispatch('startPolling'); }; +export const filterByStatus = ({ commit, dispatch }, status) => { + commit(types.SET_STATUS_FILTER, status); + dispatch('stopPolling'); + dispatch('startPolling'); +}; + export const sortByField = ({ commit, dispatch }, field) => { commit(types.SET_CURSOR, null); commit(types.SET_SORT_FIELD, field); diff --git a/app/assets/javascripts/error_tracking/store/list/mutation_types.js b/app/assets/javascripts/error_tracking/store/list/mutation_types.js index 23495cbf01d..872ac8ea8fc 100644 --- a/app/assets/javascripts/error_tracking/store/list/mutation_types.js +++ b/app/assets/javascripts/error_tracking/store/list/mutation_types.js @@ -10,3 +10,4 @@ export const SET_SORT_FIELD = 'SET_SORT_FIELD'; export const SET_SEARCH_QUERY = 'SET_SEARCH_QUERY'; export const SET_CURSOR = 'SET_CURSOR'; export const REMOVE_IGNORED_RESOLVED_ERRORS = 'REMOVE_IGNORED_RESOLVED_ERRORS'; +export const SET_STATUS_FILTER = 'SET_STATUS_FILTER'; diff --git a/app/assets/javascripts/error_tracking/store/list/mutations.js b/app/assets/javascripts/error_tracking/store/list/mutations.js index 38d156263fb..be0cd4de78d 100644 --- a/app/assets/javascripts/error_tracking/store/list/mutations.js +++ b/app/assets/javascripts/error_tracking/store/list/mutations.js @@ -62,4 +62,7 @@ export default { [types.REMOVE_IGNORED_RESOLVED_ERRORS](state, error) { state.errors = state.errors.filter(err => err.id !== error); }, + [types.SET_STATUS_FILTER](state, query) { + state.statusFilter = query; + }, }; diff --git a/app/assets/javascripts/error_tracking/store/list/state.js b/app/assets/javascripts/error_tracking/store/list/state.js index 225a805e709..eb983fde9e0 100644 --- a/app/assets/javascripts/error_tracking/store/list/state.js +++ b/app/assets/javascripts/error_tracking/store/list/state.js @@ -3,6 +3,7 @@ export default () => ({ loading: true, endpoint: null, sortField: 'last_seen', + statusFilter: 'unresolved', searchQuery: null, indexPath: '', recentSearches: [], diff --git a/app/assets/stylesheets/pages/error_tracking_list.scss b/app/assets/stylesheets/pages/error_tracking_list.scss index cd1adb9a754..cc391ca6c97 100644 --- a/app/assets/stylesheets/pages/error_tracking_list.scss +++ b/app/assets/stylesheets/pages/error_tracking_list.scss @@ -1,5 +1,5 @@ .error-list { - .sort-dropdown { + .dropdown { min-width: auto; } } diff --git a/changelogs/unreleased/209207-spinner-appears-to-be-broken.yml b/changelogs/unreleased/209207-spinner-appears-to-be-broken.yml new file mode 100644 index 00000000000..0b1ae5d1838 --- /dev/null +++ b/changelogs/unreleased/209207-spinner-appears-to-be-broken.yml @@ -0,0 +1,5 @@ +--- +title: Fix spinner in Create MR dropdown +merge_request: 26679 +author: +type: fixed diff --git a/changelogs/unreleased/filter-sentry-error-list.yml b/changelogs/unreleased/filter-sentry-error-list.yml new file mode 100644 index 00000000000..36d752ca8ba --- /dev/null +++ b/changelogs/unreleased/filter-sentry-error-list.yml @@ -0,0 +1,5 @@ +--- +title: Filter sentry error list by status (unresolved/ignored/resolved) +merge_request: 26205 +author: +type: added diff --git a/doc/administration/geo/replication/high_availability.md b/doc/administration/geo/replication/high_availability.md index 5c124e9c6dc..8dd04cdace1 100644 --- a/doc/administration/geo/replication/high_availability.md +++ b/doc/administration/geo/replication/high_availability.md @@ -25,10 +25,17 @@ The **primary** and **secondary** Geo deployments must be able to communicate to ## Redis and PostgreSQL High Availability -The **primary** and **secondary** Redis and PostgreSQL should be configured -for high availability. Because of the additional complexity involved -in setting up this configuration for PostgreSQL and Redis, -it is not covered by this Geo HA documentation. +Geo supports: + +- Redis and PostgreSQL on the **primary** node configured for high availability +- Redis on **secondary** nodes configured for high availability. + +NOTE: **Note:** +Support for PostgreSQL on **secondary** nodes in high availability configuration +[is planned](https://gitlab.com/groups/gitlab-org/-/epics/2536). + +Because of the additional complexity involved in setting up this configuration +for PostgreSQL and Redis, it is not covered by this Geo HA documentation. For more information about setting up a highly available PostgreSQL cluster and Redis cluster using the omnibus package see the high availability documentation for [PostgreSQL](../../high_availability/database.md) and @@ -37,10 +44,17 @@ For more information about setting up a highly available PostgreSQL cluster and NOTE: **Note:** It is possible to use cloud hosted services for PostgreSQL and Redis, but this is beyond the scope of this document. -## Prerequisites: A working GitLab HA cluster +## Prerequisites: Two working GitLab HA clusters + +One cluster will serve as the **primary** node. Use the +[GitLab HA documentation](../../high_availability/README.md) to set this up. If +you already have a working GitLab instance that is in-use, it can be used as a +**primary**. -This cluster will serve as the **primary** node. Use the +The second cluster will serve as the **secondary** node. Again, use the [GitLab HA documentation](../../high_availability/README.md) to set this up. +It's a good idea to log in and test it, however, note that its data will be +wiped out as part of the process of replicating from the **primary**. ## Configure the GitLab cluster to be the **primary** node @@ -99,7 +113,11 @@ major differences: various resources. Therefore, we will set up the HA components one-by-one, and include deviations -from the normal HA setup. +from the normal HA setup. However, we highly recommend first configuring a +brand-new cluster as if it were not part of a Geo setup so that it can be +tested and verified as a working cluster. And only then should it be modified +for use as a Geo **secondary**. This helps to separate problems that are related +and are not related to Geo setup. ### Step 1: Configure the Redis and Gitaly services on the **secondary** node @@ -118,7 +136,8 @@ recommended. ### Step 2: Configure the main read-only replica PostgreSQL database on the **secondary** node NOTE: **Note:** The following documentation assumes the database will be run on -a single node only, rather than as a PostgreSQL cluster. +a single node only. PostgreSQL HA on **secondary** nodes is +[not currently supported](https://gitlab.com/groups/gitlab-org/-/epics/2536). Configure the [**secondary** database](database.md) as a read-only replica of the **primary** database. Use the following as a guide. @@ -167,6 +186,11 @@ the **primary** database. Use the following as a guide. ## the tracking database IP is in postgresql['md5_auth_cidr_addresses'] above. ## geo_postgresql['enable'] = false + + ## + ## Disable `geo_logcursor` service so Rails doesn't get configured here + ## + geo_logcursor['enable'] = false ``` After making these changes, [reconfigure GitLab][gitlab-reconfigure] so the changes take effect. @@ -335,6 +359,13 @@ On the secondary the following GitLab frontend services will be enabled: Verify these services by running `sudo gitlab-ctl status` on the frontend application servers. +You may wish to run backend application services on backend-specific servers. +For example, you can disable the `geo-logcursor` service with +`geo_logcursor['enable'] = false` and run it on application servers not +attached to the load balancer. On those backend application servers, you would +disable Unicorn with `unicorn['enable'] = false`. You might also choose to do +the same thing with the `sidekiq` service. + ### Step 5: Set up the LoadBalancer for the **secondary** node In this topology, a load balancer is required at each geographic location to diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index 1b8d2ee434a..5e9d62c169a 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -248,6 +248,133 @@ The following details should be included: - Include suggested titles of any pages or subsection headings, if applicable. - List any documentation that should be cross-linked, if applicable. +### Including docs with code + +Currently, the Technical Writing team strongly encourages including documentation in +the same merge request as the code that it relates to, but this is not strictly mandatory. +It's still common for documentation to be added in an MR separate from the feature MR. + +Engineering teams may elect to adopt a workflow where it is **mandatory** that docs +are included in the code MR, as part of their [definition of done](../contributing/merge_request_workflow.md#definition-of-done). +When a team adopts this workflow, that team's engineers must include their docs in the **same** +MR as their feature code, at all times. + +#### Downsides of separate docs MRs + +A workflow that has documentation separated into its own MR has many downsides. + +If the documentation merges **before** the feature: + +- GitLab.com users might try to use the feature before it's released, driving support tickets. +- If the feature is delayed, the documentation might not be pulled/reverted in time and could be + accidentally included in the self-managed package for that release. + +If the documentation merges **after** the feature: + +- The feature might be included in the self-managed package, but without any documentation + if the docs MR misses the cutoff. +- A feature might show up in the GitLab.com UI before any documentation exists for it. + Users surprised by this feature will search for documentation and won't find it, possibly driving + support tickets. + +Having two separate MRs means: + +- Two different people might be responsible for merging one feature, which is not workable + with an asynchronous work style. The feature might merge while the technical writer is asleep, + creating a potentially lengthy delay between the two merges. +- If the docs MR is assigned to the same maintainer as responsible for the feature + code MR, they will have to review and juggle two MRs instead of dealing with just one. + +Documentation quality might be lower, because: + +- Having docs in a separate MR will mean far fewer people will see and verify them, + increasing the likelihood that issues will be missed. +- In a "split" workflow, engineers might only create the documentation MR once the + feature MR is ready, or almost ready. This gives the technical writer little time + to learn about the feature in order to do a good review. It also increases pressure + on them to review and merge faster than desired, letting problems slip in due to haste. + +#### Benefits of always including docs with code + +Including docs with code (and doing it early in the development process) has many benefits: + +- There are no timing issues connected to releases: + - If a feature slips to the next release, the documentation slips too. + - If the feature *just* makes it into a release, the docs *just* make it in too. + - If a feature makes it to GitLab.com early, the documentation will be ready for + our early adopters. +- Only a single person will be responsible for merging the feature (the code maintainer). +- The technical writer will have more time to gain an understanding of the feature + and will be better able to verify the content of the docs in the Review App or GDK. + They will also be able to offer advice for improving the UI text or offer additional use cases. +- The documentation will have increased visibility: + - Everyone involved in the merge request will see the docs. This could include product + managers, multiple engineers with deep domain knowledge, as well as the code reviewers + and maintainer. They will be more likely to catch issues with examples, as well + as background or concepts that the technical writer may not be aware of. + - Increasing visibility of the documentation also has the side effect of improving + *other* engineers' documentation. By reviewing each other's MRs, each engineer's + own documentation skills will improve. +- Thinking about the documentation early can help engineers generate better examples, + as they will need to think about what examples a user will want, and will need to + make sure the code they write implements that example properly. + +#### Docs with code as a workflow + +In order to have docs included with code as a mandatory workflow, some changes might +need to happen to a team's current workflow: + +- The engineers must strive to include the docs early in the development process, + to give ample time for review, not just from the technical writer, but also the + code reviewer and maintainer. +- Reviewers and maintainers must also review the docs during code reviews, to make + sure the described processes match the expected use of the feature, and that examples + are correct. They do *not* need to worry about style, grammar, and so on. +- The technical writer must be assigned the MR directly and not only pinged. Thanks + to the ability to have [multiple assignees for any MR](../../user/project/merge_requests/getting_started.md#multiple-assignees-starter), + this can be done at any time, but must be before the code maintainer review. It's + common to have both the docs and code reviews happening at the same time, with the + author, reviewer and technical writer discussing the docs together. +- When the docs are ready, the technical writer will click **Approve** and usually + will no longer be involved in the MR. If the feature changes during code review and + the docs are updated, the technical writer must be reassigned the MR to verify the + update. +- Maintainers are allowed to merge features with the docs "as-is", even if the technical + writer has not given final approval yet. The **docs reviews must not be blockers**. Therefore + it's important to get the docs included and assigned to the technical writers early. + If the feature is merged before final docs approval, the maintainer must create + a [post-merge follow-up issue](#post-merge-reviews), and assign it to both the engineer + and technical writer. + +Maintainers are allowed to merge features with the docs "as-is" even if the +technical writer has not given final approval yet but the merge request has +all other required approvals. + +You can visualize the parallel workflow for code and docs reviews as: + +```mermaid +graph TD + A("Feature MR Created (Engineer)") --> |Assign| B("Code Review (reviewer)") + B --> |"Approve / Reassign"| C("Code Review (maintainer)") + C --> |Approve| F("Merge (maintainer)") + A --> D("Docs Added (Engineer)") + D --> |Assign| E("Docs Review (Tech Writer)") + E --> |Approve| F +``` + +For complex features split over multiple merge requests: + +- If a merge request is implementing components for a future feature, but the components + are not accessible to users yet, then no documentation should be included. +- If a merge request will expose a feature to users in any way, such as an enabled + UI element, an API endpoint, or anything similar, then that MR **must** have docs. + Note that this may mean multiple docs additions could happen in the buildup to the + implementation of a single large feature, for example API docs and feature usage docs. +- If it's unclear which engineer should add the feature documentation into their + MR, the engineering manager should decide during planning, and tie the documentation + to the last MR that must be merged before a feature is considered released. + This is often, but not always, a frontend MR. + ## For all other documentation These documentation changes are not associated with the release of a new or updated feature, and are diff --git a/doc/user/project/operations/error_tracking.md b/doc/user/project/operations/error_tracking.md index e7565835be7..bc1256a1c50 100644 --- a/doc/user/project/operations/error_tracking.md +++ b/doc/user/project/operations/error_tracking.md @@ -41,8 +41,8 @@ You may also want to enable Sentry's GitLab integration by following the steps i NOTE: **Note:** You will need at least Reporter [permissions](../../permissions.md) to view the Error Tracking list. -The Error Tracking list may be found at **Operations > Error Tracking** in your project's sidebar. -Errors can be filtered by title or sorted by Frequency, First Seen or Last Seen. Errors are always sorted in descending order by the field specified. +You can find the Error Tracking list at **Operations > Error Tracking** in your project's sidebar. +Here, you can filter errors by title or by status (one of Ignored , Resolved, or Unresolved) and sort in descending order by Frequency, First Seen, or Last Seen. By default, the error list is ordered by Last Seen and filtered to Unresolved errors.  diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4d917cbe943..3c9520fc58f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10533,6 +10533,9 @@ msgstr "" msgid "Ignore" msgstr "" +msgid "Ignored" +msgstr "" + msgid "Image %{imageName} was scheduled for deletion from the registry." msgstr "" @@ -21104,6 +21107,9 @@ msgstr "" msgid "Unresolve thread" msgstr "" +msgid "Unresolved" +msgstr "" + msgid "UnscannedProjects|15 or more days" msgstr "" diff --git a/spec/features/error_tracking/user_filters_errors_by_status_spec.rb b/spec/features/error_tracking/user_filters_errors_by_status_spec.rb new file mode 100644 index 00000000000..51e29e2a5ec --- /dev/null +++ b/spec/features/error_tracking/user_filters_errors_by_status_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'When a user filters Sentry errors by status', :js, :use_clean_rails_memory_store_caching, :sidekiq_inline do + include_context 'sentry error tracking context feature' + + let_it_be(:issues_response_body) { fixture_file('sentry/issues_sample_response.json') } + let_it_be(:filtered_errors_by_status_response) { JSON.parse(issues_response_body).filter { |error| error['status'] == 'ignored' }.to_json } + let(:issues_api_url) { "#{sentry_api_urls.issues_url}?limit=20&query=is:unresolved" } + let(:issues_api_url_filter) { "#{sentry_api_urls.issues_url}?limit=20&query=is:ignored" } + let(:auth_token) {{ 'Authorization' => 'Bearer access_token_123' }} + let(:return_header) {{ 'Content-Type' => 'application/json' }} + + before do + stub_request(:get, issues_api_url).with(headers: auth_token) + .to_return(status: 200, body: issues_response_body, headers: return_header) + + stub_request(:get, issues_api_url_filter).with(headers: auth_token) + .to_return(status: 200, body: filtered_errors_by_status_response, headers: return_header) + end + + it 'displays the results' do + sign_in(project.owner) + visit project_error_tracking_index_path(project) + page.within(find('.gl-table')) do + results = page.all('.table-row') + expect(results.count).to be(3) + end + + find('.status-dropdown .dropdown-toggle').click + find('.dropdown-item', text: 'Ignored').click + + page.within(find('.gl-table')) do + results = page.all('.table-row') + expect(results.count).to be(1) + expect(results.first).to have_content(filtered_errors_by_status_response[0]['title']) + end + end +end diff --git a/spec/features/error_tracking/user_searches_sentry_errors_spec.rb b/spec/features/error_tracking/user_searches_sentry_errors_spec.rb index 690c60a3c3f..c5559081feb 100644 --- a/spec/features/error_tracking/user_searches_sentry_errors_spec.rb +++ b/spec/features/error_tracking/user_searches_sentry_errors_spec.rb @@ -26,7 +26,7 @@ describe 'When a user searches for Sentry errors', :js, :use_clean_rails_memory_ page.within(find('.gl-table')) do results = page.all('.table-row') - expect(results.count).to be(2) + expect(results.count).to be(3) end find('.gl-form-input').set('NotFound').native.send_keys(:return) diff --git a/spec/fixtures/sentry/issues_sample_response.json b/spec/fixtures/sentry/issues_sample_response.json index 495562ac960..980e2fb3bd6 100644 --- a/spec/fixtures/sentry/issues_sample_response.json +++ b/spec/fixtures/sentry/issues_sample_response.json @@ -82,5 +82,47 @@ "name": "Internal" }, "statusDetails": {} + }, + { + "lastSeen": "2018-12-31T12:00:11Z", + "numComments": 0, + "userCount": 0, + "stats": { + "24h": [ + [ + 1546437600, + 0 + ] + ] + }, + "culprit": "sentry.tasks.reports.deliver_organization_user_report", + "title": "Service unknown", + "id": "12", + "assignedTo": null, + "logger": null, + "type": "error", + "annotations": [], + "metadata": { + "type": "gaierror", + "value": "Service unknown" + }, + "status": "ignored", + "subscriptionDetails": null, + "isPublic": false, + "hasSeen": false, + "shortId": "INTERNAL-4", + "shareId": null, + "firstSeen": "2018-12-17T12:00:14Z", + "count": "70", + "permalink": "35.228.54.90/sentry/internal/issues/12/", + "level": "error", + "isSubscribed": true, + "isBookmarked": false, + "project": { + "slug": "internal", + "id": "1", + "name": "Internal" + }, + "statusDetails": {} } ] diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js new file mode 100644 index 00000000000..ecc28c78fb7 --- /dev/null +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -0,0 +1,179 @@ +import { mount } from '@vue/test-utils'; +import { TEST_HOST } from 'helpers/test_constants'; +import { trimText } from 'helpers/text_helper'; +import { getTimeago } from '~/lib/utils/datetime_utility'; +import Component from '~/diffs/components/commit_item.vue'; +import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; +import getDiffWithCommit from '../mock_data/diff_with_commit'; + +jest.mock('~/user_popovers'); + +const TEST_AUTHOR_NAME = 'test'; +const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; +const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; +const TEST_SIGNATURE_HTML = '<a>Legit commit</a>'; +const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; + +describe('diffs/components/commit_item', () => { + let wrapper; + + const timeago = getTimeago(); + const { commit } = getDiffWithCommit(); + + const getTitleElement = () => wrapper.find('.commit-row-message.item-title'); + const getDescElement = () => wrapper.find('pre.commit-row-description'); + const getDescExpandElement = () => + wrapper.find('.commit-content .text-expander.js-toggle-button'); + const getShaElement = () => wrapper.find('.commit-sha-group'); + const getAvatarElement = () => wrapper.find('.user-avatar-link'); + const getCommitterElement = () => wrapper.find('.committer'); + const getCommitActionsElement = () => wrapper.find('.commit-actions'); + const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus); + + const defaultProps = { + commit: getDiffWithCommit().commit, + }; + const mountComponent = (propsData = defaultProps) => { + wrapper = mount(Component, { + propsData, + stubs: { + CommitPipelineStatus: true, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('default state', () => { + beforeEach(() => { + mountComponent(); + }); + + it('renders commit title', () => { + const titleElement = getTitleElement(); + + expect(titleElement.attributes('href')).toBe(commit.commit_url); + expect(titleElement.text()).toBe(commit.title_html); + }); + + it('renders commit description', () => { + const descElement = getDescElement(); + const descExpandElement = getDescExpandElement(); + + const expected = commit.description_html.replace(/
/g, ''); + + expect(trimText(descElement.text())).toEqual(trimText(expected)); + expect(descExpandElement.exists()).toBe(true); + }); + + it('renders commit sha', () => { + const shaElement = getShaElement(); + const labelElement = shaElement.find('.label'); + const buttonElement = shaElement.find('button'); + + expect(labelElement.text()).toEqual(commit.short_id); + expect(buttonElement.props('text')).toBe(commit.id); + }); + + it('renders author avatar', () => { + const avatarElement = getAvatarElement(); + const imgElement = avatarElement.find('img'); + + expect(avatarElement.attributes('href')).toBe(commit.author.web_url); + expect(imgElement.classes()).toContain('s40'); + expect(imgElement.attributes('alt')).toBe(commit.author.name); + expect(imgElement.attributes('src')).toBe(commit.author.avatar_url); + }); + + it('renders committer text', () => { + const committerElement = getCommitterElement(); + const nameElement = committerElement.find('a'); + + const expectTimeText = timeago.format(commit.authored_date); + const expectedText = `${commit.author.name} authored ${expectTimeText}`; + + expect(trimText(committerElement.text())).toEqual(expectedText); + expect(nameElement.attributes('href')).toBe(commit.author.web_url); + expect(nameElement.text()).toBe(commit.author.name); + expect(nameElement.classes()).toContain('js-user-link'); + expect(nameElement.attributes('data-user-id')).toEqual(commit.author.id.toString()); + }); + }); + + describe('without commit description', () => { + beforeEach(() => { + mountComponent({ defaultProps, commit: { ...defaultProps.commit, description_html: '' } }); + }); + + it('hides description', () => { + const descElement = getDescElement(); + const descExpandElement = getDescExpandElement(); + + expect(descElement.exists()).toBeFalsy(); + expect(descExpandElement.exists()).toBeFalsy(); + }); + }); + + describe('with no matching user', () => { + beforeEach(() => { + mountComponent({ + defaultProps, + commit: { + ...defaultProps.commit, + author: null, + author_email: TEST_AUTHOR_EMAIL, + author_name: TEST_AUTHOR_NAME, + author_gravatar_url: TEST_AUTHOR_GRAVATAR, + }, + }); + }); + + it('renders author avatar', () => { + const avatarElement = getAvatarElement(); + const imgElement = avatarElement.find('img'); + + expect(avatarElement.attributes('href')).toBe(`mailto:${TEST_AUTHOR_EMAIL}`); + expect(imgElement.attributes('alt')).toBe(TEST_AUTHOR_NAME); + expect(imgElement.attributes('src')).toBe(TEST_AUTHOR_GRAVATAR); + }); + + it('renders committer text', () => { + const committerElement = getCommitterElement(); + const nameElement = committerElement.find('a'); + + expect(nameElement.attributes('href')).toBe(`mailto:${TEST_AUTHOR_EMAIL}`); + expect(nameElement.text()).toBe(TEST_AUTHOR_NAME); + }); + }); + + describe('with signature', () => { + beforeEach(() => { + mountComponent({ + defaultProps, + commit: { ...defaultProps.commit, signature_html: TEST_SIGNATURE_HTML }, + }); + }); + + it('renders signature html', () => { + const actionsElement = getCommitActionsElement(); + + expect(actionsElement.html()).toContain(TEST_SIGNATURE_HTML); + }); + }); + + describe('with pipeline status', () => { + beforeEach(() => { + mountComponent({ + defaultProps, + commit: { ...defaultProps.commit, pipeline_status_path: TEST_PIPELINE_STATUS_PATH }, + }); + }); + + it('renders pipeline status', () => { + expect(getCommitPipelineStatus().exists()).toBe(true); + }); + }); +}); diff --git a/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js index f852a3091aa..cd6dd5c7519 100644 --- a/spec/frontend/error_tracking/components/error_tracking_list_spec.js +++ b/spec/frontend/error_tracking/components/error_tracking_list_spec.js @@ -1,6 +1,6 @@ import { createLocalVue, mount } from '@vue/test-utils'; import Vuex from 'vuex'; -import { GlEmptyState, GlLoadingIcon, GlFormInput, GlPagination } from '@gitlab/ui'; +import { GlEmptyState, GlLoadingIcon, GlFormInput, GlPagination, GlDropdown } from '@gitlab/ui'; import stubChildren from 'helpers/stub_children'; import ErrorTrackingList from '~/error_tracking/components/error_tracking_list.vue'; import errorsList from './list_mock.json'; @@ -15,9 +15,19 @@ describe('ErrorTrackingList', () => { const findErrorListTable = () => wrapper.find('table'); const findErrorListRows = () => wrapper.findAll('tbody tr'); - const findSortDropdown = () => wrapper.find('.sort-dropdown'); + const dropdownsArray = () => wrapper.findAll(GlDropdown); const findRecentSearchesDropdown = () => - wrapper.find('.filtered-search-history-dropdown-wrapper'); + dropdownsArray() + .at(0) + .find(GlDropdown); + const findStatusFilterDropdown = () => + dropdownsArray() + .at(1) + .find(GlDropdown); + const findSortDropdown = () => + dropdownsArray() + .at(2) + .find(GlDropdown); const findLoadingIcon = () => wrapper.find(GlLoadingIcon); const findPagination = () => wrapper.find(GlPagination); @@ -60,6 +70,7 @@ describe('ErrorTrackingList', () => { fetchPaginatedResults: jest.fn(), updateStatus: jest.fn(), removeIgnoredResolvedErrors: jest.fn(), + filterByStatus: jest.fn(), }; const state = { @@ -167,10 +178,16 @@ describe('ErrorTrackingList', () => { }); it('it sorts by fields', () => { - const findSortItem = () => wrapper.find('.dropdown-item'); + const findSortItem = () => findSortDropdown().find('.dropdown-item'); findSortItem().trigger('click'); expect(actions.sortByField).toHaveBeenCalled(); }); + + it('it filters by status', () => { + const findStatusFilter = () => findStatusFilterDropdown().find('.dropdown-item'); + findStatusFilter().trigger('click'); + expect(actions.filterByStatus).toHaveBeenCalled(); + }); }); }); @@ -215,7 +232,7 @@ describe('ErrorTrackingList', () => { expect(wrapper.find(GlEmptyState).exists()).toBe(true); expect(findLoadingIcon().exists()).toBe(false); expect(findErrorListTable().exists()).toBe(false); - expect(findSortDropdown().exists()).toBe(false); + expect(dropdownsArray().length).toBe(0); }); }); diff --git a/spec/frontend/error_tracking/store/list/actions_spec.js b/spec/frontend/error_tracking/store/list/actions_spec.js index 54fdde88818..3cb740bf05d 100644 --- a/spec/frontend/error_tracking/store/list/actions_spec.js +++ b/spec/frontend/error_tracking/store/list/actions_spec.js @@ -88,6 +88,20 @@ describe('error tracking actions', () => { }); }); + describe('filterByStatus', () => { + it('should search errors by status', () => { + const status = 'ignored'; + + testAction( + actions.filterByStatus, + status, + {}, + [{ type: types.SET_STATUS_FILTER, payload: status }], + [{ type: 'stopPolling' }, { type: 'startPolling' }], + ); + }); + }); + describe('sortByField', () => { it('should search by query', () => { const field = 'frequency'; diff --git a/spec/frontend/error_tracking/store/list/mutation_spec.js b/spec/frontend/error_tracking/store/list/mutation_spec.js index 65f11aeeda1..a326a6c55c0 100644 --- a/spec/frontend/error_tracking/store/list/mutation_spec.js +++ b/spec/frontend/error_tracking/store/list/mutation_spec.js @@ -6,6 +6,7 @@ const ADD_RECENT_SEARCH = mutations[types.ADD_RECENT_SEARCH]; const CLEAR_RECENT_SEARCHES = mutations[types.CLEAR_RECENT_SEARCHES]; const LOAD_RECENT_SEARCHES = mutations[types.LOAD_RECENT_SEARCHES]; const REMOVE_IGNORED_RESOLVED_ERRORS = mutations[types.REMOVE_IGNORED_RESOLVED_ERRORS]; +const SET_STATUS_FILTER = mutations[types.SET_STATUS_FILTER]; describe('Error tracking mutations', () => { describe('SET_ERRORS', () => { @@ -139,5 +140,15 @@ describe('Error tracking mutations', () => { expect(state.errors).not.toContain(ignoredError); }); }); + + describe('SET_STATUS_FILTER', () => { + it('sets the filter to ignored, resolved or unresolved', () => { + state.statusFilter = 'unresolved'; + + SET_STATUS_FILTER(state, 'ignored'); + + expect(state.statusFilter).toBe('ignored'); + }); + }); }); }); diff --git a/spec/javascripts/diffs/components/commit_item_spec.js b/spec/javascripts/diffs/components/commit_item_spec.js deleted file mode 100644 index 9fe31a5e727..00000000000 --- a/spec/javascripts/diffs/components/commit_item_spec.js +++ /dev/null @@ -1,168 +0,0 @@ -import Vue from 'vue'; -import { TEST_HOST } from 'spec/test_constants'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; -import { trimText } from 'spec/helpers/text_helper'; -import { getTimeago } from '~/lib/utils/datetime_utility'; -import CommitItem from '~/diffs/components/commit_item.vue'; -import getDiffWithCommit from '../mock_data/diff_with_commit'; - -const TEST_AUTHOR_NAME = 'test'; -const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; -const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; -const TEST_SIGNATURE_HTML = '<a>Legit commit</a>'; -const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; - -const getTitleElement = vm => vm.$el.querySelector('.commit-row-message.item-title'); -const getDescElement = vm => vm.$el.querySelector('pre.commit-row-description'); -const getDescExpandElement = vm => - vm.$el.querySelector('.commit-content .text-expander.js-toggle-button'); -const getShaElement = vm => vm.$el.querySelector('.commit-sha-group'); -const getAvatarElement = vm => vm.$el.querySelector('.user-avatar-link'); -const getCommitterElement = vm => vm.$el.querySelector('.committer'); -const getCommitActionsElement = vm => vm.$el.querySelector('.commit-actions'); - -describe('diffs/components/commit_item', () => { - const Component = Vue.extend(CommitItem); - const timeago = getTimeago(); - const { commit } = getDiffWithCommit(); - - let vm; - - beforeEach(() => { - vm = mountComponent(Component, { - commit: getDiffWithCommit().commit, - }); - }); - - it('renders commit title', () => { - const titleElement = getTitleElement(vm); - - expect(titleElement).toHaveAttr('href', commit.commit_url); - expect(titleElement).toHaveText(commit.title_html); - }); - - // https://gitlab.com/gitlab-org/gitlab/issues/197139 - // eslint-disable-next-line jasmine/no-disabled-tests - xit('renders commit description', () => { - const descElement = getDescElement(vm); - const descExpandElement = getDescExpandElement(vm); - - const expected = commit.description_html.replace(/
/g, ''); - - expect(trimText(descElement.innerHTML)).toEqual(trimText(expected)); - expect(descExpandElement).not.toBeNull(); - }); - - it('renders commit sha', () => { - const shaElement = getShaElement(vm); - const labelElement = shaElement.querySelector('.label'); - const buttonElement = shaElement.querySelector('button'); - - expect(labelElement.textContent).toEqual(commit.short_id); - expect(buttonElement).toHaveData('clipboard-text', commit.id); - }); - - it('renders author avatar', () => { - const avatarElement = getAvatarElement(vm); - const imgElement = avatarElement.querySelector('img'); - - expect(avatarElement).toHaveAttr('href', commit.author.web_url); - expect(imgElement).toHaveClass('s40'); - expect(imgElement).toHaveAttr('alt', commit.author.name); - expect(imgElement).toHaveAttr('src', commit.author.avatar_url); - }); - - it('renders committer text', () => { - const committerElement = getCommitterElement(vm); - const nameElement = committerElement.querySelector('a'); - - const expectTimeText = timeago.format(commit.authored_date); - const expectedText = `${commit.author.name} authored ${expectTimeText}`; - - expect(trimText(committerElement.textContent)).toEqual(expectedText); - expect(nameElement).toHaveAttr('href', commit.author.web_url); - expect(nameElement).toHaveText(commit.author.name); - expect(nameElement).toHaveClass('js-user-link'); - expect(nameElement.dataset.userId).toEqual(commit.author.id.toString()); - }); - - describe('without commit description', () => { - beforeEach(done => { - vm.commit.description_html = ''; - - vm.$nextTick() - .then(done) - .catch(done.fail); - }); - - it('hides description', () => { - const descElement = getDescElement(vm); - const descExpandElement = getDescExpandElement(vm); - - expect(descElement).toBeNull(); - expect(descExpandElement).toBeNull(); - }); - }); - - describe('with no matching user', () => { - beforeEach(done => { - vm.commit.author = null; - vm.commit.author_email = TEST_AUTHOR_EMAIL; - vm.commit.author_name = TEST_AUTHOR_NAME; - vm.commit.author_gravatar_url = TEST_AUTHOR_GRAVATAR; - - vm.$nextTick() - .then(done) - .catch(done.fail); - }); - - it('renders author avatar', () => { - const avatarElement = getAvatarElement(vm); - const imgElement = avatarElement.querySelector('img'); - - expect(avatarElement).toHaveAttr('href', `mailto:${TEST_AUTHOR_EMAIL}`); - expect(imgElement).toHaveAttr('alt', TEST_AUTHOR_NAME); - expect(imgElement).toHaveAttr('src', TEST_AUTHOR_GRAVATAR); - }); - - it('renders committer text', () => { - const committerElement = getCommitterElement(vm); - const nameElement = committerElement.querySelector('a'); - - expect(nameElement).toHaveAttr('href', `mailto:${TEST_AUTHOR_EMAIL}`); - expect(nameElement).toHaveText(TEST_AUTHOR_NAME); - }); - }); - - describe('with signature', () => { - beforeEach(done => { - vm.commit.signature_html = TEST_SIGNATURE_HTML; - - vm.$nextTick() - .then(done) - .catch(done.fail); - }); - - it('renders signature html', () => { - const actionsElement = getCommitActionsElement(vm); - - expect(actionsElement).toContainHtml(TEST_SIGNATURE_HTML); - }); - }); - - describe('with pipeline status', () => { - beforeEach(done => { - vm.commit.pipeline_status_path = TEST_PIPELINE_STATUS_PATH; - - vm.$nextTick() - .then(done) - .catch(done.fail); - }); - - it('renders pipeline status', () => { - const actionsElement = getCommitActionsElement(vm); - - expect(actionsElement).toContainElement('.ci-status-link'); - }); - }); -}); diff --git a/spec/lib/sentry/client/issue_spec.rb b/spec/lib/sentry/client/issue_spec.rb index d35e4b83d7f..62cbfbf0b30 100644 --- a/spec/lib/sentry/client/issue_spec.rb +++ b/spec/lib/sentry/client/issue_spec.rb @@ -49,7 +49,7 @@ describe Sentry::Client::Issue do it_behaves_like 'calls sentry api' it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues have correct length', 2 + it_behaves_like 'issues have correct length', 3 shared_examples 'has correct external_url' do context 'external_url' do @@ -184,7 +184,7 @@ describe Sentry::Client::Issue do it_behaves_like 'calls sentry api' it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues have correct length', 2 + it_behaves_like 'issues have correct length', 3 end context 'when cursor is present' do @@ -194,7 +194,7 @@ describe Sentry::Client::Issue do it_behaves_like 'calls sentry api' it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error - it_behaves_like 'issues have correct length', 2 + it_behaves_like 'issues have correct length', 3 end end |