Commit 5c548f39 authored by Francisco Javier López's avatar Francisco Javier López Committed by Paul Slaughter

Enabling snippets with multiple files

Removing the FF snippet_multiple_files and releasing
snippet with multiples files.
parent f2e77c33
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
import { GlButton } from '@gitlab/ui'; import { GlButton } from '@gitlab/ui';
import { cloneDeep } from 'lodash'; import { cloneDeep } from 'lodash';
import { s__, sprintf } from '~/locale'; import { s__, sprintf } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import SnippetBlobEdit from './snippet_blob_edit.vue'; import SnippetBlobEdit from './snippet_blob_edit.vue';
import { SNIPPET_MAX_BLOBS } from '../constants'; import { SNIPPET_MAX_BLOBS } from '../constants';
import { createBlob, decorateBlob, diffAll } from '../utils/blob'; import { createBlob, decorateBlob, diffAll } from '../utils/blob';
...@@ -12,7 +11,6 @@ export default { ...@@ -12,7 +11,6 @@ export default {
SnippetBlobEdit, SnippetBlobEdit,
GlButton, GlButton,
}, },
mixins: [glFeatureFlagsMixin()],
props: { props: {
initBlobs: { initBlobs: {
type: Array, type: Array,
...@@ -52,12 +50,6 @@ export default { ...@@ -52,12 +50,6 @@ export default {
canAdd() { canAdd() {
return this.count < SNIPPET_MAX_BLOBS; return this.count < SNIPPET_MAX_BLOBS;
}, },
hasMultiFilesEnabled() {
return this.glFeatures.snippetMultipleFiles;
},
filesLabel() {
return this.hasMultiFilesEnabled ? s__('Snippets|Files') : s__('Snippets|File');
},
firstInputId() { firstInputId() {
const blobId = this.blobIds[0]; const blobId = this.blobIds[0];
...@@ -132,19 +124,17 @@ export default { ...@@ -132,19 +124,17 @@ export default {
</script> </script>
<template> <template>
<div class="form-group file-editor"> <div class="form-group file-editor">
<label :for="firstInputId">{{ filesLabel }}</label> <label :for="firstInputId">{{ s__('Snippets|Files') }}</label>
<snippet-blob-edit <snippet-blob-edit
v-for="(blobId, index) in blobIds" v-for="(blobId, index) in blobIds"
:key="blobId" :key="blobId"
:class="{ 'gl-mt-3': index > 0 }" :class="{ 'gl-mt-3': index > 0 }"
:blob="blobs[blobId]" :blob="blobs[blobId]"
:can-delete="canDelete" :can-delete="canDelete"
:show-delete="hasMultiFilesEnabled"
@blob-updated="updateBlob(blobId, $event)" @blob-updated="updateBlob(blobId, $event)"
@delete="deleteBlob(blobId)" @delete="deleteBlob(blobId)"
/> />
<gl-button <gl-button
v-if="hasMultiFilesEnabled"
:disabled="!canAdd" :disabled="!canAdd"
data-testid="add_button" data-testid="add_button"
class="gl-my-3" class="gl-my-3"
......
...@@ -28,7 +28,7 @@ export default { ...@@ -28,7 +28,7 @@ export default {
showDelete: { showDelete: {
type: Boolean, type: Boolean,
required: false, required: false,
default: false, default: true,
}, },
}, },
computed: { computed: {
......
...@@ -14,10 +14,6 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController ...@@ -14,10 +14,6 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
before_action :authorize_update_snippet!, only: [:edit, :update] before_action :authorize_update_snippet!, only: [:edit, :update]
before_action :authorize_admin_snippet!, only: [:destroy] before_action :authorize_admin_snippet!, only: [:destroy]
before_action do
push_frontend_feature_flag(:snippet_multiple_files, current_user)
end
def index def index
@snippet_counts = ::Snippets::CountService @snippet_counts = ::Snippets::CountService
.new(current_user, project: @project) .new(current_user, project: @project)
......
...@@ -17,10 +17,6 @@ class SnippetsController < Snippets::ApplicationController ...@@ -17,10 +17,6 @@ class SnippetsController < Snippets::ApplicationController
layout 'snippets' layout 'snippets'
before_action do
push_frontend_feature_flag(:snippet_multiple_files, current_user)
end
def index def index
if params[:username].present? if params[:username].present?
@user = UserFinder.new(params[:username]).find_by_username! @user = UserFinder.new(params[:username]).find_by_username!
......
...@@ -19,7 +19,6 @@ class Snippet < ApplicationRecord ...@@ -19,7 +19,6 @@ class Snippet < ApplicationRecord
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
MAX_FILE_COUNT = 10 MAX_FILE_COUNT = 10
MAX_SINGLE_FILE_COUNT = 1
cache_markdown_field :title, pipeline: :single_line cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description cache_markdown_field :description
...@@ -175,8 +174,8 @@ class Snippet < ApplicationRecord ...@@ -175,8 +174,8 @@ class Snippet < ApplicationRecord
Snippet.find_by(id: id, project: project) Snippet.find_by(id: id, project: project)
end end
def self.max_file_limit(user) def self.max_file_limit
Feature.enabled?(:snippet_multiple_files, user) ? MAX_FILE_COUNT : MAX_SINGLE_FILE_COUNT MAX_FILE_COUNT
end end
def initialize(attributes = {}) def initialize(attributes = {})
......
...@@ -25,10 +25,6 @@ class SnippetBlobPresenter < BlobPresenter ...@@ -25,10 +25,6 @@ class SnippetBlobPresenter < BlobPresenter
private private
def snippet_multiple_files?
blob.container.repository_exists? && Feature.enabled?(:snippet_multiple_files, current_user)
end
def snippet def snippet
blob.container blob.container
end end
...@@ -52,8 +48,6 @@ class SnippetBlobPresenter < BlobPresenter ...@@ -52,8 +48,6 @@ class SnippetBlobPresenter < BlobPresenter
end end
def snippet_blob_raw_route(only_path: false) def snippet_blob_raw_route(only_path: false)
return gitlab_raw_snippet_blob_url(snippet, blob.path, only_path: only_path) if snippet_multiple_files? gitlab_raw_snippet_blob_url(snippet, blob.path, only_path: only_path)
gitlab_raw_snippet_url(snippet, only_path: only_path)
end end
end end
...@@ -52,7 +52,7 @@ module Snippets ...@@ -52,7 +52,7 @@ module Snippets
def check_file_count! def check_file_count!
file_count = repository.ls_files(snippet.default_branch).size file_count = repository.ls_files(snippet.default_branch).size
limit = Snippet.max_file_limit(current_user) limit = Snippet.max_file_limit
if file_count > limit if file_count > limit
raise RepositoryValidationError, _('Repository files count over the limit') raise RepositoryValidationError, _('Repository files count over the limit')
......
---
title: Enable snippet multiple files
merge_request: 43246
author:
type: added
---
name: snippet_multiple_files
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/32416
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/217809
group: group::editor
type: development
default_enabled: false
...@@ -17,7 +17,7 @@ module API ...@@ -17,7 +17,7 @@ module API
expose :file_name do |snippet| expose :file_name do |snippet|
snippet.file_name_on_repo || snippet.file_name snippet.file_name_on_repo || snippet.file_name
end end
expose :files, if: ->(snippet, options) { snippet_multiple_files?(snippet, options[:current_user]) } do |snippet, options| expose :files do |snippet, options|
snippet.list_files.map do |file| snippet.list_files.map do |file|
{ {
path: file, path: file,
...@@ -25,10 +25,6 @@ module API ...@@ -25,10 +25,6 @@ module API
} }
end end
end end
def snippet_multiple_files?(snippet, current_user)
::Feature.enabled?(:snippet_multiple_files, current_user) && snippet.repository_exists?
end
end end
end end
end end
...@@ -93,7 +93,7 @@ module API ...@@ -93,7 +93,7 @@ module API
def validate_params_for_multiple_files(snippet) def validate_params_for_multiple_files(snippet)
return unless params[:content] || params[:file_name] return unless params[:content] || params[:file_name]
if Feature.enabled?(:snippet_multiple_files, current_user) && snippet.multiple_files? if snippet.multiple_files?
render_api_error!({ error: _('To update Snippets with multiple files, you must use the `files` parameter') }, 400) render_api_error!({ error: _('To update Snippets with multiple files, you must use the `files` parameter') }, 400)
end end
end end
......
...@@ -115,7 +115,7 @@ module Gitlab ...@@ -115,7 +115,7 @@ module Gitlab
override :check_single_change_access override :check_single_change_access
def check_single_change_access(change, _skip_lfs_integrity_check: false) def check_single_change_access(change, _skip_lfs_integrity_check: false)
Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, logger: logger).validate! Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate! Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
rescue Checks::TimedLogger::TimeoutError rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message raise TimeoutError, logger.full_message
end end
......
# frozen_string_literal: true # frozen_string_literal: true
module QA module QA
RSpec.describe 'Create', :requires_admin do RSpec.describe 'Create' do
describe 'Multiple file snippet' do describe 'Multiple file snippet' do
before do
Runtime::Feature.enable('snippet_multiple_files')
end
after do
Runtime::Feature.disable('snippet_multiple_files')
end
it 'creates a personal snippet with multiple files', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/842' do it 'creates a personal snippet with multiple files', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/842' do
Flow::Login.sign_in Flow::Login.sign_in
......
...@@ -9,6 +9,7 @@ exports[`Snippet Blob Edit component with loaded blob matches snapshot 1`] = ` ...@@ -9,6 +9,7 @@ exports[`Snippet Blob Edit component with loaded blob matches snapshot 1`] = `
candelete="true" candelete="true"
data-qa-selector="file_name_field" data-qa-selector="file_name_field"
id="blob_local_7_file_path" id="blob_local_7_file_path"
showdelete="true"
value="foo/bar/test.md" value="foo/bar/test.md"
/> />
......
...@@ -19,17 +19,12 @@ const TEST_BLOBS_UNLOADED = TEST_BLOBS.map(blob => ({ ...blob, content: '', isLo ...@@ -19,17 +19,12 @@ const TEST_BLOBS_UNLOADED = TEST_BLOBS.map(blob => ({ ...blob, content: '', isLo
describe('snippets/components/snippet_blob_actions_edit', () => { describe('snippets/components/snippet_blob_actions_edit', () => {
let wrapper; let wrapper;
const createComponent = (props = {}, snippetMultipleFiles = true) => { const createComponent = (props = {}) => {
wrapper = shallowMount(SnippetBlobActionsEdit, { wrapper = shallowMount(SnippetBlobActionsEdit, {
propsData: { propsData: {
initBlobs: TEST_BLOBS, initBlobs: TEST_BLOBS,
...props, ...props,
}, },
provide: {
glFeatures: {
snippetMultipleFiles,
},
},
}); });
}; };
...@@ -69,28 +64,24 @@ describe('snippets/components/snippet_blob_actions_edit', () => { ...@@ -69,28 +64,24 @@ describe('snippets/components/snippet_blob_actions_edit', () => {
wrapper = null; wrapper = null;
}); });
describe.each` describe('multi-file snippets rendering', () => {
featureFlag | label | showDelete | showAdd
${true} | ${'Files'} | ${true} | ${true}
${false} | ${'File'} | ${false} | ${false}
`('with feature flag = $featureFlag', ({ featureFlag, label, showDelete, showAdd }) => {
beforeEach(() => { beforeEach(() => {
createComponent({}, featureFlag); createComponent();
}); });
it('renders label', () => { it('renders label', () => {
expect(findLabel().text()).toBe(label); expect(findLabel().text()).toBe('Files');
}); });
it(`renders delete button (show=${showDelete})`, () => { it(`renders delete button (show=true)`, () => {
expect(findFirstBlobEdit().props()).toMatchObject({ expect(findFirstBlobEdit().props()).toMatchObject({
showDelete, showDelete: true,
canDelete: true, canDelete: true,
}); });
}); });
it(`renders add button (show=${showAdd})`, () => { it(`renders add button (show=true)`, () => {
expect(findAddButton().exists()).toBe(showAdd); expect(findAddButton().exists()).toBe(true);
}); });
}); });
......
...@@ -156,7 +156,7 @@ describe('Snippet Blob Edit component', () => { ...@@ -156,7 +156,7 @@ describe('Snippet Blob Edit component', () => {
}); });
it('shows blob header', () => { it('shows blob header', () => {
const { canDelete = true, showDelete = false } = props; const { canDelete = true, showDelete = true } = props;
expect(findHeader().props()).toMatchObject({ expect(findHeader().props()).toMatchObject({
canDelete, canDelete,
......
...@@ -21,16 +21,6 @@ RSpec.describe ::API::Entities::Snippet do ...@@ -21,16 +21,6 @@ RSpec.describe ::API::Entities::Snippet do
it { expect(subject[:visibility]).to eq snippet.visibility } it { expect(subject[:visibility]).to eq snippet.visibility }
it { expect(subject).to include(:author) } it { expect(subject).to include(:author) }
context 'with snippet_multiple_files feature disabled' do
before do
stub_feature_flags(snippet_multiple_files: false)
end
it 'does not return files' do
expect(subject).not_to include(:files)
end
end
describe 'file_name' do describe 'file_name' do
it 'returns attribute from repository' do it 'returns attribute from repository' do
expect(subject[:file_name]).to eq snippet.blobs.first.path expect(subject[:file_name]).to eq snippet.blobs.first.path
...@@ -77,14 +67,6 @@ RSpec.describe ::API::Entities::Snippet do ...@@ -77,14 +67,6 @@ RSpec.describe ::API::Entities::Snippet do
let(:blob) { snippet.blobs.first } let(:blob) { snippet.blobs.first }
let(:ref) { blob.repository.root_ref } let(:ref) { blob.repository.root_ref }
context 'when repository does not exist' do
it 'does not include the files attribute' do
allow(snippet).to receive(:repository_exists?).and_return(false)
expect(subject).not_to include(:files)
end
end
shared_examples 'snippet files' do shared_examples 'snippet files' do
let(:file) { subject[:files].first } let(:file) { subject[:files].first }
...@@ -99,6 +81,14 @@ RSpec.describe ::API::Entities::Snippet do ...@@ -99,6 +81,14 @@ RSpec.describe ::API::Entities::Snippet do
it 'has the raw url' do it 'has the raw url' do
expect(file[:raw_url]).to match(raw_url) expect(file[:raw_url]).to match(raw_url)
end end
context 'when repository does not exist' do
it 'returns empty array' do
allow(snippet.repository).to receive(:empty?).and_return(true)
expect(subject[:files]).to be_empty
end
end
end end
context 'with PersonalSnippet' do context 'with PersonalSnippet' do
......
...@@ -260,7 +260,7 @@ RSpec.describe Gitlab::GitAccessSnippet do ...@@ -260,7 +260,7 @@ RSpec.describe Gitlab::GitAccessSnippet do
service = double service = double
expect(service).to receive(:validate!).and_return(nil) expect(service).to receive(:validate!).and_return(nil)
expect(Snippet).to receive(:max_file_limit).with(user).and_return(5) expect(Snippet).to receive(:max_file_limit).and_return(5)
expect(Gitlab::Checks::PushFileCountCheck).to receive(:new).with(anything, hash_including(limit: 5)).and_return(service) expect(Gitlab::Checks::PushFileCountCheck).to receive(:new).with(anything, hash_including(limit: 5)).and_return(service)
push_access_check push_access_check
......
...@@ -717,19 +717,11 @@ RSpec.describe Snippet do ...@@ -717,19 +717,11 @@ RSpec.describe Snippet do
end end
describe '.max_file_limit' do describe '.max_file_limit' do
subject { described_class.max_file_limit(nil) } subject { described_class.max_file_limit }
it "returns #{Snippet::MAX_FILE_COUNT}" do it "returns #{Snippet::MAX_FILE_COUNT}" do
expect(subject).to eq Snippet::MAX_FILE_COUNT expect(subject).to eq Snippet::MAX_FILE_COUNT
end end
context 'when feature flag :snippet_multiple_files is disabled' do
it "returns #{described_class::MAX_SINGLE_FILE_COUNT}" do
stub_feature_flags(snippet_multiple_files: false)
expect(subject).to eq described_class::MAX_SINGLE_FILE_COUNT
end
end
end end
describe '#list_files' do describe '#list_files' do
......
...@@ -133,28 +133,6 @@ RSpec.describe SnippetBlobPresenter do ...@@ -133,28 +133,6 @@ RSpec.describe SnippetBlobPresenter do
subject { described_class.new(snippet.blobs.first, current_user: user).raw_path } subject { described_class.new(snippet.blobs.first, current_user: user).raw_path }
it_behaves_like 'snippet blob raw path' it_behaves_like 'snippet blob raw path'
context 'with snippet_multiple_files feature disabled' do
before do
stub_feature_flags(snippet_multiple_files: false)
end
context 'with ProjectSnippet' do
let(:snippet) { project_snippet }
it 'returns the raw path' do
expect(subject).to eq "/#{snippet.project.full_path}/-/snippets/#{snippet.id}/raw"
end
end
context 'with PersonalSnippet' do
let(:snippet) { personal_snippet }
it 'returns the raw path' do
expect(subject).to eq "/-/snippets/#{snippet.id}/raw"
end
end
end
end end
describe '#raw_url' do describe '#raw_url' do
...@@ -165,28 +143,6 @@ RSpec.describe SnippetBlobPresenter do ...@@ -165,28 +143,6 @@ RSpec.describe SnippetBlobPresenter do
end end
it_behaves_like 'snippet blob raw url' it_behaves_like 'snippet blob raw url'
context 'with snippet_multiple_files feature disabled' do
before do
stub_feature_flags(snippet_multiple_files: false)
end
context 'with ProjectSnippet' do
let(:snippet) { project_snippet }
it 'returns the raw project snippet url' do
expect(subject).to eq("http://test.host/#{project_snippet.project.full_path}/-/snippets/#{project_snippet.id}/raw")
end
end
context 'with PersonalSnippet' do
let(:snippet) { personal_snippet }
it 'returns the raw personal snippet url' do
expect(subject).to eq("http://test.host/-/snippets/#{personal_snippet.id}/raw")
end
end
end
end end
end end
......
...@@ -116,10 +116,6 @@ RSpec.describe API::ProjectSnippets do ...@@ -116,10 +116,6 @@ RSpec.describe API::ProjectSnippets do
let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123", user) } let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123", user) }
end end
end end
it_behaves_like 'snippet_multiple_files feature disabled' do
subject { get api("/projects/#{project.id}/snippets/#{snippet.id}", user) }
end
end end
describe 'POST /projects/:project_id/snippets/' do describe 'POST /projects/:project_id/snippets/' do
......
...@@ -167,8 +167,6 @@ RSpec.describe API::Snippets do ...@@ -167,8 +167,6 @@ RSpec.describe API::Snippets do
it_behaves_like 'snippet access with different users' do it_behaves_like 'snippet access with different users' do
let(:path) { "/snippets/#{snippet.id}" } let(:path) { "/snippets/#{snippet.id}" }
end end
it_behaves_like 'snippet_multiple_files feature disabled'
end end
describe 'POST /snippets/' do describe 'POST /snippets/' do
...@@ -251,10 +249,6 @@ RSpec.describe API::Snippets do ...@@ -251,10 +249,6 @@ RSpec.describe API::Snippets do
it_behaves_like 'snippet creation' it_behaves_like 'snippet creation'
it_behaves_like 'snippet_multiple_files feature disabled' do
let(:snippet) { Snippet.find(json_response["id"]) }
end
context 'with an external user' do context 'with an external user' do
let(:user) { create(:user, :external) } let(:user) { create(:user, :external) }
......
...@@ -40,7 +40,7 @@ RSpec.describe Snippets::RepositoryValidationService do ...@@ -40,7 +40,7 @@ RSpec.describe Snippets::RepositoryValidationService do
end end
it 'returns error when the repository has more file than the limit' do it 'returns error when the repository has more file than the limit' do
limit = Snippet.max_file_limit(user) + 1 limit = Snippet.max_file_limit + 1
files = Array.new(limit) { FFaker::Filesystem.file_name } files = Array.new(limit) { FFaker::Filesystem.file_name }
allow(repository).to receive(:ls_files).and_return(files) allow(repository).to receive(:ls_files).and_return(files)
......
...@@ -99,18 +99,6 @@ RSpec.shared_examples 'snippet blob content' do ...@@ -99,18 +99,6 @@ RSpec.shared_examples 'snippet blob content' do
end end
end end
RSpec.shared_examples 'snippet_multiple_files feature disabled' do
before do
stub_feature_flags(snippet_multiple_files: false)
subject
end
it 'does not return files attributes' do
expect(json_response).not_to have_key('files')
end
end
RSpec.shared_examples 'snippet creation with files parameter' do RSpec.shared_examples 'snippet creation with files parameter' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment