diff --git a/config/feature_flags/beta/vulnerability_report_advanced_filtering.yml b/config/feature_flags/beta/vulnerability_report_advanced_filtering.yml deleted file mode 100644 index 53b37ef8d3905dd86c694b1c29cc1b885972c4ef..0000000000000000000000000000000000000000 --- a/config/feature_flags/beta/vulnerability_report_advanced_filtering.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: vulnerability_report_advanced_filtering -feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/3429 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/140984 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437128 -milestone: '16.9' -group: group::threat insights -type: beta -default_enabled: true diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/filtered_search/tokens/tool_token.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/filtered_search/tokens/tool_token.vue index 1bccd81ce6f3148a802b7e6c4196d53e61fc6b4c..ee4303115670d1d7e227df6e2d1ef7e4a251a422 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/filtered_search/tokens/tool_token.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/filtered_search/tokens/tool_token.vue @@ -216,7 +216,9 @@ export default { @complete="emitFiltersChanged" > <template #view> - {{ toggleText }} + <span data-testid="tool-token-value"> + {{ toggleText }} + </span> </template> <template #suggestions> <template v-if="isSimpleTool"> diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_report.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_report.vue index 6f60e867d992daeb3c65330db13ca12898ae9123..eb95a4852f15f722b65de695abbb2ee0d563c13a 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_report.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_report.vue @@ -177,7 +177,11 @@ export default { return this.filterDropdowns.includes(FILTERS.PROJECT); }, shouldShowAdvancedFiltering() { - return this.glFeatures?.vulnerabilityReportAdvancedFiltering; + return ( + this.isProjectVulnerabilityReport || + this.isGroupVulnerabilityReport || + this.isInstanceVulnerabilityReport + ); }, shouldShowGroupByButton() { if (!this.isVisible) { @@ -486,10 +490,7 @@ export default { class="gl-pb-2 gl-mt-4 gl-mb-4" @filters-changed="updateGraphqlFilters" /> - <div - v-if="shouldShowGroupByButton" - class="gl-p-5 gl-bg-gray-10 gl-border-b-1 gl-border-b-solid gl-border-b-gray-100" - > + <div v-else class="gl-p-5 gl-bg-gray-10 gl-border-b-1 gl-border-b-solid gl-border-b-gray-100"> <div ref="groupByContainer" class="gl-inline-flex gl-align-items-center"> <label for="group-by" class="gl-m-0">{{ __('Group by:') }}</label> <gl-collapsible-listbox @@ -501,7 +502,6 @@ export default { /> </div> <filtered-search - v-if="shouldShowAdvancedFiltering" :available-filters="filterDropdowns" class="gl-pb-2 gl-mt-4" @filters-changed="updateGraphqlFilters" diff --git a/ee/app/controllers/groups/security/vulnerabilities_controller.rb b/ee/app/controllers/groups/security/vulnerabilities_controller.rb index 2b0c841b6a65b7a3d90478060cf3b76e9ba7e5df..083c773d103fe0bfbd80cc86c61ba471141ee2c4 100644 --- a/ee/app/controllers/groups/security/vulnerabilities_controller.rb +++ b/ee/app/controllers/groups/security/vulnerabilities_controller.rb @@ -14,7 +14,6 @@ class VulnerabilitiesController < Groups::ApplicationController before_action do push_frontend_feature_flag(:group_level_vulnerability_report_grouping, @group) push_frontend_feature_flag(:vulnerability_owasp_top_10_group, @group) - push_frontend_feature_flag(:vulnerability_report_advanced_filtering, @group, type: :beta) end def index diff --git a/ee/app/controllers/projects/security/vulnerability_report_controller.rb b/ee/app/controllers/projects/security/vulnerability_report_controller.rb index d9b79947795ff7a7425b7d8a7fc820425c25de4f..9bff2588c439baf730a7fea67fef11f4daaf6101 100644 --- a/ee/app/controllers/projects/security/vulnerability_report_controller.rb +++ b/ee/app/controllers/projects/security/vulnerability_report_controller.rb @@ -7,7 +7,6 @@ class VulnerabilityReportController < Projects::ApplicationController before_action do authorize_read_vulnerability! - push_frontend_feature_flag(:vulnerability_report_advanced_filtering, @project, type: :beta) push_frontend_feature_flag(:vulnerability_report_owasp_2021, @project, type: :beta) push_frontend_feature_flag(:container_scanning_for_registry_flag, @project) end diff --git a/ee/app/controllers/security/vulnerabilities_controller.rb b/ee/app/controllers/security/vulnerabilities_controller.rb index 12847c919be869f9856f8db0d70129e01078f582..2f2b94dfab621051061e5e5dc75a29c656d2f251 100644 --- a/ee/app/controllers/security/vulnerabilities_controller.rb +++ b/ee/app/controllers/security/vulnerabilities_controller.rb @@ -9,7 +9,6 @@ class VulnerabilitiesController < ::Security::ApplicationController before_action do push_frontend_feature_flag(:group_level_vulnerability_report_grouping, @user, type: :development) - push_frontend_feature_flag(:vulnerability_report_advanced_filtering, @user, type: :beta) end private diff --git a/ee/spec/features/projects/security/vulnerability_report_spec.rb b/ee/spec/features/projects/security/vulnerability_report_spec.rb index 810710fd04e423a7aa39f67c8c4c973dc3bf895b..b4c1d10d62389caadc3f63f5f01beec627416832 100644 --- a/ee/spec/features/projects/security/vulnerability_report_spec.rb +++ b/ee/spec/features/projects/security/vulnerability_report_spec.rb @@ -57,7 +57,6 @@ end before do - stub_feature_flags(vulnerability_report_advanced_filtering: false) stub_licensed_features( security_dashboard: true, sast: true @@ -66,32 +65,58 @@ sign_in(user) end - it "shows the Vulnerability report" do - visit(project_security_vulnerability_report_index_path(project)) + def click_search_button + button = find_by_testid('filtered-search-term') + button.click + button.click # Second click clears the dropdown + end + + def clear_and_focus_filtered_search + find_by_testid('clear-icon').click + find_by_testid('filtered-search-term').click + end + it "shows the Vulnerability report", :aggregate_failures do + visit(project_security_vulnerability_report_index_path(project)) expect(page).to have_content "Vulnerability report" expect(page).to have_content sast_vulnerability.title expect(page).to have_content dast_vulnerability.title + end - # This click opens the filter - click_on 'All tools' + it "shows the Vulnerability report with a filter", :aggregate_failures do + visit(project_security_vulnerability_report_index_path(project)) + expect(page).to have_content "Vulnerability report" + + clear_and_focus_filtered_search - find_by_testid('listbox-item-SAST').click + click_link('Tool') + click_link('SAST Scanner') + click_search_button expect(page).to have_content sast_vulnerability.title expect(page).to have_no_content dast_vulnerability.title - # The filter stays open, so it doesn't need to be opened again - find_by_testid('listbox-item-DAST').click - # Clicking an item in the filter toggles it, so this deselects 'SAST' - find_by_testid('listbox-item-SAST').click + clear_and_focus_filtered_search + + click_link('Tool') + click_link('DAST Scanner') + click_search_button expect(page).to have_no_content sast_vulnerability.title expect(page).to have_content dast_vulnerability.title end context "when a scanner has produced more than one type of finding" do + let_it_be(:secret_detection_scanner) do + create( + :vulnerabilities_scanner, + name: 'Secret Detection', + vendor: 'GitLab', + project: project + ) + end + let_it_be(:secret_detection_vulnerability) do create(:vulnerability, :secret_detection, title: "Secret Detection vulnerability", project: project) end @@ -106,7 +131,7 @@ :identifier, vulnerability: secret_detection_vulnerability, report_type: :secret_detection, - scanner: sast_vulnerabilities_scanner, + scanner: secret_detection_scanner, project: project ) end @@ -119,15 +144,23 @@ expect(page).to have_content dast_vulnerability.title expect(page).to have_content secret_detection_vulnerability.title - click_on 'All tools' + clear_and_focus_filtered_search + + click_link('Tool') + click_link('SAST Scanner') + click_search_button - find_by_testid('listbox-item-SAST').click expect(page).to have_content sast_vulnerability.title expect(page).to have_no_content dast_vulnerability.title expect(page).to have_no_content secret_detection_vulnerability.title - find_by_testid('listbox-item-SECRET_DETECTION').click - find_by_testid('listbox-item-SAST').click # Deselects SAST from the filter + # Now that we have a value selected, the dropdown won't show the tool link anymore + # Therefore click on the selected value to re-open the dropdown + find_by_testid('tool-token-value').click + click_link('Secret Detection') + click_link('SAST Scanner') # Deselects SAST from the filter + click_search_button + expect(page).to have_no_content sast_vulnerability.title expect(page).to have_no_content dast_vulnerability.title expect(page).to have_content secret_detection_vulnerability.title @@ -151,7 +184,10 @@ expect(page).to have_no_content(vulnerability.title) end - filter_by(status: :dismissed) + clear_and_focus_filtered_search + + click_link('Status') + click_link('Needs triage') vulnerabilities.each do |vulnerability| expect(page).to have_content(vulnerability.title) @@ -159,13 +195,6 @@ end end - def filter_by(status:) - click_on "Needs triage +1 more" - find_by_testid("listbox-item-#{status.to_s.upcase}").click - find_by_testid('vulnerability-report-header').click - wait_for_all_requests - end - def change_all(state:, reason:, comment:) find_by_testid('vulnerability-checkbox-all').check diff --git a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_report_spec.js b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_report_spec.js index 8277831876dc387679cd9e6b6f41607b8b8bf003..eddd5eed9748f3de3fa59c4623e8dd08cffa3957 100644 --- a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_report_spec.js +++ b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_report_spec.js @@ -90,7 +90,8 @@ describe('Vulnerability report component', () => { const findCounts = () => wrapper.findComponent(VulnerabilityCounts); const findPortalTarget = () => wrapper.findComponent(PortalTarget); const findFiltersContainer = () => wrapper.findByTestId('filters-container'); - const findFilters = () => wrapper.findComponent(VulnerabilityFilters); + const findFilters = () => wrapper.findComponent(FilteredSearch); + const findOldFilters = () => wrapper.findComponent(VulnerabilityFilters); const findList = () => wrapper.findComponent(VulnerabilityListGraphql); const findListTopDiv = () => wrapper.findByTestId('vulnerability-list-top'); const findGroupByButton = () => wrapper.findComponent(GlCollapsibleListbox); @@ -125,20 +126,20 @@ describe('Vulnerability report component', () => { }); }); - describe('filters', () => { + describe('old filters', () => { it('gets the expected filters prop', () => { const filterDropdowns = []; - createWrapper({ filterDropdowns }); + createWrapper({ filterDropdowns, dashboardType: DASHBOARD_TYPES.PIPELINE }); - expect(findFilters().props('filters')).toBe(filterDropdowns); + expect(findOldFilters().props('filters')).toBe(filterDropdowns); }); it('calls the filter transform function when the filters are changed', async () => { const fnData = { b: 3, d: 4 }; const filterFn = jest.fn().mockImplementation(() => fnData); - createWrapper({ filterFn }); + createWrapper({ filterFn, dashboardType: DASHBOARD_TYPES.PIPELINE }); const data = { a: 1, b: 2 }; - findFilters().vm.$emit('filters-changed', data); + findOldFilters().vm.$emit('filters-changed', data); await nextTick(); // We test that the filter function is called with the data emitted by VulnerabilityFilters. @@ -773,27 +774,15 @@ describe('Vulnerability report component', () => { describe('enhanced filters', () => { const findFilteredSearch = () => wrapper.findComponent(FilteredSearch); - it.each` - featureFlagValue | shouldShow - ${true} | ${true} - ${false} | ${false} - `( - 'should display the enhanced filters: $shouldShow', - async ({ featureFlagValue, shouldShow }) => { - await createWrapper({ - glFeatures: { - vulnerabilityReportAdvancedFiltering: featureFlagValue, - }, - }); + it('should display the enhanced filters: $shouldShow', async () => { + await createWrapper({}); - expect(findFilteredSearch().exists()).toBe(shouldShow); - }, - ); + expect(findFilteredSearch().exists()).toBe(true); + }); it('should display the enhanced filters when groupLevelVulnerabilityReportGrouping is enabled', () => { createWrapper({ glFeatures: { - vulnerabilityReportAdvancedFiltering: true, groupLevelVulnerabilityReportGrouping: true, }, }); @@ -804,7 +793,6 @@ describe('Vulnerability report component', () => { it('should still display the enhanced filters when groupLevelVulnerabilityReportGrouping is not enabled', async () => { await createWrapper({ glFeatures: { - vulnerabilityReportAdvancedFiltering: true, groupLevelVulnerabilityReportGrouping: false, }, }); @@ -814,12 +802,7 @@ describe('Vulnerability report component', () => { it('passes the available filters', () => { const filterDropdowns = []; - createWrapper({ - filterDropdowns, - glFeatures: { - vulnerabilityReportAdvancedFiltering: true, - }, - }); + createWrapper({ filterDropdowns }); expect(findFilteredSearch().props('availableFilters')).toBe(filterDropdowns); }); @@ -828,12 +811,7 @@ describe('Vulnerability report component', () => { const fnData = { b: 3, d: 4 }; const filterFn = jest.fn().mockImplementation(() => fnData); - createWrapper({ - filterFn, - glFeatures: { - vulnerabilityReportAdvancedFiltering: true, - }, - }); + createWrapper({ filterFn }); const data = { a: 1, b: 2 }; findFilteredSearch().vm.$emit('filters-changed', data); diff --git a/qa/qa/ee/page/component/secure_report.rb b/qa/qa/ee/page/component/secure_report.rb index 795fafc75f634f004e13315f9f946e0c8ebec21d..6cc8b1e1030a21905a93bdcc7831f191aaaf5fa6 100644 --- a/qa/qa/ee/page/component/secure_report.rb +++ b/qa/qa/ee/page/component/secure_report.rb @@ -74,32 +74,12 @@ def filter_by_status(statuses) end end - if has_element?('filtered-search-token', wait: 10) - filter_by_status_new(statuses) - else - filter_by_status_old(statuses) - end + filter_by_status_new(statuses) state = statuses_list(statuses).map { |item| "state=#{item}" }.join("&") raise 'Status unchanged in the URL' unless page.current_url.downcase.include?(state) end - def filter_by_status_old(statuses) - wait_until(max_duration: 30, message: "Waiting for status dropdown element to appear") do - has_element?('filter-status-dropdown') - end - # Retry on exception to avoid ElementNotFound errors when clicks are sent too fast for the UI to update - retry_on_exception(sleep_interval: 2, message: "Retrying status click until current url matches state") do - find(status_dropdown_button_selector, wait: 5).click - find(status_item_selector('ALL')).click - statuses_list(statuses).each do |status| - find(status_item_selector(status)).click - wait_for_requests # It takes a moment to update the page after changing selections - end - find(status_dropdown_button_selector, wait: 5).click - end - end - def filter_by_status_new(statuses) click_element('clear-icon') click_element('filtered-search-token-segment') diff --git a/qa/qa/specs/features/ee/browser_ui/10_govern/security_reports_spec.rb b/qa/qa/specs/features/ee/browser_ui/10_govern/security_reports_spec.rb index c98e1d0eb9190ce1ebdd801af43517d4ce78b65e..96dab4abcfa34e7376541b5ce1388c7151ca599e 100644 --- a/qa/qa/specs/features/ee/browser_ui/10_govern/security_reports_spec.rb +++ b/qa/qa/specs/features/ee/browser_ui/10_govern/security_reports_spec.rb @@ -268,12 +268,7 @@ def filter_report_and_perform(page:, filter_report:) page.filter_report_type(filter_report) yield - if page.has_element?("filtered-search-term", wait: 1) - # This code to be removed after vulnerability_report_advanced_filtering is enabled by default - page.clear_filter_token('tool') - else - page.filter_report_type(filter_report) - end + page.clear_filter_token('tool') end def push_security_reports