Commit 50200fc4 authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents d55e240b c7cc77ff
<script> <script>
/* eslint-disable @gitlab/require-string-literal-i18n-helpers */ /* eslint-disable @gitlab/require-string-literal-i18n-helpers */
import { GlButton } from '@gitlab/ui'; import { GlButton, GlSprintf } from '@gitlab/ui';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { __, n__, sprintf, s__ } from '~/locale'; import { __, n__, s__ } from '~/locale';
const mergeCommitCount = s__('mrWidgetCommitsAdded|1 merge commit');
export default { export default {
mergeCommitCount,
components: { components: {
GlButton, GlButton,
GlSprintf,
}, },
props: { props: {
isSquashEnabled: { isSquashEnabled: {
...@@ -47,22 +51,15 @@ export default { ...@@ -47,22 +51,15 @@ export default {
ariaLabel() { ariaLabel() {
return this.expanded ? __('Collapse') : __('Expand'); return this.expanded ? __('Collapse') : __('Expand');
}, },
targetBranchEscaped() {
return escape(this.targetBranch);
},
message() { message() {
const message = this.isFastForwardEnabled return this.isFastForwardEnabled
? s__('mrWidgetCommitsAdded|%{commitCount} will be added to %{targetBranch}.') ? s__('mrWidgetCommitsAdded|%{commitCount} will be added to %{targetBranch}.')
: s__( : s__(
'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.', 'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.',
); );
return sprintf(
message,
{
commitCount: `<strong class="commits-count-message">${this.commitsCountMessage}</strong>`,
mergeCommitCount: `<strong>${s__('mrWidgetCommitsAdded|1 merge commit')}</strong>`,
targetBranch: `<span class="label-branch">${escape(this.targetBranch)}</span>`,
},
false,
);
}, },
}, },
methods: { methods: {
...@@ -89,10 +86,19 @@ export default { ...@@ -89,10 +86,19 @@ export default {
/> />
<span v-if="expanded">{{ __('Collapse') }}</span> <span v-if="expanded">{{ __('Collapse') }}</span>
<span v-else> <span v-else>
<span <span class="vertical-align-middle">
class="vertical-align-middle" <gl-sprintf :message="message">
v-html="message /* eslint-disable-line vue/no-v-html */" <template #commitCount>
></span> <strong class="commits-count-message">{{ commitsCountMessage }}</strong>
</template>
<template #mergeCommitCount>
<strong>{{ $options.mergeCommitCount }}</strong>
</template>
<template #targetBranch>
<span class="label-branch">{{ targetBranchEscaped }}</span>
</template>
</gl-sprintf>
</span>
<gl-button variant="link" class="modify-message-button"> <gl-button variant="link" class="modify-message-button">
{{ modifyLinkMessage }} {{ modifyLinkMessage }}
</gl-button> </gl-button>
......
...@@ -82,6 +82,19 @@ AND (EXISTS ( ...@@ -82,6 +82,19 @@ AND (EXISTS (
While this worked without schema changes, and did improve readability somewhat, While this worked without schema changes, and did improve readability somewhat,
it did not improve query performance. it did not improve query performance.
### Attempt A2: use label IDs in the WHERE EXISTS clause
In [merge request #34503](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34503), we followed a similar approach to A1. But this time, we
did a separate query to fetch the IDs of the labels used in the filter so that we avoid the `JOIN` in the `EXISTS` clause and filter directly by
`label_links.label_id`. We also added a new index on `label_links` for the `target_id`, `label_id`, and `target_type` columns to speed up this query.
Finding the label IDs wasn't straightforward because there could be multiple labels with the same title within a single root namespace. We solved
this by grouping the label IDs by title and then using the array of IDs in the `EXISTS` clauses.
This resulted in a significant performance improvement. However, this optimization could not be applied to the dashboard pages
where we do not have a project or group context. We could not easily search for the label IDs here because that would mean searching across all
projects and groups that the user has access to.
## Attempt B: Denormalize using an array column ## Attempt B: Denormalize using an array column
Having [removed MySQL support in GitLab 12.1](https://about.gitlab.com/blog/2019/06/27/removing-mysql-support/), Having [removed MySQL support in GitLab 12.1](https://about.gitlab.com/blog/2019/06/27/removing-mysql-support/),
...@@ -159,9 +172,8 @@ However, at present, the disadvantages outweigh the advantages. ...@@ -159,9 +172,8 @@ However, at present, the disadvantages outweigh the advantages.
## Conclusion ## Conclusion
We have yet to find a method that is demonstrably better than the current We found a method A2 that does not need denormalization and improves the query performance significantly. This
method, when considering: did not apply to all cases, but we were able to apply method A1 to the rest of the cases so that we remove the
`GROUP BY` and `HAVING` clauses in all scenarios.
1. Query performance. This simplified the query and improved the performance in the most common cases.
1. Readability.
1. Ease of maintaining schema consistency.
...@@ -340,6 +340,9 @@ WARNING: ...@@ -340,6 +340,9 @@ WARNING:
HyperLogLog (HLL) is a probabilistic algorithm and its **results always includes some small error**. According to [Redis documentation](https://redis.io/commands/pfcount), data from HyperLogLog (HLL) is a probabilistic algorithm and its **results always includes some small error**. According to [Redis documentation](https://redis.io/commands/pfcount), data from
used HLL implementation is "approximated with a standard error of 0.81%". used HLL implementation is "approximated with a standard error of 0.81%".
NOTE:
A user's consent for usage_stats (`User.single_user&.requires_usage_stats_consent?`) is not checked during the data tracking stage due to performance reasons. Keys corresponding to those counters are present in Redis even if `usage_stats_consent` is still required. However, no metric is collected from Redis and reported back to GitLab as long as `usage_stats_consent` is required.
With `Gitlab::UsageDataCounters::HLLRedisCounter` we have available data structures used to count unique values. With `Gitlab::UsageDataCounters::HLLRedisCounter` we have available data structures used to count unique values.
Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PFCOUNT](https://redis.io/commands/pfcount). Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PFCOUNT](https://redis.io/commands/pfcount).
......
...@@ -101,7 +101,12 @@ export default { ...@@ -101,7 +101,12 @@ export default {
> >
{{ $options.i18n.editPolicyProjectButtonText }} {{ $options.i18n.editPolicyProjectButtonText }}
</gl-button> </gl-button>
<gl-button data-testid="new-policy-button" variant="confirm" :href="newPolicyPath"> <gl-button
data-testid="new-policy-button"
data-qa-selector="new_policy_button"
variant="confirm"
:href="newPolicyPath"
>
{{ $options.i18n.newPolicyButtonText }} {{ $options.i18n.newPolicyButtonText }}
</gl-button> </gl-button>
<scan-new-policy-modal <scan-new-policy-modal
......
...@@ -283,6 +283,7 @@ export default { ...@@ -283,6 +283,7 @@ export default {
<gl-table <gl-table
ref="policiesTable" ref="policiesTable"
data-qa-selector="policies_list"
:busy="isLoadingPolicies" :busy="isLoadingPolicies"
:items="policies" :items="policies"
:fields="fields" :fields="fields"
......
...@@ -88,6 +88,7 @@ export default { ...@@ -88,6 +88,7 @@ export default {
<gl-form-group :label="s__('SecurityOrchestration|Policy type')" label-for="policyType"> <gl-form-group :label="s__('SecurityOrchestration|Policy type')" label-for="policyType">
<gl-form-select <gl-form-select
id="policyType" id="policyType"
data-qa-selector="policy_type_form_select"
:value="policyOptions.value" :value="policyOptions.value"
:options="policyTypes" :options="policyTypes"
:disabled="!shouldAllowPolicyTypeSelection" :disabled="!shouldAllowPolicyTypeSelection"
......
# frozen_string_literal: true
module QA
module EE
module Page
module Project
module Policies
class Index < QA::Page::Base
view 'ee/app/assets/javascripts/threat_monitoring/components/policies/policies_list.vue' do
element :policies_list
end
view 'ee/app/assets/javascripts/threat_monitoring/components/policies/policies_header.vue' do
element :new_policy_button
end
def has_policies_list?
has_element?(:policies_list)
end
def click_new_policy_button
click_element(:new_policy_button)
end
end
end
end
end
end
end
# frozen_string_literal: true
module QA
module EE
module Page
module Project
module Policies
class PolicyEditor < QA::Page::Base
view 'ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_editor.vue' do
element :policy_type_form_select
end
def has_policy_type_form_select?
has_element?(:policy_type_form_select)
end
end
end
end
end
end
end
...@@ -30,6 +30,14 @@ module QA ...@@ -30,6 +30,14 @@ module QA
end end
end end
def click_on_policies
hover_security_compliance do
within_submenu do
click_element(:sidebar_menu_item_link, menu_item: 'Policies')
end
end
end
def click_on_vulnerability_report def click_on_vulnerability_report
hover_security_compliance do hover_security_compliance do
within_submenu do within_submenu do
......
...@@ -8,13 +8,11 @@ module QA ...@@ -8,13 +8,11 @@ module QA
class Index < QA::Page::Base class Index < QA::Page::Base
TAB_INDEX = { TAB_INDEX = {
alerts: 1, alerts: 1,
policies: 2, statistics: 2 # it hasn't been added yet
statistics: 3 # it hasn't been added yet
}.freeze }.freeze
view 'ee/app/assets/javascripts/threat_monitoring/components/app.vue' do view 'ee/app/assets/javascripts/threat_monitoring/components/app.vue' do
element :alerts_tab element :alerts_tab
element :policies_tab
element :threat_monitoring_container element :threat_monitoring_container
end end
...@@ -22,16 +20,6 @@ module QA ...@@ -22,16 +20,6 @@ module QA
has_element?(:alerts_tab) has_element?(:alerts_tab)
end end
def has_policies_tab?
has_element?(:policies_tab)
end
def click_policies_tab
within_element(:threat_monitoring_container) do
find(tab_element_for(:policies)).click
end
end
private private
def tab_element_for(tab_name) def tab_element_for(tab_name)
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module QA module QA
RSpec.describe 'Protect' do RSpec.describe 'Protect' do
describe 'Threat Monitoring Policy List page' do describe 'Policies List page' do
let!(:project) do let!(:project) do
Resource::Project.fabricate_via_api! do |project| Resource::Project.fabricate_via_api! do |project|
project.name = Runtime::Env.auto_devops_project_name || 'project-with-protect' project.name = Runtime::Env.auto_devops_project_name || 'project-with-protect'
...@@ -23,13 +23,24 @@ module QA ...@@ -23,13 +23,24 @@ module QA
project.visit! project.visit!
end end
it 'can load Threat Monitoring page and view the policy alert list', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1892' do it 'can load Policies page and view the policies list', :smoke, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1892' do
Page::Project::Menu.perform(&:click_on_threat_monitoring) Page::Project::Menu.perform(&:click_on_policies)
EE::Page::Project::ThreatMonitoring::AlertsList.perform do |alerts_list| EE::Page::Project::Policies::Index.perform do |policies_page|
aggregate_failures do aggregate_failures do
expect(alerts_list).to have_alerts_tab expect(policies_page).to have_policies_list
expect(alerts_list).to have_alerts_list end
end
end
it 'can navigate to Policy Editor page', :smoke, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1892' do
Page::Project::Menu.perform(&:click_on_policies)
EE::Page::Project::Policies::Index.perform(&:click_new_policy_button)
EE::Page::Project::Policies::PolicyEditor.perform do |policy_editor|
aggregate_failures do
expect(policy_editor).to have_policy_type_form_select
end end
end end
end end
...@@ -64,7 +75,7 @@ module QA ...@@ -64,7 +75,7 @@ module QA
cluster.remove! cluster.remove!
end end
it 'loads a sample network policy under policies tab on the Threat Monitoring page', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1925' do it 'loads a sample network policy under policies page', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1925' do
Resource::KubernetesCluster::ProjectCluster.fabricate_via_browser_ui! do |k8s_cluster| Resource::KubernetesCluster::ProjectCluster.fabricate_via_browser_ui! do |k8s_cluster|
k8s_cluster.project = project k8s_cluster.project = project
k8s_cluster.cluster = cluster k8s_cluster.cluster = cluster
...@@ -87,11 +98,10 @@ module QA ...@@ -87,11 +98,10 @@ module QA
cluster.add_sample_policy(project, policy_name: policy_name) cluster.add_sample_policy(project, policy_name: policy_name)
Page::Project::Menu.perform(&:click_on_threat_monitoring) Page::Project::Menu.perform(&:click_on_policies)
EE::Page::Project::ThreatMonitoring::Index.perform do |index| EE::Page::Project::Policies::Index.perform do |index|
index.click_policies_tab
aggregate_failures do aggregate_failures do
expect(index).to have_policies_tab expect(policies_list).to have_policies_list
expect(index.has_content?(policy_name)).to be true expect(index.has_content?(policy_name)).to be true
end end
end end
......
# frozen_string_literal: true
module QA
RSpec.describe 'Protect' do
describe 'Threat Monitoring Policy Alert List page' do
let!(:project) do
Resource::Project.fabricate_via_api! do |project|
project.name = Runtime::Env.auto_devops_project_name || 'project-with-protect'
project.description = 'Project with Protect'
project.auto_devops_enabled = true
project.initialize_with_readme = true
project.template_name = 'express'
end
end
after do
project.remove_via_api!
end
context 'without k8s cluster' do
before do
Flow::Login.sign_in
project.visit!
end
it 'can load Threat Monitoring page and view the policy alert list', :smoke, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1892' do
Page::Project::Menu.perform(&:click_on_threat_monitoring)
EE::Page::Project::ThreatMonitoring::AlertsList.perform do |alerts_list|
aggregate_failures do
expect(alerts_list).to have_alerts_tab
expect(alerts_list).to have_alerts_list
end
end
end
end
end
end
end
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { GlSprintf } from '@gitlab/ui';
import CommitsHeader from '~/vue_merge_request_widget/components/states/commits_header.vue'; import CommitsHeader from '~/vue_merge_request_widget/components/states/commits_header.vue';
describe('Commits header component', () => { describe('Commits header component', () => {
...@@ -6,6 +7,9 @@ describe('Commits header component', () => { ...@@ -6,6 +7,9 @@ describe('Commits header component', () => {
const createComponent = (props) => { const createComponent = (props) => {
wrapper = shallowMount(CommitsHeader, { wrapper = shallowMount(CommitsHeader, {
stubs: {
GlSprintf,
},
propsData: { propsData: {
isSquashEnabled: false, isSquashEnabled: false,
targetBranch: 'main', targetBranch: 'main',
......
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