Commit 912545f3 authored by Thiago Figueiró's avatar Thiago Figueiró

Revert "Merge branch '210327-custom-scanner-filter' into 'master'"

This reverts merge request !46540
parent 0ce72a03
...@@ -63,8 +63,6 @@ job finishes but the DAST job fails, the security dashboard doesn't show SAST re ...@@ -63,8 +63,6 @@ job finishes but the DAST job fails, the security dashboard doesn't show SAST re
the analyzer outputs an the analyzer outputs an
[exit code](../../../development/integrations/secure.md#exit-code). [exit code](../../../development/integrations/secure.md#exit-code).
You can filter the vulnerabilities list by selecting from the **Severity** and **Scanner** dropdowns.
## Project Security Dashboard ## Project Security Dashboard
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/235558) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.6. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/235558) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.6.
...@@ -107,11 +105,6 @@ You can filter the vulnerabilities by one or more of the following: ...@@ -107,11 +105,6 @@ You can filter the vulnerabilities by one or more of the following:
| Severity | Critical, High, Medium, Low, Info, Unknown | | Severity | Critical, High, Medium, Low, Info, Unknown |
| Scanner | [Available Scanners](../index.md#security-scanning-tools) | | Scanner | [Available Scanners](../index.md#security-scanning-tools) |
You can filter the vulnerabilities list by selecting from the **Status**, **Severity**, and
**Scanner** dropdowns. In the **Scanner** dropdown, select individual scanners or scanner groups to
toggle those scanners. The **Scanner** dropdown includes both GitLab scanners, and in GitLab 13.6
and later, custom scanners.
You can also dismiss vulnerabilities in the table: You can also dismiss vulnerabilities in the table:
1. Select the checkbox for each vulnerability you want to dismiss. 1. Select the checkbox for each vulnerability you want to dismiss.
...@@ -267,11 +260,6 @@ You can filter which vulnerabilities the vulnerability report displays by: ...@@ -267,11 +260,6 @@ You can filter which vulnerabilities the vulnerability report displays by:
| Scanner | [Available Scanners](../index.md#security-scanning-tools) | | Scanner | [Available Scanners](../index.md#security-scanning-tools) |
| Project | Projects configured in the Security Center settings | | Project | Projects configured in the Security Center settings |
You can filter the vulnerabilities list by selecting from the **Status**, **Severity**, and
**Scanner**, and **Project** dropdowns. In the **Scanner** dropdown, select individual scanners or
scanner groups to toggle those scanners. The **Scanner** dropdown includes both GitLab scanners, and
in GitLab 13.6 and later, custom scanners.
Clicking any vulnerability in the table takes you to its Clicking any vulnerability in the table takes you to its
[Vulnerability Details](../vulnerabilities) page to see more information on that vulnerability. [Vulnerability Details](../vulnerabilities) page to see more information on that vulnerability.
To create an issue associated with the vulnerability, click the **Create Issue** button. To create an issue associated with the vulnerability, click the **Create Issue** button.
......
...@@ -58,17 +58,15 @@ export default { ...@@ -58,17 +58,15 @@ export default {
toggle-class="gl-w-full" toggle-class="gl-w-full"
> >
<template #button-content> <template #button-content>
<slot name="button-content"> <gl-truncate
<gl-truncate :text="firstSelectedOption"
:text="firstSelectedOption" class="gl-min-w-0 gl-mr-2"
class="gl-min-w-0 gl-mr-2" :data-qa-selector="qaSelector"
:data-qa-selector="qaSelector" />
/> <span v-if="extraOptionCount" class="gl-mr-2">
<span v-if="extraOptionCount" class="gl-mr-2"> {{ n__('+%d more', '+%d more', extraOptionCount) }}
{{ n__('+%d more', '+%d more', extraOptionCount) }} </span>
</span> <gl-icon name="chevron-down" class="gl-flex-shrink-0 gl-ml-auto" />
<gl-icon name="chevron-down" class="gl-flex-shrink-0 gl-ml-auto" />
</slot>
</template> </template>
<gl-search-box-by-type <gl-search-box-by-type
......
<script>
import { GlDropdownDivider, GlDropdownItem, GlTruncate, GlLoadingIcon, GlIcon } from '@gitlab/ui';
import { assignWith, groupBy, union, uniq, without } from 'lodash';
import FilterBody from './filter_body.vue';
import FilterItem from './filter_item.vue';
import StandardFilter from './standard_filter.vue';
import createFlash from '~/flash';
import { s__ } from '~/locale';
import { scannerFilterResultsKeyMap, dashboardTypeQuery } from '../../constants';
export default {
components: {
GlDropdownDivider,
GlDropdownItem,
GlTruncate,
GlLoadingIcon,
GlIcon,
FilterBody,
FilterItem,
},
extends: StandardFilter,
props: {
fullPath: {
type: String,
required: false,
default: '',
},
},
inject: ['dashboardType'],
apollo: {
customScanners: {
query() {
return dashboardTypeQuery[this.dashboardType];
},
variables() {
return { fullPath: this.fullPath };
},
update(data) {
let nodes = data[this.scannerFilterResultsKey]?.vulnerabilityScanners.nodes;
nodes = nodes?.map(node => ({ ...node, id: `${node.externalId}.${node.reportType}` }));
return groupBy(nodes, 'vendor');
},
error() {
createFlash({
message: s__(
'Could not retrieve custom scanners for scanner filter. Please try again later.',
),
});
},
},
},
data() {
return {
customScanners: {},
};
},
computed: {
options() {
const customerScannerOptions = Object.values(this.customScanners).flatMap(x => x);
return this.filter.options.concat(customerScannerOptions);
},
filterObject() {
const reportType = uniq(this.selectedOptions.map(x => x.reportType));
const scanner = uniq(this.selectedOptions.map(x => x.externalId));
return { reportType, scanner };
},
groups() {
const defaultGroup = { GitLab: this.filter.options };
// If the group already exists in defaultGroup, combine it with the one from customScanners.
return assignWith(defaultGroup, this.customScanners, (original = [], updated) =>
original.concat(updated),
);
},
scannerFilterResultsKey() {
return scannerFilterResultsKeyMap[this.dashboardType];
},
},
watch: {
customScanners() {
// Update the selected options from the querystring when the custom scanners finish loading.
this.selectedOptions = this.routeQueryOptions;
},
},
methods: {
toggleGroup(groupName) {
const options = this.groups[groupName];
// If every option is selected, de-select all of them. Otherwise, select all of them.
if (options.every(option => this.selectedSet.has(option))) {
this.selectedOptions = without(this.selectedOptions, ...options);
} else {
this.selectedOptions = union(this.selectedOptions, options);
}
this.updateRouteQuery();
},
},
};
</script>
<template>
<filter-body
v-model.trim="searchTerm"
:name="filter.name"
:selected-options="selectedOptionsOrAll"
:show-search-box="showSearchBox"
>
<template v-if="$apollo.queries.customScanners.loading" #button-content>
<gl-loading-icon />
<gl-icon name="chevron-down" class="gl-flex-shrink-0 gl-ml-auto" />
</template>
<filter-item
:text="filter.allOption.name"
:is-checked="!selectedOptions.length"
data-testid="allOption"
@click="deselectAllOptions"
/>
<template v-for="[groupName, groupOptions] in Object.entries(groups)">
<gl-dropdown-divider :key="`${groupName}:divider`" />
<gl-dropdown-item
:key="`${groupName}:header`"
:data-testid="`${groupName}Header`"
@click.native.capture.stop="toggleGroup(groupName)"
>
<gl-truncate class="gl-font-weight-bold" :text="groupName" />
</gl-dropdown-item>
<filter-item
v-for="option in groupOptions"
:key="option.id"
:text="option.name"
data-testid="option"
:is-checked="isSelected(option)"
@click="toggleOption(option)"
/>
</template>
<gl-loading-icon v-if="$apollo.queries.customScanners.loading" class="gl-py-3" />
</filter-body>
</template>
...@@ -23,9 +23,6 @@ export default { ...@@ -23,9 +23,6 @@ export default {
}; };
}, },
computed: { computed: {
options() {
return this.filter.options;
},
selectedSet() { selectedSet() {
return new Set(this.selectedOptions); return new Set(this.selectedOptions);
}, },
...@@ -44,7 +41,7 @@ export default { ...@@ -44,7 +41,7 @@ export default {
return { [this.filter.id]: this.selectedOptions.map(x => x.id) }; return { [this.filter.id]: this.selectedOptions.map(x => x.id) };
}, },
filteredOptions() { filteredOptions() {
return this.options.filter(option => return this.filter.options.filter(option =>
option.name.toLowerCase().includes(this.searchTerm.toLowerCase()), option.name.toLowerCase().includes(this.searchTerm.toLowerCase()),
); );
}, },
...@@ -53,7 +50,7 @@ export default { ...@@ -53,7 +50,7 @@ export default {
return Array.isArray(ids) ? ids : [ids]; return Array.isArray(ids) ? ids : [ids];
}, },
routeQueryOptions() { routeQueryOptions() {
const options = this.options.filter(x => this.routeQueryIds.includes(x.id)); const options = this.filter.options.filter(x => this.routeQueryIds.includes(x.id));
const hasAllId = this.routeQueryIds.includes(this.filter.allOption.id); const hasAllId = this.routeQueryIds.includes(this.filter.allOption.id);
if (options.length && !hasAllId) { if (options.length && !hasAllId) {
......
...@@ -77,11 +77,7 @@ export default { ...@@ -77,11 +77,7 @@ export default {
</header> </header>
</template> </template>
<template #sticky> <template #sticky>
<filters <filters :projects="projects" @filterChange="handleFilterChange" />
:projects="projects"
:full-path="groupFullPath"
@filterChange="handleFilterChange"
/>
</template> </template>
<group-security-vulnerabilities :group-full-path="groupFullPath" :filters="filters" /> <group-security-vulnerabilities :group-full-path="groupFullPath" :filters="filters" />
</security-dashboard-layout> </security-dashboard-layout>
......
...@@ -10,6 +10,8 @@ import VulnerabilitiesCountList from './vulnerability_count_list.vue'; ...@@ -10,6 +10,8 @@ import VulnerabilitiesCountList from './vulnerability_count_list.vue';
import Filters from './first_class_vulnerability_filters.vue'; import Filters from './first_class_vulnerability_filters.vue';
import CsvExportButton from './csv_export_button.vue'; import CsvExportButton from './csv_export_button.vue';
export const BANNER_COOKIE_KEY = 'hide_vulnerabilities_introduction_banner';
export default { export default {
components: { components: {
AutoFixUserCallout, AutoFixUserCallout,
...@@ -39,21 +41,21 @@ export default { ...@@ -39,21 +41,21 @@ export default {
}, },
}, },
data() { data() {
const shouldShowAutoFixUserCallout = const shoudShowAutoFixUserCallout =
this.glFeatures.securityAutoFix && !Cookies.get('auto_fix_user_callout_dismissed'); this.glFeatures.securityAutoFix && !Cookies.get('auto_fix_user_callout_dismissed');
return { return {
filters: {}, filters: {},
shouldShowAutoFixUserCallout, shoudShowAutoFixUserCallout,
}; };
}, },
inject: ['dashboardDocumentation', 'autoFixDocumentation', 'projectFullPath'], inject: ['dashboardDocumentation', 'autoFixDocumentation'],
methods: { methods: {
handleFilterChange(filters) { handleFilterChange(filters) {
this.filters = filters; this.filters = filters;
}, },
handleAutoFixUserCalloutClose() { handleAutoFixUserCalloutClose() {
Cookies.set('auto_fix_user_callout_dismissed', 'true'); Cookies.set('auto_fix_user_callout_dismissed', 'true');
this.shouldShowAutoFixUserCallout = false; this.shoudShowAutoFixUserCallout = false;
}, },
}, },
}; };
...@@ -63,7 +65,7 @@ export default { ...@@ -63,7 +65,7 @@ export default {
<div> <div>
<template v-if="pipeline.id"> <template v-if="pipeline.id">
<auto-fix-user-callout <auto-fix-user-callout
v-if="shouldShowAutoFixUserCallout" v-if="shoudShowAutoFixUserCallout"
:help-page-path="autoFixDocumentation" :help-page-path="autoFixDocumentation"
@close="handleAutoFixUserCalloutClose" @close="handleAutoFixUserCalloutClose"
/> />
...@@ -77,7 +79,7 @@ export default { ...@@ -77,7 +79,7 @@ export default {
<vulnerabilities-count-list :filters="filters" /> <vulnerabilities-count-list :filters="filters" />
</template> </template>
<template #sticky> <template #sticky>
<filters :full-path="projectFullPath" @filterChange="handleFilterChange" /> <filters @filterChange="handleFilterChange" />
</template> </template>
<project-vulnerabilities-app <project-vulnerabilities-app
:dashboard-documentation="dashboardDocumentation" :dashboard-documentation="dashboardDocumentation"
......
...@@ -2,14 +2,15 @@ ...@@ -2,14 +2,15 @@
import { debounce } from 'lodash'; import { debounce } from 'lodash';
import { stateFilter, severityFilter, scannerFilter, getProjectFilter } from '../helpers'; import { stateFilter, severityFilter, scannerFilter, getProjectFilter } from '../helpers';
import StandardFilter from './filters/standard_filter.vue'; import StandardFilter from './filters/standard_filter.vue';
import ScannerFilter from './filters/scanner_filter.vue';
const searchBoxOptionCount = 20; // Number of options before the search box is shown. const searchBoxOptionCount = 20; // Number of options before the search box is shown.
export default { export default {
components: {
StandardFilter,
},
props: { props: {
projects: { type: Array, required: false, default: undefined }, projects: { type: Array, required: false, default: undefined },
fullPath: { type: String, required: false, default: '' },
}, },
data: () => ({ data: () => ({
filterQuery: {}, filterQuery: {},
...@@ -31,9 +32,6 @@ export default { ...@@ -31,9 +32,6 @@ export default {
emitFilterChange: debounce(function emit() { emitFilterChange: debounce(function emit() {
this.$emit('filterChange', this.filterQuery); this.$emit('filterChange', this.filterQuery);
}), }),
getFilterComponent(filter) {
return filter.id === 'reportType' ? ScannerFilter : StandardFilter;
},
}, },
searchBoxOptionCount, searchBoxOptionCount,
}; };
...@@ -42,14 +40,12 @@ export default { ...@@ -42,14 +40,12 @@ export default {
<template> <template>
<div class="dashboard-filters border-bottom bg-gray-light"> <div class="dashboard-filters border-bottom bg-gray-light">
<div class="row mx-0 p-2"> <div class="row mx-0 p-2">
<component <standard-filter
:is="getFilterComponent(filter)"
v-for="filter in filters" v-for="filter in filters"
:key="filter.id" :key="filter.id"
class="col-sm-6 col-md-4 col-lg-2 p-2" class="col-sm-6 col-md-4 col-lg-2 p-2"
:filter="filter" :filter="filter"
:full-path="fullPath" :data-testid="filter.id"
:data-testid="`${filter.id}Filter`"
:show-search-box="filter.options.length >= $options.searchBoxOptionCount" :show-search-box="filter.options.length >= $options.searchBoxOptionCount"
@filter-changed="updateFilterQuery" @filter-changed="updateFilterQuery"
/> />
......
import projectSpecificScanners from './graphql/project_specific_scanners.query.graphql';
import groupSpecificScanners from './graphql/group_specific_scanners.query.graphql';
import instanceSpecificScanners from './graphql/instance_specific_scanners.query.graphql';
export const COLLAPSE_SECURITY_REPORTS_SUMMARY_LOCAL_STORAGE_KEY = export const COLLAPSE_SECURITY_REPORTS_SUMMARY_LOCAL_STORAGE_KEY =
'hide_pipelines_security_reports_summary_details'; 'hide_pipelines_security_reports_summary_details';
export const scannerFilterResultsKeyMap = {
instance: 'instanceSecurityDashboard',
project: 'project',
group: 'group',
};
export const dashboardTypeQuery = {
project: projectSpecificScanners,
group: groupSpecificScanners,
instance: instanceSpecificScanners,
};
export default () => ({}); export default () => ({});
...@@ -80,7 +80,6 @@ export default (el, dashboardType) => { ...@@ -80,7 +80,6 @@ export default (el, dashboardType) => {
notEnabledScannersHelpPath: el.dataset.notEnabledScannersHelpPath, notEnabledScannersHelpPath: el.dataset.notEnabledScannersHelpPath,
noPipelineRunScannersHelpPath: el.dataset.noPipelineRunScannersHelpPath, noPipelineRunScannersHelpPath: el.dataset.noPipelineRunScannersHelpPath,
hasVulnerabilities: parseBoolean(el.dataset.hasVulnerabilities), hasVulnerabilities: parseBoolean(el.dataset.hasVulnerabilities),
dashboardType,
...provide, ...provide,
}), }),
render(createElement) { render(createElement) {
......
#import "./vulnerability_scanner.fragment.graphql" #import "./vulnerablity_scanner.fragment.graphql"
query groupSpecificScanners($fullPath: ID!) { query groupSpecificScanners($fullPath: ID!) {
group(fullPath: $fullPath) { group(fullPath: $fullPath) {
......
#import "./vulnerability_scanner.fragment.graphql" #import "./vulnerablity_scanner.fragment.graphql"
query instanceSpecificScanners { query instanceSpecificScanners {
instanceSecurityDashboard { instanceSecurityDashboard {
......
#import "./vulnerability_scanner.fragment.graphql" #import "./vulnerablity_scanner.fragment.graphql"
query projectSpecificScanners($fullPath: ID!) { query projectSpecificScanners($fullpath: id!) {
project(fullPath: $fullPath) { project(fullPath: $fullPath) {
vulnerabilityScanners { vulnerabilityScanners {
nodes { nodes {
......
...@@ -30,17 +30,10 @@ export const severityFilter = { ...@@ -30,17 +30,10 @@ export const severityFilter = {
defaultOptions: [], defaultOptions: [],
}; };
const scannerFilterOptions = parseOptions(REPORT_TYPES).map(option => ({
...option,
id: `GitLab.${option.id}`,
reportType: option.id,
externalId: 'GitLab',
}));
export const scannerFilter = { export const scannerFilter = {
name: s__('Reports|Scanner'), name: s__('Reports|Scanner'),
id: 'reportType', id: 'reportType',
options: scannerFilterOptions, options: parseOptions(REPORT_TYPES),
allOption: BASE_FILTERS.report_type, allOption: BASE_FILTERS.report_type,
defaultOptions: [], defaultOptions: [],
}; };
......
---
title: Add custom security scanner to vulnerability list filter
merge_request: 46540
author:
type: added
import ScannerFilter from 'ee/security_dashboard/components/filters/scanner_filter.vue';
import { createLocalVue, shallowMount } from '@vue/test-utils';
import VueRouter from 'vue-router';
import { uniq, sampleSize, difference } from 'lodash';
import { GlLoadingIcon } from '@gitlab/ui';
const localVue = createLocalVue();
localVue.use(VueRouter);
const router = new VueRouter();
const createOptions = (groupName, length) =>
Array.from({ length }).map((_, i) => ({
id: `${groupName}${i}`,
name: `${groupName}-${i}`,
reportType: `${groupName}Report`,
externalId: groupName,
}));
const gitLabOptions = createOptions('GitLab', 8);
const filter = {
id: 'scanner',
name: 'scanner',
options: gitLabOptions.slice(0, 5),
allOption: { id: 'allOptionId' },
defaultOptions: [],
};
const customScanners = {
GitLab: gitLabOptions.slice(5),
Custom: createOptions('Custom', 3),
};
const customOptions = Object.values(customScanners).flatMap(x => x);
describe('Scanner Filter component', () => {
let wrapper;
const findItemWithName = name => wrapper.find(`[text="${name}"]`);
const findHeaderWithName = name => wrapper.find(`[data-testid="${name}Header"]`);
const expectSelectedItems = items => {
const dropdownItems = wrapper.findAll('[data-testid="option"]');
const checkedItems = dropdownItems.wrappers
.filter(x => x.props('isChecked'))
.map(x => x.props('text'));
const expectedItems = items.map(x => x.name);
expect(checkedItems.sort()).toEqual(expectedItems.sort());
};
const createWrapper = options => {
wrapper = shallowMount(ScannerFilter, {
localVue,
router,
propsData: { filter },
provide: { dashboardType: '' },
data: () => ({ customScanners }),
mocks: {
$apollo: {
queries: {
customScanners: {},
},
},
},
...options,
});
};
afterEach(() => {
wrapper.destroy();
});
it('has the default and custom option items', () => {
createWrapper();
filter.options.concat(customOptions).forEach(option => {
expect(findItemWithName(option.name).exists()).toBe(true);
});
});
it('toggles selection of all items in a group when the group header is clicked', async () => {
const selectedOptions = sampleSize(filter.options.concat(customOptions), 7);
createWrapper();
wrapper.setData({ selectedOptions });
await wrapper.vm.$nextTick();
expectSelectedItems(selectedOptions);
const clickAndCheck = async expectedOptions => {
findHeaderWithName('GitLab').trigger('click');
await wrapper.vm.$nextTick();
expectSelectedItems(expectedOptions);
};
await clickAndCheck(uniq(gitLabOptions.concat(selectedOptions))); // First click selects all.
await clickAndCheck(difference(selectedOptions, gitLabOptions)); // Second check unselects all.
await clickAndCheck(uniq(gitLabOptions.concat(selectedOptions))); // Third click selects all again.
});
it('updates selected options when customScanner is changed', async () => {
const selectedOptions = sampleSize(customOptions, 4);
router.replace({ query: { [filter.id]: selectedOptions.map(x => x.id) } });
createWrapper();
wrapper.setData({ selectedOptions });
await wrapper.vm.$nextTick();
expectSelectedItems(selectedOptions);
});
it('shows loading icon when Apollo query is loading', () => {
const mocks = { $apollo: { queries: { customScanners: { loading: true } } } };
createWrapper({ mocks });
expect(wrapper.find(GlLoadingIcon).exists()).toBe(true);
});
it('emits filter-changed event with expected data when selected options is changed', async () => {
const selectedOptions = sampleSize(customOptions, 4);
createWrapper();
wrapper.setData({ selectedOptions });
await wrapper.vm.$nextTick();
expect(wrapper.emitted('filter-changed')[0][0]).toEqual({
reportType: expect.arrayContaining(['GitLabReport', 'CustomReport']),
scanner: expect.arrayContaining(['GitLab', 'Custom']),
});
});
});
import VueRouter from 'vue-router'; import VueRouter from 'vue-router';
import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils';
import Filters from 'ee/security_dashboard/components/first_class_vulnerability_filters.vue'; import Filters from 'ee/security_dashboard/components/first_class_vulnerability_filters.vue';
import StandardFilter from 'ee/security_dashboard/components/filters/standard_filter.vue';
const router = new VueRouter(); const router = new VueRouter();
const localVue = createLocalVue(); const localVue = createLocalVue();
...@@ -14,9 +15,9 @@ describe('First class vulnerability filters component', () => { ...@@ -14,9 +15,9 @@ describe('First class vulnerability filters component', () => {
{ id: 'gid://gitlab/Project/12', name: 'GitLab Com' }, { id: 'gid://gitlab/Project/12', name: 'GitLab Com' },
]; ];
const findFilters = () => wrapper.findAll('[data-testid$="Filter"]'); const findFilters = () => wrapper.findAll(StandardFilter);
const findStateFilter = () => wrapper.find('[data-testid="stateFilter"]'); const findStateFilter = () => wrapper.find('[data-testid="state"]');
const findProjectFilter = () => wrapper.find('[data-testid="projectIdFilter"]'); const findProjectFilter = () => wrapper.find('[data-testid="projectId"]');
const createComponent = ({ propsData, listeners } = {}) => { const createComponent = ({ propsData, listeners } = {}) => {
return shallowMount(Filters, { localVue, router, propsData, listeners }); return shallowMount(Filters, { localVue, router, propsData, listeners });
......
...@@ -7710,9 +7710,6 @@ msgstr "" ...@@ -7710,9 +7710,6 @@ msgstr ""
msgid "Could not restore the group" msgid "Could not restore the group"
msgstr "" msgstr ""
msgid "Could not retrieve custom scanners for scanner filter. Please try again later."
msgstr ""
msgid "Could not revoke impersonation token %{token_name}." msgid "Could not revoke impersonation token %{token_name}."
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