From 081c20decabe2154802daedc9cd1f7fcee2165ed Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 21 Dec 2020 09:10:07 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- app/assets/javascripts/diffs/utils/diff_file.js | 11 ++- .../javascripts/diffs/utils/merge_request.js | 18 +++- app/helpers/visibility_level_helper.rb | 110 +++++---------------- .../numerous_undo_possibilities_in_git/index.md | 2 + lib/gitlab/danger/teammate.rb | 1 + spec/frontend/diffs/utils/diff_file_spec.js | 30 +++++- spec/frontend/diffs/utils/merge_request_spec.js | 22 ++++- spec/helpers/visibility_level_helper_spec.rb | 38 +++---- spec/lib/gitlab/danger/teammate_spec.rb | 8 ++ 9 files changed, 123 insertions(+), 117 deletions(-) diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index 69d0e49e501..2cb88e34856 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -4,6 +4,7 @@ import { DIFF_FILE_MANUAL_COLLAPSE, DIFF_FILE_AUTOMATIC_COLLAPSE, } from '../constants'; +import { getDerivedMergeRequestInformation } from './merge_request'; import { uuids } from './uuids'; function fileSymlinkInformation(file, fileList) { @@ -34,8 +35,12 @@ function collapsed(file) { } function identifier(file) { + const { userOrGroup, project, id } = getDerivedMergeRequestInformation({ + endpoint: file.load_collapsed_diff_url, + }); + return uuids({ - seeds: [file.file_identifier_hash, file.blob?.id], + seeds: [userOrGroup, project, id, file.file_identifier_hash, file.blob?.id], })[0]; } @@ -48,10 +53,10 @@ export function prepareRawDiffFile({ file, allFiles, meta = false }) { }, }; - // It's possible, but not confirmed, that `content_sha` isn't available sometimes + // It's possible, but not confirmed, that `blob.id` isn't available sometimes // See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49506#note_464692057 // We don't want duplicate IDs if that's the case, so we just don't assign an ID - if (!meta && file.blob?.id) { + if (!meta && file.blob?.id && file.load_collapsed_diff_url) { additionalProperties.id = identifier(file); } diff --git a/app/assets/javascripts/diffs/utils/merge_request.js b/app/assets/javascripts/diffs/utils/merge_request.js index 10e2a4b2cf2..edb4304f558 100644 --- a/app/assets/javascripts/diffs/utils/merge_request.js +++ b/app/assets/javascripts/diffs/utils/merge_request.js @@ -1,10 +1,20 @@ +const endpointRE = /^(\/?(.+?)\/(.+?)\/-\/merge_requests\/(\d+)).*$/i; + export function getDerivedMergeRequestInformation({ endpoint } = {}) { - const mrPath = endpoint - ?.split('/') - .slice(0, -1) - .join('/'); + let mrPath; + let userOrGroup; + let project; + let id; + const matches = endpointRE.exec(endpoint); + + if (matches) { + [, mrPath, userOrGroup, project, id] = matches; + } return { mrPath, + userOrGroup, + project, + id, }; } diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 0a37257e124..696f29164fd 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -26,76 +26,6 @@ module VisibilityLevelHelper end end - def project_visibility_level_description(level) - case level - when Gitlab::VisibilityLevel::PRIVATE - _("Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.") - when Gitlab::VisibilityLevel::INTERNAL - _("The project can be accessed by any logged in user except external users.") - when Gitlab::VisibilityLevel::PUBLIC - _("The project can be accessed without any authentication.") - end - end - - def group_visibility_level_description(level) - case level - when Gitlab::VisibilityLevel::PRIVATE - _("The group and its projects can only be viewed by members.") - when Gitlab::VisibilityLevel::INTERNAL - _("The group and any internal projects can be viewed by any logged in user except external users.") - when Gitlab::VisibilityLevel::PUBLIC - _("The group and any public projects can be viewed without any authentication.") - end - end - - # Note: these messages closely mirror the form validation strings found in the project - # model and any changes or additons to these may also need to be made there. - def disallowed_project_visibility_level_description(level, project) - level_name = Gitlab::VisibilityLevel.level_name(level).downcase - reasons = [] - instructions = [] - - unless project.visibility_level_allowed_as_fork?(level) - reasons << "the fork source project has lower visibility" - end - - unless project.visibility_level_allowed_by_group?(level) - errors = visibility_level_errors_for_group(project.group, level_name) - - reasons << errors[:reason] - instructions << errors[:instruction] - end - - reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' - "This project cannot be #{level_name}#{reasons}.#{instructions.join}".html_safe - end - - # Note: these messages closely mirror the form validation strings found in the group - # model and any changes or additons to these may also need to be made there. - def disallowed_group_visibility_level_description(level, group) - level_name = Gitlab::VisibilityLevel.level_name(level).downcase - reasons = [] - instructions = [] - - unless group.visibility_level_allowed_by_projects?(level) - reasons << "it contains projects with higher visibility" - end - - unless group.visibility_level_allowed_by_sub_groups?(level) - reasons << "it contains sub-groups with higher visibility" - end - - unless group.visibility_level_allowed_by_parent?(level) - errors = visibility_level_errors_for_group(group.parent, level_name) - - reasons << errors[:reason] - instructions << errors[:instruction] - end - - reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' - "This group cannot be #{level_name}#{reasons}.#{instructions.join}".html_safe - end - def visibility_icon_description(form_model) if form_model.respond_to?(:visibility_level_allowed_as_fork?) project_visibility_icon_description(form_model.visibility_level) @@ -104,14 +34,6 @@ module VisibilityLevelHelper end end - def group_visibility_icon_description(level) - "#{visibility_level_label(level)} - #{group_visibility_level_description(level)}" - end - - def project_visibility_icon_description(level) - "#{visibility_level_label(level)} - #{project_visibility_level_description(level)}" - end - def visibility_level_label(level) # The visibility level can be: # 'VisibilityLevel|Private', 'VisibilityLevel|Internal', 'VisibilityLevel|Public' @@ -203,11 +125,33 @@ module VisibilityLevelHelper current_level end - def visibility_level_errors_for_group(group, level_name) - group_name = link_to group.name, group_path(group) - change_visibility = link_to 'change the visibility', edit_group_path(group) + def project_visibility_level_description(level) + case level + when Gitlab::VisibilityLevel::PRIVATE + _("Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.") + when Gitlab::VisibilityLevel::INTERNAL + _("The project can be accessed by any logged in user except external users.") + when Gitlab::VisibilityLevel::PUBLIC + _("The project can be accessed without any authentication.") + end + end + + def group_visibility_level_description(level) + case level + when Gitlab::VisibilityLevel::PRIVATE + _("The group and its projects can only be viewed by members.") + when Gitlab::VisibilityLevel::INTERNAL + _("The group and any internal projects can be viewed by any logged in user except external users.") + when Gitlab::VisibilityLevel::PUBLIC + _("The group and any public projects can be viewed without any authentication.") + end + end - { reason: "the visibility of #{group_name} is #{group.visibility}", - instruction: " To make this group #{level_name}, you must first #{change_visibility} of the parent group." } + def project_visibility_icon_description(level) + "#{visibility_level_label(level)} - #{project_visibility_level_description(level)}" + end + + def group_visibility_icon_description(level) + "#{visibility_level_label(level)} - #{group_visibility_level_description(level)}" end end diff --git a/doc/topics/git/numerous_undo_possibilities_in_git/index.md b/doc/topics/git/numerous_undo_possibilities_in_git/index.md index 8fc2259c83e..f6571c7b277 100644 --- a/doc/topics/git/numerous_undo_possibilities_in_git/index.md +++ b/doc/topics/git/numerous_undo_possibilities_in_git/index.md @@ -493,6 +493,8 @@ An alternative is the open source community-maintained tool [BFG](https://rtyley Keep in mind that these tools are faster because they do not provide the same feature set as `git filter-branch` does, but focus on specific use cases. +Refer [Reduce repository size](../../../user/project/repository/reducing_the_repo_size_using_git.md) page to know more about purging files from repository history & GitLab storage. + ## Conclusion There are various options of undoing your work with any version control system, but diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb index 4481977db15..287b12b7fd5 100644 --- a/lib/gitlab/danger/teammate.rb +++ b/lib/gitlab/danger/teammate.rb @@ -94,6 +94,7 @@ module Gitlab when :engineering_productivity return false unless role[/Engineering Productivity/] return true if kind == :reviewer + return true if capabilities(project).include?("#{kind} engineering_productivity") capabilities(project).include?("#{kind} backend") else diff --git a/spec/frontend/diffs/utils/diff_file_spec.js b/spec/frontend/diffs/utils/diff_file_spec.js index 2e6247b8c07..2de8db28e71 100644 --- a/spec/frontend/diffs/utils/diff_file_spec.js +++ b/spec/frontend/diffs/utils/diff_file_spec.js @@ -1,6 +1,8 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; function getDiffFiles() { + const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc'; + return [ { blob: { @@ -8,6 +10,7 @@ function getDiffFiles() { }, file_hash: 'ABC', // This file is just a normal file file_identifier_hash: 'ABC1', + load_collapsed_diff_url: loadFull, }, { blob: { @@ -15,6 +18,7 @@ function getDiffFiles() { }, file_hash: 'DEF', // This file replaces a symlink file_identifier_hash: 'DEF1', + load_collapsed_diff_url: loadFull, a_mode: '0', b_mode: '0755', }, @@ -24,6 +28,7 @@ function getDiffFiles() { }, file_hash: 'DEF', // This symlink is replaced by a file file_identifier_hash: 'DEF2', + load_collapsed_diff_url: loadFull, a_mode: '120000', b_mode: '0', }, @@ -33,6 +38,7 @@ function getDiffFiles() { }, file_hash: 'GHI', // This symlink replaces a file file_identifier_hash: 'GHI1', + load_collapsed_diff_url: loadFull, a_mode: '0', b_mode: '120000', }, @@ -42,6 +48,7 @@ function getDiffFiles() { }, file_hash: 'GHI', // This file is replaced by a symlink file_identifier_hash: 'GHI2', + load_collapsed_diff_url: loadFull, a_mode: '0755', b_mode: '0', }, @@ -86,11 +93,11 @@ describe('diff_file utilities', () => { it.each` fileIndex | id - ${0} | ${'8dcd585e-a421-4dab-a04e-6f88c81b7b4c'} - ${1} | ${'3f178b78-392b-44a4-bd7d-5d6192208a97'} - ${2} | ${'3d9e1354-cddf-4a11-8234-f0413521b2e5'} - ${3} | ${'460f005b-d29d-43c1-9a08-099a7c7f08de'} - ${4} | ${'d8c89733-6ce1-4455-ae3d-f8aad6ee99f9'} + ${0} | ${'68296a4f-f1c7-445a-bd0e-6e3b02c4eec0'} + ${1} | ${'051c9bb8-cdba-4eb7-b8d1-508906e6d8ba'} + ${2} | ${'ed3d53d5-5da0-412d-a3c6-7213f84e88d3'} + ${3} | ${'39d998dc-bc69-4b19-a6af-41e4369c2bd5'} + ${4} | ${'7072d115-ce39-423c-8346-9fcad58cd68e'} `('sets the file id properly { id: $id } on normal diff files', ({ fileIndex, id }) => { const preppedFile = prepareRawDiffFile({ file: files[fileIndex], @@ -122,5 +129,18 @@ describe('diff_file utilities', () => { expect(preppedFile).not.toHaveProp('id'); }); + + it('does not set the id property if the file is missing a `load_collapsed_diff_url` property', () => { + const fileMissingContentSha = { ...files[0] }; + + delete fileMissingContentSha.load_collapsed_diff_url; + + const preppedFile = prepareRawDiffFile({ + file: fileMissingContentSha, + allFiles: files, + }); + + expect(preppedFile).not.toHaveProp('id'); + }); }); }); diff --git a/spec/frontend/diffs/utils/merge_request_spec.js b/spec/frontend/diffs/utils/merge_request_spec.js index e949a70dfe7..8c7b1e1f2a5 100644 --- a/spec/frontend/diffs/utils/merge_request_spec.js +++ b/spec/frontend/diffs/utils/merge_request_spec.js @@ -2,16 +2,28 @@ import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request'; import { diffMetadata } from '../mock_data/diff_metadata'; describe('Merge Request utilities', () => { + const derivedMrInfo = { + mrPath: '/gitlab-org/gitlab-test/-/merge_requests/4', + userOrGroup: 'gitlab-org', + project: 'gitlab-test', + id: '4', + }; + const unparseableEndpoint = { + mrPath: undefined, + userOrGroup: undefined, + project: undefined, + id: undefined, + }; + describe('getDerivedMergeRequestInformation', () => { const endpoint = `${diffMetadata.latest_version_path}.json?searchParam=irrelevant`; - const mrPath = diffMetadata.latest_version_path.replace(/\/diffs$/, ''); it.each` argument | response - ${{ endpoint }} | ${{ mrPath }} - ${{}} | ${{ mrPath: undefined }} - ${{ endpoint: undefined }} | ${{ mrPath: undefined }} - ${{ endpoint: null }} | ${{ mrPath: undefined }} + ${{ endpoint }} | ${derivedMrInfo} + ${{}} | ${unparseableEndpoint} + ${{ endpoint: undefined }} | ${unparseableEndpoint} + ${{ endpoint: null }} | ${unparseableEndpoint} `('generates the correct derived results based on $argument', ({ argument, response }) => { expect(getDerivedMergeRequestInformation(argument)).toStrictEqual(response); }); diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 86b0693af92..10e0815918f 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -35,29 +35,33 @@ RSpec.describe VisibilityLevelHelper do describe 'visibility_level_description' do context 'used with a Project' do - it 'delegates projects to #project_visibility_level_description' do - expect(visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, project)) - .to match /project/i + let(:descriptions) do + [ + visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, project), + visibility_level_description(Gitlab::VisibilityLevel::INTERNAL, project), + visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project) + ] end - end - context 'used with a Group' do - it 'delegates groups to #group_visibility_level_description' do - expect(visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) - .to match /group/i + it 'returns different project related descriptions depending on visibility level' do + expect(descriptions.uniq.size).to eq(descriptions.size) + expect(descriptions).to all match /project/i end end - end - describe "#project_visibility_level_description" do - it "describes private projects" do - expect(project_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE)) - .to eq _('Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.') - end + context 'used with a Group' do + let(:descriptions) do + [ + visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group), + visibility_level_description(Gitlab::VisibilityLevel::INTERNAL, group), + visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, group) + ] + end - it "describes public projects" do - expect(project_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC)) - .to eq _('The project can be accessed without any authentication.') + it 'returns different group related descriptions depending on visibility level' do + expect(descriptions.uniq.size).to eq(descriptions.size) + expect(descriptions).to all match /group/i + end end end diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index eebe14ed5e1..9c066ba4c1b 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -121,6 +121,14 @@ RSpec.describe Gitlab::Danger::Teammate do end end + context 'when capabilities include maintainer engineering productivity' do + let(:capabilities) { ['maintainer engineering_productivity'] } + + it '#maintainer? returns true' do + expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_truthy + end + end + context 'when capabilities include trainee_maintainer backend' do let(:capabilities) { ['trainee_maintainer backend'] } -- cgit v1.2.1