Commit 37bc2274 authored by Max Woolf's avatar Max Woolf

Add support for failure status status checks

Previously, the existence of a status check
response was taken to mean an approval.

Adds a new status parameter, which defaults
to approved to avoid any breaking changes but
also allows explicitly failing.

Updates frontend components to handle new status.

Changelog: added
EE: true
parent 5e223920
...@@ -18,6 +18,7 @@ export const ICON_WARNING = 'warning'; ...@@ -18,6 +18,7 @@ export const ICON_WARNING = 'warning';
export const ICON_SUCCESS = 'success'; export const ICON_SUCCESS = 'success';
export const ICON_NOTFOUND = 'notfound'; export const ICON_NOTFOUND = 'notfound';
export const ICON_PENDING = 'pending'; export const ICON_PENDING = 'pending';
export const ICON_FAILED = 'failed';
export const status = { export const status = {
LOADING, LOADING,
......
# frozen_string_literal: true
class AddStatusToStatusCheckResponses < Gitlab::Database::Migration[1.0]
def change
add_column :status_check_responses, :status, :integer, default: 0, null: false, limit: 2
end
end
d2d236e9ee5fa6e9c1ee97431543e871b78e469b812444bd9386dfecf849947b
\ No newline at end of file
...@@ -20656,7 +20656,8 @@ CREATE TABLE status_check_responses ( ...@@ -20656,7 +20656,8 @@ CREATE TABLE status_check_responses (
merge_request_id bigint NOT NULL, merge_request_id bigint NOT NULL,
external_approval_rule_id bigint, external_approval_rule_id bigint,
sha bytea NOT NULL, sha bytea NOT NULL,
external_status_check_id bigint NOT NULL external_status_check_id bigint NOT NULL,
status smallint DEFAULT 0 NOT NULL
); );
CREATE SEQUENCE status_check_responses_id_seq CREATE SEQUENCE status_check_responses_id_seq
...@@ -44,6 +44,9 @@ GET /projects/:id/merge_requests/:merge_request_iid/status_checks ...@@ -44,6 +44,9 @@ GET /projects/:id/merge_requests/:merge_request_iid/status_checks
## Set status of an external status check ## Set status of an external status check
> - Introduced in GitLab 14.9, `passed` status to pass external status checks.
> - `pass` status to pass checks is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/339039) in GitLab 14.9. Replaced with `passed`.
For a single merge request, use the API to inform GitLab that a merge request has passed a check by an external service. For a single merge request, use the API to inform GitLab that a merge request has passed a check by an external service.
To set the status of an external check, the personal access token used must belong to a user with at least the developer role on the target project of the merge request. To set the status of an external check, the personal access token used must belong to a user with at least the developer role on the target project of the merge request.
...@@ -55,13 +58,14 @@ POST /projects/:id/merge_requests/:merge_request_iid/status_check_responses ...@@ -55,13 +58,14 @@ POST /projects/:id/merge_requests/:merge_request_iid/status_check_responses
**Parameters:** **Parameters:**
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
| -------------------------- | ------- | -------- | ------------------------------------- | | -------------------------- | ------- | -------- | ---------------------------------------------------------------------------- |
| `id` | integer | yes | ID of a project | | `id` | integer | yes | ID of a project |
| `merge_request_iid` | integer | yes | IID of a merge request | | `merge_request_iid` | integer | yes | IID of a merge request |
| `sha` | string | yes | SHA at `HEAD` of the source branch | | `sha` | string | yes | SHA at `HEAD` of the source branch |
| `external_status_check_id` | integer | yes | ID of an external status check | | `external_status_check_id` | integer | yes | ID of an external status check |
| `status` | string | no | Set to `pass` to pass the check | | `status` | string | no | Set to `passed` to pass the check or `failed` to fail it (GitLab 14.9 and later with feature flag enabled) |
| `status` | string | no | Set to `pass` to pass the check (GitLab 14.0 to GitLab 14.8) |
NOTE: NOTE:
`sha` must be the SHA at the `HEAD` of the merge request's source branch. `sha` must be the SHA at the `HEAD` of the merge request's source branch.
......
export const PASSED = 'passed';
export const APPROVED = 'approved'; export const APPROVED = 'approved';
export const PENDING = 'pending'; export const PENDING = 'pending';
export const FAILED = 'failed';
<script> <script>
import { GlLink, GlSprintf } from '@gitlab/ui';
import * as Sentry from '@sentry/browser'; import * as Sentry from '@sentry/browser';
import { componentNames } from 'ee/reports/components/issue_body'; import { componentNames } from 'ee/reports/components/issue_body';
import { helpPagePath } from '~/helpers/help_page_helper'; import { helpPagePath } from '~/helpers/help_page_helper';
...@@ -7,13 +6,11 @@ import axios from '~/lib/utils/axios_utils'; ...@@ -7,13 +6,11 @@ import axios from '~/lib/utils/axios_utils';
import { sprintf, s__ } from '~/locale'; import { sprintf, s__ } from '~/locale';
import ReportSection from '~/reports/components/report_section.vue'; import ReportSection from '~/reports/components/report_section.vue';
import { status } from '~/reports/constants'; import { status } from '~/reports/constants';
import { APPROVED, PENDING } from './constants'; import { APPROVED, PASSED, PENDING, FAILED } from './constants';
export default { export default {
name: 'StatusChecksReportsApp', name: 'StatusChecksReportsApp',
components: { components: {
GlLink,
GlSprintf,
ReportSection, ReportSection,
}, },
componentNames, componentNames,
...@@ -31,21 +28,26 @@ export default { ...@@ -31,21 +28,26 @@ export default {
}, },
computed: { computed: {
approvedStatusChecks() { approvedStatusChecks() {
return this.statusChecks.filter((s) => s.status === APPROVED); return this.statusChecks.filter((s) => [PASSED, APPROVED].includes(s.status));
}, },
pendingStatusChecks() { pendingStatusChecks() {
return this.statusChecks.filter((s) => s.status === PENDING); return this.statusChecks.filter((s) => s.status === PENDING);
}, },
failedStatusChecks() {
return this.statusChecks.filter((s) => s.status === FAILED);
},
hasStatusChecks() { hasStatusChecks() {
return this.statusChecks.length > 0; return this.statusChecks.length > 0;
}, },
headingReportText() { headingReportText() {
if (this.pendingStatusChecks.length > 0) { if (this.approvedStatusChecks.length === this.statusChecks.length) {
return sprintf(s__('StatusCheck|%{pending} pending'), { return s__('StatusCheck|All passed');
pending: this.pendingStatusChecks.length,
});
} }
return s__('StatusCheck|All passed');
return sprintf(s__('StatusCheck| %{failed} failed, and %{pending} pending'), {
pending: this.pendingStatusChecks.length,
failed: this.failedStatusChecks.length,
});
}, },
}, },
mounted() { mounted() {
...@@ -67,9 +69,6 @@ export default { ...@@ -67,9 +69,6 @@ export default {
}, },
i18n: { i18n: {
heading: s__('StatusCheck|Status checks'), heading: s__('StatusCheck|Status checks'),
subHeading: s__(
'StatusCheck|When this merge request is updated, a call is sent to the following APIs to confirm their status. %{linkStart}Learn more%{linkEnd}.',
),
errorText: s__('StatusCheck|Failed to load status checks.'), errorText: s__('StatusCheck|Failed to load status checks.'),
}, },
docsLink: helpPagePath('user/project/merge_requests/status_checks.md', { docsLink: helpPagePath('user/project/merge_requests/status_checks.md', {
...@@ -85,6 +84,7 @@ export default { ...@@ -85,6 +84,7 @@ export default {
:error-text="$options.i18n.errorText" :error-text="$options.i18n.errorText"
:has-issues="hasStatusChecks" :has-issues="hasStatusChecks"
:resolved-issues="approvedStatusChecks" :resolved-issues="approvedStatusChecks"
:unresolved-issues="failedStatusChecks"
:neutral-issues="pendingStatusChecks" :neutral-issues="pendingStatusChecks"
:component="$options.componentNames.StatusCheckIssueBody" :component="$options.componentNames.StatusCheckIssueBody"
:show-report-section-status-icon="false" :show-report-section-status-icon="false"
...@@ -99,15 +99,5 @@ export default { ...@@ -99,15 +99,5 @@ export default {
<strong class="gl-p-1">{{ headingReportText }}</strong> <strong class="gl-p-1">{{ headingReportText }}</strong>
</p> </p>
</template> </template>
<template #sub-heading>
<span class="gl-text-gray-500 gl-font-sm">
<gl-sprintf :message="$options.i18n.subHeading">
<template #link="{ content }">
<gl-link class="gl-font-sm" :href="$options.docsLink">{{ content }}</gl-link>
</template>
</gl-sprintf>
</span>
</template>
</report-section> </report-section>
</template> </template>
<script> <script>
import SummaryRow from '~/reports/components/summary_row.vue'; import SummaryRow from '~/reports/components/summary_row.vue';
import { ICON_SUCCESS, ICON_PENDING } from '~/reports/constants'; import { ICON_SUCCESS, ICON_PENDING, ICON_FAILED } from '~/reports/constants';
import { APPROVED } from '../../reports/status_checks_report/constants'; import { PASSED, APPROVED, FAILED } from '../../reports/status_checks_report/constants';
export default { export default {
name: 'StatusCheckIssueBody', name: 'StatusCheckIssueBody',
...@@ -16,10 +16,16 @@ export default { ...@@ -16,10 +16,16 @@ export default {
}, },
computed: { computed: {
statusIcon() { statusIcon() {
if (this.issue.status === APPROVED) { switch (this.issue.status) {
return ICON_SUCCESS; case APPROVED:
return ICON_SUCCESS;
case PASSED:
return ICON_SUCCESS;
case FAILED:
return ICON_FAILED;
default:
return ICON_PENDING;
} }
return ICON_PENDING;
}, },
}, },
}; };
...@@ -29,7 +35,7 @@ export default { ...@@ -29,7 +35,7 @@ export default {
<div class="gl-w-full" :data-testid="`mr-status-check-issue-${issue.id}`"> <div class="gl-w-full" :data-testid="`mr-status-check-issue-${issue.id}`">
<summary-row :status-icon="statusIcon" nested-summary> <summary-row :status-icon="statusIcon" nested-summary>
<template #summary> <template #summary>
<span>{{ issue.name }}, {{ issue.external_url }}</span> <span>{{ issue.name }}: {{ issue.external_url }}</span>
</template> </template>
</summary-row> </summary-row>
</div> </div>
......
...@@ -5,7 +5,7 @@ import { ...@@ -5,7 +5,7 @@ import {
EXTENSION_SUMMARY_FAILED_CLASS, EXTENSION_SUMMARY_FAILED_CLASS,
EXTENSION_SUMMARY_NEUTRAL_CLASS, EXTENSION_SUMMARY_NEUTRAL_CLASS,
} from '~/vue_merge_request_widget/constants'; } from '~/vue_merge_request_widget/constants';
import { APPROVED, PENDING } from 'ee/reports/status_checks_report/constants'; import { APPROVED, PASSED, PENDING } from 'ee/reports/status_checks_report/constants';
export default { export default {
name: 'WidgetStatusChecks', name: 'WidgetStatusChecks',
...@@ -48,7 +48,7 @@ export default { ...@@ -48,7 +48,7 @@ export default {
return { return {
subject: s__('StatusCheck|Status checks'), subject: s__('StatusCheck|Status checks'),
meta: reports.join(__(', and ')), meta: reports.join(__(', ')),
}; };
}, },
statusIcon({ pending = [], failed = [] }) { statusIcon({ pending = [], failed = [] }) {
...@@ -102,6 +102,9 @@ export default { ...@@ -102,6 +102,9 @@ export default {
statusChecks.forEach((statusCheck) => { statusChecks.forEach((statusCheck) => {
switch (statusCheck.status) { switch (statusCheck.status) {
case PASSED:
approved.push(this.createReportRow(statusCheck, EXTENSION_ICONS.success));
break;
case APPROVED: case APPROVED:
approved.push(this.createReportRow(statusCheck, EXTENSION_ICONS.success)); approved.push(this.createReportRow(statusCheck, EXTENSION_ICONS.success));
break; break;
......
...@@ -18,6 +18,7 @@ module EE ...@@ -18,6 +18,7 @@ module EE
push_frontend_feature_flag(:missing_mr_security_scan_types, @project) push_frontend_feature_flag(:missing_mr_security_scan_types, @project)
push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml) push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml)
push_frontend_feature_flag(:refactor_mr_widgets_extensions_user, current_user, default_enabled: :yaml) push_frontend_feature_flag(:refactor_mr_widgets_extensions_user, current_user, default_enabled: :yaml)
push_frontend_feature_flag(:status_checks_add_status_field, default_enabled: :yaml)
end end
before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports, before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports,
......
...@@ -27,6 +27,10 @@ module MergeRequests ...@@ -27,6 +27,10 @@ module MergeRequests
ApprovalRules::ExternalApprovalRulePayloadWorker.perform_async(self.id, payload_data(data)) ApprovalRules::ExternalApprovalRulePayloadWorker.perform_async(self.id, payload_data(data))
end end
def status(merge_request, sha)
merge_request.status_check_responses.where(external_status_check: self, sha: sha).last&.status || 'pending'
end
def approved?(merge_request, sha) def approved?(merge_request, sha)
merge_request.status_check_responses.where(external_status_check: self, sha: sha).exists? merge_request.status_check_responses.where(external_status_check: self, sha: sha).exists?
end end
......
...@@ -11,6 +11,8 @@ module MergeRequests ...@@ -11,6 +11,8 @@ module MergeRequests
belongs_to :merge_request belongs_to :merge_request
belongs_to :external_status_check, class_name: 'MergeRequests::ExternalStatusCheck' belongs_to :external_status_check, class_name: 'MergeRequests::ExternalStatusCheck'
enum status: %w(passed failed)
validates :merge_request, presence: true validates :merge_request, presence: true
validates :external_status_check, presence: true validates :external_status_check, presence: true
validates :sha, presence: true validates :sha, presence: true
......
---
name: status_checks_add_status_field
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81005
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353836
milestone: '14.9'
type: development
group: group::compliance
default_enabled: false
...@@ -10,7 +10,11 @@ module API ...@@ -10,7 +10,11 @@ module API
expose :status expose :status
def status def status
object.approved?(options[:merge_request], options[:sha]) ? 'approved' : 'pending' if ::Feature.enabled?(:status_checks_add_status_field, object.project, default_enabled: :yaml)
object.status(options[:merge_request], options[:sha])
else
object.approved?(options[:merge_request], options[:sha]) ? 'approved' : 'pending'
end
end end
end end
end end
......
...@@ -106,16 +106,19 @@ module API ...@@ -106,16 +106,19 @@ module API
requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request' requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
requires :external_status_check_id, type: Integer, desc: 'The ID of a external status check' requires :external_status_check_id, type: Integer, desc: 'The ID of a external status check'
requires :sha, type: String, desc: 'The current SHA at HEAD of the merge request.' requires :sha, type: String, desc: 'The current SHA at HEAD of the merge request.'
optional :status, type: String, desc: 'Status of the merge request status check', default: 'passed', values: %w(passed failed)
end end
post 'status_check_responses' do post 'status_check_responses' do
merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request)
status = ::Feature.enabled?(:status_checks_add_status_field, merge_request.project, default_enabled: :yaml) ? params[:status] : 'passed'
status_check = merge_request.project.external_status_checks.find(params[:external_status_check_id]) status_check = merge_request.project.external_status_checks.find(params[:external_status_check_id])
check_sha_param!(params, merge_request) check_sha_param!(params, merge_request)
not_found! unless current_user.can?(:provide_status_check_response, merge_request) not_found! unless current_user.can?(:provide_status_check_response, merge_request)
approval = merge_request.status_check_responses.create!(external_status_check: status_check, sha: params[:sha]) approval = merge_request.status_check_responses.create!(external_status_check: status_check, sha: params[:sha], status: status)
present(approval, with: Entities::MergeRequests::StatusCheckResponse) present(approval, with: Entities::MergeRequests::StatusCheckResponse)
end end
......
...@@ -4,5 +4,7 @@ FactoryBot.define do ...@@ -4,5 +4,7 @@ FactoryBot.define do
factory :status_check_response, class: 'MergeRequests::StatusCheckResponse' do factory :status_check_response, class: 'MergeRequests::StatusCheckResponse' do
merge_request merge_request
external_status_check external_status_check
sha { 'aabccddee' }
status { 'passed' }
end end
end end
...@@ -3,13 +3,17 @@ ...@@ -3,13 +3,17 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Merge request > User sees status checks widget', :js do RSpec.describe 'Merge request > User sees status checks widget', :js do
using RSpec::Parameterized::TableSyntax
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let_it_be(:check1) { create(:external_status_check, project: project) } let_it_be(:check_pending) { create(:external_status_check, project: project) }
let_it_be(:check2) { create(:external_status_check, project: project) } let_it_be(:check_failed) { create(:external_status_check, project: project) }
let_it_be(:check_passed) { create(:external_status_check, project: project) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:merge_request) { create(:merge_request, source_project: project) }
let_it_be(:status_check_response) { create(:status_check_response, external_status_check: check1, merge_request: merge_request, sha: merge_request.source_branch_sha) } let_it_be(:status_check_response_passed) { create(:status_check_response, external_status_check: check_passed, merge_request: merge_request, sha: merge_request.source_branch_sha, status: 'passed') }
let_it_be(:status_check_response_failed) { create(:status_check_response, external_status_check: check_failed, merge_request: merge_request, sha: merge_request.source_branch_sha, status: 'failed') }
shared_examples 'no status checks widget' do shared_examples 'no status checks widget' do
it 'does not show the widget' do it 'does not show the widget' do
...@@ -19,13 +23,12 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do ...@@ -19,13 +23,12 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do
before do before do
stub_licensed_features(external_status_checks: true) stub_licensed_features(external_status_checks: true)
stub_feature_flags(refactor_mr_widgets_extensions: false)
stub_feature_flags(refactor_mr_widgets_extensions_user: false)
end end
context 'user is authorized' do context 'user is authorized' do
before do before do
stub_feature_flags(refactor_mr_widgets_extensions: false)
stub_feature_flags(refactor_mr_widgets_extensions_user: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
...@@ -33,45 +36,25 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do ...@@ -33,45 +36,25 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do
end end
it 'shows the widget' do it 'shows the widget' do
expect(page).to have_content('Status checks 1 pending') expect(page).to have_content('Status checks 1 failed, and 1 pending')
end
it 'shows the status check issues', :aggregate_failures do
within '[data-test-id="mr-status-checks"]' do
click_button 'Expand'
end
[check1, check2].each do |rule|
within "[data-testid='mr-status-check-issue-#{rule.id}']" do
icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'pending'
expect(page).to have_css(".ci-status-icon-#{icon_type}")
expect(page).to have_content("#{rule.name}, #{rule.external_url}")
end
end
end
end
context 'widget extension flag is enabled' do
before do
project.add_maintainer(user)
sign_in(user)
visit project_merge_request_path(project, merge_request)
end end
it 'shows the widget' do where(:check, :icon_class) do
expect(page).to have_content('Status checks 1 pending') lazy { check_pending } | '.ci-status-icon-pending'
lazy { check_passed } | '.ci-status-icon-success'
lazy { check_failed } | '.ci-status-icon-failed'
end end
it 'shows the status check issues', :aggregate_failures do with_them do
within '[data-testid="widget-extension"]' do it 'is rendered correctly', :aggregate_failures do
find('[data-testid="toggle-button"]').click within '[data-test-id="mr-status-checks"]' do
end click_button 'Expand'
end
[check1, check2].each do |rule| within "[data-testid='mr-status-check-issue-#{check.id}']" do
icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'neutral' expect(page).to have_css(icon_class)
expect(page).to have_css("[data-testid='status-#{icon_type}-icon']") expect(page).to have_content("#{check.name}: #{check.external_url}")
expect(page).to have_content("#{rule.name}: #{rule.external_url}") end
end end
end end
end end
......
...@@ -9,7 +9,7 @@ exports[`Grouped test reports app when mounted matches the default state compone ...@@ -9,7 +9,7 @@ exports[`Grouped test reports app when mounted matches the default state compone
<div class=\\"gl-display-flex gl-align-items-center\\"> <div class=\\"gl-display-flex gl-align-items-center\\">
<p class=\\"gl-line-height-normal gl-m-0\\">Status checks</p> <p class=\\"gl-line-height-normal gl-m-0\\">Status checks</p>
<!----> <!---->
</div> <span class=\\"gl-text-gray-500 gl-font-sm\\">When this merge request is updated, a call is sent to the following APIs to confirm their status. <gl-link-stub href=\\"/help/user/project/merge_requests/status_checks.md#status-checks-widget\\" class=\\"gl-font-sm\\">Learn more</gl-link-stub>.</span> </div>
</div> </div>
<!----> <!---->
</div> </div>
......
...@@ -3,7 +3,7 @@ export const approvedChecks = [ ...@@ -3,7 +3,7 @@ export const approvedChecks = [
id: 1, id: 1,
name: 'Foo', name: 'Foo',
external_url: 'http://foo', external_url: 'http://foo',
status: 'approved', status: 'passed',
}, },
]; ];
...@@ -28,3 +28,5 @@ export const failedChecks = [ ...@@ -28,3 +28,5 @@ export const failedChecks = [
export const pendingAndFailedChecks = [...pendingChecks, ...failedChecks]; export const pendingAndFailedChecks = [...pendingChecks, ...failedChecks];
export const approvedAndPendingChecks = [...approvedChecks, ...pendingChecks]; export const approvedAndPendingChecks = [...approvedChecks, ...pendingChecks];
export const approvedFailedAndPending = [...approvedChecks, ...failedChecks, ...pendingChecks];
...@@ -8,7 +8,13 @@ import axios from '~/lib/utils/axios_utils'; ...@@ -8,7 +8,13 @@ import axios from '~/lib/utils/axios_utils';
import httpStatus from '~/lib/utils/http_status'; import httpStatus from '~/lib/utils/http_status';
import ReportSection from '~/reports/components/report_section.vue'; import ReportSection from '~/reports/components/report_section.vue';
import { status as reportStatus } from '~/reports/constants'; import { status as reportStatus } from '~/reports/constants';
import { approvedChecks, pendingChecks, approvedAndPendingChecks } from './mock_data'; import {
approvedChecks,
pendingChecks,
failedChecks,
approvedAndPendingChecks,
pendingAndFailedChecks,
} from './mock_data';
jest.mock('~/flash'); jest.mock('~/flash');
...@@ -34,6 +40,9 @@ describe('Grouped test reports app', () => { ...@@ -34,6 +40,9 @@ describe('Grouped test reports app', () => {
beforeEach(() => { beforeEach(() => {
mock = new MockAdapter(axios); mock = new MockAdapter(axios);
window.gon.features = {
statusChecksAddStatusField: true,
};
}); });
afterEach(() => { afterEach(() => {
...@@ -73,29 +82,34 @@ describe('Grouped test reports app', () => { ...@@ -73,29 +82,34 @@ describe('Grouped test reports app', () => {
}; };
describe.each` describe.each`
state | response | text | resolvedIssues | neutralIssues state | response | text | resolvedIssues | neutralIssues | unresolvedIssues
${'approved'} | ${approvedChecks} | ${'All passed'} | ${approvedChecks} | ${[]} ${'approved'} | ${approvedChecks} | ${'All passed'} | ${approvedChecks} | ${[]} | ${[]}
${'pending'} | ${pendingChecks} | ${'1 pending'} | ${[]} | ${pendingChecks} ${'pending'} | ${pendingChecks} | ${'0 failed, and 1 pending'} | ${[]} | ${pendingChecks} | ${[]}
${'mixed'} | ${approvedAndPendingChecks} | ${'1 pending'} | ${approvedChecks} | ${pendingChecks} ${'approved and pending'} | ${approvedAndPendingChecks} | ${'0 failed, and 1 pending'} | ${approvedChecks} | ${pendingChecks} | ${[]}
`('and the status checks are $state', ({ response, text, resolvedIssues, neutralIssues }) => { ${'pending and failed'} | ${pendingAndFailedChecks} | ${'1 failed, and 1 pending'} | ${[]} | ${pendingChecks} | ${failedChecks}
beforeEach(() => { `(
return mountWithResponse(httpStatus.OK, response); 'and the status checks are $state',
}); ({ response, text, resolvedIssues, neutralIssues, unresolvedIssues }) => {
beforeEach(() => {
it('sets the report status to success', () => { return mountWithResponse(httpStatus.OK, response);
expect(findReport().props('status')).toBe(reportStatus.SUCCESS); });
});
it('sets the report status to success', () => {
it('sets the issues on the report', () => { expect(findReport().props('status')).toBe(reportStatus.SUCCESS);
expect(findReport().props('hasIssues')).toBe(true); });
expect(findReport().props('resolvedIssues')).toStrictEqual(resolvedIssues);
expect(findReport().props('neutralIssues')).toStrictEqual(neutralIssues); it('sets the issues on the report', () => {
}); expect(findReport().props('hasIssues')).toBe(true);
expect(findReport().props('unresolvedIssues')).toStrictEqual(unresolvedIssues);
it(`renders '${text}' in the report section`, () => { expect(findReport().props('resolvedIssues')).toStrictEqual(resolvedIssues);
expect(findReport().text()).toContain(text); expect(findReport().props('neutralIssues')).toStrictEqual(neutralIssues);
}); });
});
it(`renders '${text}' in the report section`, () => {
expect(findReport().text()).toContain(text);
});
},
);
describe('and an error occurred', () => { describe('and an error occurred', () => {
beforeEach(() => { beforeEach(() => {
......
...@@ -31,13 +31,15 @@ describe('status check issue body', () => { ...@@ -31,13 +31,15 @@ describe('status check issue body', () => {
}); });
it('renders the status check name and external URL', () => { it('renders the status check name and external URL', () => {
expect(wrapper.text()).toBe(`${defaultStatusCheck.name}, ${defaultStatusCheck.external_url}`); expect(wrapper.text()).toBe(`${defaultStatusCheck.name}: ${defaultStatusCheck.external_url}`);
}); });
it.each` it.each`
status | icon status | icon
${'passed'} | ${'success'}
${'approved'} | ${'success'} ${'approved'} | ${'success'}
${'pending'} | ${'pending'} ${'pending'} | ${'pending'}
${'failed'} | ${'failed'}
`('sets the status-icon to $icon when the check status is $status', ({ status, icon }) => { `('sets the status-icon to $icon when the check status is $status', ({ status, icon }) => {
createComponent({ status }); createComponent({ status });
......
...@@ -78,7 +78,7 @@ describe('Status checks extension', () => { ...@@ -78,7 +78,7 @@ describe('Status checks extension', () => {
${'approved'} | ${approvedChecks} | ${'Status checks all passed'} ${'approved'} | ${approvedChecks} | ${'Status checks all passed'}
${'pending'} | ${pendingChecks} | ${'1 pending'} ${'pending'} | ${pendingChecks} | ${'1 pending'}
${'approved and pending'} | ${approvedAndPendingChecks} | ${'1 pending'} ${'approved and pending'} | ${approvedAndPendingChecks} | ${'1 pending'}
${'failed and pending'} | ${pendingAndFailedChecks} | ${'1 failed, and 1 pending'} ${'failed and pending'} | ${pendingAndFailedChecks} | ${'1 failed, 1 pending'}
`('and the status checks are $state', ({ response, text }) => { `('and the status checks are $state', ({ response, text }) => {
beforeEach(async () => { beforeEach(async () => {
await setupWithResponse(httpStatus.OK, response); await setupWithResponse(httpStatus.OK, response);
......
...@@ -120,5 +120,71 @@ RSpec.describe MergeRequests::ExternalStatusCheck, type: :model do ...@@ -120,5 +120,71 @@ RSpec.describe MergeRequests::ExternalStatusCheck, type: :model do
it { is_expected.to be false } it { is_expected.to be false }
end end
describe 'status' do
let_it_be(:rule) { create(:external_status_check) }
let_it_be(:merge_request) { create(:merge_request) }
let(:project) { merge_request.source_project }
subject { rule.status(merge_request, merge_request.source_branch_sha) }
context 'when a rule has a positive status check response' do
let_it_be(:status_check_response) { create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: merge_request.source_branch_sha, status: 'passed') }
it { is_expected.to eq('passed') }
context 'when a rule also has a positive check response from an old sha' do
before do
create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: 'abc1234', status: 'passed')
end
it { is_expected.to eq('passed') }
end
end
context 'when a rule has a negative status check response' do
let_it_be(:status_check_response) { create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: merge_request.source_branch_sha, status: 'failed') }
it { is_expected.to eq('failed') }
end
context 'when a rule has no status check response' do
it { is_expected.to eq('pending') }
end
end
describe 'status' do
let_it_be(:rule) { create(:external_status_check) }
let_it_be(:merge_request) { create(:merge_request) }
let(:project) { merge_request.source_project }
subject { rule.status(merge_request, merge_request.source_branch_sha) }
context 'when a rule has a positive status check response' do
let_it_be(:status_check_response) { create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: merge_request.source_branch_sha, status: 'passed') }
it { is_expected.to eq('passed') }
context 'when a rule also has a positive check response from an old sha' do
before do
create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: 'abc1234', status: 'passed')
end
it { is_expected.to eq('passed') }
end
end
context 'when a rule has a negative status check response' do
let_it_be(:status_check_response) { create(:status_check_response, merge_request: merge_request, external_status_check: rule, sha: merge_request.source_branch_sha, status: 'failed') }
it { is_expected.to eq('failed') }
end
context 'when a rule has no status check response' do
it { is_expected.to eq('pending') }
end
end
end end
end end
...@@ -8,6 +8,8 @@ RSpec.describe MergeRequests::StatusCheckResponse, type: :model do ...@@ -8,6 +8,8 @@ RSpec.describe MergeRequests::StatusCheckResponse, type: :model do
it { is_expected.to belong_to(:merge_request) } it { is_expected.to belong_to(:merge_request) }
it { is_expected.to belong_to(:external_status_check).class_name('MergeRequests::ExternalStatusCheck') } it { is_expected.to belong_to(:external_status_check).class_name('MergeRequests::ExternalStatusCheck') }
it { is_expected.to define_enum_for(:status) }
it { is_expected.to validate_presence_of(:merge_request) } it { is_expected.to validate_presence_of(:merge_request) }
it { is_expected.to validate_presence_of(:external_status_check) } it { is_expected.to validate_presence_of(:external_status_check) }
it { is_expected.to validate_presence_of(:sha) } it { is_expected.to validate_presence_of(:sha) }
......
...@@ -16,6 +16,7 @@ RSpec.describe API::StatusChecks do ...@@ -16,6 +16,7 @@ RSpec.describe API::StatusChecks do
let(:single_object_url) { "/projects/#{project.id}/external_status_checks/#{rule.id}" } let(:single_object_url) { "/projects/#{project.id}/external_status_checks/#{rule.id}" }
let(:collection_url) { "/projects/#{project.id}/external_status_checks" } let(:collection_url) { "/projects/#{project.id}/external_status_checks" }
let(:sha) { merge_request.source_branch_sha } let(:sha) { merge_request.source_branch_sha }
let(:status) { '' }
subject { get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_checks", user), params: { external_status_check_id: rule.id, sha: sha } } subject { get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_checks", user), params: { external_status_check_id: rule.id, sha: sha } }
...@@ -46,16 +47,32 @@ RSpec.describe API::StatusChecks do ...@@ -46,16 +47,32 @@ RSpec.describe API::StatusChecks do
it 'has the correct status values' do it 'has the correct status values' do
subject subject
expect(json_response[0]["status"]).to eq('approved') expect(json_response[0]["status"]).to eq('passed')
expect(json_response[1]["status"]).to eq('pending') expect(json_response[1]["status"]).to eq('pending')
expect(json_response[2]["status"]).to eq('pending') expect(json_response[2]["status"]).to eq('pending')
end end
context 'when status_checks_add_status_field is disabled' do
before do
stub_feature_flags(status_checks_add_status_field: false)
end
it 'has the correct status values' do
subject
expect(json_response[0]["status"]).to eq('approved')
expect(json_response[1]["status"]).to eq('pending')
expect(json_response[2]["status"]).to eq('pending')
end
end
end end
end end
end end
describe 'POST :id/:merge_requests/:merge_request_iid/status_check_responses' do describe 'POST :id/:merge_requests/:merge_request_iid/status_check_responses' do
subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_check_responses", user), params: { external_status_check_id: rule.id, sha: sha } } subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_check_responses", user), params: { external_status_check_id: rule.id, sha: sha, status: status } }
let(:status) { 'passed' }
context 'permissions' do context 'permissions' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
...@@ -94,6 +111,22 @@ RSpec.describe API::StatusChecks do ...@@ -94,6 +111,22 @@ RSpec.describe API::StatusChecks do
project.add_user(user, :maintainer) project.add_user(user, :maintainer)
end end
context 'when status_checks_add_status_field flag is disabled' do
before do
stub_feature_flags(status_checks_add_status_field: false)
end
context 'status is failed' do
let(:status) { 'failed' }
it 'is overridden to passed' do
subject
expect(MergeRequests::StatusCheckResponse.last.status).to eq("passed")
end
end
end
context 'when external status check ID does not belong to the requested project' do context 'when external status check ID does not belong to the requested project' do
let_it_be(:rule) { create(:external_status_check) } let_it_be(:rule) { create(:external_status_check) }
......
...@@ -1279,7 +1279,7 @@ msgstr "" ...@@ -1279,7 +1279,7 @@ msgstr ""
msgid "+%{tags} more" msgid "+%{tags} more"
msgstr "" msgstr ""
msgid ", and " msgid ", "
msgstr "" msgstr ""
msgid ", or " msgid ", or "
...@@ -35084,6 +35084,9 @@ msgstr "" ...@@ -35084,6 +35084,9 @@ msgstr ""
msgid "Status: %{title}" msgid "Status: %{title}"
msgstr "" msgstr ""
msgid "StatusCheck| %{failed} failed, and %{pending} pending"
msgstr ""
msgid "StatusCheck|%{failed} failed" msgid "StatusCheck|%{failed} failed"
msgstr "" msgstr ""
...@@ -35156,9 +35159,6 @@ msgstr "" ...@@ -35156,9 +35159,6 @@ msgstr ""
msgid "StatusCheck|Update status check" msgid "StatusCheck|Update status check"
msgstr "" msgstr ""
msgid "StatusCheck|When this merge request is updated, a call is sent to the following APIs to confirm their status. %{linkStart}Learn more%{linkEnd}."
msgstr ""
msgid "StatusCheck|You are about to remove the %{name} status check." msgid "StatusCheck|You are about to remove the %{name} status check."
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment