Merge branch 'sh-fix-personal-snippet-uploads-object-storage' into 'master'

Fix incorrect prefix used in new uploads for personal snippets

Closes #61671

See merge request gitlab-org/gitlab-ce!28337

1 files changed, 40 insertions, 20 deletions

diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb
index 97758f0243e..d9f0e2f3cb7 100644
--- a/spec/uploaders/personal_file_uploader_spec.rb
+++ b/spec/uploaders/personal_file_uploader_spec.rb
@@ -7,33 +7,19 @@ describe PersonalFileUploader do
 
   subject { uploader }
 
-  it_behaves_like 'builds correct paths',
-                  store_dir: %r[uploads/-/system/personal_snippet/\d+],
-                  upload_path: %r[\h+/\S+],
-                  absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet\/\d+\/\h+\/\S+$]
-
-  context "object_store is REMOTE" do
+  shared_examples '#base_dir' do
     before do
-      stub_uploads_object_storage
+      subject.instance_variable_set(:@secret, 'secret')
     end
 
-    include_context 'with storage', described_class::Store::REMOTE
-
-    it_behaves_like 'builds correct paths',
-                    store_dir: %r[\d+/\h+],
-                    upload_path: %r[^personal_snippet\/\d+\/\h+\/<filename>]
-  end
-
-  describe '#upload_paths' do
-    it 'builds correct paths for both local and remote storage' do
-      paths = uploader.upload_paths('test.jpg')
+    it 'is prefixed with uploads/-/system' do
+      allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name'))
 
-      expect(paths.first).to match(%r[\h+\/test.jpg])
-      expect(paths.second).to match(%r[^personal_snippet\/\d+\/\h+\/test.jpg])
+      expect(described_class.base_dir(model)).to eq("uploads/-/system/personal_snippet/#{model.id}")
     end
   end
 
-  describe '#to_h' do
+  shared_examples '#to_h' do
     before do
       subject.instance_variable_set(:@secret, 'secret')
     end
@@ -50,6 +36,40 @@ describe PersonalFileUploader do
     end
   end
 
+  describe '#upload_paths' do
+    it 'builds correct paths for both local and remote storage' do
+      paths = uploader.upload_paths('test.jpg')
+
+      expect(paths.first).to match(%r[\h+\/test.jpg])
+      expect(paths.second).to match(%r[^personal_snippet\/\d+\/\h+\/test.jpg])
+    end
+  end
+
+  context 'object_store is LOCAL' do
+    it_behaves_like 'builds correct paths',
+                    store_dir: %r[uploads/-/system/personal_snippet/\d+/\h+],
+                    upload_path: %r[\h+/\S+],
+                    absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet\/\d+\/\h+\/\S+$]
+
+    it_behaves_like '#base_dir'
+    it_behaves_like '#to_h'
+  end
+
+  context "object_store is REMOTE" do
+    before do
+      stub_uploads_object_storage
+    end
+
+    include_context 'with storage', described_class::Store::REMOTE
+
+    it_behaves_like 'builds correct paths',
+                    store_dir: %r[\d+/\h+],
+                    upload_path: %r[^personal_snippet\/\d+\/\h+\/<filename>]
+
+    it_behaves_like '#base_dir'
+    it_behaves_like '#to_h'
+  end
+
   describe "#migrate!" do
     before do
       uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))