From 2763994b8f9e44c8ee9bd7197622e6bcba00a4db Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 22 Jul 2022 09:11:01 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../issuable/issuable_template_selector.js | 16 ++- .../issuables_description_templates_helper.rb | 31 +++-- .../destroy/destroyed_issuable_service.rb | 7 +- doc/administration/get_started.md | 2 +- glfm_specification/example_snapshots/html.yml | 29 +++- .../example_snapshots/markdown.yml | 4 +- .../example_snapshots/prosemirror_json.yml | 8 +- .../glfm_canonical_examples.txt | 24 +++- glfm_specification/output/spec.txt | 24 +++- scripts/lib/glfm/update_example_snapshots.rb | 10 ++ .../projects/issuable_templates_spec.rb | 28 ++++ ...ables_description_templates_helper_spec.rb | 127 +++++++++++++----- .../broadcast_messages_controller_spec.rb | 8 +- .../destroyed_issuable_service_spec.rb | 57 +++++--- 14 files changed, 282 insertions(+), 93 deletions(-) rename spec/{controllers => requests}/admin/broadcast_messages_controller_spec.rb (55%) diff --git a/app/assets/javascripts/issuable/issuable_template_selector.js b/app/assets/javascripts/issuable/issuable_template_selector.js index cce903d388d..6b8f3de8d49 100644 --- a/app/assets/javascripts/issuable/issuable_template_selector.js +++ b/app/assets/javascripts/issuable/issuable_template_selector.js @@ -17,7 +17,15 @@ export default class IssuableTemplateSelector extends TemplateSelector { name: this.dropdown.data('selected'), }; - if (initialQuery.name) this.requestFile(initialQuery); + // Only use the default template if we don't have description data from autosave + if (!initialQuery.name && this.dropdown.data('default') && !this.editor.getValue().length) { + initialQuery.name = this.dropdown.data('default'); + } + + if (initialQuery.name) { + this.requestFile(initialQuery); + this.setToggleText(initialQuery.name); + } $('.reset-template', this.dropdown.parent()).on('click', () => { this.setInputValueToTemplateContent(); @@ -53,10 +61,14 @@ export default class IssuableTemplateSelector extends TemplateSelector { } this.setInputValueToTemplateContent(); - $('.dropdown-toggle-text', this.dropdown).text(__('Choose a template')); + this.setToggleText(__('Choose a template')); this.previousSelectedIndex = null; } + setToggleText(text) { + $('.dropdown-toggle-text', this.dropdown).text(text); + } + setSelectedIndex() { this.previousSelectedIndex = this.dropdown.data('deprecatedJQueryDropdown').selectedIndex; } diff --git a/app/helpers/issuables_description_templates_helper.rb b/app/helpers/issuables_description_templates_helper.rb index a82a5ac0fb0..58b86dca1e0 100644 --- a/app/helpers/issuables_description_templates_helper.rb +++ b/app/helpers/issuables_description_templates_helper.rb @@ -5,8 +5,12 @@ module IssuablesDescriptionTemplatesHelper include GitlabRoutingHelper def template_dropdown_tag(issuable, &block) - selected_template = selected_template(issuable) - title = selected_template || _('Choose a template') + template_names = template_names(issuable) + + selected_template = selected_template_name(template_names) + default_template = default_template_name(template_names, issuable) + title = _('Choose a template') + options = { toggle_class: 'js-issuable-selector', title: title, @@ -17,6 +21,7 @@ module IssuablesDescriptionTemplatesHelper data: issuable_templates(ref_project, issuable.to_ability_name), field_name: 'issuable_template', selected: selected_template, + default: default_template, project_id: ref_project.id } } @@ -32,19 +37,19 @@ module IssuablesDescriptionTemplatesHelper @template_types[project.id][issuable_type] ||= TemplateFinder.all_template_names(project, issuable_type.pluralize) end - def selected_template(issuable) - all_templates = issuable_templates(ref_project, issuable.to_ability_name) + def selected_template_name(template_names) + template_names.find { |tmpl_name| tmpl_name == params[:issuable_template] } + end + def default_template_name(template_names, issuable) + return if issuable.description.present? || issuable.persisted? + + template_names.find { |tmpl_name| tmpl_name.casecmp?('default') } + end + + def template_names(issuable) # Only local templates will be listed if licenses for inherited templates are not present - all_templates = all_templates.values.flatten.map { |tpl| tpl[:name] }.compact.uniq - - template = all_templates.find { |tmpl_name| tmpl_name == params[:issuable_template] } - - unless issuable.description.present? - template ||= all_templates.find { |tmpl_name| tmpl_name.casecmp?('default') } - end - - template + issuable_templates(ref_project, issuable.to_ability_name).values.flatten.map { |tpl| tpl[:name] }.compact.uniq end def available_service_desk_templates_for(project) diff --git a/app/services/todos/destroy/destroyed_issuable_service.rb b/app/services/todos/destroy/destroyed_issuable_service.rb index 7a85b59eeea..759c430ec7a 100644 --- a/app/services/todos/destroy/destroyed_issuable_service.rb +++ b/app/services/todos/destroy/destroyed_issuable_service.rb @@ -5,9 +5,14 @@ module Todos class DestroyedIssuableService BATCH_SIZE = 100 + # Since we are moving towards work items, in some instances we create todos with + # `target_type: WorkItem` in other instances we still create todos with `target_type: Issue` + # So when an issue/work item is deleted, we just make sure to delete todos for both target types + BOUND_TARGET_TYPES = %w(Issue WorkItem).freeze + def initialize(target_id, target_type) @target_id = target_id - @target_type = target_type + @target_type = BOUND_TARGET_TYPES.include?(target_type) ? BOUND_TARGET_TYPES : target_type end def execute diff --git a/doc/administration/get_started.md b/doc/administration/get_started.md index b2bdd876499..01b555a2a06 100644 --- a/doc/administration/get_started.md +++ b/doc/administration/get_started.md @@ -144,7 +144,7 @@ You can restore a backup only to **the exact same version and type** (Community ### Back up GitLab SaaS -Backups of GitLab databases and filesystems are taken every 24 hours, and are kept for two weeks on a rolling schedule. All backups are encrypted. +Backups of GitLab databases and file systems are taken every 24 hours, and are kept for two weeks on a rolling schedule. All backups are encrypted. - GitLab SaaS creates backups to ensure your data is secure, but you can't use these methods to export or back up your data yourself. - Issues are stored in the database. They can't be stored in Git itself. diff --git a/glfm_specification/example_snapshots/html.yml b/glfm_specification/example_snapshots/html.yml index 6c05fbe744b..376a4bc72ca 100644 --- a/glfm_specification/example_snapshots/html.yml +++ b/glfm_specification/example_snapshots/html.yml @@ -7556,16 +7556,35 @@ wysiwyg: |-

Multiple spaces

07_01__gitlab_specific_markdown__footnotes__001: - canonical: "" + canonical: | +

+ footnote reference tag + + + 1 + + +

+
+
    +
  1. +

    + footnote text + + +

    +
    2. +
+
static: |- -

footnote reference tag 1

+

footnote reference tag 1

    -
  1. -

    footnote text ↩

    +
  2. +

    footnote text ↩

wysiwyg: |- -

footnote reference tag 1

+

footnote reference tag fortytwo

footnote text

diff --git a/glfm_specification/example_snapshots/markdown.yml b/glfm_specification/example_snapshots/markdown.yml index de0e66f7e08..c4c30dcb513 100644 --- a/glfm_specification/example_snapshots/markdown.yml +++ b/glfm_specification/example_snapshots/markdown.yml @@ -2190,6 +2190,6 @@ 06_15__inlines__textual_content__003: | Multiple spaces 07_01__gitlab_specific_markdown__footnotes__001: | - footnote reference tag [^1] + footnote reference tag [^fortytwo] - [^1]: footnote text + [^fortytwo]: footnote text diff --git a/glfm_specification/example_snapshots/prosemirror_json.yml b/glfm_specification/example_snapshots/prosemirror_json.yml index 0a8c15e9650..0b468945042 100644 --- a/glfm_specification/example_snapshots/prosemirror_json.yml +++ b/glfm_specification/example_snapshots/prosemirror_json.yml @@ -19218,8 +19218,8 @@ { "type": "footnoteReference", "attrs": { - "identifier": "1", - "label": "1" + "identifier": "fortytwo", + "label": "fortytwo" } } ] @@ -19227,8 +19227,8 @@ { "type": "footnoteDefinition", "attrs": { - "identifier": "1", - "label": "1" + "identifier": "fortytwo", + "label": "fortytwo" }, "content": [ { diff --git a/glfm_specification/input/gitlab_flavored_markdown/glfm_canonical_examples.txt b/glfm_specification/input/gitlab_flavored_markdown/glfm_canonical_examples.txt index e4d01b4a813..d0d450b66bf 100644 --- a/glfm_specification/input/gitlab_flavored_markdown/glfm_canonical_examples.txt +++ b/glfm_specification/input/gitlab_flavored_markdown/glfm_canonical_examples.txt @@ -14,7 +14,27 @@ See [the footnotes section of the user-facing documentation for GitLab Flavored Markdown](https://docs.gitlab.com/ee/user/markdown.html#footnotes). ```````````````````````````````` example gitlab footnote -footnote reference tag [^1] +footnote reference tag [^fortytwo] -[^1]: footnote text +[^fortytwo]: footnote text +. +

+footnote reference tag + + +1 + + +

+
+
    +
  1. +

    +footnote text + + +

    +
    2. +
+
```````````````````````````````` diff --git a/glfm_specification/output/spec.txt b/glfm_specification/output/spec.txt index c81daf7f689..3fc27efdc34 100644 --- a/glfm_specification/output/spec.txt +++ b/glfm_specification/output/spec.txt @@ -9616,9 +9616,29 @@ See [the footnotes section of the user-facing documentation for GitLab Flavored Markdown](https://docs.gitlab.com/ee/user/markdown.html#footnotes). ```````````````````````````````` example gitlab footnote -footnote reference tag [^1] +footnote reference tag [^fortytwo] -[^1]: footnote text +[^fortytwo]: footnote text +. +

+footnote reference tag + + +1 + + +

+
+
    +
  1. +

    +footnote text + + +

    +
    2. +
+
```````````````````````````````` diff --git a/scripts/lib/glfm/update_example_snapshots.rb b/scripts/lib/glfm/update_example_snapshots.rb index 893d8d9c014..d8d6cf3cdbc 100644 --- a/scripts/lib/glfm/update_example_snapshots.rb +++ b/scripts/lib/glfm/update_example_snapshots.rb @@ -29,6 +29,8 @@ module Glfm def process(skip_static_and_wysiwyg: false) output('Updating example snapshots...') + setup_environment + output('(Skipping static HTML generation)') if skip_static_and_wysiwyg output("Reading #{GLFM_SPEC_TXT_PATH}...") @@ -47,6 +49,14 @@ module Glfm private + def setup_environment + # Set 'GITLAB_TEST_FOOTNOTE_ID' in order to override random number generation in + # Banzai::Filter::FootnoteFilter#random_number, and thus avoid the need to + # perform normalization on the value. See: + # https://docs.gitlab.com/ee/development/gitlab_flavored_markdown/specification_guide/#normalization + ENV['GITLAB_TEST_FOOTNOTE_ID'] = '42' + end + def add_example_names(all_examples) # NOTE: This method and the parse_examples method assume: # 1. Section 2 is the first section which contains examples diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index 12c5820a69d..ac83de3e765 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -90,6 +90,34 @@ RSpec.describe 'issuable templates', :js do end end + context 'user creates an issue with a default template from the repo' do + let(:template_content) { 'this is the default template' } + + before do + project.repository.create_file( + user, + '.gitlab/issue_templates/default.md', + template_content, + message: 'added default issue template', + branch_name: 'master' + ) + end + + it 'does not overwrite autosaved description' do + visit new_project_issue_path project + wait_for_requests + + assert_template # default template is loaded the first time + + fill_in 'issue_description', with: 'my own description', fill_options: { clear: :backspace } + + visit new_project_issue_path project + wait_for_requests + + assert_template(expected_content: 'my own description') + end + end + context 'user creates a merge request using templates' do let(:template_content) { 'this is a test "feature-proposal" template' } let(:bug_template_content) { 'this is merge request bug template' } diff --git a/spec/helpers/issuables_description_templates_helper_spec.rb b/spec/helpers/issuables_description_templates_helper_spec.rb index 768ce5975c1..bd8af384d40 100644 --- a/spec/helpers/issuables_description_templates_helper_spec.rb +++ b/spec/helpers/issuables_description_templates_helper_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do - include_context 'project issuable templates context' - describe '#issuable_templates' do + include_context 'project issuable templates context' + let_it_be(:inherited_from) { nil } let_it_be(:user) { create(:user) } let_it_be(:parent_group, reload: true) { create(:group) } @@ -44,7 +44,7 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do end end - describe '#selected_template' do + describe '#available_service_desk_templates_for' do let_it_be(:project) { build(:project) } before do @@ -72,40 +72,9 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do ].to_json expect(helper.available_service_desk_templates_for(@project)).to eq(value) end - - context 'when no issuable_template parameter or default template is present' do - it 'does not select a template' do - expect(helper.selected_template(project)).to be(nil) - end - end - - context 'when an issuable_template parameter has been provided' do - before do - allow(helper).to receive(:params).and_return({ issuable_template: 'another_issue_template' }) - end - - it 'selects the issuable template' do - expect(helper.selected_template(project)).to eq('another_issue_template') - end - end - - context 'when there is a default template' do - let(:templates) do - { - "" => [ - { name: "another_issue_template", id: "another_issue_template", project_id: project.id }, - { name: "default", id: "default", project_id: project.id } - ] - } - end - - it 'selects the default template' do - expect(helper.selected_template(project)).to eq('default') - end - end end - context 'when there are not templates in the project' do + context 'when there are no templates in the project' do let(:templates) { {} } it 'returns empty array' do @@ -114,4 +83,92 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do end end end + + describe '#selected_template_name' do + let(:template_names) { %w(another_issue_template custom_issue_template) } + + context 'when no issuable_template parameter is provided' do + it 'does not select a template' do + expect(helper.selected_template_name(template_names)).to be_nil + end + end + + context 'when an issuable_template parameter has been provided' do + before do + allow(helper).to receive(:params).and_return({ issuable_template: template_param_value }) + end + + context 'when param matches existing templates' do + let(:template_param_value) { 'another_issue_template' } + + it 'returns the matching issuable template' do + expect(helper.selected_template_name(template_names)).to eq('another_issue_template') + end + end + + context 'when param does not match any templates' do + let(:template_param_value) { 'non_matching_issue_template' } + + it 'returns nil' do + expect(helper.selected_template_name(template_names)).to be_nil + end + end + end + end + + describe '#default_template_name' do + context 'when a default template is available' do + let(:template_names) { %w(another_issue_template deFault) } + + it 'returns the default template' do + issue = build(:issue) + + expect(helper.default_template_name(template_names, issue)).to be('deFault') + end + + it 'returns nil when issuable has a description set' do + issue = build(:issue, description: 'from template in project settings') + + expect(helper.default_template_name(template_names, issue)).to be_nil + end + + it 'returns nil when issuable is persisted' do + issue = create(:issue) + + expect(helper.default_template_name(template_names, issue)).to be_nil + end + end + + context 'when there is no default template' do + let(:template_names) { %w(another_issue_template) } + + it 'returns nil' do + expect(helper.default_template_name(template_names, build(:issue))).to be_nil + end + end + end + + describe '#template_names' do + let(:project) { build(:project) } + let(:templates) do + { + "Project templates" => [ + { name: "another_issue_template", id: "another_issue_template", project_id: project.id }, + { name: "custom_issue_template", id: "custom_issue_template", project_id: project.id } + ], + "Group templates" => [ + { name: "another_issue_template", id: "another_issue_template", project_id: project.id } + ] + } + end + + before do + allow(helper).to receive(:ref_project).and_return(project) + allow(helper).to receive(:issuable_templates).and_return(templates) + end + + it 'returns unique list of template names' do + expect(helper.template_names(build(:issue))).to contain_exactly('another_issue_template', 'custom_issue_template') + end + end end diff --git a/spec/controllers/admin/broadcast_messages_controller_spec.rb b/spec/requests/admin/broadcast_messages_controller_spec.rb similarity index 55% rename from spec/controllers/admin/broadcast_messages_controller_spec.rb rename to spec/requests/admin/broadcast_messages_controller_spec.rb index 66a54dc10ac..9101370d42d 100644 --- a/spec/controllers/admin/broadcast_messages_controller_spec.rb +++ b/spec/requests/admin/broadcast_messages_controller_spec.rb @@ -2,16 +2,14 @@ require 'spec_helper' -RSpec.describe Admin::BroadcastMessagesController do +RSpec.describe Admin::BroadcastMessagesController, :enable_admin_mode do before do sign_in(create(:admin)) end - describe 'GET /preview' do - render_views - + describe 'POST /preview' do it 'renders preview partial' do - get :preview, params: { broadcast_message: { message: "Hello, world!" } } + post preview_admin_broadcast_messages_path, params: { broadcast_message: { message: "Hello, world!" } } expect(response).to have_gitlab_http_status(:ok) expect(response.body).to render_template(:_preview) diff --git a/spec/services/todos/destroy/destroyed_issuable_service_spec.rb b/spec/services/todos/destroy/destroyed_issuable_service_spec.rb index 24f74bae7c8..6d6abe06d1c 100644 --- a/spec/services/todos/destroy/destroyed_issuable_service_spec.rb +++ b/spec/services/todos/destroy/destroyed_issuable_service_spec.rb @@ -4,31 +4,46 @@ require 'spec_helper' RSpec.describe Todos::Destroy::DestroyedIssuableService do describe '#execute' do - let_it_be(:target) { create(:merge_request) } - let_it_be(:pending_todo) { create(:todo, :pending, project: target.project, target: target, user: create(:user)) } - let_it_be(:done_todo) { create(:todo, :done, project: target.project, target: target, user: create(:user)) } + let_it_be(:user) { create(:user) } - def execute - described_class.new(target.id, target.class.name).execute + subject { described_class.new(target.id, target.class.name).execute } + + context 'when target is merge request' do + let_it_be(:target) { create(:merge_request) } + let_it_be(:pending_todo) { create(:todo, :pending, project: target.project, target: target, user: user) } + let_it_be(:done_todo) { create(:todo, :done, project: target.project, target: target, user: user) } + + it 'deletes todos for specified target ID and type' do + control_count = ActiveRecord::QueryRecorder.new { subject }.count + + # Create more todos for the target + create(:todo, :pending, project: target.project, target: target, user: user) + create(:todo, :pending, project: target.project, target: target, user: user) + create(:todo, :done, project: target.project, target: target, user: user) + create(:todo, :done, project: target.project, target: target, user: user) + + expect { subject }.not_to exceed_query_limit(control_count) + end + + it 'invalidates todos cache counts of todo users', :use_clean_rails_redis_caching do + expect { subject } + .to change { pending_todo.user.todos_pending_count }.from(1).to(0) + .and change { done_todo.user.todos_done_count }.from(1).to(0) + end end - it 'deletes todos for specified target ID and type' do - control_count = ActiveRecord::QueryRecorder.new { execute }.count + context 'when target is an work item' do + let_it_be(:target) { create(:work_item) } + let_it_be(:todo1) { create(:todo, :pending, project: target.project, target: target, user: user) } + let_it_be(:todo2) { create(:todo, :done, project: target.project, target: target, user: user) } + # rubocop: disable Cop/AvoidBecomes + let_it_be(:todo3) { create(:todo, :pending, project: target.project, target: target.becomes(Issue), user: user) } + let_it_be(:todo4) { create(:todo, :done, project: target.project, target: target.becomes(Issue), user: user) } + # rubocop: enable Cop/AvoidBecomes - # Create more todos for the target - create(:todo, :pending, project: target.project, target: target, user: create(:user)) - create(:todo, :pending, project: target.project, target: target, user: create(:user)) - create(:todo, :done, project: target.project, target: target, user: create(:user)) - create(:todo, :done, project: target.project, target: target, user: create(:user)) - - expect { execute }.not_to exceed_query_limit(control_count) - expect(target.reload.todos.count).to eq(0) - end - - it 'invalidates todos cache counts of todo users', :use_clean_rails_redis_caching do - expect { execute } - .to change { pending_todo.user.todos_pending_count }.from(1).to(0) - .and change { done_todo.user.todos_done_count }.from(1).to(0) + it 'deletes todos' do + expect { subject }.to change(Todo, :count).by(-4) + end end end end