From 7e5cacee269c973c71dc2e01227132c7d94ccdfb Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 26 Mar 2025 13:12:24 +0800 Subject: [PATCH] Remove legacy note attachment code These have been deprecated and migrated years ago so these code can now be removed --- .rubocop_todo/gitlab/namespaced_class.yml | 1 - app/controllers/projects/notes_controller.rb | 11 +- app/models/note.rb | 18 +-- app/models/project.rb | 4 +- app/serializers/note_attachment_entity.rb | 7 - app/serializers/note_entity.rb | 2 - app/serializers/project_note_entity.rb | 4 - app/views/events/event/_note.html.haml | 9 -- app/views/shared/notes/_note.html.haml | 12 -- config/routes/project.rb | 1 - config/routes/snippets.rb | 6 +- config/routes/uploads.rb | 12 +- .../raketasks/uploads/migrate.md | 2 - doc/api/notes.md | 5 - doc/api/openapi/openapi_v2.yaml | 2 - doc/development/file_storage.md | 1 - .../api/schemas/vulnerability_notes.json | 3 +- .../epics/issue_promote_service_spec.rb | 18 --- lib/api/entities/note.rb | 1 - .../import_export/project/import_export.yml | 1 - lib/gitlab/path_regex.rb | 1 - locale/gitlab.pot | 6 - spec/controllers/uploads_controller_spec.rb | 120 +----------------- spec/factories/notes.rb | 12 -- spec/factories/uploads.rb | 4 +- .../merge_request/user_posts_notes_spec.rb | 25 +--- .../api/schemas/entities/discussion.json | 7 - .../api/schemas/public_api/v4/notes.json | 7 - .../pipelines/snippets_pipeline_spec.rb | 2 +- .../gitlab/cleanup/project_uploads_spec.rb | 6 +- spec/lib/gitlab/path_regex_spec.rb | 2 +- spec/models/note_spec.rb | 21 --- spec/models/upload_spec.rb | 9 +- .../note_entity_shared_examples.rb | 1 - spec/uploaders/attachment_uploader_spec.rb | 14 +- 35 files changed, 33 insertions(+), 324 deletions(-) delete mode 100644 app/serializers/note_attachment_entity.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index b572d8b5faea1e..e5e12c286f0f1e 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -558,7 +558,6 @@ Gitlab/NamespacedClass: - 'app/serializers/move_to_project_serializer.rb' - 'app/serializers/namespace_basic_entity.rb' - 'app/serializers/namespace_serializer.rb' - - 'app/serializers/note_attachment_entity.rb' - 'app/serializers/note_entity.rb' - 'app/serializers/note_user_entity.rb' - 'app/serializers/paginated_diff_entity.rb' diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index e5ddb082a25b93..776098d1cc8ae0 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -14,7 +14,7 @@ class Projects::NotesController < Projects::ApplicationController before_action :authorize_create_note!, only: [:create] before_action :authorize_resolve_note!, only: [:resolve, :unresolve] - feature_category :team_planning, [:index, :create, :update, :destroy, :delete_attachment, :toggle_award_emoji] + feature_category :team_planning, [:index, :create, :update, :destroy, :toggle_award_emoji] feature_category :code_review_workflow, [:resolve, :unresolve, :outdated_line_change] urgency :low @@ -37,15 +37,6 @@ def feature_category_override_for_target_type(target_type) end end - def delete_attachment - note.remove_attachment! - note.update_attribute(:attachment, nil) - - respond_to do |format| - format.js { head :ok } - end - end - def resolve return render_404 unless note.resolvable? diff --git a/app/models/note.rb b/app/models/note.rb index 2d029801b59ffa..f1d1a18e5048f3 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -29,6 +29,8 @@ class Note < ApplicationRecord include EachBatch include Spammable + ignore_column :attachment, remove_with: '18.1', remove_after: '2025-05-15' + cache_markdown_field :note, pipeline: :note, issuable_reference_expansion_enabled: true redact_field :note @@ -97,9 +99,6 @@ class Note < ApplicationRecord validates :project, presence: true, if: :for_project_noteable? validates :namespace, presence: true - # Attachments are deprecated and are handled by Markdown uploader - validates :attachment, file_size: { maximum: :max_attachment_size } - validates :noteable_type, presence: true validates :noteable_id, presence: true, unless: [:for_commit?, :importing?] validates :commit_id, presence: true, if: :for_commit? @@ -117,11 +116,6 @@ class Note < ApplicationRecord validate :does_not_exceed_notes_limit?, on: :create, unless: [:system?, :importing?] - # @deprecated attachments are handled by the Upload model. - # - # https://gitlab.com/gitlab-org/gitlab/-/issues/20830 - mount_uploader :attachment, AttachmentUploader - # Scopes scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } scope :system, -> { where(system: true) } @@ -313,10 +307,6 @@ def active? true end - def max_attachment_size - Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i - end - def hook_attrs Gitlab::HookData::NoteBuilder.new(self).build end @@ -723,10 +713,6 @@ def attribute_names_for_serialization attributes.keys end - def uploads_sharding_key - { namespace_id: namespace_id } - end - private def trigger_note_subscription? diff --git a/app/models/project.rb b/app/models/project.rb index 5c38539405989a..772919f2919144 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -277,9 +277,7 @@ def self.integration_association_name(name) has_one :forked_from_project, through: :fork_network_member # Projects with a very large number of notes may time out destroying them - # through the foreign key. Additionally, the deprecated attachment uploader - # for notes requires us to use dependent: :destroy to avoid orphaning uploaded - # files. + # through the foreign key. # # https://gitlab.com/gitlab-org/gitlab/-/issues/207222 # Order of this association is important for project deletion. diff --git a/app/serializers/note_attachment_entity.rb b/app/serializers/note_attachment_entity.rb deleted file mode 100644 index dc801e2bf4e936..00000000000000 --- a/app/serializers/note_attachment_entity.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class NoteAttachmentEntity < Grape::Entity - expose :url - expose :filename - expose :image?, as: :image -end diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb index c3e65114a9df82..892befd968888a 100644 --- a/app/serializers/note_entity.rb +++ b/app/serializers/note_entity.rb @@ -93,8 +93,6 @@ class NoteEntity < API::Entities::Note new_project_issue_path(note.project, merge_request_to_resolve_discussions_of: note.noteable.iid, discussion_to_resolve: note.discussion_id) end - expose :attachment, using: NoteAttachmentEntity, if: ->(note, _) { note.attachment? } - expose :cached_markdown_version # Correctly rendering a note requires some background information about any diff --git a/app/serializers/project_note_entity.rb b/app/serializers/project_note_entity.rb index 3c209f7809200a..738c894e58f5c9 100644 --- a/app/serializers/project_note_entity.rb +++ b/app/serializers/project_note_entity.rb @@ -20,10 +20,6 @@ class ProjectNoteEntity < NoteEntity expose :path, if: ->(note, _) { note.id } do |note| project_note_path(note.project, note) end - - expose :delete_attachment_path, if: ->(note, _) { note.attachment? } do |note| - delete_attachment_project_note_path(note.project, note) - end end ProjectNoteEntity.prepend_mod_with('ProjectNoteEntity') diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index 64858fde355036..c25654d4f20a01 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -21,12 +21,3 @@ .event-note .md = first_line_in_markdown(event.target, :note, 150, project: event.project) - - note = event.target - - if note.attachment.url - - if note.attachment.image? - = link_to note.attachment.url, target: '_blank' do - = image_tag note.attachment.url, class: 'note-image-attach col-lg-4' - - else - = link_to note.attachment.url, target: '_blank', class: 'note-file-attach' do - = sprite_icon("paperclip") - = note.attachment_identifier diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 3607919e996e25..b50e4b4d39024c 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -69,15 +69,3 @@ = _("Toggle commit list") = sprite_icon('chevron-down', css_class: 'js-chevron-down gl-ml-1 gl-align-text-bottom') = sprite_icon('chevron-up', css_class: 'js-chevron-up gl-ml-1 gl-align-text-bottom gl-hidden') - - if note.attachment.url - .note-attachment - - if note.attachment.image? - = link_to note.attachment.url, target: '_blank' do - = image_tag note.attachment.url, class: 'note-image-attach col-lg-4' - .attachment - = link_to note.attachment.url, target: '_blank' do - = sprite_icon('paperclip') - = note.attachment_identifier - = link_to delete_attachment_project_note_path(note.project, note), - title: _('Delete this attachment'), method: :delete, remote: true, data: { confirm: _('Are you sure you want to remove the attachment?') }, class: 'danger js-note-attachment-delete' do - = sprite_icon('remove', css_class: 'gl-fill-icon-danger') diff --git a/config/routes/project.rb b/config/routes/project.rb index 4a8ed14ebb59a0..b1ddd3fbd9acd4 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -592,7 +592,6 @@ resources :notes, only: [:create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do # rubocop: disable Cop/PutProjectRoutesUnderScope member do - delete :delete_attachment # rubocop:todo Cop/PutProjectRoutesUnderScope post :resolve # rubocop:todo Cop/PutProjectRoutesUnderScope delete :resolve, action: :unresolve # rubocop:todo Cop/PutProjectRoutesUnderScope get :outdated_line_change # rubocop:todo Cop/PutProjectRoutesUnderScope diff --git a/config/routes/snippets.rb b/config/routes/snippets.rb index 4b76ebd5ea5542..e639416b85bd66 100644 --- a/config/routes/snippets.rb +++ b/config/routes/snippets.rb @@ -11,11 +11,7 @@ end scope module: :snippets do - resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do - member do - delete :delete_attachment - end - end + resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } end end diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index 1ad973c76d0367..eed87ff78aebcc 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -1,12 +1,13 @@ # frozen_string_literal: true scope path: :uploads do - # Note attachments and User/Group/Project/Topic avatars + # User/Group/Project/Topic avatars get "-/system/:model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { - model: %r{note|user|group|project|projects\/topic|achievements\/achievement|organizations\/organization_detail}, - mounted_as: /avatar|attachment/, filename: %r{[^/]+} + model: %r{user|group|project|projects\/topic|achievements\/achievement|organizations\/organization_detail}, + mounted_as: 'avatar', + filename: %r{[^/]+} } # show uploads for models, snippets (notes) available for now @@ -47,8 +48,3 @@ constraints: { model: /abuse_report/, mounted_as: /screenshot/, filename: %r{[^/]+} }, as: 'abuse_report_screenshot' end - -# Redirect old note attachments path to new uploads path. -get "files/note/:id/:filename", - to: redirect("uploads/note/attachment/%{id}/%{filename}"), - constraints: { filename: %r{[^/]+} } diff --git a/doc/administration/raketasks/uploads/migrate.md b/doc/administration/raketasks/uploads/migrate.md index b461cd89c55139..1f79204d4eb3fb 100644 --- a/doc/administration/raketasks/uploads/migrate.md +++ b/doc/administration/raketasks/uploads/migrate.md @@ -121,7 +121,6 @@ gitlab-rake "gitlab:uploads:migrate[AvatarUploader, Group, :avatar]" gitlab-rake "gitlab:uploads:migrate[AvatarUploader, User, :avatar]" # Attachments -gitlab-rake "gitlab:uploads:migrate[AttachmentUploader, Note, :attachment]" gitlab-rake "gitlab:uploads:migrate[AttachmentUploader, Appearance, :logo]" gitlab-rake "gitlab:uploads:migrate[AttachmentUploader, Appearance, :header_logo]" @@ -153,7 +152,6 @@ sudo -u git -H bundle exec rake "gitlab:uploads:migrate[AvatarUploader, Group, : sudo -u git -H bundle exec rake "gitlab:uploads:migrate[AvatarUploader, User, :avatar]" # Attachments -sudo -u git -H bundle exec rake "gitlab:uploads:migrate[AttachmentUploader, Note, :attachment]" sudo -u git -H bundle exec rake "gitlab:uploads:migrate[AttachmentUploader, Appearance, :logo]" sudo -u git -H bundle exec rake "gitlab:uploads:migrate[AttachmentUploader, Appearance, :header_logo]" diff --git a/doc/api/notes.md b/doc/api/notes.md index d12ce49271e96a..6e7935bd94e164 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -77,7 +77,6 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at { "id": 302, "body": "closed", - "attachment": null, "author": { "id": 1, "username": "pipin", @@ -102,7 +101,6 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at { "id": 305, "body": "Text of the comment\r\n", - "attachment": null, "author": { "id": 1, "username": "pipin", @@ -266,7 +264,6 @@ Parameters: { "id": 302, "body": "closed", - "attachment": null, "author": { "id": 1, "username": "pipin", @@ -404,7 +401,6 @@ Parameters: { "id": 301, "body": "Comment for MR", - "attachment": null, "author": { "id": 1, "username": "pipin", @@ -573,7 +569,6 @@ Parameters: { "id": 302, "body": "Epic note", - "attachment": null, "author": { "id": 1, "username": "pipin", diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index cf37f5a6a461a2..aa668fab219619 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -60043,8 +60043,6 @@ definitions: type: string body: type: string - attachment: - type: string author: "$ref": "#/definitions/API_Entities_UserBasic" created_at: diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index 9cec2407b9f5a6..7ffca6a5a5e326 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -45,7 +45,6 @@ they are still not 100% standardized. You can see them below: | Project avatars | yes | `uploads/-/system/project/avatar/:id/:filename` | `AvatarUploader` | Project | | Topic avatars | yes | `uploads/-/system/projects/topic/avatar/:id/:filename` | `AvatarUploader` | Topic | | Issues/MR/Notes Markdown attachments | yes | `uploads/:hash_project_id/:random_hex/:filename` | `FileUploader` | Project | -| Issues/MR/Notes Legacy Markdown attachments | no | `uploads/-/system/note/attachment/:id/:filename` | `AttachmentUploader` | Note | | Design Management design thumbnails | yes | `uploads/-/system/design_management/action/image_v432x230/:id/:filename` | `DesignManagement::DesignV432x230Uploader` | DesignManagement::Action | | CI Artifacts (CE) | yes | `shared/artifacts/:disk_hash[0..1]/:disk_hash[2..3]/:disk_hash/:year_:month_:date/:job_id/:job_artifact_id` (`:disk_hash` is SHA256 digest of `project_id`) | `JobArtifactUploader` | Ci::JobArtifact | | LFS Objects (CE) | yes | `shared/lfs-objects/:hex/:hex/:object_hash` | `LfsObjectUploader` | LfsObject | diff --git a/ee/spec/fixtures/api/schemas/vulnerability_notes.json b/ee/spec/fixtures/api/schemas/vulnerability_notes.json index f390cf1882483c..867e1804569b6f 100644 --- a/ee/spec/fixtures/api/schemas/vulnerability_notes.json +++ b/ee/spec/fixtures/api/schemas/vulnerability_notes.json @@ -12,7 +12,6 @@ "required": [ "id", "type", - "attachment", "author", "created_at", "updated_at", @@ -124,4 +123,4 @@ } }, "additionalProperties": false -} \ No newline at end of file +} diff --git a/ee/spec/services/epics/issue_promote_service_spec.rb b/ee/spec/services/epics/issue_promote_service_spec.rb index 695dcb377fff36..cf5e9ff2f40392 100644 --- a/ee/spec/services/epics/issue_promote_service_spec.rb +++ b/ee/spec/services/epics/issue_promote_service_spec.rb @@ -342,24 +342,6 @@ weight: 3 ) end - - it 'copies note attachments' do - create(:discussion_note_on_issue, :with_attachment, noteable: issue, project: issue.project) - - epic = subject.execute(issue) - - expect(epic.notes.user.first.attachment).to be_kind_of(AttachmentUploader) - expect_snowplow_event( - category: 'epics', - action: 'promote', - property: 'issue_id', - value: issue.id, - project: project, - user: user, - namespace: group, - weight: 3 - ) - end end context 'on other issue types' do diff --git a/lib/api/entities/note.rb b/lib/api/entities/note.rb index 40c4643f9686ec..f92bf8a19613ec 100644 --- a/lib/api/entities/note.rb +++ b/lib/api/entities/note.rb @@ -13,7 +13,6 @@ class Note < Grape::Entity NotePresenter.new(note, current_user: options[:current_user]).note end - expose :attachment_identifier, as: :attachment expose :author, using: Entities::UserBasic expose :created_at, :updated_at expose :system?, as: :system diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 8234a2f2138812..4c91c7bc3584dd 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -474,7 +474,6 @@ included_attributes: - :created_at - :updated_at - :project_id - - :attachment - :line_code - :commit_id - :system diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index e5668a8c729e53..09bf048ce901ab 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -30,7 +30,6 @@ module PathRegex explore favicon.ico favicon.png - files groups health_check help diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cf0eb2382c15cb..dc5d87178ee05a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8282,9 +8282,6 @@ msgstr "" msgid "Are you sure you want to remove %{topic_name}?" msgstr "" -msgid "Are you sure you want to remove the attachment?" -msgstr "" - msgid "Are you sure you want to remove the license?" msgstr "" @@ -20038,9 +20035,6 @@ msgstr "" msgid "Delete the pipeline" msgstr "" -msgid "Delete this attachment" -msgstr "" - msgid "Delete this epic and release all child items?" msgstr "" diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 3fef9d8aa99161..d313c6a17015e8 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -196,12 +196,13 @@ describe "GET show" do context 'Content-Disposition security measures' do - let(:expected_disposition) { 'inline;' } - let(:project) { create(:project, :public) } + let_it_be(:snippet) { create(:personal_snippet, :public) } + + let(:upload) { create(:upload, :personal_snippet_upload, :with_file, model: snippet, filename: filename) } shared_examples_for 'uploaded file with disposition' do it 'returns correct Content-Disposition' do - get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: filename } + get :show, params: { model: 'personal_snippet', id: snippet.id, secret: upload.secret, filename: filename } expect(response['Content-Disposition']).to start_with(expected_disposition) end @@ -210,7 +211,6 @@ context 'for PNG files' do let(:filename) { 'dk.png' } let(:expected_disposition) { 'inline;' } - let(:note) { create(:note, :with_attachment, project: project) } it_behaves_like 'uploaded file with disposition' end @@ -218,7 +218,6 @@ context 'for PDF files' do let(:filename) { 'sample.pdf' } let(:expected_disposition) { 'inline;' } - let(:note) { create(:note, :with_pdf_attachment, project: project) } it_behaves_like 'uploaded file with disposition' end @@ -226,7 +225,6 @@ context 'for SVG files' do let(:filename) { 'unsanitized.svg' } let(:expected_disposition) { 'attachment;' } - let(:note) { create(:note, :with_svg_attachment, project: project) } it_behaves_like 'uploaded file with disposition' end @@ -507,116 +505,6 @@ end end - context "when viewing a note attachment" do - let!(:note) { create(:note, :with_attachment) } - let(:project) { note.project } - - context "when the project is public" do - before do - project.update_attribute(:visibility_level, Project::PUBLIC) - end - - context "when not signed in" do - it "responds with status 200" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:ok) - end - - it_behaves_like 'content not cached' do - subject do - get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } - - response - end - end - end - - context "when signed in" do - before do - sign_in(user) - end - - it "responds with status 200" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:ok) - end - - it_behaves_like 'content not cached' do - subject do - get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } - - response - end - end - end - end - - context "when the project is private" do - before do - project.update_attribute(:visibility_level, Project::PRIVATE) - end - - context "when not signed in" do - it "responds with status 401" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context "when signed in" do - before do - sign_in(user) - end - - context "when the user has access to the project" do - before do - project.add_maintainer(user) - end - - context "when the user is blocked" do - before do - user.block - project.add_maintainer(user) - end - - it "responds with status 401" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context "when the user isn't blocked" do - it "responds with status 200" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:ok) - end - - it_behaves_like 'content not cached' do - subject do - get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } - - response - end - end - end - end - - context "when the user doesn't have access to the project" do - it "responds with status 404" do - get :show, params: { model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png" } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - end - context "when viewing a topic avatar" do let!(:topic) { create(:topic, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index c70ace7c3c378b..6d5f4c5166228e 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -206,18 +206,6 @@ note { AwardEmoji::THUMBS_UP } end - trait :with_attachment do - attachment { fixture_file_upload("spec/fixtures/dk.png", "image/png") } - end - - trait :with_svg_attachment do - attachment { fixture_file_upload("spec/fixtures/unsanitized.svg", "image/svg+xml") } - end - - trait :with_pdf_attachment do - attachment { fixture_file_upload("spec/fixtures/sample.pdf", "application/pdf") } - end - trait :confidential do confidential { true } end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index abe2ec0d5b1dbc..36d58e6eb7ba94 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -62,8 +62,8 @@ end trait :attachment_upload do - mount_point { :attachment } - model { association(:note) } + mount_point { :logo } + model { association(:appearance) } uploader { "AttachmentUploader" } end diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index 420537f8e06419..5f462e994e4fdd 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -12,7 +12,7 @@ end let!(:note) do - create(:note_on_merge_request, :with_attachment, noteable: merge_request, project: project) + create(:note_on_merge_request, noteable: merge_request, project: project) end before do @@ -203,28 +203,5 @@ end end end - - describe 'deleting attachment on legacy diff note' do - before do - find('.note').hover - - find('.js-note-edit').click - end - - # TODO: https://gitlab.com/gitlab-org/gitlab-foss/issues/48034 - xit 'shows the delete link' do - page.within('.note-attachment') do - is_expected.to have_css('.js-note-attachment-delete') - end - end - - # TODO: https://gitlab.com/gitlab-org/gitlab-foss/issues/48034 - xit 'removes the attachment div and resets the edit form' do - accept_confirm { find('.js-note-attachment-delete').click } - is_expected.not_to have_css('.note-attachment') - is_expected.not_to have_css('.current-note-edit-form') - wait_for_requests - end - end end end diff --git a/spec/fixtures/api/schemas/entities/discussion.json b/spec/fixtures/api/schemas/entities/discussion.json index 644c3de4da7c17..81e4f01f220328 100644 --- a/spec/fixtures/api/schemas/entities/discussion.json +++ b/spec/fixtures/api/schemas/entities/discussion.json @@ -29,12 +29,6 @@ "body": { "type": "string" }, - "attachment": { - "type": [ - "string", - "null" - ] - }, "award_emoji": { "type": "array" }, @@ -223,7 +217,6 @@ }, "required": [ "id", - "attachment", "author", "created_at", "updated_at", diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json index 7132186e64bb2e..9fd3e26cbc25e2 100644 --- a/spec/fixtures/api/schemas/public_api/v4/notes.json +++ b/spec/fixtures/api/schemas/public_api/v4/notes.json @@ -15,12 +15,6 @@ "body": { "type": "string" }, - "attachment": { - "type": [ - "string", - "null" - ] - }, "imported": { "type": "boolean" }, @@ -123,7 +117,6 @@ "required": [ "id", "body", - "attachment", "author", "created_at", "updated_at", diff --git a/spec/lib/bulk_imports/projects/pipelines/snippets_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/snippets_pipeline_spec.rb index 8570bcecc7f164..aabe94c2e754a3 100644 --- a/spec/lib/bulk_imports/projects/pipelines/snippets_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/snippets_pipeline_spec.rb @@ -97,7 +97,7 @@ # object, then parse it right away. We expected that some attrs like Datetimes be # converted to Strings. let(:exported_snippet) { Gitlab::Json.parse(note.noteable.attributes.merge('notes' => notes).to_json) } - let(:note) { create(:note_on_project_snippet, :with_attachment) } + let(:note) { create(:note_on_project_snippet) } let(:notes) { [note.attributes.merge('author' => { 'name' => note.author.name })] } it 'restores the notes' do diff --git a/spec/lib/gitlab/cleanup/project_uploads_spec.rb b/spec/lib/gitlab/cleanup/project_uploads_spec.rb index 92fe34a16c2c9d..1473084feee551 100644 --- a/spec/lib/gitlab/cleanup/project_uploads_spec.rb +++ b/spec/lib/gitlab/cleanup/project_uploads_spec.rb @@ -239,11 +239,13 @@ end it 'does not move any non-project (FileUploader) uploads' do + appearance = create(:appearance) + paths = [] orphaned1 = create(:upload, :personal_snippet_upload, :with_file) orphaned2 = create(:upload, :namespace_upload, :with_file) - orphaned3 = create(:upload, :attachment_upload, :with_file) - orphaned4 = create(:upload, :favicon_upload, :with_file) + orphaned3 = create(:upload, :attachment_upload, :with_file, model: appearance) + orphaned4 = create(:upload, :favicon_upload, :with_file, model: appearance) paths << orphaned1.absolute_path paths << orphaned2.absolute_path paths << orphaned3.absolute_path diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 786f9f989789ea..434a2e153720a2 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -194,7 +194,7 @@ def failure_message(constant_name, migration_helper, missing_words: [], addition # We ban new items in this list, see https://gitlab.com/gitlab-org/gitlab/-/issues/215362 it 'does not allow expansion' do - expect(described_class::TOP_LEVEL_ROUTES.size).to eq(39) + expect(described_class::TOP_LEVEL_ROUTES.size).to eq(38) end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index edb853f5b35480..7abb7664f4273a 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1992,18 +1992,6 @@ def update_note(note, **attributes) end end - describe '#attachment' do - it 'is cleaned up correctly when project is destroyed' do - note = create(:note_on_issue, :with_attachment) - - attachment = note.attachment - - note.project.destroy! - - expect(attachment).not_to be_exist - end - end - describe '#post_processed_cache_key' do let(:note) { build(:note) } @@ -2110,13 +2098,4 @@ def update_note(note, **attributes) end end end - - describe '#uploads_sharding_key' do - it 'returns namespace_id' do - namespace = build_stubbed(:namespace) - note = build_stubbed(:note, namespace: namespace) - - expect(note.uploads_sharding_key).to eq(namespace_id: namespace.id) - end - end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index b92999cbfe6065..90dfb3a33f1f12 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -86,14 +86,11 @@ let_it_be(:project) { create(:project) } describe '.for_model_type_and_id' do - let(:avatar_uploads) { create_list(:upload, 2) } - let(:attachment_uploads) { create_list(:upload, 2, :attachment_upload) } + let(:snippet_uploads) { create_list(:upload, 2, :personal_snippet_upload) } it 'returns records matching the given model_type and ids' do - model_ids = [avatar_uploads, attachment_uploads].map { |uploads| uploads.first.model_id } - - expect(described_class.for_model_type_and_id(Note, model_ids)) - .to contain_exactly(attachment_uploads.first) + expect(described_class.for_model_type_and_id(Snippet, snippet_uploads.first.model_id)) + .to contain_exactly(snippet_uploads.first) end end diff --git a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb index e8238480ced078..d1f1fd7ccc71d8 100644 --- a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb +++ b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb @@ -6,7 +6,6 @@ context 'basic note' do it 'exposes correct elements' do expect(subject).to include( - :attachment, :author, :award_emoji, :base_discussion, diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb index e4e96aa15b7777..335c443839e9ce 100644 --- a/spec/uploaders/attachment_uploader_spec.rb +++ b/spec/uploaders/attachment_uploader_spec.rb @@ -3,16 +3,16 @@ require 'spec_helper' RSpec.describe AttachmentUploader do - let(:note) { create(:note, :with_attachment) } - let(:uploader) { note.attachment } + let(:appearance) { create(:appearance, :with_logo) } + let(:uploader) { appearance.logo } let(:upload) { create(:upload, :attachment_upload, model: uploader.model) } subject { uploader } it_behaves_like 'builds correct paths', - store_dir: %r{uploads/-/system/note/attachment/}, - upload_path: %r{uploads/-/system/note/attachment/}, - absolute_path: %r{#{CarrierWave.root}/uploads/-/system/note/attachment/} + store_dir: %r{uploads/-/system/appearance/logo/}, + upload_path: %r{uploads/-/system/appearance/logo/}, + absolute_path: %r{#{CarrierWave.root}/uploads/-/system/appearance/logo/} context "object_store is REMOTE" do before do @@ -22,8 +22,8 @@ include_context 'with storage', described_class::Store::REMOTE it_behaves_like 'builds correct paths', - store_dir: %r{note/attachment/}, - upload_path: %r{note/attachment/} + store_dir: %r{appearance/logo/}, + upload_path: %r{appearance/logo/} end describe "#migrate!" do -- GitLab