diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-15 09:10:17 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-15 09:10:17 +0300 |
commit | d35de87f96f580fede92e6b43352fbff8316e2c3 (patch) | |
tree | 9b0ebb12a3ce148f35f1e05a3dba2675adc97f99 | |
parent | 03c5d7f2c175acedafcb1b233ec1e40e9fcc8d1b (diff) |
Add latest changes from gitlab-org/gitlab@master
60 files changed, 1042 insertions, 284 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 121e45502fd..45c7fe35f03 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -354,7 +354,11 @@ export default { v-if="!diffFile.submodule && addMergeRequestButtons" class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start" > - <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" /> + <diff-stats + :diff-file="diffFile" + :added-lines="diffFile.added_lines" + :removed-lines="diffFile.removed_lines" + /> <gl-form-checkbox v-if="isReviewable && showLocalFileReviews" v-gl-tooltip.hover diff --git a/app/assets/javascripts/diffs/components/diff_stats.vue b/app/assets/javascripts/diffs/components/diff_stats.vue index 0303700f42a..af2bbc82543 100644 --- a/app/assets/javascripts/diffs/components/diff_stats.vue +++ b/app/assets/javascripts/diffs/components/diff_stats.vue @@ -2,10 +2,16 @@ import { GlIcon } from '@gitlab/ui'; import { isNumber } from 'lodash'; import { n__ } from '~/locale'; +import { isNotDiffable, stats } from '../utils/diff_file'; export default { components: { GlIcon }, props: { + diffFile: { + type: Object, + required: false, + default: () => null, + }, addedLines: { type: Number, required: true, @@ -33,6 +39,12 @@ export default { hasDiffFiles() { return isNumber(this.diffFilesLength) && this.diffFilesLength >= 0; }, + notDiffable() { + return isNotDiffable(this.diffFile); + }, + fileStats() { + return stats(this.diffFile); + }, }, }; </script> @@ -45,23 +57,28 @@ export default { 'd-none d-sm-inline-flex': !isCompareVersionsHeader, }" > - <div v-if="hasDiffFiles" class="diff-stats-group"> - <gl-icon name="doc-code" class="diff-stats-icon text-secondary" /> - <span class="text-secondary bold">{{ diffFilesCountText }} {{ filesText }}</span> - </div> - <div - class="diff-stats-group cgreen d-flex align-items-center" - :class="{ bold: isCompareVersionsHeader }" - > - <span>+</span> - <span class="js-file-addition-line">{{ addedLines }}</span> + <div v-if="notDiffable" :class="fileStats.classes"> + {{ fileStats.text }} </div> - <div - class="diff-stats-group cred d-flex align-items-center" - :class="{ bold: isCompareVersionsHeader }" - > - <span>-</span> - <span class="js-file-deletion-line">{{ removedLines }}</span> + <div v-else class="diff-stats-contents"> + <div v-if="hasDiffFiles" class="diff-stats-group"> + <gl-icon name="doc-code" class="diff-stats-icon text-secondary" /> + <span class="text-secondary bold">{{ diffFilesCountText }} {{ filesText }}</span> + </div> + <div + class="diff-stats-group cgreen d-flex align-items-center" + :class="{ bold: isCompareVersionsHeader }" + > + <span>+</span> + <span class="js-file-addition-line">{{ addedLines }}</span> + </div> + <div + class="diff-stats-group cred d-flex align-items-center" + :class="{ bold: isCompareVersionsHeader }" + > + <span>-</span> + <span class="js-file-deletion-line">{{ removedLines }}</span> + </div> </div> </div> </template> diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index a96c1207a04..363ace2b7c6 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -1,3 +1,5 @@ +import { diffViewerModes as viewerModes } from '~/ide/constants'; +import { changeInPercent, numberToHumanSize } from '~/lib/utils/number_utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { uuids } from '~/lib/utils/uuids'; @@ -46,6 +48,8 @@ function identifier(file) { })[0]; } +export const isNotDiffable = (file) => file?.viewer?.name === viewerModes.not_diffable; + export function prepareRawDiffFile({ file, allFiles, meta = false }) { const additionalProperties = { brokenSymlink: fileSymlinkInformation(file, allFiles), @@ -84,3 +88,35 @@ export function isCollapsed(file) { export function getShortShaFromFile(file) { return file.content_sha ? truncateSha(String(file.content_sha)) : null; } + +export function stats(file) { + let valid = false; + let classes = ''; + let sign = ''; + let text = ''; + let percent = 0; + let diff = 0; + + if (file) { + percent = changeInPercent(file.old_size, file.new_size); + diff = file.new_size - file.old_size; + sign = diff >= 0 ? '+' : ''; + text = `${sign}${numberToHumanSize(diff)} (${sign}${percent}%)`; + valid = true; + + if (diff > 0) { + classes = 'cgreen'; + } else if (diff < 0) { + classes = 'cred'; + } + } + + return { + changed: diff, + text, + percent, + classes, + sign, + valid, + }; +} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue index 55fa24fb51a..07821b01dd5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue @@ -32,9 +32,9 @@ export default { :href="helpPath" :title="__('About this feature')" target="_blank" - class="d-flex-center pl-1" + class="d-flex-center" > - <gl-icon name="question" /> + <gl-icon name="question-o" class="gl-ml-3" /> </gl-link> </div> </template> diff --git a/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue b/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue index 26bc9b5d60e..eed1c86c318 100644 --- a/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue +++ b/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue @@ -53,6 +53,6 @@ export default { </span> <gl-link v-else target="_blank" :href="helpPath" :aria-label="$options.i18n.securityReportsHelp"> - <gl-icon name="question" /> + <gl-icon name="question-o" /> </gl-link> </template> diff --git a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue index b7f283b8fd9..d7a3d4e611e 100644 --- a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue +++ b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue @@ -191,6 +191,7 @@ export default { <security-summary :message="groupedSummaryText" /> <help-icon + class="gl-ml-3" :help-path="securityReportsDocsPath" :discover-project-security-path="discoverProjectSecurityPath" /> @@ -219,6 +220,7 @@ export default { {{ $options.i18n.scansHaveRun }} <help-icon + class="gl-ml-3" :help-path="securityReportsDocsPath" :discover-project-security-path="discoverProjectSecurityPath" /> diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss index 222bfa97b17..c0e9289309a 100644 --- a/app/assets/stylesheets/framework/diffs.scss +++ b/app/assets/stylesheets/framework/diffs.scss @@ -646,6 +646,10 @@ table.code { align-items: center; padding: 0 1rem; + .diff-stats-contents { + display: contents; + } + .diff-stats-group { padding: 0 0.25rem; } diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 136b674f0a9..66816d4c587 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -183,7 +183,12 @@ class GroupsController < Groups::ApplicationController def download_export if @group.export_file_exists? - send_upload(@group.export_file, attachment: @group.export_file.filename) + if @group.export_archive_exists? + send_upload(@group.export_file, attachment: @group.export_file.filename) + else + redirect_to edit_group_path(@group), + alert: _('The file containing the export is not available yet; it may still be transferring. Please try again later.') + end else redirect_to edit_group_path(@group), alert: _('Group export link has expired. Please generate a new export from your group settings.') diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b37d5466757..53d80b8be58 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -226,7 +226,14 @@ class ProjectsController < Projects::ApplicationController def download_export if @project.export_file_exists? - send_upload(@project.export_file, attachment: @project.export_file.filename) + if @project.export_archive_exists? + send_upload(@project.export_file, attachment: @project.export_file.filename) + else + redirect_to( + edit_project_path(@project, anchor: 'js-export-project'), + alert: _("The file containing the export is not available yet; it may still be transferring. Please try again later.") + ) + end else redirect_to( edit_project_path(@project, anchor: 'js-export-project'), diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 46414b49f37..efdad22fa54 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -338,6 +338,8 @@ module ApplicationSettingsHelper :version_check_enabled, :web_ide_clientside_preview_enabled, :diff_max_patch_bytes, + :diff_max_files, + :diff_max_lines, :commit_email_hostname, :protected_ci_variables, :local_markdown_version, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 65800e40d6c..f8047ed9b78 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -273,6 +273,18 @@ class ApplicationSetting < ApplicationRecord greater_than_or_equal_to: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, less_than_or_equal_to: Gitlab::Git::Diff::MAX_PATCH_BYTES_UPPER_BOUND } + validates :diff_max_files, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_FILES_SETTING_UPPER_BOUND } + + validates :diff_max_lines, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_LINES_SETTING_UPPER_BOUND } + validates :user_default_internal_regex, js_regex: true, allow_nil: true validates :personal_access_token_prefix, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index bf9df3b9efc..b613e698471 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -60,6 +60,8 @@ module ApplicationSettingImplementation default_projects_limit: Settings.gitlab['default_projects_limit'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, + diff_max_files: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + diff_max_lines: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, disable_feed_token: false, disabled_oauth_sign_in_sources: [], dns_rebinding_protection_enabled: true, diff --git a/app/models/commit.rb b/app/models/commit.rb index 23c1dffcc63..a1ed5eb9ab9 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -33,6 +33,12 @@ class Commit # Used by GFM to match and present link extensions on node texts and hrefs. LINK_EXTENSION_PATTERN = /(patch)/.freeze + DEFAULT_MAX_DIFF_LINES_SETTING = 50_000 + DEFAULT_MAX_DIFF_FILES_SETTING = 1_000 + MAX_DIFF_LINES_SETTING_UPPER_BOUND = 100_000 + MAX_DIFF_FILES_SETTING_UPPER_BOUND = 3_000 + DIFF_SAFE_LIMIT_FACTOR = 10 + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :full_title, pipeline: :single_line, limit: 1.kilobyte cache_markdown_field :description, pipeline: :commit_description, limit: 1.megabyte @@ -78,20 +84,24 @@ class Commit end def diff_safe_lines(project: nil) - Gitlab::Git::DiffCollection.default_limits(project: project)[:max_lines] + diff_safe_max_lines(project: project) end - def diff_hard_limit_files(project: nil) + def diff_max_files(project: nil) if Feature.enabled?(:increased_diff_limits, project) 3000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_files else 1000 end end - def diff_hard_limit_lines(project: nil) + def diff_max_lines(project: nil) if Feature.enabled?(:increased_diff_limits, project) 100000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_lines else 50000 end @@ -99,11 +109,19 @@ class Commit def max_diff_options(project: nil) { - max_files: diff_hard_limit_files(project: project), - max_lines: diff_hard_limit_lines(project: project) + max_files: diff_max_files(project: project), + max_lines: diff_max_lines(project: project) } end + def diff_safe_max_files(project: nil) + diff_max_files(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + + def diff_safe_max_lines(project: nil) + diff_max_lines(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + def from_hash(hash, container) raw_commit = Gitlab::Git::Commit.new(container.repository.raw, hash) new(raw_commit, container) diff --git a/app/models/group.rb b/app/models/group.rb index 6c04bbdb032..1ce2fc18979 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -647,13 +647,17 @@ class Group < Namespace end def export_file_exists? - export_file&.file + import_export_upload&.export_file_exists? end def export_file import_export_upload&.export_file end + def export_archive_exists? + import_export_upload&.export_archive_exists? + end + def adjourned_deletion? false end diff --git a/app/models/import_export_upload.rb b/app/models/import_export_upload.rb index fce4ef40902..bc363cce8dd 100644 --- a/app/models/import_export_upload.rb +++ b/app/models/import_export_upload.rb @@ -11,10 +11,42 @@ class ImportExportUpload < ApplicationRecord mount_uploader :import_file, ImportExportUploader mount_uploader :export_file, ImportExportUploader + # This causes CarrierWave v1 and v3 (but not v2) to upload the file to + # object storage *after* the database entry has been committed to the + # database. This avoids idling in a transaction. + if Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_STORE_EXPORT_FILE_AFTER_COMMIT', true)) + skip_callback :save, :after, :store_export_file! + set_callback :commit, :after, :store_export_file! + end + scope :updated_before, ->(date) { where('updated_at < ?', date) } scope :with_export_file, -> { where.not(export_file: nil) } def retrieve_upload(_identifier, paths) Upload.find_by(model: self, path: paths) end + + def export_file_exists? + !!carrierwave_export_file + end + + # This checks if the export archive is actually stored on disk. It + # requires a HEAD request if object storage is used. + def export_archive_exists? + !!carrierwave_export_file&.exists? + # Handle any HTTP unexpected error + # https://github.com/excon/excon/blob/bbb5bd791d0bb2251593b80e3bce98dbec6e8f24/lib/excon/error.rb#L129-L169 + rescue Excon::Error => e + # The HEAD request will fail with a 403 Forbidden if the file does not + # exist, and the user does not have permission to list the object + # storage bucket. + Gitlab::ErrorTracking.track_exception(e) + false + end + + private + + def carrierwave_export_file + export_file&.file + end end diff --git a/app/models/project.rb b/app/models/project.rb index 3a89a85d65d..a28389c359f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1987,7 +1987,11 @@ class Project < ApplicationRecord end def export_file_exists? - export_file&.file + import_export_upload&.export_file_exists? + end + + def export_archive_exists? + import_export_upload&.export_archive_exists? end def export_file diff --git a/app/views/admin/application_settings/_diff_limits.html.haml b/app/views/admin/application_settings/_diff_limits.html.haml index 5351ac5abd1..6a51d2e39d4 100644 --- a/app/views/admin/application_settings/_diff_limits.html.haml +++ b/app/views/admin/application_settings/_diff_limits.html.haml @@ -3,11 +3,30 @@ %fieldset .form-group - = f.label :diff_max_patch_bytes, _('Maximum diff patch size in bytes'), class: 'label-light' + = f.label :diff_max_patch_bytes, _('Maximum diff patch size (Bytes)'), class: 'label-light' = f.number_field :diff_max_patch_bytes, class: 'form-control gl-form-input' %span.form-text.text-muted - = _("Collapse diffs larger than this size, and show a 'too large' message instead.") + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") = link_to sprite_icon('question-o'), - help_page_path('user/admin_area/diff_limits') + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_files, _('Maximum files in a diff'), class: 'label-light' + = f.number_field :diff_max_files, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_lines, _('Maximum lines in a diff'), class: 'label-light' + = f.number_field :diff_max_lines, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') = f.submit _('Save changes'), class: 'gl-button btn btn-confirm' diff --git a/app/views/admin/services/_service_templates_deprecated_alert.html.haml b/app/views/admin/services/_service_templates_deprecated_alert.html.haml index 0cc44099049..eac2f9c7f4e 100644 --- a/app/views/admin/services/_service_templates_deprecated_alert.html.haml +++ b/app/views/admin/services/_service_templates_deprecated_alert.html.haml @@ -2,7 +2,9 @@ - settings_link_start = "<a href=\"#{integrations_admin_application_settings_path}\">".html_safe .gl-alert.gl-alert-danger.gl-mt-5{ role: 'alert' } - = sprite_icon('error', css_class: 'gl-alert-icon gl-alert-icon-no-title') - %h4.gl-alert-title= s_('AdminSettings|Service templates are deprecated and will be removed in GitLab 14.0.') - .gl-alert-body - = html_escape_once(s_("AdminSettings|You can't add new templates. To migrate or remove a Service template, create a new integration at %{settings_link_start}Settings > Integrations%{link_end}. Learn more about %{doc_link_start}Project integration management%{link_end}.")).html_safe % { settings_link_start: settings_link_start, doc_link_start: doc_link_start, link_end: '</a>'.html_safe } + .gl-alert-container + = sprite_icon('error', css_class: 'gl-alert-icon gl-alert-icon-no-title') + .gl-alert-content + %h4.gl-alert-title= s_('AdminSettings|Service templates are deprecated and will be removed in GitLab 14.0.') + .gl-alert-body + = html_escape_once(s_("AdminSettings|You can't add new templates. To migrate or remove a Service template, create a new integration at %{settings_link_start}Settings > Integrations%{link_end}. Learn more about %{doc_link_start}Project integration management%{link_end}.")).html_safe % { settings_link_start: settings_link_start, doc_link_start: doc_link_start, link_end: '</a>'.html_safe } diff --git a/app/views/layouts/nav/projects_dropdown/_show.html.haml b/app/views/layouts/nav/projects_dropdown/_show.html.haml index 85a5486ccdc..46070975566 100644 --- a/app/views/layouts/nav/projects_dropdown/_show.html.haml +++ b/app/views/layouts/nav/projects_dropdown/_show.html.haml @@ -18,10 +18,10 @@ - experiment(:new_repo, user: current_user) do |e| - e.use do = nav_link(path: 'projects/new#blank_project', html_options: { class: 'gl-border-0 gl-border-t-1 gl-border-solid gl-border-gray-100' }) do - = link_to new_project_path(anchor: 'blank_project'), data: { track_label: "projects_dropdown_blank_project", track_event: "click_link", track_experiment: "new_repo" } do + = link_to new_project_path(anchor: 'blank_project'), data: { track_label: "projects_dropdown_blank_project", track_event: "click_link", track_experiment: "new_repo", qa_selector: "create_project_link" } do = _('Create blank project') = nav_link(path: 'projects/new#import_project') do - = link_to new_project_path(anchor: 'import_project'), data: { track_label: "projects_dropdown_import_project", track_event: "click_link", track_experiment: "new_repo" } do + = link_to new_project_path(anchor: 'import_project'), data: { track_label: "projects_dropdown_import_project", track_event: "click_link", track_experiment: "new_repo", qa_selector: "import_project_link" } do = _('Import project') - e.try do = nav_link(path: 'projects/new#blank_project', html_options: { class: 'gl-border-0 gl-border-t-1 gl-border-solid gl-border-gray-100' }) do diff --git a/config/feature_flags/development/configurable_diff_limits.yml b/config/feature_flags/development/configurable_diff_limits.yml new file mode 100644 index 00000000000..e73d45fac65 --- /dev/null +++ b/config/feature_flags/development/configurable_diff_limits.yml @@ -0,0 +1,8 @@ +--- +name: configurable_diff_limits +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56722 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332194 +milestone: '14.0' +type: development +group: group::code review +default_enabled: false diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 06d2929cd54..c8ea494bc41 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -2,15 +2,21 @@ require 'digest/md5' -MESSAGE = <<MARKDOWN +REVIEW_ROULETTE_SECTION = <<MARKDOWN ## Reviewer roulette +MARKDOWN + +CATEGORY_TABLE = <<MARKDOWN + +Changes that require review have been detected! -Changes that require review have been detected! A merge request is normally -reviewed by both a reviewer and a maintainer in its primary category (e.g. -~frontend or ~backend), and by a maintainer in all other categories. +Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category: + +| Category | Reviewer | Maintainer | +| -------- | -------- | ---------- | MARKDOWN -CATEGORY_TABLE_HEADER = <<MARKDOWN +POST_TABLE_MESSAGE = <<MARKDOWN To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to @@ -26,12 +32,15 @@ Please consider assigning a reviewer or maintainer who is a Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you. +MARKDOWN -| Category | Reviewer | Maintainer | -| -------- | -------- | ---------- | +NO_SUGGESTIONS = <<MARKDOWN + +There are no reviewer and maintainer suggestions for the changes in this MR. MARKDOWN UNKNOWN_FILES_MESSAGE = <<MARKDOWN +### Uncategorised files These files couldn't be categorised, so Danger was unable to suggest a reviewer. Please consider creating a merge request to @@ -99,8 +108,14 @@ if changes.any? markdown_row_for_spins(spin.category, [spin]) end - markdown(MESSAGE) - markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty? + markdown(REVIEW_ROULETTE_SECTION) + + if rows.empty? + markdown(NO_SUGGESTIONS) + else + markdown(CATEGORY_TABLE + rows.join("\n")) + markdown(POST_TABLE_MESSAGE) + end unknown = changes.fetch(:unknown, []) markdown(UNKNOWN_FILES_MESSAGE + helper.markdown_list(unknown)) unless unknown.empty? diff --git a/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb b/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb new file mode 100644 index 00000000000..9c1cd94dbaa --- /dev/null +++ b/db/migrate/20210527133919_add_diff_max_lines_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDiffMaxLinesToApplicationSettings < ActiveRecord::Migration[6.0] + def change + add_column(:application_settings, + :diff_max_lines, + :integer, + default: 50000, + null: false) + end +end diff --git a/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb b/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb new file mode 100644 index 00000000000..60b1f74cfd0 --- /dev/null +++ b/db/migrate/20210527134019_add_diff_max_files_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDiffMaxFilesToApplicationSettings < ActiveRecord::Migration[6.0] + def change + add_column(:application_settings, + :diff_max_files, + :integer, + default: 1000, + null: false) + end +end diff --git a/db/schema_migrations/20210527133919 b/db/schema_migrations/20210527133919 new file mode 100644 index 00000000000..559860de55d --- /dev/null +++ b/db/schema_migrations/20210527133919 @@ -0,0 +1 @@ +aaf5936c945451fa98df7c21ab34c9aa7190dcf301f536c259e5b1fe54407f36
\ No newline at end of file diff --git a/db/schema_migrations/20210527134019 b/db/schema_migrations/20210527134019 new file mode 100644 index 00000000000..de757dd355e --- /dev/null +++ b/db/schema_migrations/20210527134019 @@ -0,0 +1 @@ +ac4522ee51d4a4cda317b680c16be3d9ef3e1619bba80c26aefe8d5dc70f013c
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 549a09ebc60..3b681ff6cdb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9517,6 +9517,8 @@ CREATE TABLE application_settings ( elasticsearch_username text, encrypted_elasticsearch_password bytea, encrypted_elasticsearch_password_iv bytea, + diff_max_lines integer DEFAULT 50000 NOT NULL, + diff_max_files integer DEFAULT 1000 NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), diff --git a/doc/api/settings.md b/doc/api/settings.md index 76ae0174496..8225713fc00 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -251,7 +251,9 @@ listed in the descriptions of the relevant settings. | `default_projects_limit` | integer | no | Project limit per user. Default is `100000`. | | `default_snippet_visibility` | string | no | What visibility level new snippets receive. Can take `private`, `internal` and `public` as a parameter. Default is `private`. | | `deletion_adjourned_period` | integer | no | **(PREMIUM SELF)** The number of days to wait before deleting a project or group that is marked for deletion. Value must be between 0 and 90. -| `diff_max_patch_bytes` | integer | no | Maximum diff patch size (Bytes). | +| `diff_max_patch_bytes` | integer | no | Maximum [diff patch size](../user/admin_area/diff_limits.md), in bytes. | +| `diff_max_files` | integer | no | Maximum [files in a diff](../user/admin_area/diff_limits.md). | +| `diff_max_lines` | integer | no | Maximum [lines in a diff](../user/admin_area/diff_limits.md). | | `disable_feed_token` | boolean | no | Disable display of RSS/Atom and calendar feed tokens ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/231493) in GitLab 13.7) | | `disabled_oauth_sign_in_sources` | array of strings | no | Disabled OAuth sign-in sources. | | `dns_rebinding_protection_enabled` | boolean | no | Enforce DNS rebinding attack protection. | diff --git a/doc/user/admin_area/diff_limits.md b/doc/user/admin_area/diff_limits.md index c6c2190b7c7..37fdb3ae195 100644 --- a/doc/user/admin_area/diff_limits.md +++ b/doc/user/admin_area/diff_limits.md @@ -12,17 +12,25 @@ You can set a maximum size for display of diff files (patches). For details about diff files, [view changes between files](../project/merge_requests/changes.md). Read more about the [built-in limits for merge requests and diffs](../../administration/instance_limits.md#merge-requests). -## Maximum diff patch size +## Configure diff limits -Diff files which exceed this value are presented as 'too large' and cannot -be expandable. Instead of an expandable view, a link to the blob view is -shown. +WARNING: +These settings are experimental. An increased maximum increases resource +consumption of your instance. Keep this in mind when adjusting the maximum. + +To speed the loading time of merge request views and branch comparison views +on your instance, you can configure three instance-level maximum values for diffs: + +- **Maximum diff patch size**: The total size, in bytes, of the entire diff. +- **Maximum diff files**: The total number of files changed in a diff. +- **Maximum diff files**: The total number of files changed in a diff. The default value is 1000. +- **Maximum diff lines**: The total number of lines changed in a diff. The default value is 50,000. -Patches greater than 10% of this size are automatically collapsed, and a -link to expand the diff is presented. -This affects merge requests and branch comparison views. +When a diff reaches 10% of any of these values, the files are shown in a +collapsed view, with a link to expand the diff. Diffs that exceed any of the +set values are presented as **Too large** are cannot be expanded in the UI. -To set the maximum diff patch size: +To configure these values: 1. On the top bar, select **Menu >** **{admin}** **Admin**. 1. In the left sidebar, select **Settings > General**. @@ -30,10 +38,6 @@ To set the maximum diff patch size: 1. Enter a value for **Maximum diff patch size**, measured in bytes. 1. Select **Save changes**. -WARNING: -This setting is experimental. An increased maximum increases resource -consumption of your instance. Keep this in mind when adjusting the maximum. - <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index 63460763533..70b64fb7e1e 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -170,6 +170,8 @@ the site during a scan could lead to inaccurate results. #### Include the DAST template +> This template was [updated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62597) to DAST_VERSION: 2 in GitLab 14.0. + If you want to manually add DAST to your application, the DAST job is defined in a CI/CD template file. Updates to the template are provided with GitLab upgrades, allowing you to benefit from any improvements and additions. diff --git a/doc/user/group/epics/epic_boards.md b/doc/user/group/epics/epic_boards.md index 343f7c496b1..2f9dc27d87f 100644 --- a/doc/user/group/epics/epic_boards.md +++ b/doc/user/group/epics/epic_boards.md @@ -6,17 +6,12 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Epic Boards **(PREMIUM)** -> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2864) in GitLab 13.10. -> - It's [deployed behind a feature flag](../../feature_flags.md), disabled by default. -> - It's disabled on GitLab.com. -> - It's not recommended for production use. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](../../../administration/feature_flags.md). - -WARNING: -This feature might not be available to you. Check the **version history** note above for details. - -The GitLab Epic Board is a software project management tool used to plan, -organize, and visualize a workflow for a feature or product release. +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5067) in GitLab 13.10. +> - [Deployed behind a feature flag](../../feature_flags.md), disabled by default. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/290039) in GitLab 14.0. +> - Enabled on GitLab.com. +> - Recommended for production use. +> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](../../../administration/feature_flags.md). Epic boards build on the existing [epic tracking functionality](index.md) and [labels](../../project/labels.md). Your epics appear as cards in vertical lists, organized by their assigned @@ -24,45 +19,156 @@ labels. To view an epic board, in a group, select **Epics > Boards**. -![GitLab epic board - Premium](img/epic_board_v13_10.png) +![GitLab epic board - Premium](img/epic_board_v14_0.png) ## Create an epic board +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. + To create a new epic board: -1. Select the dropdown with the current board name in the upper left corner of the Epic Boards page. +1. Go to your group and select **Epics > Boards**. +1. In the upper left corner, select the dropdown with the current board name. 1. Select **Create new board**. -1. Enter the new board's name and select **Create**. +1. Enter the new board's title. +1. Optional. To hide the Open or Closed lists, clear the **Show the Open list** and + **Show the Closed list** checkboxes. +1. Optional. Set board scope: + 1. Next to **Scope**, select **Expand**. + 1. Next to **Labels**, select **Edit** and select the labels to use as board scope. +1. Select **Create board**. + +Now you can [add some lists](#create-a-new-list). +To change these options later, [edit the board](#edit-the-scope-of-an-epic-board). + +## Delete an epic board + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5079) in GitLab 14.0. + +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. +- A minimum of two boards present in a group. + +To delete the active epic board: + +1. Select the dropdown with the current board name in the upper left corner of the Epic Boards page. +1. Select **Delete board**. +1. Select **Delete**. + +## Actions you can take on an epic board + +- [Create a new list](#create-a-new-list). +- [Remove an existing list](#remove-a-list). +- [Filter epics](#filter-epics). +- Create workflows, like when using [issue boards](../../project/issue_board.md#create-workflows). +- [Move epics and lists](#move-epics-and-lists). +- Change epic labels (by dragging an epic between lists). +- Close an epic (by dragging it to the **Closed** list). +- [Edit the scope of a board](#edit-the-scope-of-an-epic-board). + +### Create a new list + +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. + +To create a new list: + +1. Go to your group and select **Epics > Boards**. +1. In the upper-right corner, select **Create list**. +1. In the **New list** column expand the **Select a label** dropdown and select the label to use as + list scope. +1. Select **Add to board**. + +### Remove a list + +Removing a list doesn't have any effect on epics and labels, as it's just the +list view that's removed. You can always create it again later if you need. + +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. + +To remove a list from an epic board: -## Limitations of epic boards +1. On the top of the list you want to remove, select the **List settings** icon (**{settings}**). + The list settings sidebar opens on the right. +1. Select **Remove list**. A confirmation dialog appears. +1. Select **OK**. -As of GitLab 13.10, these limitations apply: +### Filter epics -- Epic Boards need to be enabled by an administrator. -- Epic Boards can be created but not deleted. -- Lists can be added to the board but not deleted. -- There is no sidebar on the board. To edit an epic, go to the epic's page. -- There is no drag and drop support yet. To move an epic between lists, edit epic labels on the epic's page. -- Epics cannot be re-ordered within the list. +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5079) in GitLab 14.0. -To learn more about the future iterations of this feature, visit -[epic 5067](https://gitlab.com/groups/gitlab-org/-/epics/5067). +Use the filters on top of your epic board to show only +the results you want. It's similar to the filtering used in the epic list, +as the metadata from the epics and labels is re-used in the epic board. -## Enable or disable Epic Boards +You can filter by the following: -Epic Boards are under development and not ready for production use. It is -deployed behind a feature flag that is **disabled by default**. +- Author +- Label + +### Move epics and lists + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5079) in GitLab 14.0. + +You can move epics and lists by dragging them. + +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. + +To move an epic, select the epic card and drag it to another position in its current list or +into another list. Learn about possible effects in [Dragging epics between lists](#dragging-epics-between-lists). + +To move a list, select its top bar, and drag it horizontally. +You can't move the **Open** and **Closed** lists, but you can hide them when editing an epic board. + +#### Dragging epics between lists + +When you drag epics between lists, the result is different depending on the source list +and the target list. + +| | To Open | To Closed | To label B list | +| --------------------- | -------------- | ---------- | ------------------------------ | +| **From Open** | - | Close epic | Add label B | +| **From Closed** | Reopen epic | - | Reopen epic and add label B | +| **From label A list** | Remove label A | Close epic | Remove label A and add label B | + +### Edit the scope of an epic board + +Prerequisites: + +- A minimum of [Reporter](../../permissions.md#group-members-permissions) access to a group in GitLab. + +To edit the scope of an epic board: + +1. In the upper-right corner, select **Edit board**. +1. Optional: + - Edit the board's title. + - Show or hide the Open and Closed columns. + - Select other labels as the board's scope. +1. Select **Save changes**. + +## Enable or disable epic boards + +Epic boards are under development but ready for production use. +It is deployed behind a feature flag that is **enabled by default**. [GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) -can enable it. +can disable it. -To enable it: +To disable it: ```ruby -Feature.enable(:epic_boards) +Feature.disable(:epic_boards) ``` -To disable it: +To enable it: ```ruby -Feature.disable(:epic_boards) +Feature.enable(:epic_boards) ``` diff --git a/doc/user/group/epics/img/epic_board_v13_10.png b/doc/user/group/epics/img/epic_board_v13_10.png Binary files differdeleted file mode 100644 index 85a131ea605..00000000000 --- a/doc/user/group/epics/img/epic_board_v13_10.png +++ /dev/null diff --git a/doc/user/group/epics/img/epic_board_v14_0.png b/doc/user/group/epics/img/epic_board_v14_0.png Binary files differnew file mode 100644 index 00000000000..e6b4e40aa05 --- /dev/null +++ b/doc/user/group/epics/img/epic_board_v14_0.png diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 1de84abb31c..b8a01021145 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -263,8 +263,9 @@ The following table lists group permissions available for each role: | Browse group | ✓ | ✓ | ✓ | ✓ | ✓ | | View group wiki pages **(PREMIUM)** | ✓ (6) | ✓ | ✓ | ✓ | ✓ | | View Insights charts **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | -| View group epic **(PREMIUM)** | ✓ | ✓ | ✓ | ✓ | ✓ | -| Create/edit group epic **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | +| View group epic **(PREMIUM)** | ✓ | ✓ | ✓ | ✓ | ✓ | +| Create/edit group epic **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | +| Create/edit/delete epic boards **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | | Manage group labels | | ✓ | ✓ | ✓ | ✓ | | See a container registry | | ✓ | ✓ | ✓ | ✓ | | Pull [packages](packages/index.md) | | ✓ | ✓ | ✓ | ✓ | @@ -288,8 +289,8 @@ The following table lists group permissions available for each role: | Create/Delete group deploy tokens | | | | | ✓ | | Manage group members | | | | | ✓ | | Delete group | | | | | ✓ | -| Delete group epic **(PREMIUM)** | | | | | ✓ | -| Edit SAML SSO Billing **(PREMIUM SAAS)** | ✓ | ✓ | ✓ | ✓ | ✓ (4) | +| Delete group epic **(PREMIUM)** | | | | | ✓ | +| Edit SAML SSO Billing **(PREMIUM SAAS)** | ✓ | ✓ | ✓ | ✓ | ✓ (4) | | View group Audit Events | | | ✓ (7) | ✓ (7) | ✓ | | Disable notification emails | | | | | ✓ | | View Contribution analytics | ✓ | ✓ | ✓ | ✓ | ✓ | diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index 18fe4e6b194..d3135d2e777 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -34,11 +34,11 @@ boards in the same project. Different issue board features are available in different [GitLab tiers](https://about.gitlab.com/pricing/), as shown in the following table: -| Tier | Number of project issue boards | Number of [group issue boards](#group-issue-boards) | [Configurable issue boards](#configurable-issue-boards) | [Assignee lists](#assignee-lists) | -|------------------|--------------------------------|------------------------------|---------------------------|----------------| -| Free | Multiple | 1 | No | No | -| Premium | Multiple | Multiple | Yes | Yes | -| Ultimate | Multiple | Multiple | Yes | Yes | +| Tier | Number of project issue boards | Number of [group issue boards](#group-issue-boards) | [Configurable issue boards](#configurable-issue-boards) | [Assignee lists](#assignee-lists) | +| -------- | ------------------------------ | --------------------------------------------------- | ------------------------------------------------------- | --------------------------------- | +| Free | Multiple | 1 | No | No | +| Premium | Multiple | Multiple | Yes | Yes | +| Ultimate | Multiple | Multiple | Yes | Yes | To learn more, visit [GitLab Enterprise features for issue boards](#gitlab-enterprise-features-for-issue-boards) below. @@ -312,7 +312,7 @@ assignee list: 1. Search and select the user you want to add as an assignee. Now that the assignee list is added, you can assign or unassign issues to that user -by [dragging issues](#drag-issues-between-lists) to and from an assignee list. +by [moving issues](#move-issues-and-lists) to and from an assignee list. To remove an assignee list, just as with a label list, click the trash icon. ![Assignee lists](img/issue_board_assignee_lists_v13_6.png) @@ -328,7 +328,7 @@ milestone, giving you more freedom and visibility on the issue board. To add a m 1. Select the **Milestone** tab. 1. Search and click the milestone. -Like the assignee lists, you're able to [drag issues](#drag-issues-between-lists) +Like the assignee lists, you're able to [drag issues](#move-issues-and-lists) to and from a milestone list to manipulate the milestone of the dragged issues. As in other list types, click the trash icon to remove a list. @@ -355,7 +355,7 @@ iteration. To add an iteration list: 1. In the dropdown, select an iteration. 1. Select **Add to board**. -Like the milestone lists, you're able to [drag issues](#drag-issues-between-lists) +Like the milestone lists, you're able to [drag issues](#move-issues-and-lists) to and from a iteration list to manipulate the iteration of the dragged issues. ![Iteration lists](img/issue_board_iteration_lists_v13_10.png) @@ -380,7 +380,20 @@ To group issues by epic in an issue board: ![Epics Swimlanes](img/epics_swimlanes_v13.6.png) -You can also [drag issues](#drag-issues-between-lists) to change their position and epic assignment: +To edit an issue without leaving this view, select the issue card (not its title), and a sidebar +appears on the right. There you can see and edit the issue's: + +- Title +- Assignees +- Epic **PREMIUM** +- Milestone +- Time tracking value (view only) +- Due date +- Labels +- Weight +- Notifications setting + +You can also [drag issues](#move-issues-and-lists) to change their position and epic assignment: - To reorder an issue, drag it to the new position within a list. - To assign an issue to another epic, drag it to the epic's horizontal lane. @@ -435,11 +448,11 @@ This feature is only supported when using the [GraphQL-based boards](#graphql-ba - [Remove an issue from a list](#remove-an-issue-from-a-list). - [Filter issues](#filter-issues) that appear across your issue board. - [Create workflows](#create-workflows). -- [Drag issues between lists](#drag-issues-between-lists). +- [Move issues and lists](#move-issues-and-lists). - [Multi-select issue cards](#multi-select-issue-cards). - Drag and reorder the lists. - Change issue labels (by dragging an issue between lists). -- Close an issue (by dragging it to the **Done** list). +- Close an issue (by dragging it to the **Closed** list). If you're not able to do some of the things above, make sure you have the right [permissions](#permissions). @@ -483,12 +496,12 @@ You can now choose it to create a list. ### Remove a list Removing a list doesn't have any effect on issues and labels, as it's just the -list view that's removed. You can always restore it later if you need. +list view that's removed. You can always create it again later if you need. To remove a list from an issue board: -1. Select the **List settings** icon (**{settings}**) on the top of the list you want to remove. The - list settings sidebar opens on the right. +1. On the top of the list you want to remove, select the **List settings** icon (**{settings}**). + The list settings sidebar opens on the right. 1. Select **Remove list**. A confirmation dialog appears. 1. Select **OK**. @@ -582,16 +595,33 @@ to another list, the label changes and a system note is recorded. ![issue board system notes](img/issue_board_system_notes_v13_6.png) -### Drag issues between lists +### Move issues and lists + +You can move issues and lists by dragging them. + +Prerequisites: + +- A minimum of [Reporter](../permissions.md#project-members-permissions) access to a project in GitLab. + +To move an issue, select the issue card and drag it to another position in its current list or +into a different list. Learn about possible effects in [Dragging issues between lists](#dragging-issues-between-lists). + +To move a list, select its top bar, and drag it horizontally. +You can't move the **Open** and **Closed** lists, but you can hide them when editing an issue board. + +#### Dragging issues between lists + +To move an issue to another list, select the issue card and drag it onto that list. -When dragging issues between lists, different behavior occurs depending on the source list and the target list. +When you drag issues between lists, the result is different depending on the source list +and the target list. -| | To Open | To Closed | To label `B` list | To assignee `Bob` list | -| ------------------------------ | ------------------ | ------------ | ---------------------------- | ------------------------------------- | -| **From Open** | - | Issue closed | `B` added | `Bob` assigned | -| **From Closed** | Issue reopened | - | Issue reopened<br/>`B` added | Issue reopened<br/>`Bob` assigned | -| **From label `A` list** | `A` removed | Issue closed | `A` removed<br/>`B` added | `Bob` assigned | -| **From assignee `Alice` list** | `Alice` unassigned | Issue closed | `B` added | `Alice` unassigned<br/>`Bob` assigned | +| | To Open | To Closed | To label B list | To assignee Bob list | +| ---------------------------- | -------------- | ----------- | ------------------------------ | ----------------------------- | +| **From Open** | - | Close issue | Add label B | Assign Bob | +| **From Closed** | Reopen issue | - | Reopen issue and add label B | Reopen issue and assign Bob | +| **From label A list** | Remove label A | Close issue | Remove label A and add label B | Assign Bob | +| **From assignee Alice list** | Unassign Alice | Close issue | Add label B | Unassign Alice and assign Bob | ### Multi-select issue cards diff --git a/lib/api/group_export.rb b/lib/api/group_export.rb index 6134515032f..7e4fdba6033 100644 --- a/lib/api/group_export.rb +++ b/lib/api/group_export.rb @@ -23,7 +23,11 @@ module API check_rate_limit! :group_download_export, [current_user, user_group] if user_group.export_file_exists? - present_carrierwave_file!(user_group.export_file) + if user_group.export_archive_exists? + present_carrierwave_file!(user_group.export_file) + else + render_api_error!('The group export file is not available yet', 404) + end else render_api_error!('404 Not found or has expired', 404) end diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 76b3dea723a..4041e130f9e 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -30,7 +30,11 @@ module API check_rate_limit! :project_download_export, [current_user, user_project] if user_project.export_file_exists? - present_carrierwave_file!(user_project.export_file) + if user_project.export_archive_exists? + present_carrierwave_file!(user_project.export_file) + else + render_api_error!('The project export file is not available yet', 404) + end else render_api_error!('404 Not found or has expired', 404) end diff --git a/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml index b355b6e36a2..a2b112b8e9f 100644 --- a/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml @@ -10,7 +10,7 @@ stages: - dast variables: - DAST_VERSION: 1 + DAST_VERSION: 2 # Setting this variable will affect all Security templates # (SAST, Dependency Scanning, ...) SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers" diff --git a/lib/gitlab/ci/templates/Security/DAST.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/DAST.latest.gitlab-ci.yml index 60581d2a31b..6834766da3d 100644 --- a/lib/gitlab/ci/templates/Security/DAST.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/DAST.latest.gitlab-ci.yml @@ -17,7 +17,7 @@ # List of available variables: https://docs.gitlab.com/ee/user/application_security/dast/#available-variables variables: - DAST_VERSION: 1 + DAST_VERSION: 2 # Setting this variable will affect all Security templates # (SAST, Dependency Scanning, ...) SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers" diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index fb947c80b7e..631624c068c 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -12,11 +12,7 @@ module Gitlab delegate :max_files, :max_lines, :max_bytes, :safe_max_files, :safe_max_lines, :safe_max_bytes, to: :limits def self.default_limits(project: nil) - if Feature.enabled?(:increased_diff_limits, project) - { max_files: 300, max_lines: 10000 } - else - { max_files: 100, max_lines: 5000 } - end + { max_files: ::Commit.diff_safe_max_files(project: project), max_lines: ::Commit.diff_safe_max_lines(project: project) } end def self.limits(options = {}) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3058acadaeb..360f5bb729a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7969,9 +7969,6 @@ msgstr "" msgid "Collapse approvers" msgstr "" -msgid "Collapse diffs larger than this size, and show a 'too large' message instead." -msgstr "" - msgid "Collapse issues" msgstr "" @@ -11417,6 +11414,9 @@ msgstr "" msgid "Didn't receive unlock instructions?" msgstr "" +msgid "Diff files surpassing this limit will be presented as 'too large' and won't be expandable." +msgstr "" + msgid "Diff limits" msgstr "" @@ -20252,7 +20252,7 @@ msgstr "" msgid "Maximum delay (Minutes)" msgstr "" -msgid "Maximum diff patch size in bytes" +msgid "Maximum diff patch size (Bytes)" msgstr "" msgid "Maximum duration of a session." @@ -20276,6 +20276,9 @@ msgstr "" msgid "Maximum file size is 2MB. Please select a smaller file." msgstr "" +msgid "Maximum files in a diff" +msgstr "" + msgid "Maximum import size (MB)" msgstr "" @@ -20288,6 +20291,9 @@ msgstr "" msgid "Maximum lifetime allowable for Personal Access Tokens is active, your expire date must be set before %{maximum_allowable_date}." msgstr "" +msgid "Maximum lines in a diff" +msgstr "" + msgid "Maximum npm package file size in bytes" msgstr "" @@ -32425,6 +32431,9 @@ msgstr "" msgid "The errors we encountered were:" msgstr "" +msgid "The file containing the export is not available yet; it may still be transferring. Please try again later." +msgstr "" + msgid "The file has been successfully created." msgstr "" diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index fa39684f053..760741a9630 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -55,13 +55,17 @@ module QA end def go_to_projects - go_to_menu_dropdown_option(:projects_dropdown) - - within_element(:menu_subview_container) do + within_projects_menu do click_element(:menu_item_link, title: 'Your projects') end end + def go_to_create_project + within_projects_menu do + click_element(:menu_item_link, title: 'Create new project') + end + end + def go_to_menu_dropdown_option(option_name) within_top_menu do click_element(:navbar_dropdown, title: 'Menu') @@ -84,11 +88,11 @@ module QA def go_to_admin_area click_admin_area - if has_text?('Enter Admin Mode', wait: 1.0) - Admin::NewSession.perform do |new_session| - new_session.set_password(Runtime::User.admin_password) - new_session.click_enter_admin_mode - end + return unless has_text?('Enter Admin Mode', wait: 1.0) + + Admin::NewSession.perform do |new_session| + new_session.set_password(Runtime::User.admin_password) + new_session.click_enter_admin_mode end end @@ -166,19 +170,15 @@ module QA private - def within_top_menu - within_element(:navbar) do - yield - end + def within_top_menu(&block) + within_element(:navbar, &block) end - def within_user_menu + def within_user_menu(&block) within_top_menu do click_element :user_avatar - within_element(:user_menu) do - yield - end + within_element(:user_menu, &block) end end @@ -188,6 +188,12 @@ module QA within_element(:menu_subview_container, &block) end + def within_projects_menu(&block) + go_to_menu_dropdown_option(:projects_dropdown) + + within_element(:menu_subview_container, &block) + end + def click_admin_area go_to_menu_dropdown_option(:admin_area_link) end diff --git a/qa/qa/page/project/import/github.rb b/qa/qa/page/project/import/github.rb index 58c82fa14c1..ba87ea520dc 100644 --- a/qa/qa/page/project/import/github.rb +++ b/qa/qa/page/project/import/github.rb @@ -32,24 +32,20 @@ module QA end def import!(full_path, name) - unless already_imported(full_path) - choose_test_namespace(full_path) - set_path(full_path, name) - import_project(full_path) - wait_for_success - end + return if already_imported(full_path) - go_to_project(name) + choose_test_namespace(full_path) + set_path(full_path, name) + import_project(full_path) + wait_for_success end private - def within_repo_path(full_path) + def within_repo_path(full_path, &block) project_import_row = find_element(:project_import_row, text: full_path) - within(project_import_row) do - yield - end + within(project_import_row, &block) end def choose_test_namespace(full_path) @@ -75,8 +71,13 @@ module QA def wait_for_success # TODO: set reload:false and remove skip_finished_loading_check_on_refresh when # https://gitlab.com/gitlab-org/gitlab/-/issues/292861 is fixed - wait_until(max_duration: 60, sleep_interval: 5.0, reload: true, skip_finished_loading_check_on_refresh: true) do - page.has_no_content?('Importing 1 repository', wait: 3.0) + wait_until( + max_duration: 60, + sleep_interval: 5.0, + reload: true, + skip_finished_loading_check_on_refresh: true + ) do + page.has_no_content?('Importing 1 repository') end end diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 4b8e15387c3..d334b9e53da 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -10,10 +10,10 @@ module QA include Visibility attr_accessor :repository_storage # requires admin access - attr_writer :initialize_with_readme - attr_writer :auto_devops_enabled - attr_writer :github_personal_access_token - attr_writer :github_repository_path + attr_writer :initialize_with_readme, + :auto_devops_enabled, + :github_personal_access_token, + :github_repository_path attribute :id attribute :name @@ -40,15 +40,11 @@ module QA end attribute :repository_ssh_location do - Page::Project::Show.perform do |show| - show.repository_clone_ssh_location - end + Page::Project::Show.perform(&:repository_clone_ssh_location) end attribute :repository_http_location do - Page::Project::Show.perform do |show| - show.repository_clone_http_location - end + Page::Project::Show.perform(&:repository_clone_http_location) end def initialize @@ -104,7 +100,7 @@ module QA def has_file?(file_path) response = repository_tree - raise ResourceNotFoundError, "#{response[:message]}" if response.is_a?(Hash) && response.has_key?(:message) + raise ResourceNotFoundError, (response[:message]).to_s if response.is_a?(Hash) && response.has_key?(:message) response.any? { |file| file[:path] == file_path } end @@ -115,14 +111,14 @@ module QA def has_branches?(branches) branches.all? do |branch| - response = get(Runtime::API::Request.new(api_client, "#{api_repository_branches_path}/#{branch}").url) + response = get(request_url("#{api_repository_branches_path}/#{branch}")) response.code == HTTP_STATUS_OK end end def has_tags?(tags) tags.all? do |tag| - response = get(Runtime::API::Request.new(api_client, "#{api_repository_tags_path}/#{tag}").url) + response = get(request_url("#{api_repository_tags_path}/#{tag}")) response.code == HTTP_STATUS_OK end end @@ -183,6 +179,10 @@ module QA "#{api_get_path}/pipeline_schedules" end + def api_issues_path + "#{api_get_path}/issues" + end + def api_put_path "/projects/#{id}" end @@ -217,19 +217,28 @@ module QA def change_repository_storage(new_storage) put_body = { repository_storage: new_storage } - response = put Runtime::API::Request.new(api_client, api_put_path).url, put_body + response = put(request_url(api_put_path), put_body) unless response.code == HTTP_STATUS_OK - raise ResourceUpdateFailedError, "Could not change repository storage to #{new_storage}. Request returned (#{response.code}): `#{response}`." + raise( + ResourceUpdateFailedError, + "Could not change repository storage to #{new_storage}. Request returned (#{response.code}): `#{response}`." + ) end - wait_until(sleep_interval: 1) { Runtime::API::RepositoryStorageMoves.has_status?(self, 'finished', new_storage) } + wait_until(sleep_interval: 1) do + Runtime::API::RepositoryStorageMoves.has_status?(self, 'finished', new_storage) + end rescue Support::Repeater::RepeaterConditionExceededError - raise Runtime::API::RepositoryStorageMoves::RepositoryStorageMovesError, 'Timed out while waiting for the repository storage move to finish' + raise( + Runtime::API::RepositoryStorageMoves::RepositoryStorageMovesError, + 'Timed out while waiting for the repository storage move to finish' + ) end def commits - parse_body(get(Runtime::API::Request.new(api_client, api_commits_path).url)) + response = get(request_url(api_commits_path)) + parse_body(response) end def default_branch @@ -237,7 +246,7 @@ module QA end def import_status - response = get Runtime::API::Request.new(api_client, "/projects/#{id}/import").url + response = get(request_url("/projects/#{id}/import")) unless response.code == HTTP_STATUS_OK raise ResourceQueryError, "Could not get import status. Request returned (#{response.code}): `#{response}`." @@ -251,7 +260,8 @@ module QA end def merge_requests - parse_body(get(Runtime::API::Request.new(api_client, api_merge_requests_path).url)) + response = get(request_url(api_merge_requests_path)) + parse_body(response) end def merge_request_with_title(title) @@ -260,42 +270,52 @@ module QA def runners(tag_list: nil) response = if tag_list - get Runtime::API::Request.new(api_client, "#{api_runners_path}?tag_list=#{tag_list.compact.join(',')}", per_page: '100').url + get(request_url("#{api_runners_path}?tag_list=#{tag_list.compact.join(',')}", per_page: '100')) else - get Runtime::API::Request.new(api_client, "#{api_runners_path}", per_page: '100').url + get(request_url(api_runners_path, per_page: '100')) end parse_body(response) end def registry_repositories - response = get Runtime::API::Request.new(api_client, "#{api_registry_repositories_path}").url + response = get(request_url(api_registry_repositories_path)) parse_body(response) end def packages - response = get Runtime::API::Request.new(api_client, "#{api_packages_path}").url + response = get(request_url(api_packages_path)) parse_body(response) end def repository_branches - parse_body(get(Runtime::API::Request.new(api_client, api_repository_branches_path).url)) + response = get(request_url(api_repository_branches_path)) + parse_body(response) end def repository_tags - parse_body(get(Runtime::API::Request.new(api_client, api_repository_tags_path).url)) + response = get(request_url(api_repository_tags_path)) + parse_body(response) end def repository_tree - parse_body(get(Runtime::API::Request.new(api_client, api_repository_tree_path).url)) + response = get(request_url(api_repository_tree_path)) + parse_body(response) end def pipelines - parse_body(get(Runtime::API::Request.new(api_client, api_pipelines_path).url)) + response = get(request_url(api_pipelines_path)) + parse_body(response) end def pipeline_schedules - parse_body(get(Runtime::API::Request.new(api_client, api_pipeline_schedules_path).url)) + response = get(request_url(api_pipeline_schedules_path)) + parse_body(response) + end + + def issues + response = get(request_url(api_issues_path)) + parse_body(response) end private @@ -307,6 +327,14 @@ module QA Git::Location.new(api_resource[:http_url_to_repo]) api_resource end + + # Get api request url + # + # @param [String] path + # @return [String] + def request_url(path, **opts) + Runtime::API::Request.new(api_client, path, **opts).url + end end end end diff --git a/qa/qa/resource/project_imported_from_github.rb b/qa/qa/resource/project_imported_from_github.rb index b06a7fe4e3d..93cd166a191 100644 --- a/qa/qa/resource/project_imported_from_github.rb +++ b/qa/qa/resource/project_imported_from_github.rb @@ -7,23 +7,19 @@ module QA class ProjectImportedFromGithub < Resource::Project def fabricate! self.import = true - super - group.visit! + Page::Main::Menu.perform(&:go_to_create_project) - Page::Group::Show.perform(&:go_to_new_project) - go_to_import_page - Page::Project::New.perform(&:click_github_link) + Page::Project::New.perform do |project_page| + project_page.click_import_project + project_page.click_github_link + end Page::Project::Import::Github.perform do |import_page| import_page.add_personal_access_token(@github_personal_access_token) import_page.import!(@github_repository_path, @name) end end - - def go_to_import_page - Page::Project::New.perform(&:click_import_project) - end end end end diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/import_github_repo_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/import_github_repo_spec.rb index 5072b6d48bf..9d36dbd5cc4 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/import_github_repo_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/import_github_repo_spec.rb @@ -3,25 +3,26 @@ module QA RSpec.describe 'Manage', :github, :requires_admin do describe 'Project import' do + let!(:api_client) { Runtime::API::Client.as_admin } + let!(:group) { Resource::Group.fabricate_via_api! { |resource| resource.api_client = api_client } } let!(:user) do Resource::User.fabricate_via_api! do |resource| - resource.api_client = Runtime::API::Client.as_admin + resource.api_client = api_client + resource.hard_delete_on_api_removal = true end end - let(:group) { Resource::Group.fabricate_via_api! } - let(:imported_project) do Resource::ProjectImportedFromGithub.fabricate_via_browser_ui! do |project| project.name = 'imported-project' project.group = group project.github_personal_access_token = Runtime::Env.github_access_token project.github_repository_path = 'gitlab-qa-github/test-project' + project.api_client = api_client end end before do - Runtime::Feature.enable(:invite_members_group_modal, group: group) group.add_member(user, Resource::Members::AccessLevel::MAINTAINER) end @@ -32,90 +33,49 @@ module QA it 'imports a GitHub repo', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1762' do Flow::Login.sign_in(as: user) - imported_project # import the project + imported_project.reload! # import the project and reload all fields - Page::Main::Menu.perform(&:go_to_projects) - Page::Dashboard::Projects.perform do |dashboard| - dashboard.go_to_project(imported_project.name) + aggregate_failures do + verify_repository_import + verify_issues_import + verify_merge_requests_import end - - Page::Project::Show.perform(&:wait_for_import) - - verify_repository_import - verify_issues_import - verify_merge_requests_import - verify_labels_import - verify_milestones_import - verify_wiki_import end def verify_repository_import - Page::Project::Show.perform do |project| - expect(project).to have_content('This test project is used for automated GitHub import by GitLab QA.') - expect(project).to have_content(imported_project.name) - end + expect(imported_project.api_response).to include( + description: 'A new repo for test', + import_status: 'finished', + import_error: nil + ) end def verify_issues_import - QA::Support::Retrier.retry_on_exception do - Page::Project::Menu.perform(&:click_issues) - - Page::Project::Issue::Show.perform do |issue_page| - expect(issue_page).to have_content('This is a sample issue') - - click_link 'This is a sample issue' - - expect(issue_page).to have_content('This is a sample first comment') - - # Comments - comment_text = 'This is a comment from @sliaquat' - - expect(issue_page).to have_comment(comment_text) - expect(issue_page).to have_label('custom new label') - expect(issue_page).to have_label('help wanted') - expect(issue_page).to have_label('good first issue') - end - end + issues = imported_project.issues + + expect(issues.length).to eq(1) + expect(issues.first).to include( + title: 'This is a sample issue', + description: "*Created by: gitlab-qa-github*\n\nThis is a sample first comment", + labels: ['custom new label', 'good first issue', 'help wanted'], + user_notes_count: 1 + ) end def verify_merge_requests_import - Page::Project::Menu.perform(&:click_merge_requests) - - Page::MergeRequest::Show.perform do |merge_request| - expect(merge_request).to have_content('Improve readme') - - click_link 'Improve readme' - - expect(merge_request).to have_content('This improves the README file a bit.') - - # Comments - expect(merge_request).to have_content('[PR comment by @sliaquat] Nice work!') - - # Diff comments - expect(merge_request).to have_content('[Single diff comment] Good riddance') - expect(merge_request).to have_content('[Single diff comment] Nice addition') - - expect(merge_request).to have_label('bug') - expect(merge_request).to have_label('documentation') - end - end - - def verify_labels_import - # TODO: Waiting on https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/19228 - # to build upon it. - end - - def verify_milestones_import - # TODO: Waiting on https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/18727 - # to build upon it. - end - - def verify_wiki_import - Page::Project::Menu.perform(&:click_wiki) - - Page::Project::Wiki::Show.perform do |wiki| - expect(wiki).to have_content('Welcome to the test-project wiki!') - end + merge_requests = imported_project.merge_requests + + expect(merge_requests.length).to eq(1) + expect(merge_requests.first).to include( + title: 'Improve readme', + state: 'opened', + target_branch: 'main', + source_branch: 'improve-readme', + labels: %w[bug documentation], + description: <<~DSC.strip + *Created by: gitlab-qa-github*\n\nThis improves the README file a bit.\r\n\r\nTODO:\r\n\r\n \r\n\r\n- [ ] Do foo\r\n- [ ] Make bar\r\n - [ ] Think about baz + DSC + ) end end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 07d8332bfd0..91b11cd46c5 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -1065,14 +1065,13 @@ RSpec.describe GroupsController, factory_default: :keep do describe 'GET #download_export' do let(:admin) { create(:admin) } + let(:export_file) { fixture_file_upload('spec/fixtures/group_export.tar.gz') } before do enable_admin_mode!(admin) end context 'when there is a file available to download' do - let(:export_file) { fixture_file_upload('spec/fixtures/group_export.tar.gz') } - before do sign_in(admin) create(:import_export_upload, group: group, export_file: export_file) @@ -1085,6 +1084,22 @@ RSpec.describe GroupsController, factory_default: :keep do end end + context 'when the file is no longer present on disk' do + before do + sign_in(admin) + + create(:import_export_upload, group: group, export_file: export_file) + group.export_file.file.delete + end + + it 'returns not found' do + get :download_export, params: { id: group.to_param } + + expect(flash[:alert]).to include('file containing the export is not available yet') + expect(response).to redirect_to(edit_group_path(group)) + end + end + context 'when there is no file available to download' do before do sign_in(admin) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 398aedfd8e2..ce229fb861a 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -7,7 +7,7 @@ RSpec.describe ProjectsController do include ProjectForksHelper using RSpec::Parameterized::TableSyntax - let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) } + let_it_be(:project, reload: true) { create(:project, :with_export, service_desk_enabled: false) } let_it_be(:public_project) { create(:project, :public) } let_it_be(:user) { create(:user) } @@ -1349,7 +1349,7 @@ RSpec.describe ProjectsController do end end - describe '#download_export' do + describe '#download_export', :clean_gitlab_redis_cache do let(:action) { :download_export } context 'object storage enabled' do @@ -1361,6 +1361,17 @@ RSpec.describe ProjectsController do end end + context 'when project export file is absent' do + it 'alerts the user and returns 302' do + project.export_file.file.delete + + get action, params: { namespace_id: project.namespace, id: project } + + expect(flash[:alert]).to include('file containing the export is not available yet') + expect(response).to have_gitlab_http_status(:found) + end + end + context 'when project export is disabled' do before do stub_application_setting(project_export_enabled?: false) diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 5d232a9d09a..bd6e37c1cef 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -58,6 +58,13 @@ FactoryBot.define do shared_runners_enabled { false } end + trait :with_export do + after(:create) do |group, _evaluator| + export_file = fixture_file_upload('spec/fixtures/group_export.tar.gz') + create(:import_export_upload, group: group, export_file: export_file) + end + end + trait :allow_descendants_override_disabled_shared_runners do allow_descendants_override_disabled_shared_runners { true } end diff --git a/spec/frontend/diffs/components/diff_stats_spec.js b/spec/frontend/diffs/components/diff_stats_spec.js index 504158fb7fc..52e68b35889 100644 --- a/spec/frontend/diffs/components/diff_stats_spec.js +++ b/spec/frontend/diffs/components/diff_stats_spec.js @@ -1,6 +1,7 @@ import { GlIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import DiffStats from '~/diffs/components/diff_stats.vue'; +import mockDiffFile from '../mock_data/diff_file'; const TEST_ADDED_LINES = 100; const TEST_REMOVED_LINES = 200; @@ -38,9 +39,37 @@ describe('diff_stats', () => { }); }); + describe('bytes changes', () => { + let file; + const getBytesContainer = () => wrapper.find('.diff-stats > div:first-child'); + + beforeEach(() => { + file = { + ...mockDiffFile, + viewer: { + ...mockDiffFile.viewer, + name: 'not_diffable', + }, + }; + + createComponent({ diffFile: file }); + }); + + it("renders the bytes changes instead of line changes when the file isn't diffable", () => { + const content = getBytesContainer(); + + expect(content.classes('cgreen')).toBe(true); + expect(content.text()).toBe('+1.00 KiB (+100%)'); + }); + }); + describe('line changes', () => { const findFileLine = (name) => wrapper.find(name); + beforeEach(() => { + createComponent(); + }); + it('shows the amount of lines added', () => { expect(findFileLine('.js-file-addition-line').text()).toBe(TEST_ADDED_LINES.toString()); }); diff --git a/spec/frontend/diffs/mock_data/diff_file.js b/spec/frontend/diffs/mock_data/diff_file.js index cef776c885a..9ebcd5ef26b 100644 --- a/spec/frontend/diffs/mock_data/diff_file.js +++ b/spec/frontend/diffs/mock_data/diff_file.js @@ -19,6 +19,8 @@ export default { renamed_file: false, old_path: 'CHANGELOG', new_path: 'CHANGELOG', + old_size: 1024, + new_size: 2048, mode_changed: false, a_mode: '100644', b_mode: '100644', diff --git a/spec/frontend/diffs/utils/diff_file_spec.js b/spec/frontend/diffs/utils/diff_file_spec.js index c6cfdfced65..ba9130966b9 100644 --- a/spec/frontend/diffs/utils/diff_file_spec.js +++ b/spec/frontend/diffs/utils/diff_file_spec.js @@ -1,4 +1,11 @@ -import { prepareRawDiffFile, getShortShaFromFile } from '~/diffs/utils/diff_file'; +import { + prepareRawDiffFile, + getShortShaFromFile, + stats, + isNotDiffable, +} from '~/diffs/utils/diff_file'; +import { diffViewerModes } from '~/ide/constants'; +import mockDiffFile from '../mock_data/diff_file'; function getDiffFiles() { const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc'; @@ -154,4 +161,73 @@ describe('diff_file utilities', () => { expect(getShortShaFromFile({ content_sha: cs })).toBe(response); }); }); + + describe('stats', () => { + const noFile = [ + "returns empty stats when the file isn't provided", + undefined, + { + text: '', + percent: 0, + changed: 0, + classes: '', + sign: '', + valid: false, + }, + ]; + const validFile = [ + 'computes the correct stats from a file', + mockDiffFile, + { + changed: 1024, + percent: 100, + classes: 'cgreen', + sign: '+', + text: '+1.00 KiB (+100%)', + valid: true, + }, + ]; + const negativeChange = [ + 'computed the correct states from a file with a negative size change', + { + ...mockDiffFile, + new_size: 0, + old_size: 1024, + }, + { + changed: -1024, + percent: -100, + classes: 'cred', + sign: '', + text: '-1.00 KiB (-100%)', + valid: true, + }, + ]; + + it.each([noFile, validFile, negativeChange])('%s', (_, file, output) => { + expect(stats(file)).toEqual(output); + }); + }); + + describe('isNotDiffable', () => { + it.each` + bool | vw + ${true} | ${diffViewerModes.not_diffable} + ${false} | ${diffViewerModes.text} + ${false} | ${diffViewerModes.image} + `('returns $bool when the viewer is $vw', ({ bool, vw }) => { + expect(isNotDiffable({ viewer: { name: vw } })).toBe(bool); + }); + + it.each` + file + ${undefined} + ${null} + ${{}} + ${{ viewer: undefined }} + ${{ viewer: null }} + `('reports `false` when the file is `$file`', ({ file }) => { + expect(isNotDiffable(file)).toBe(false); + }); + }); }); diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index c13d83d1685..4e72d558b52 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -990,6 +990,34 @@ RSpec.describe ApplicationSetting do end end end + + describe '#diff_max_files' do + context 'validations' do + it { is_expected.to validate_presence_of(:diff_max_files) } + + specify do + is_expected + .to validate_numericality_of(:diff_max_files) + .only_integer + .is_greater_than_or_equal_to(Commit::DEFAULT_MAX_DIFF_FILES_SETTING) + .is_less_than_or_equal_to(Commit::MAX_DIFF_FILES_SETTING_UPPER_BOUND) + end + end + end + + describe '#diff_max_lines' do + context 'validations' do + it { is_expected.to validate_presence_of(:diff_max_lines) } + + specify do + is_expected + .to validate_numericality_of(:diff_max_lines) + .only_integer + .is_greater_than_or_equal_to(Commit::DEFAULT_MAX_DIFF_LINES_SETTING) + .is_less_than_or_equal_to(Commit::MAX_DIFF_LINES_SETTING_UPPER_BOUND) + end + end + end end describe '#sourcegraph_url_is_com?' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 693e754c53d..8ffc198fc4d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -672,6 +672,92 @@ eos it_behaves_like '#uri_type' end + describe '.diff_max_files' do + subject(:diff_max_files) { described_class.diff_max_files } + + let(:increased_diff_limits) { false } + let(:configurable_diff_limits) { false } + + before do + stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits) + end + + context 'when increased_diff_limits is enabled' do + let(:increased_diff_limits) { true } + + it 'returns 3000' do + expect(diff_max_files).to eq(3000) + end + end + + context 'when configurable_diff_limits is enabled' do + let(:configurable_diff_limits) { true } + + it 'returns the current settings' do + Gitlab::CurrentSettings.update!(diff_max_files: 1234) + expect(diff_max_files).to eq(1234) + end + end + + context 'when neither feature flag is enabled' do + it 'returns 1000' do + expect(diff_max_files).to eq(1000) + end + end + end + + describe '.diff_max_lines' do + subject(:diff_max_lines) { described_class.diff_max_lines } + + let(:increased_diff_limits) { false } + let(:configurable_diff_limits) { false } + + before do + stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits) + end + + context 'when increased_diff_limits is enabled' do + let(:increased_diff_limits) { true } + + it 'returns 100000' do + expect(diff_max_lines).to eq(100000) + end + end + + context 'when configurable_diff_limits is enabled' do + let(:configurable_diff_limits) { true } + + it 'returns the current settings' do + Gitlab::CurrentSettings.update!(diff_max_lines: 65321) + expect(diff_max_lines).to eq(65321) + end + end + + context 'when neither feature flag is enabled' do + it 'returns 50000' do + expect(diff_max_lines).to eq(50000) + end + end + end + + describe '.diff_safe_max_files' do + subject(:diff_safe_max_files) { described_class.diff_safe_max_files } + + it 'returns the commit diff max divided by the limit factor of 10' do + expect(::Commit).to receive(:diff_max_files).and_return(10) + expect(diff_safe_max_files).to eq(1) + end + end + + describe '.diff_safe_max_lines' do + subject(:diff_safe_max_lines) { described_class.diff_safe_max_lines } + + it 'returns the commit diff max divided by the limit factor of 10' do + expect(::Commit).to receive(:diff_max_lines).and_return(100) + expect(diff_safe_max_lines).to eq(10) + end + end + describe '.from_hash' do subject { described_class.from_hash(commit.to_hash, container) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c8acc85cfd9..c2bc581d519 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2613,4 +2613,16 @@ RSpec.describe Group do expect(group.activity_path).to eq(expected_path) end end + + context 'with export' do + let(:group) { create(:group, :with_export) } + + it '#export_file_exists returns true' do + expect(group.export_file_exists?).to be true + end + + it '#export_archive_exists? returns true' do + expect(group.export_archive_exists?).to be true + end + end end diff --git a/spec/models/import_export_upload_spec.rb b/spec/models/import_export_upload_spec.rb index f82c8da379f..e13f504b82a 100644 --- a/spec/models/import_export_upload_spec.rb +++ b/spec/models/import_export_upload_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe ImportExportUpload do - subject { described_class.new(project: create(:project)) } + let(:project) { create(:project) } + + subject { described_class.new(project: project) } shared_examples 'stores the Import/Export file' do |method| it 'stores the import file' do @@ -43,4 +45,80 @@ RSpec.describe ImportExportUpload do end end end + + context 'ActiveRecord callbacks' do + let(:after_save_callbacks) { described_class._save_callbacks.select { |cb| cb.kind == :after } } + let(:after_commit_callbacks) { described_class._commit_callbacks.select { |cb| cb.kind == :after } } + + def find_callback(callbacks, key) + callbacks.find { |cb| cb.instance_variable_get(:@key) == key } + end + + it 'export file is stored in after_commit callback' do + expect(find_callback(after_commit_callbacks, :store_export_file!)).to be_present + expect(find_callback(after_save_callbacks, :store_export_file!)).to be_nil + end + + it 'import file is stored in after_save callback' do + expect(find_callback(after_save_callbacks, :store_import_file!)).to be_present + expect(find_callback(after_commit_callbacks, :store_import_file!)).to be_nil + end + end + + describe 'export file' do + it '#export_file_exists? returns false' do + expect(subject.export_file_exists?).to be false + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be false + end + + context 'with export' do + let(:project_with_export) { create(:project, :with_export) } + + subject { described_class.with_export_file.find_by(project: project_with_export) } + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be true + end + + context 'when object file does not exist' do + before do + subject.export_file.file.delete + end + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be false + end + end + + context 'when checking object existence raises a error' do + let(:exception) { Excon::Error::Forbidden.new('not allowed') } + + before do + file = double + allow(file).to receive(:exists?).and_raise(exception) + allow(subject).to receive(:carrierwave_export_file).and_return(file) + end + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception) + expect(subject.export_archive_exists?).to be false + end + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 242d5947d26..e012fcac810 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4376,6 +4376,18 @@ RSpec.describe Project, factory_default: :keep do end end + context 'with export' do + let(:project) { create(:project, :with_export) } + + it '#export_file_exists? returns true' do + expect(project.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(project.export_archive_exists?).to be true + end + end + describe '#forks_count' do it 'returns the number of forks' do project = build(:project) @@ -6638,7 +6650,7 @@ RSpec.describe Project, factory_default: :keep do context 'when project export is completed' do before do finish_job(project_export_job) - allow(project).to receive(:export_file).and_return(double(ImportExportUploader, file: 'exists.zip')) + allow(project).to receive(:export_file_exists?).and_return(true) end it { expect(project.export_status).to eq :finished } @@ -6649,7 +6661,7 @@ RSpec.describe Project, factory_default: :keep do before do finish_job(project_export_job) - allow(project).to receive(:export_file).and_return(double(ImportExportUploader, file: 'exists.zip')) + allow(project).to receive(:export_file_exists?).and_return(true) end it { expect(project.export_status).to eq :regeneration_in_progress } diff --git a/spec/requests/api/group_export_spec.rb b/spec/requests/api/group_export_spec.rb index ee88d1eb1e9..31eef21654a 100644 --- a/spec/requests/api/group_export_spec.rb +++ b/spec/requests/api/group_export_spec.rb @@ -64,6 +64,23 @@ RSpec.describe API::GroupExport do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'when object is not present' do + let(:other_group) { create(:group, :with_export) } + let(:other_download_path) { "/groups/#{other_group.id}/export/download" } + + before do + other_group.add_owner(user) + other_group.export_file.file.delete + end + + it 'returns 404' do + get api(other_download_path, user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('The group export file is not available yet') + end + end end context 'when export file does not exist' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index eb6e78aeda2..038c3bc552a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1353,7 +1353,7 @@ RSpec.describe API::MergeRequests do context 'when a merge request has more than the changes limit' do it "returns a string indicating that more changes were made" do - allow(Commit).to receive(:diff_hard_limit_files).and_return(5) + allow(Commit).to receive(:diff_max_files).and_return(5) merge_request_overflow = create(:merge_request, :simple, author: user, diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index ac24aeee52c..06f4475ef79 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -196,6 +196,19 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do end end + context 'when export object is not present' do + before do + project_after_export.export_file.file.delete + end + + it 'returns 404' do + get api(download_path_export_action, user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('The project export file is not available yet') + end + end + context 'when upload complete' do before do project_after_export.remove_exports diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 66c0dcaa36c..4a4aeaea714 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -113,6 +113,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do terms: 'Hello world!', performance_bar_allowed_group_path: group.full_path, diff_max_patch_bytes: 300_000, + diff_max_files: 2000, + diff_max_lines: 50000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, local_markdown_version: 3, allow_local_requests_from_web_hooks_and_services: true, @@ -159,6 +161,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['terms']).to eq('Hello world!') expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) expect(json_response['diff_max_patch_bytes']).to eq(300_000) + expect(json_response['diff_max_files']).to eq(2000) + expect(json_response['diff_max_lines']).to eq(50000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(json_response['local_markdown_version']).to eq(3) expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true) |