kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2021-08-20 06:11:03 +00:00
parent a7c6b8d269
commit 0c51dc19cb
20 changed files with 150 additions and 265 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
app/models/diff_note.rb
View File

@ -22,7 +22,7 @@ class DiffNote < Note
validate :verify_supported, unless: :importing?
before_validation :set_line_code, if: :on_text?, unless: :importing?
after_save :keep_around_commits, unless: :importing?
after_save :keep_around_commits, unless: -> { importing? || skip_keep_around_commits }
NoteDiffFileCreationError = Class.new(StandardError)
@ -115,6 +115,20 @@ class DiffNote < Note
position&.multiline?
end
def shas
[
self.original_position.base_sha,
self.original_position.start_sha,
self.original_position.head_sha
].tap do |a|
if self.position != self.original_position
a << self.position.base_sha
a << self.position.start_sha
a << self.position.head_sha
end
end
end
private
def enqueue_diff_file_creation_job
@ -173,18 +187,6 @@ class DiffNote < Note
end
def keep_around_commits
shas = [
self.original_position.base_sha,
self.original_position.start_sha,
self.original_position.head_sha
]
if self.position != self.original_position
shas << self.position.base_sha
shas << self.position.start_sha
shas << self.position.head_sha
end
repository.keep_around(*shas)
end

3
app/models/note.rb
View File

@ -48,6 +48,9 @@ class Note < ApplicationRecord
# Attribute used to store the attributes that have been changed by quick actions.
attr_accessor :commands_changes
# Attribute used to determine whether keep_around_commits will be skipped for diff notes.
attr_accessor :skip_keep_around_commits
default_value_for :system, false
attr_mentionable :note, pipeline: :note

6
app/models/shard.rb
View File

@ -18,10 +18,6 @@ class Shard < ApplicationRecord
end
def self.by_name(name)
transaction(requires_new: true) do
find_or_create_by(name: name)
end
rescue ActiveRecord::RecordNotUnique
retry
safe_find_or_create_by(name: name)
end
end

20
app/services/draft_notes/publish_service.rb
View File

@ -34,22 +34,24 @@ module DraftNotes
created_notes = draft_notes.map do |draft_note|
draft_note.review = review
create_note_from_draft(draft_note, skip_capture_diff_note_position: true)
create_note_from_draft(draft_note, skip_capture_diff_note_position: true, skip_keep_around_commits: true)
end
capture_diff_note_positions(created_notes)
keep_around_commits(created_notes)
draft_notes.delete_all
set_reviewed
notification_service.async.new_review(review)
MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request)
end
def create_note_from_draft(draft, skip_capture_diff_note_position: false)
def create_note_from_draft(draft, skip_capture_diff_note_position: false, skip_keep_around_commits: false)
# Make sure the diff file is unfolded in order to find the correct line
# codes.
draft.diff_file&.unfold_diff_lines(draft.original_position)
note = Notes::CreateService.new(draft.project, draft.author, draft.publish_params).execute(
note_params = draft.publish_params.merge(skip_keep_around_commits: skip_keep_around_commits)
note = Notes::CreateService.new(draft.project, draft.author, note_params).execute(
skip_capture_diff_note_position: skip_capture_diff_note_position
)
@ -86,5 +88,17 @@ module DraftNotes
capture_service.execute(note.discussion) if note.diff_note? && note.start_of_discussion?
end
end
def keep_around_commits(notes)
shas = notes.flat_map do |note|
note.shas if note.diff_note?
end.uniq
# We are allowing this since gitaly call will be created for each sha and
# even though they're unique, there will still be multiple Gitaly calls.
Gitlab::GitalyClient.allow_n_plus_1_calls do
project.repository.keep_around(*shas)
end
end
end
end

27
app/workers/all_queues.yml
View File

@ -673,24 +673,6 @@
:idempotent:
:tags:
- :exclude_from_kubernetes
- :name: deployment:deployments_finished
:worker_name: Deployments::FinishedWorker
:feature_category: :continuous_delivery
:has_external_dependencies:
:urgency: :low
:resource_boundary: :cpu
:weight: 3
:idempotent:
:tags: []
- :name: deployment:deployments_forward_deployment
:worker_name: Deployments::ForwardDeploymentWorker
:feature_category: :continuous_delivery
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 3
:idempotent:
:tags: []
- :name: deployment:deployments_hooks
:worker_name: Deployments::HooksWorker
:feature_category: :continuous_delivery
@ -709,15 +691,6 @@
:weight: 3
:idempotent: true
:tags: []
- :name: deployment:deployments_success
:worker_name: Deployments::SuccessWorker
:feature_category: :continuous_delivery
:has_external_dependencies:
:urgency: :low
:resource_boundary: :cpu
:weight: 3
:idempotent:
:tags: []
- :name: deployment:deployments_update_environment
:worker_name: Deployments::UpdateEnvironmentWorker
:feature_category: :continuous_delivery

2
app/workers/build_success_worker.rb
View File

@ -21,8 +21,6 @@ class BuildSuccessWorker # rubocop:disable Scalability/IdempotentWorker
private
##
# TODO: This should be processed in DeploymentSuccessWorker once we started storing `action` value in `deployments` records
def stop_environment(build)
build.persisted_environment.fire_state_event(:stop)
end

24
app/workers/deployments/finished_worker.rb
View File

@ -1,24 +0,0 @@
# frozen_string_literal: true
# This worker is deprecated and will be removed in 14.0
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381
module Deployments
class FinishedWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
data_consistency :always
sidekiq_options retry: 3
queue_namespace :deployment
feature_category :continuous_delivery
worker_resource_boundary :cpu
def perform(deployment_id)
if (deploy = Deployment.find_by_id(deployment_id))
LinkMergeRequestsService.new(deploy).execute
deploy.execute_hooks(Time.current)
end
end
end
end

20
app/workers/deployments/forward_deployment_worker.rb
View File

@ -1,20 +0,0 @@
# frozen_string_literal: true
# This worker is deprecated and will be removed in 14.0
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381
module Deployments
class ForwardDeploymentWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
data_consistency :always
sidekiq_options retry: 3
queue_namespace :deployment
feature_category :continuous_delivery
def perform(deployment_id)
Deployments::OlderDeploymentsDropService.new(deployment_id).execute
end
end
end

25
app/workers/deployments/success_worker.rb
View File

@ -1,25 +0,0 @@
# frozen_string_literal: true
# This worker is deprecated and will be removed in 14.0
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/266381
module Deployments
class SuccessWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
data_consistency :always
sidekiq_options retry: 3
queue_namespace :deployment
feature_category :continuous_delivery
worker_resource_boundary :cpu
def perform(deployment_id)
Deployment.find_by_id(deployment_id).try do |deployment|
break unless deployment.success?
Deployments::UpdateEnvironmentService.new(deployment).execute
end
end
end
end

22
doc/development/documentation/styleguide/index.md
View File

@ -1456,27 +1456,9 @@ Follow these styles when you're describing user interface elements in an
application:
- For elements with a visible label, use that label in bold with matching case.
For example, `the **Cancel** button`.
For example, `Select **Cancel**`.
- For elements with a tooltip or hover label, use that label in bold with
matching case. For example, `the **Add status emoji** button`.
### Verbs for UI elements
Use these verbs for specific uses with user interface
elements:
| Recommended | Used for | Replaces |
|:--------------------|:--------------------------------------|:----------------------|
| select | buttons, links, menu items, dropdowns | click, press, hit |
| select or clear | checkboxes | enable, click, press |
| expand | expandable sections | open |
| turn on or turn off | toggles | flip, enable, disable |
### Other Verbs
| Recommended | Used for | Replaces |
|:------------|:--------------------------------|:----------------------|
| go to | making a browser go to location | navigate to, open |
matching case. For example, `Select **Add status emoji**`.
## GitLab versions

37
doc/development/documentation/styleguide/word_list.md
View File

@ -75,10 +75,17 @@ Use lowercase for **boards**, **issue boards**, and **epic boards**.
One word, **checkbox**. Do not use **check box**.
You **select** (not **check** or **enable**) and **clear** (not **deselect** or **disable**) checkboxes.
## CI/CD
Always uppercase. No need to spell out on first use.
## click
Do not use. Instead, use **select** with buttons, links, menu items, and lists.
**Select** applies to more devices, while **click** is more specific to a mouse.
## currently
Do not use when talking about the product or its features. The documentation describes the product as it is today. ([Vale](../testing.md#vale) rule: [`CurrentStatus.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/CurrentStatus.yml))
@ -117,6 +124,10 @@ Do not use. If the user doesn't find the process to be easy, we lose their trust
Do not use Latin abbreviations. Use **for example**, **such as**, **for instance**, or **like** instead. ([Vale](../testing.md#vale) rule: [`LatinTerms.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/LatinTerms.yml))
## expand
Use instead of **open** when you are talking about expanding or collapsing a section in the UI.
## email
Do not use **e-mail** with a hyphen. When plural, use **emails** or **email messages**.
@ -203,6 +214,13 @@ Do not use when talking about version numbers.
- Avoid: In GitLab 14.1 and higher.
- Use instead: In GitLab 14.1 and later.
## hit
Don't use to mean **press**.
- Avoid: Hit the **ENTER** button.
- Use instead: Press **ENTER**.
## I
Do not use first-person singular. Use **you**, **we**, or **us** instead. ([Vale](../testing.md#vale) rule: [`FirstPerson.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/FirstPerson.yml))
@ -293,6 +311,13 @@ Lowercase. If you use **MR** as the acronym, spell it out on first use.
Lowercase.
## navigate
Do not use. Use **go** instead. For example:
- Go to this webpage.
- Open a terminal and go to the `runner` directory.
## need to, should
Try to avoid. If something is required, use **must**.
@ -334,6 +359,12 @@ Do not use roles and permissions interchangeably. Each user is assigned a role.
Do not use. For details, see the [Microsoft style guide](https://docs.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/p/please).
## press
Use when talking about keyboard keys. For example:
- To stop the command, press <kbd>Control</kbd>+<kbd>C</kbd>.
## profanity
Do not use. Doing so may negatively affect other users and contributors, which is contrary to the GitLab value of [Diversity, Inclusion, and Belonging](https://about.gitlab.com/handbook/values/#diversity-inclusion).
@ -454,6 +485,12 @@ Use lowercase. ([Vale](../testing.md#vale) rule: [`ToDo.yml`](https://gitlab.com
Use title case. ([Vale](../testing.md#vale) rule: [`ToDo.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/ToDo.yml))
## toggle
You **turn on** or **turn off** a toggle. For example:
- Turn on the **blah** toggle.
## useful
Do not use. If the user doesn't find the process to be useful, we lose their trust.

8
locale/gitlab.pot
View File

@ -1539,10 +1539,10 @@ msgstr ""
msgid "API?"
msgstr ""
msgid "APIFuzzing|$VariableWithPassword"
msgid "APIFuzzing|$VARIABLE_WITH_PASSWORD"
msgstr ""
msgid "APIFuzzing|$VariableWithUsername"
msgid "APIFuzzing|$VARIABLE_WITH_USERNAME"
msgstr ""
msgid "APIFuzzing|API Fuzzing Configuration"
@ -1569,10 +1569,10 @@ msgstr ""
msgid "APIFuzzing|Enable authentication"
msgstr ""
msgid "APIFuzzing|Enter the name of the variable containing the password. For example, $VariableWithPassword."
msgid "APIFuzzing|Enter the name of the CI variable containing the password. For example, $VARIABLE_WITH_PASSWORD."
msgstr ""
msgid "APIFuzzing|Enter the name of the variable containing the username. For example, $VariableWithUsername."
msgid "APIFuzzing|Enter the name of the CI variable containing the username. For example, $VARIABLE_WITH_USERNAME."
msgstr ""
msgid "APIFuzzing|File path or URL to APIs to be tested. For example, folder/example_fuzz.har. HAR files may contain sensitive information such as authentication tokens, API keys, and session cookies. We recommend that you review the HAR files' contents before adding them to a repository."

2
qa/qa/specs/features/browser_ui/3_create/merge_request/merge_when_pipeline_succeeds_spec.rb
View File

@ -30,7 +30,7 @@ module QA
content: <<~EOF
test:
tags: ["runner-for-#{project.name}"]
script: sleep 10
script: sleep 20
only:
- merge_requests
EOF

12
spec/features/cycle_analytics_spec.rb
View File

@ -43,6 +43,10 @@ RSpec.describe 'Value Stream Analytics', :js do
end
context "when there's value stream analytics data" do
around do |example|
travel_to(5.days.ago) { example.run }
end
before do
project.add_maintainer(user)
@ -53,10 +57,10 @@ RSpec.describe 'Value Stream Analytics', :js do
merge_request = issue.merge_requests_closing_issues.first.merge_request
merge_request.update!(created_at: issue.metrics.first_associated_with_milestone_at + 1.hour)
merge_request.metrics.update!(
latest_build_started_at: 4.hours.ago,
latest_build_finished_at: 3.hours.ago,
merged_at: merge_request.created_at + 1.hour,
first_deployed_to_production_at: merge_request.created_at + 2.hours
latest_build_started_at: merge_request.created_at + 3.hours,
latest_build_finished_at: merge_request.created_at + 4.hours,
merged_at: merge_request.created_at + 4.hours,
first_deployed_to_production_at: merge_request.created_at + 5.hours
)
sign_in(user)

2
spec/lib/gitlab/ci/parsers/security/common_spec.rb
View File

@ -1,7 +1,5 @@
# frozen_string_literal: true
# TODO remove duplication from spec/lib/gitlab/ci/parsers/security/common_spec.rb and spec/lib/gitlab/ci/parsers/security/common_spec.rb
# See https://gitlab.com/gitlab-org/gitlab/-/issues/336589
require 'spec_helper'
RSpec.describe Gitlab::Ci::Parsers::Security::Common do

27
spec/models/diff_note_spec.rb
View File

@ -558,4 +558,31 @@ RSpec.describe DiffNote do
it { is_expected.to eq('note') }
end
describe '#shas' do
it 'returns list of SHAs based on original_position' do
expect(subject.shas).to match_array([
position.base_sha,
position.start_sha,
position.head_sha
])
end
context 'when position changes' do
before do
subject.position = new_position
end
it 'includes the new position SHAs' do
expect(subject.shas).to match_array([
position.base_sha,
position.start_sha,
position.head_sha,
new_position.base_sha,
new_position.start_sha,
new_position.head_sha
])
end
end
end
end

20
spec/models/shard_spec.rb
View File

@ -33,19 +33,21 @@ RSpec.describe Shard do
expect(result.name).to eq('foo')
end
it 'retries if creation races' do
it 'returns existing record if creation races' do
shard_created_by_others = double(described_class)
expect(described_class)
.to receive(:find_or_create_by)
.with(name: 'default')
.to receive(:find_by)
.with(name: 'new_shard')
.and_return(nil, shard_created_by_others)
expect(described_class)
.to receive(:create)
.with(name: 'new_shard')
.and_raise(ActiveRecord::RecordNotUnique, 'fail')
.once
expect(described_class)
.to receive(:find_or_create_by)
.with(name: 'default')
.and_call_original
expect(described_class.by_name('default')).to eq(default_shard)
expect(described_class.by_name('new_shard')).to eq(shard_created_by_others)
end
end
end

27
spec/services/draft_notes/publish_service_spec.rb
View File

@ -33,7 +33,8 @@ RSpec.describe DraftNotes::PublishService do
end
it 'does not skip notification', :sidekiq_might_not_need_inline do
expect(Notes::CreateService).to receive(:new).with(project, user, drafts.first.publish_params).and_call_original
note_params = drafts.first.publish_params.merge(skip_keep_around_commits: false)
expect(Notes::CreateService).to receive(:new).with(project, user, note_params).and_call_original
expect_next_instance_of(NotificationService) do |notification_service|
expect(notification_service).to receive(:new_note)
end
@ -127,12 +128,17 @@ RSpec.describe DraftNotes::PublishService do
publish
end
context 'capturing diff notes positions' do
context 'capturing diff notes positions and keeping around commits' do
before do
# Need to execute this to ensure that we'll be able to test creation of
# DiffNotePosition records as that only happens when the `MergeRequest#merge_ref_head`
# is present. This service creates that for the specified merge request.
MergeRequests::MergeToRefService.new(project: project, current_user: user).execute(merge_request)
# Need to re-stub this and call original as we are stubbing
# `Gitlab::Git::KeepAround#execute` in spec_helper for performance reason.
# Enabling it here so we can test the Gitaly calls it makes.
allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
end
it 'creates diff_note_positions for diff notes' do
@ -143,11 +149,26 @@ RSpec.describe DraftNotes::PublishService do
expect(notes.last.diff_note_positions).to be_any
end
it 'keeps around the commits of each published note' do
publish
repository = project.repository
notes = merge_request.notes.order(id: :asc)
notes.first.shas.each do |sha|
expect(repository.ref_exists?("refs/keep-around/#{sha}")).to be_truthy
end
notes.last.shas.each do |sha|
expect(repository.ref_exists?("refs/keep-around/#{sha}")).to be_truthy
end
end
it 'does not requests a lot from Gitaly', :request_store do
# NOTE: This should be reduced as we work on reducing Gitaly calls.
# Gitaly requests shouldn't go above this threshold as much as possible
# as it may add more to the Gitaly N+1 issue we are experiencing.
expect { publish }.to change { Gitlab::GitalyClient.get_request_count }.by(11)
expect { publish }.to change { Gitlab::GitalyClient.get_request_count }.by(21)
end
end

65
spec/workers/deployments/finished_worker_spec.rb
View File

@ -1,65 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Deployments::FinishedWorker do
let(:worker) { described_class.new }
describe '#perform' do
before do
allow(ProjectServiceWorker).to receive(:perform_async)
end
it 'links merge requests to the deployment' do
deployment = create(:deployment)
service = instance_double(Deployments::LinkMergeRequestsService)
expect(Deployments::LinkMergeRequestsService)
.to receive(:new)
.with(deployment)
.and_return(service)
expect(service).to receive(:execute)
worker.perform(deployment.id)
end
it 'executes project services for deployment_hooks' do
deployment = create(:deployment)
project = deployment.project
service = create(:service, type: 'SlackService', project: project, deployment_events: true, active: true)
worker.perform(deployment.id)
expect(ProjectServiceWorker).to have_received(:perform_async).with(service.id, an_instance_of(Hash))
end
it 'does not execute an inactive service' do
deployment = create(:deployment)
project = deployment.project
create(:service, type: 'SlackService', project: project, deployment_events: true, active: false)
worker.perform(deployment.id)
expect(ProjectServiceWorker).not_to have_received(:perform_async)
end
it 'does nothing if a deployment with the given id does not exist' do
worker.perform(0)
expect(ProjectServiceWorker).not_to have_received(:perform_async)
end
it 'execute webhooks' do
deployment = create(:deployment)
project = deployment.project
web_hook = create(:project_hook, deployment_events: true, project: project)
expect_next_instance_of(WebHookService, web_hook, an_instance_of(Hash), "deployment_hooks") do |service|
expect(service).to receive(:async_execute)
end
worker.perform(deployment.id)
end
end
end

38
spec/workers/deployments/success_worker_spec.rb
View File

@ -1,38 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Deployments::SuccessWorker do
subject { described_class.new.perform(deployment&.id) }
context 'when successful deployment' do
let(:deployment) { create(:deployment, :success) }
it 'executes Deployments::UpdateEnvironmentService' do
expect(Deployments::UpdateEnvironmentService)
.to receive(:new).with(deployment).and_call_original
subject
end
end
context 'when canceled deployment' do
let(:deployment) { create(:deployment, :canceled) }
it 'does not execute Deployments::UpdateEnvironmentService' do
expect(Deployments::UpdateEnvironmentService).not_to receive(:new)
subject
end
end
context 'when deploy record does not exist' do
let(:deployment) { nil }
it 'does not execute Deployments::UpdateEnvironmentService' do
expect(Deployments::UpdateEnvironmentService).not_to receive(:new)
subject
end
end
end