diff --git a/back/app/controllers/web_api/v1/files_controller.rb b/back/app/controllers/web_api/v1/files_controller.rb index b3aecb752865..fdf4ec83034d 100644 --- a/back/app/controllers/web_api/v1/files_controller.rb +++ b/back/app/controllers/web_api/v1/files_controller.rb @@ -5,121 +5,60 @@ class WebApi::V1::FilesController < ApplicationController skip_before_action :authenticate_user - CONSTANTIZER = { - 'Idea' => { - container_class: Idea, - file_class: IdeaFile, - policy_scope_class: IdeaFilePolicy::Scope, - file_relationship: :idea_files, - container_id: :idea_id - }, - 'Project' => { - container_class: Project, - file_class: ProjectFile, - policy_scope_class: ProjectFilePolicy::Scope, - file_relationship: :project_files, - container_id: :project_id - }, - 'ProjectFolder' => { - container_class: ProjectFolders::Folder, - file_class: ProjectFolders::File, - policy_scope_class: ProjectFolders::FilePolicy::Scope, - file_relationship: :files, - container_id: :project_folder_id - }, - 'Event' => { - container_class: Event, - file_class: EventFile, - policy_scope_class: EventFilePolicy::Scope, - file_relationship: :event_files, - container_id: :event_id - }, - 'Phase' => { - container_class: Phase, - file_class: PhaseFile, - policy_scope_class: PhaseFilePolicy::Scope, - file_relationship: :phase_files, - container_id: :phase_id - }, - 'StaticPage' => { - container_class: StaticPage, - file_class: StaticPageFile, - policy_scope_class: StaticPageFilePolicy::Scope, - file_relationship: :static_page_files, - container_id: :static_page_id - } + # Maps container types (as used in params) to their corresponding container classes and + # the key of the container ID in params. Ideally, the ID parameter should be the same + # for all container types (for example, +container_id+). + CONTAINER_MAPPINGS = { + 'Idea' => [Idea, :idea_id], + 'Project' => [Project, :project_id], + 'ProjectFolder' => [ProjectFolders::Folder, :project_folder_id], + 'Event' => [Event, :event_id], + 'Phase' => [Phase, :phase_id], + 'StaticPage' => [StaticPage, :static_page_id] } - before_action :set_container, only: %i[index create] - before_action :set_file, only: %i[show destroy update] - skip_after_action :verify_policy_scoped - def index - files = @container.send(secure_constantize(:file_relationship)).order(:ordering) - - if files.empty? - file_attachments = policy_scope(@container.file_attachments.includes(:file)) - render json: WebApi::V1::FileAttachmentSerializerAsLegacyFile.new( - file_attachments, - params: jsonapi_serializer_params - ).serializable_hash - else - files = secure_constantize(:policy_scope_class).new(pundit_user, files).resolve - render json: WebApi::V1::FileSerializer.new( - files, - params: jsonapi_serializer_params - ).serializable_hash - end + file_attachments = policy_scope(container.file_attachments.includes(:file)) + + render json: WebApi::V1::FileAttachmentSerializerAsLegacyFile.new( + file_attachments, + params: jsonapi_serializer_params + ).serializable_hash end def show - render json: serialize_file(@file) + render json: serialize_file(file_attachment) end def create - container_files = @container.send(secure_constantize(:file_relationship)) - - # If there exist files using the legacy file class, continue using it. We'll migrate - # all the files of a given container at once. They cannot be mixed. - file = if container_files.exists? - @container.send(secure_constantize(:file_relationship)).new(create_file_params) - else - build_file_attachment - end - - authorize(file) - - if file.save - render json: serialize_file(file), status: :created - else - render json: { errors: transform_carrierwave_error_details(file.errors, :file) }, status: :unprocessable_entity - end + file_attachment = authorize(build_file_attachment) + file_attachment.file.save! + file_attachment.save! + + render json: serialize_file(file_attachment), status: :created + rescue ActiveRecord::RecordInvalid + errors = format_errors(file_attachment) + render json: { errors: }, status: :unprocessable_entity end def update - new_ordering = params.dig(:file, :ordering) - return render(json: serialize_file(@file)) unless new_ordering + position = params.dig(:file, :ordering) - if @file.is_a?(Files::FileAttachment) - @file.position = new_ordering + if file_attachment.update(position:) + render json: serialize_file(file_attachment) else - @file.ordering = new_ordering - end - - if @file.save - render json: serialize_file(@file) - else - render json: { errors: transform_carrierwave_error_details(@file.errors, :file) }, status: :unprocessable_entity + errors = format_errors(file_attachment) + render json: { errors: }, status: :unprocessable_entity end end def destroy - @file.destroy! - SideFxFileService.new.after_destroy(@file) + file_attachment.destroy! + SideFxFileService.new.after_destroy(file_attachment) head :ok rescue ActiveRecord::RecordNotDestroyed - render json: { errors: @file.errors.details }, status: :unprocessable_entity + render json: { errors: file_attachment.errors.details }, status: :unprocessable_entity end private @@ -141,32 +80,25 @@ def create_file_params returned_file_params end - def update_file_params - params.require(:file).permit(:ordering) - end - - def set_file - # First attempt to find using legacy file class - @file = secure_constantize(:file_class).find(params[:id]) - authorize @file - rescue ActiveRecord::RecordNotFound - # Fall back to FileAttachment lookup - @file = Files::FileAttachment.includes(:file).find(params[:id]) - authorize @file - end - - def set_container - container_id = params[secure_constantize(:container_id)] - @container = secure_constantize(:container_class).find container_id + def file_attachment + @file_attachment ||= authorize( + # TODO: (tech-debt) Ideally, we should use the id of the file, not the id of the + # file attachment. + Files::FileAttachment.includes(:file).find(params[:id]) + ) end - def secure_constantize(key) - CONSTANTIZER.fetch(params[:container_type])[key] + def container + @container ||= begin + container_class, id_key = CONTAINER_MAPPINGS.fetch(params[:container_type]) + container_class.find(params[id_key]) + end end def serialize_file(file) - serializer_class = file.is_a?(Files::FileAttachment) ? WebApi::V1::FileAttachmentSerializerAsLegacyFile : WebApi::V1::FileSerializer - serializer_class.new(file, params: jsonapi_serializer_params).serializable_hash + WebApi::V1::FileAttachmentSerializerAsLegacyFile + .new(file, params: jsonapi_serializer_params) + .serializable_hash end def build_file_attachment @@ -177,14 +109,22 @@ def build_file_attachment uploader: current_user ) - if (project = @container.try(:project)) + if (project = container.try(:project)) files_file.files_projects.build(project: project) end Files::FileAttachment.new( file: files_file, - attachable: @container, + attachable: container, position: file_params[:ordering] ) end + + def format_errors(file_attachment) + # Surface the file errors in the response. + file_errors = transform_carrierwave_error_details(file_attachment.file.errors, :content) + # TODO: Adapt the front-end to use "content" instead of "file". + file_errors[:file] = file_errors.delete(:content) + file_errors.merge(file_attachment.errors.details) + end end diff --git a/back/spec/acceptance/event_files_spec.rb b/back/spec/acceptance/event_files_spec.rb index 4134e36b66e8..a20e63c034bf 100644 --- a/back/spec/acceptance/event_files_spec.rb +++ b/back/spec/acceptance/event_files_spec.rb @@ -3,15 +3,18 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'EventFile' do - explanation 'File attachments.' +resource 'File attachment as legacy EventFile' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +EventFile+ model. + EXPLANATION before do header 'Content-Type', 'application/json' admin_header_token @project = create(:project) @event = create(:event, project: @project) - create_list(:event_file, 2, event: @event) + create_list(:file_attachment, 2, attachable: @event) end get 'web_api/v1/events/:event_id/files' do @@ -19,19 +22,29 @@ example_request 'List all file attachments of an event' do assert_status 200 - json_response = json_parse(response_body) - expect(json_response[:data].size).to eq 2 + expect(response_data.size).to eq 2 end end get 'web_api/v1/events/:event_id/files/:file_id' do let(:event_id) { @event.id } - let(:file_id) { EventFile.first.id } + let(:file_id) { @event.file_attachments.first.id } example_request 'Get one file of an event' do assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present + + expect(response_data).to include( + type: 'file', + id: file_id, + attributes: hash_including( + file: hash_including(url: end_with('.pdf')), + ordering: nil, + name: be_a(String), + size: be_an(Integer), + created_at: be_a(String), + updated_at: be_a(String) + ) + ) end end @@ -41,14 +54,12 @@ end let(:event_id) { @event.id } - let(:file_id) { EventFile.first.id } + let(:file_id) { @event.file_attachments.first.id } let(:ordering) { 3 } example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(3) end end @@ -58,60 +69,73 @@ parameter :name, 'The name of the file, including the file extension', required: true parameter :ordering, 'An integer that is used to order the file attachments within an event', required: false end - ValidationErrorHelper.new.error_fields(self, EventFile) + ValidationErrorHelper.new.error_fields(self, Files::File) let(:event_id) { @event.id } let(:ordering) { 1 } let(:name) { 'afvalkalender.pdf' } let(:file) { file_as_base64 name, 'application/pdf' } - example_request 'Add a file attachment to an event' do + example 'Add a file attachment to an event' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and not_change(EventFile, :count) + expect(response_status).to eq 201 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq(1) - expect(json_response.dig(:data, :attributes, :name)).to eq(name) - expect(json_response.dig(:data, :attributes, :size)).to be_present + + expect(response_data[:attributes]).to include( + file: be_present, + ordering: 1, + name: name, + size: be_present + ) end - describe do + describe 'Add a file with an unsupported file extension', pending: <<~REASON do + Currently, the +Files::FileUploader+ allows all file extensions. + REASON let(:file) { file_as_base64 'keylogger.exe', 'application/octet-stream' } let(:name) { 'keylogger.exe' } - example_request '[error] Add an unsupported file extension as attachment to an event' do + example_request '[error]' do assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'extension_whitelist_error') + expect(json_response_body).to include_response_error(:file, 'extension_whitelist_error') end end - describe do + describe 'Add a file of which the size is too small' do let(:file) { 'data:application/pdf;base64,===' } - example_request '[error] Add a file of which the size is too small' do + example_request '[error]' do assert_status 422 end end - describe do - example '[error] Add a file of which the size is too large' do - # mock the size_range method of EventFileUploader to have 3 bytes as maximum size - expect_any_instance_of(EventFileUploader).to receive(:size_range).and_return(1..3) + describe 'Add a file that is too large' do + example '[error]' do + # Mock the `size_range` method of `Files::FileUploader` to set the maximum size to 3 bytes. + expect_any_instance_of(Files::FileUploader).to receive(:size_range).and_return(1..3) do_request assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'max_size_error') + expect(json_response_body).to include_response_error(:file, 'max_size_error') end end end delete 'web_api/v1/events/:event_id/files/:file_id' do let(:event_id) { @event.id } - let(:file_id) { EventFile.first.id } + let(:file_id) { @event.file_attachments.first.id } - example_request 'Delete a file attachment from an event' do - assert_status 200 - expect { EventFile.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) + example 'Delete file attachment from an event' do + expect { do_request } + .to change(Files::FileAttachment, :count).by(-1) + .and not_change(EventFile, :count) + + expect(response_status).to eq 200 + + expect { Files::FileAttachment.find(file_id) } + .to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/back/spec/acceptance/files/file_attachments_as_legacy_files_spec.rb b/back/spec/acceptance/files/file_attachments_as_legacy_files_spec.rb deleted file mode 100644 index 6ce421018a68..000000000000 --- a/back/spec/acceptance/files/file_attachments_as_legacy_files_spec.rb +++ /dev/null @@ -1,214 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' -require 'rspec_api_documentation/dsl' - -require_relative '../shared/errors_examples' - -resource 'FileAttachments as legacy files' do - header 'Content-Type', 'application/json' - before { admin_header_token } - - let_it_be(:project) { create(:project) } - let(:project_id) { project.id } - - get 'web_api/v1/projects/:project_id/files' do - let_it_be(:file_attachment) { create_pair(:file_attachment, attachable: project) } - - context 'when there are legacy files' do - let!(:legacy_file) { create(:project_file, project: project) } - - example_request 'List only the legacy files' do - assert_status 200 - expect(response_data.size).to eq(1) - expect(response_ids).to eq [legacy_file.id] - end - end - - context 'when there are only file attachments' do - example 'List the file attachments' do - do_request - assert_status 200 - expect(response_data.size).to eq(2) - expect(response_ids).to match_array file_attachment.map(&:id) - end - end - end - - get 'web_api/v1/projects/:project_id/files/:id' do - let_it_be(:file_attachment) { create(:file_attachment, attachable: project) } - let_it_be(:legacy_file) { create(:project_file, project: project) } - - let(:project_id) { project.id } - let(:id) { file_attachment.id } - - example 'Get a file attachment by id' do - do_request(id: file_attachment.id) - assert_status 200 - - expect(response_data).to match hash_including( - id: file_attachment.id, - type: 'file', - attributes: hash_including( - ordering: file_attachment.position, - file: { url: file_attachment.file.content.url }, - name: file_attachment.file.name, - size: file_attachment.file.size, - created_at: file_attachment.file.created_at.iso8601(3), - updated_at: file_attachment.file.updated_at.iso8601(3) - ) - ) - end - - example 'Get a legacy file by id' do - do_request(id: legacy_file.id) - assert_status 200 - expect(response_data[:id]).to eq(legacy_file.id) - end - end - - patch 'web_api/v1/projects/:project_id/files/:id' do - with_options scope: :file do - parameter :ordering, 'An integer to update the order of the file attachments', required: false - end - - example 'Update the ordering of a file attachment by id' do - attachment1 = create(:file_attachment, attachable: project, position: 1) - attachment2 = create(:file_attachment, attachable: project, position: 2) - - expect(attachment1.position).to eq(1) - expect(attachment2.position).to eq(2) - - do_request(id: attachment2.id, ordering: 1) - assert_status 200 - expect(response_data.dig(:attributes, :ordering)).to eq(1) - - # The front-end has full control over the ordering of file attachments which can - # lead to inconsistencies. This will be reworked in the future. - # See ticket TAN-5126. - expect(attachment1.reload.position).to eq(1) - expect(attachment2.reload.position).to eq(1) - end - - example 'Update the ordering of a legacy file by id' do - # We only need one file to test reordering because the backend does not manage - # ordering for legacy files. The value can be set arbitrarily by the frontend. - file = create(:project_file, project: project, ordering: 1) - # This should not be taken into account. In principle, file attachments and legacy - # files should not be mixed, but we're testing it anyway. - attachment = create(:file_attachment, attachable: project, position: 1) - - do_request(id: file.id, ordering: 2) - assert_status 200 - expect(response_data.dig(:attributes, :ordering)).to eq(2) - - expect(file.reload.ordering).to eq(2) - expect(attachment.reload.position).to eq(1) - end - end - - delete 'web_api/v1/projects/:project_id/files/:id' do - let_it_be(:legacy_file) { create(:project_file, project: project) } - - context 'when attaching a file to an Idea' do - let!(:file_attachment) { create(:file_attachment, to: :idea) } - - example 'Delete a file attachment by id and the file' do - do_request(id: file_attachment.id) - - assert_status 200 - expect { file_attachment.file.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { file_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - end - - context 'when attaching a file to a non-Idea resource' do - let!(:file_attachment) { create(:file_attachment, to: :event) } - - example 'Delete a file attachment by id' do - do_request(id: file_attachment.id) - - assert_status 200 - expect { file_attachment.file.reload }.not_to raise_error - expect { file_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - end - - example 'Delete a legacy file by id' do - do_request(id: legacy_file.id) - assert_status 200 - expect { ProjectFile.find(legacy_file.id) } - .to raise_error(ActiveRecord::RecordNotFound) - end - end - - post 'web_api/v1/projects/:project_id/files' do - with_options scope: :file do - parameter :file, 'The base64 encoded file', required: true - parameter :name, 'The name of the file, including the file extension', required: true - parameter :ordering, 'An integer that is used to order the file attachments within a project', required: false - end - - ValidationErrorHelper.new.error_fields(self, ProjectFile) - - let(:ordering) { 1 } - let(:name) { 'minimal_pdf.pdf' } - let(:file) { file_as_base64 name, 'application/pdf' } - - context 'when there are no legacy files' do - let!(:file_attachment) { create(:file_attachment, attachable: project, position: 1) } - - example 'Create a file as a file attachment' do - expect { do_request } - .to change(Files::File, :count).by(1) - .and(change(Files::FileAttachment, :count).by(1)) - .and(change(Files::FilesProject, :count).by(1)) - .and not_change(ProjectFile, :count) - .and not_change(file_attachment.reload, :position) - - assert_status 201 - - expect(response_data).to match hash_including( - id: be_present, - type: 'file', - attributes: { - ordering: ordering, - file: { url: be_present }, - name: name, - size: 130, - created_at: be_present, - updated_at: be_present - } - ) - - attachment = Files::FileAttachment.find(response_data[:id]) - expect(attachment.file.projects).to contain_exactly(project) - end - end - - context 'when there are legacy files' do - before { create(:project_file, project: project) } - - example 'Create a file as a legacy file' do - expect { do_request } - .to change(ProjectFile, :count).by(1) - - assert_status 201 - expect(response_data[:id]).to be_present - - expect(response_data).to match hash_including( - id: be_present, - type: 'file', - attributes: { - ordering: ordering, - file: { url: be_present }, - name: name, - size: 130, - created_at: be_present, - updated_at: be_present - } - ) - end - end - end -end diff --git a/back/spec/acceptance/idea_files_spec.rb b/back/spec/acceptance/idea_files_spec.rb index bd8097b1e889..4be29fdd172a 100644 --- a/back/spec/acceptance/idea_files_spec.rb +++ b/back/spec/acceptance/idea_files_spec.rb @@ -3,8 +3,11 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'IdeaFile' do - explanation 'File attachments.' +resource 'File attachment as legacy IdeaFile' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +IdeaFile+ model. + EXPLANATION before do header 'Content-Type', 'application/json' @@ -12,27 +15,37 @@ header_token_for @user @project = create(:single_phase_ideation_project) @idea = create(:idea, author: @user, project: @project) - create_list(:idea_file, 2, idea: @idea) + create_list(:file_attachment, 2, attachable: @idea) end get 'web_api/v1/ideas/:idea_id/files' do let(:idea_id) { @idea.id } example_request 'List all file attachments of an idea' do - expect(status).to eq(200) - json_response = json_parse(response_body) - expect(json_response[:data].size).to eq 2 + assert_status 200 + expect(response_data.size).to eq 2 end end get 'web_api/v1/ideas/:idea_id/files/:file_id' do let(:idea_id) { @idea.id } - let(:file_id) { IdeaFile.first.id } + let(:file_id) { @idea.file_attachments.first.id } example_request 'Get one file attachment of an idea by id' do expect(status).to eq(200) - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present + + expect(response_data).to include( + type: 'file', + id: file_id, + attributes: hash_including( + file: hash_including(url: end_with('.pdf')), + ordering: nil, + name: be_a(String), + size: be_an(Integer), + created_at: be_a(String), + updated_at: be_a(String) + ) + ) end end @@ -42,14 +55,12 @@ end let(:idea_id) { @idea.id } - let(:file_id) { IdeaFile.first.id } + let(:file_id) { @idea.file_attachments.first.id } let(:ordering) { 3 } example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(3) end end @@ -59,34 +70,46 @@ parameter :name, 'The name of the file, including the file extension', required: true parameter :ordering, 'An integer that is used to order the files within an idea', required: false end - ValidationErrorHelper.new.error_fields(self, IdeaFile) + ValidationErrorHelper.new.error_fields(self, Files::File) let(:idea_id) { @idea.id } - let(:file) { encode_file_as_base64('afvalkalender.pdf') } - let(:ordering) { 1 } let(:name) { 'afvalkalender.pdf' } + let(:file) { file_as_base64 name, 'application/pdf' } + let(:ordering) { 1 } + + example 'Add a file attachment to an idea' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and not_change(IdeaFile, :count) - example_request 'Add a file attachment to an idea' do - expect(response_status).to eq 201 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq(1) - expect(json_response.dig(:data, :attributes, :name)).to eq(name) + assert_status 201 + + expect(response_data[:attributes]).to include( + file: be_present, + ordering: 1, + name: name + ) end end delete 'web_api/v1/ideas/:idea_id/files/:file_id' do let(:idea_id) { @idea.id } - let(:file_id) { IdeaFile.first.id } + let(:file_attachment) { @idea.file_attachments.first } + let(:file_id) { file_attachment.id } - example_request 'Delete a file attachment from an idea' do - expect(response_status).to eq 200 - expect { IdeaFile.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + example 'Delete the file attachment by id and its underlying file' do + file_id = file_attachment.file_id - private + expect { do_request } + .to change(Files::FileAttachment, :count).by(-1) + # Special case: idea files are automatically deleted when detached from their idea + .and change(Files::File, :count).by(-1) + .and not_change(IdeaFile, :count) - def encode_file_as_base64(filename) - "data:application/pdf;base64,#{Base64.encode64(Rails.root.join('spec', 'fixtures', filename).read)}" + assert_status 200 + + expect { Files::File.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { file_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound) + end end end diff --git a/back/spec/acceptance/phase_files_spec.rb b/back/spec/acceptance/phase_files_spec.rb index 9aa89272d837..a32f06f78ddb 100644 --- a/back/spec/acceptance/phase_files_spec.rb +++ b/back/spec/acceptance/phase_files_spec.rb @@ -3,15 +3,18 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'PhaseFile' do - explanation 'File attachments.' +resource 'File attachment as legacy PhaseFile' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +PhaseFile+ model. + EXPLANATION before do header 'Content-Type', 'application/json' admin_header_token @project = create(:project) @phase = create(:phase, project: @project) - create_list(:phase_file, 2, phase: @phase) + create_list(:file_attachment, 2, attachable: @phase) end get 'web_api/v1/phases/:phase_id/files' do @@ -19,19 +22,29 @@ example_request 'List all file attachments of a phase' do assert_status 200 - json_response = json_parse(response_body) - expect(json_response[:data].size).to eq 2 + expect(response_data.size).to eq 2 end end get 'web_api/v1/phases/:phase_id/files/:file_id' do let(:phase_id) { @phase.id } - let(:file_id) { PhaseFile.first.id } + let(:file_id) { @phase.file_attachments.first.id } example_request 'Get one file of a phase' do assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present + + expect(response_data).to include( + type: 'file', + id: file_id, + attributes: hash_including( + file: hash_including(url: end_with('.pdf')), + ordering: nil, + name: be_a(String), + size: be_an(Integer), + created_at: be_a(String), + updated_at: be_a(String) + ) + ) end end @@ -41,14 +54,12 @@ end let(:phase_id) { @phase.id } - let(:file_id) { PhaseFile.first.id } + let(:file_id) { @phase.file_attachments.first.id } let(:ordering) { 3 } example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(3) end end @@ -58,52 +69,65 @@ parameter :name, 'The name of the file, including the file extension', required: true parameter :ordering, 'An integer that is used to order the file attachments within a phase', required: false end - ValidationErrorHelper.new.error_fields(self, PhaseFile) + ValidationErrorHelper.new.error_fields(self, Files::File) let(:phase_id) { @phase.id } let(:ordering) { 1 } let(:name) { 'afvalkalender.pdf' } let(:file) { file_as_base64 name, 'application/pdf' } - example_request 'Add a file attachment to a phase' do + example 'Add a file attachment to a phase' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and not_change(PhaseFile, :count) + assert_status 201 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq(1) - expect(json_response.dig(:data, :attributes, :name)).to eq(name) - expect(json_response.dig(:data, :attributes, :size)).to be_present + + expect(response_data[:attributes]).to include( + file: be_present, + ordering: 1, + name: name, + size: be_present + ) end - describe do + describe 'Add a file with an unsupported file extension', pending: <<~REASON do + Currently, the +Files::FileUploader+ allows all file extensions. + REASON let(:file) { file_as_base64 'keylogger.exe', 'application/octet-stream' } let(:name) { 'keylogger.exe' } - example_request '[error] Add an unsupported file extension as attachment to a phase' do + example_request '[error]' do assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'extension_whitelist_error') + expect(json_response_body).to include_response_error(:file, 'extension_whitelist_error') end end - describe do - example '[error] Add a file of which the size is too large' do - # mock the size_range method of PhaseFileUploader to have 3 bytes as maximum size - expect_any_instance_of(PhaseFileUploader).to receive(:size_range).and_return(1..3) + describe 'Add a file that is too large' do + example '[error]' do + # Mock the `size_range` method of `Files::FileUploader` to set the maximum size to 3 bytes. + expect_any_instance_of(Files::FileUploader).to receive(:size_range).and_return(1..3) do_request assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'max_size_error') + expect(json_response_body).to include_response_error(:file, 'max_size_error') end end end delete 'web_api/v1/phases/:phase_id/files/:file_id' do let(:phase_id) { @phase.id } - let(:file_id) { PhaseFile.first.id } + let(:file_id) { @phase.file_attachments.first.id } + + example 'Delete file attachment from a phase' do + expect { do_request } + .to change(Files::FileAttachment, :count).by(-1) + .and not_change(PhaseFile, :count) - example_request 'Delete a file attachment from a phase' do expect(response_status).to eq 200 - expect { PhaseFile.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) + + expect { Files::FileAttachment.find(file_id) } + .to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/back/spec/acceptance/project_files_spec.rb b/back/spec/acceptance/project_files_spec.rb index f9ee6a4779a3..6f3a014af032 100644 --- a/back/spec/acceptance/project_files_spec.rb +++ b/back/spec/acceptance/project_files_spec.rb @@ -3,59 +3,78 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'ProjectFile' do - explanation 'File attachments.' - - before do - header 'Content-Type', 'application/json' - admin_header_token - @project = create(:project) - create_list(:project_file, 2, project: @project) - end +resource 'File attachment as legacy ProjectFile' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +ProjectFile+ model. + EXPLANATION - get 'web_api/v1/projects/:project_id/files' do - let(:project_id) { @project.id } + header 'Content-Type', 'application/json' + + before { admin_header_token } - example_request 'List all file attachments of a project' do + let_it_be(:project) { create(:project) } + let(:project_id) { project.id } + + get 'web_api/v1/projects/:project_id/files' do + example 'List the file attachments' do + create_list(:file_attachment, 2, attachable: project) + do_request assert_status 200 - json_response = json_parse(response_body) - expect(json_response[:data].size).to eq 2 + expect(response_data.size).to eq 2 + end + + context 'when there are no file attachments' do + example_request 'Returns an empty list' do + assert_status 200 + expect(json_response_body[:data]).to be_empty + end end end - get 'web_api/v1/projects/:project_id/files/:file_id' do - let(:project_id) { @project.id } - let(:file_id) { ProjectFile.first.id } + get 'web_api/v1/projects/:project_id/files/:id' do + let!(:file_attachment) { create(:file_attachment, attachable: project) } + let(:id) { file_attachment.id } - example_request 'Get one file of a project' do + example_request 'Get a file attachment by id' do assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - end - context 'when a legacy file is migrated' do - example 'Get a migrated legacy file by id [NOT FOUND]' do - migrated_file = create(:project_file, project_id: project_id, migrated_file: create(:file)) - do_request(file_id: migrated_file.id) - assert_status 404 - end + expect(response_data).to match hash_including( + id: id, + type: 'file', + attributes: hash_including( + ordering: file_attachment.position, + file: { url: file_attachment.file.content.url }, + name: file_attachment.file.name, + size: file_attachment.file.size, + created_at: file_attachment.file.created_at.iso8601(3), + updated_at: file_attachment.file.updated_at.iso8601(3) + ) + ) end end - patch 'web_api/v1/projects/:project_id/files/:file_id' do + patch 'web_api/v1/projects/:project_id/files/:id' do with_options scope: :file do parameter :ordering, 'An integer to update the order of the file attachments', required: false end - let(:project_id) { @project.id } - let(:file_id) { ProjectFile.first.id } - let(:ordering) { 3 } + example 'Update the ordering of a file attachment by id' do + attachment1 = create(:file_attachment, attachable: project, position: 1) + attachment2 = create(:file_attachment, attachable: project, position: 2) + + expect(attachment1.position).to eq(1) + expect(attachment2.position).to eq(2) - example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) + do_request(id: attachment2.id, ordering: 1) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(1) + + # The front-end has full control over the ordering of file attachments which can + # lead to inconsistencies. This will be reworked in the future. + # See ticket TAN-5126. + expect(attachment1.reload.position).to eq(1) + expect(attachment2.reload.position).to eq(1) end end @@ -65,154 +84,77 @@ parameter :name, 'The name of the file, including the file extension', required: true parameter :ordering, 'An integer that is used to order the file attachments within a project', required: false end - ValidationErrorHelper.new.error_fields(self, ProjectFile) - let(:project_id) { @project.id } - let(:ordering) { 1 } - let(:name) { 'afvalkalender.pdf' } - let(:file) { file_as_base64 name, 'application/pdf' } - - context 'when there are legacy files' do - example 'Add a file to a project' do - expect { do_request } - .to change(@project.project_files.reload, :count).by(1) - .and not_change(@project.file_attachments.reload, :count) - - assert_status 201 - - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq(1) - expect(json_response.dig(:data, :attributes, :name)).to eq(name) - expect(json_response.dig(:data, :attributes, :size)).to be_present - end - end - context 'when there are only migrated legacy files' do - before do - @project.project_files.each do |file| - file.update!(migrated_file: create(:file)) - end - end + ValidationErrorHelper.new.error_fields(self, Files::File) - example 'Create a file as a file attachment' do - expect { do_request } - .to change(Files::File, :count).by(1) - .and(change(Files::FileAttachment, :count).by(1)) - .and not_change(ProjectFile, :count) + let(:ordering) { 1 } + let(:name) { 'minimal_pdf.pdf' } + let(:file) { file_as_base64 name, 'application/pdf' } - assert_status 201 - end + let!(:file_attachment) { create(:file_attachment, attachable: project, position: 1) } + + example 'Add a file to the project' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and(change(Files::FilesProject, :count).by(1)) + .and not_change(ProjectFile, :count) + .and not_change(file_attachment.reload, :position) + + assert_status 201 + + expect(response_data).to match hash_including( + id: be_present, + type: 'file', + attributes: { + ordering: ordering, + file: { url: be_present }, + name: name, + size: 130, + created_at: be_present, + updated_at: be_present + } + ) + + attachment = Files::FileAttachment.find(response_data[:id]) + expect(attachment.file.projects).to contain_exactly(project) end - describe do + # [TODO] Currently, the +Files::FileUploader+ allows all file extensions. + describe 'Add a file with an unsupported file extension', pending: <<~REASON do + Currently, the +Files::FileUploader+ allows all file extensions. + REASON let(:file) { file_as_base64 'keylogger.exe', 'application/octet-stream' } let(:name) { 'keylogger.exe' } - example_request '[error] Add an unsupported file extension as attachment to a project' do + example_request '[error]' do assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'extension_whitelist_error') + expect(json_response_body).to include_response_error(:file, 'extension_whitelist_error') end end - describe do - example '[error] Add a file of which the size is too large' do - # mock the size_range method of ProjectFileUploader to have 3 bytes as maximum size - expect_any_instance_of(ProjectFileUploader).to receive(:size_range).and_return(1..3) + describe 'Add a file that is too large' do + example '[error]' do + # Mock the `size_range` method of `Files::FileUploader` to set the maximum size to 3 bytes. + expect_any_instance_of(Files::FileUploader).to receive(:size_range).and_return(1..3) do_request assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'max_size_error') + expect(json_response_body).to include_response_error(:file, 'max_size_error') end end end - delete 'web_api/v1/projects/:project_id/files/:file_id' do - let(:project_id) { @project.id } - let(:file_id) { ProjectFile.first.id } + delete 'web_api/v1/projects/:project_id/files/:id' do + let!(:file_attachment) { create(:file_attachment, attachable: project) } + let(:id) { file_attachment.id } - example_request 'Delete a file attachment from a project' do - expect(response_status).to eq 200 - expect { ProjectFile.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) - end - - context 'when the legacy file is migrated' do - example 'Delete a migrated legacy file by id [NOT FOUND]' do - migrated_file = create(:project_file, project_id: project_id, migrated_file: create(:file)) - do_request(file_id: migrated_file.id) - assert_status 404 - end - end - end - - get 'web_api/v1/projects/:project_id/files' do - parameter :project_id, 'Project ID', required: true - let(:project) { create(:project) } - let(:project_id) { project.id } - - context 'when CONSTANTIZER search returns no legacy files but file attachments exist' do - let!(:file) { create(:file, name: 'test-attachment.pdf', projects: [project]) } - let!(:file_attachment) { create(:file_attachment, attachable: project, file: file) } - - example_request 'Returns file attachments with exact same JSON structure as legacy files' do - assert_status 200 - json_response = json_parse(response_body) - - expect(json_response[:data].size).to eq(1) - - # Verify JSON API format - file_data = json_response[:data][0] - expect(file_data[:type]).to eq('file') - expect(file_data[:id]).to be_present - - # Verify all required attributes are present - attributes = file_data[:attributes] - expect(attributes).to have_key(:file) - expect(attributes).to have_key(:ordering) - expect(attributes).to have_key(:name) - expect(attributes).to have_key(:size) - expect(attributes).to have_key(:created_at) - expect(attributes).to have_key(:updated_at) - - # Verify attribute values and types - expect(attributes[:name]).to eq('test-attachment.pdf') - expect(attributes[:ordering]).to be_nil - expect(attributes[:size]).to be_a(Integer) - expect(attributes[:created_at]).to be_a(String) - expect(attributes[:updated_at]).to be_a(String) - - # Verify file attribute structure (should contain url) - expect(attributes[:file]).to be_a(Hash) - expect(attributes[:file]).to have_key(:url) - expect(attributes[:file][:url]).to be_a(String) - expect(attributes[:file][:url]).to include('.pdf') - end - end + example 'Delete a file attachment by id' do + do_request(id: file_attachment.id) - context 'when both CONSTANTIZER and file attachments have files' do - let!(:project_file) { create(:project_file, project: project) } - let!(:file) { create(:file, name: 'test-attachment.pdf', projects: [project]) } - let!(:file_attachment) { create(:file_attachment, file: file, attachable: project) } - - example_request 'Prioritizes CONSTANTIZER results over file attachments' do - assert_status 200 - json_response = json_parse(response_body) - - expect(json_response[:data].size).to eq(1) - - # Should return CONSTANTIZER result, not file attachment - expect(json_response[:data][0][:id]).to eq(project_file.id) - end - end - - context 'when neither CONSTANTIZER nor file attachments return results' do - example_request 'Returns empty results' do - assert_status 200 - json_response = json_parse(response_body) - - expect(json_response[:data]).to be_empty - end + assert_status 200 + expect { file_attachment.file.reload }.not_to raise_error + expect { file_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/back/spec/acceptance/project_folder_files_spec.rb b/back/spec/acceptance/project_folder_files_spec.rb index 4a7c09a53d5b..cf2db6a8eacb 100644 --- a/back/spec/acceptance/project_folder_files_spec.rb +++ b/back/spec/acceptance/project_folder_files_spec.rb @@ -3,14 +3,17 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'ProjectFolderFile' do - explanation 'File attachments.' +resource 'File attachment as legacy ProjectFolders::File' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +ProjectFolders::File+ model. + EXPLANATION before do header 'Content-Type', 'application/json' admin_header_token @folder = create(:project_folder) - create_list(:project_folder_file, 2, project_folder: @folder) + create_list(:file_attachment, 2, attachable: @folder) end get 'web_api/v1/project_folders/:folder_id/files' do @@ -18,19 +21,29 @@ example_request 'List all file attachments of a folder' do expect(status).to eq(200) - json_response = json_parse(response_body) - expect(json_response[:data].size).to eq 2 + expect(response_data.size).to eq 2 end end get 'web_api/v1/project_folders/:folder_id/files/:file_id' do let(:folder_id) { @folder.id } - let(:file_id) { ProjectFolders::File.first.id } + let(:file_id) { @folder.file_attachments.first.id } example_request 'Get one file of a folder' do expect(status).to eq(200) - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present + + expect(response_data).to include( + type: 'file', + id: file_id, + attributes: hash_including( + file: hash_including(url: end_with('.pdf')), + ordering: nil, + name: be_a(String), + size: be_an(Integer), + created_at: be_a(String), + updated_at: be_a(String) + ) + ) end end @@ -40,14 +53,12 @@ end let(:folder_id) { @folder.id } - let(:file_id) { ProjectFolders::File.first.id } + let(:file_id) { @folder.file_attachments.first.id } let(:ordering) { 3 } example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(3) end end @@ -57,52 +68,65 @@ parameter :name, 'The name of the file, including the file extension', required: true parameter :ordering, 'An integer that is used to order the file attachments within a folder', required: false end - ValidationErrorHelper.new.error_fields(self, ProjectFolders::File) + ValidationErrorHelper.new.error_fields(self, Files::File) let(:folder_id) { @folder.id } let(:ordering) { 1 } let(:name) { 'afvalkalender.pdf' } let(:file) { file_as_base64 name, 'application/pdf' } - example_request 'Add a file attachment to a folder' do + example 'Add a file attachment to a folder' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and not_change(ProjectFolders::File, :count) + assert_status 201 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq(1) - expect(json_response.dig(:data, :attributes, :name)).to eq(name) - expect(json_response.dig(:data, :attributes, :size)).to be_present + + expect(response_data[:attributes]).to include( + file: be_present, + ordering: 1, + name: name, + size: be_present + ) end - describe do + describe 'Add a file with an unsupported file extension', pending: <<~REASON do + Currently, the +Files::FileUploader+ allows all file extensions. + REASON + let(:file) { file_as_base64 'keylogger.exe', 'application/octet-stream' } let(:name) { 'keylogger.exe' } - let(:file) { file_as_base64 name, 'application/octet-stream' } - example_request '[error] Add an unsupported file extension as attachment to a folder' do + example_request '[error]' do assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'extension_whitelist_error') + expect(json_response_body).to include_response_error(:file, 'extension_whitelist_error') end end - describe do - example '[error] Add a file of which the size is too large' do - # mock the size_range method of ProjectFolderFileUploader to have 3 bytes as maximum size - expect_any_instance_of(ProjectFolders::FileUploader).to receive(:size_range).and_return(1..3) + describe 'Add a file that is too large' do + example '[error]' do + # Mock the `size_range` method of `Files::FileUploader` to set the maximum size to 3 bytes. + expect_any_instance_of(Files::FileUploader).to receive(:size_range).and_return(1..3) do_request assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'max_size_error') + expect(json_response_body).to include_response_error(:file, 'max_size_error') end end end delete 'web_api/v1/project_folders/:folder_id/files/:file_id' do let(:folder_id) { @folder.id } - let(:file_id) { ProjectFolders::File.first.id } + let(:file_id) { @folder.file_attachments.first.id } + + example 'Delete file attachment from a folder' do + expect { do_request } + .to change(Files::FileAttachment, :count).by(-1) + .and not_change(ProjectFolders::File, :count) - example_request 'Delete a file attachment from a folder' do expect(response_status).to eq 200 - expect { ProjectFolders::File.find(file_id) }.to raise_error(ActiveRecord::RecordNotFound) + + expect { Files::FileAttachment.find(file_id) } + .to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/back/spec/acceptance/static_page_files_spec.rb b/back/spec/acceptance/static_page_files_spec.rb index d9b64fd9ef5c..0abddee41c40 100644 --- a/back/spec/acceptance/static_page_files_spec.rb +++ b/back/spec/acceptance/static_page_files_spec.rb @@ -3,14 +3,17 @@ require 'rails_helper' require 'rspec_api_documentation/dsl' -resource 'StaticPageFile' do - explanation 'StaticPage attachments.' +resource 'File attachment as legacy StaticPageFile' do + explanation <<~EXPLANATION + The implementation of this API has been updated to use the +Files:FileAttachment+ + model behind the scenes, instead of the legacy +StaticPageFile+ model. + EXPLANATION before do header 'Content-Type', 'application/json' admin_header_token @page = create(:static_page) - create_list(:static_page_file, 2, static_page: @page) + create_list(:file_attachment, 2, attachable: @page) end get 'web_api/v1/static_pages/:page_id/files' do @@ -18,19 +21,29 @@ example_request 'List all file attachments of a static page' do assert_status 200 - json_response = json_parse response_body - expect(json_response[:data].size).to eq 2 + expect(response_data.size).to eq 2 end end get 'web_api/v1/static_pages/:page_id/files/:file_id' do let(:page_id) { @page.id } - let(:file_id) { StaticPageFile.first.id } + let(:file_id) { @page.file_attachments.first.id } example_request 'Get one file of a static page' do assert_status 200 - json_response = json_parse response_body - expect(json_response.dig(:data, :attributes, :file)).to be_present + + expect(response_data).to include( + type: 'file', + id: file_id, + attributes: hash_including( + file: hash_including(url: end_with('.pdf')), + ordering: nil, + name: be_a(String), + size: be_an(Integer), + created_at: be_a(String), + updated_at: be_a(String) + ) + ) end end @@ -40,69 +53,80 @@ end let(:page_id) { @page.id } - let(:file_id) { StaticPageFile.first.id } + let(:file_id) { @page.file_attachments.first.id } let(:ordering) { 3 } example_request 'Update the ordering of a file attachment' do - do_request(ordering: ordering) assert_status 200 - json_response = json_parse(response_body) - expect(json_response.dig(:data, :attributes, :ordering)).to eq(3) + expect(response_data.dig(:attributes, :ordering)).to eq(3) end end post 'web_api/v1/static_pages/:page_id/files' do with_options scope: :file do parameter :file, 'The base64 encoded file', required: true - parameter :ordering, 'An integer that is used to order the file attachments within a page', required: false parameter :name, 'The name of the file, including the file extension', required: true + parameter :ordering, 'An integer that is used to order the file attachments within a page', required: false end - ValidationErrorHelper.new.error_fields self, StaticPageFile + ValidationErrorHelper.new.error_fields(self, Files::File) let(:page_id) { @page.id } let(:ordering) { 1 } let(:name) { 'afvalkalender.pdf' } let(:file) { file_as_base64 name, 'application/pdf' } - example_request 'Add a file attachment to a page' do + example 'Add a file attachment to a page' do + expect { do_request } + .to change(Files::File, :count).by(1) + .and(change(Files::FileAttachment, :count).by(1)) + .and not_change(StaticPageFile, :count) + assert_status 201 - json_response = json_parse response_body - expect(json_response.dig(:data, :attributes, :file)).to be_present - expect(json_response.dig(:data, :attributes, :ordering)).to eq 1 - expect(json_response.dig(:data, :attributes, :name)).to eq name - expect(json_response.dig(:data, :attributes, :size)).to be_present + + expect(response_data[:attributes]).to include( + file: be_present, + ordering: 1, + name: name, + size: be_present + ) end - describe do + describe 'Add a file with an unsupported file extension', pending: <<~REASON do + Currently, the +Files::FileUploader+ allows all file extensions. + REASON + let(:file) { file_as_base64 'keylogger.exe', 'application/octet-stream' } let(:name) { 'keylogger.exe' } - let(:file) { file_as_base64 name, 'application/octet-stream' } - example_request '[error] Add an unsupported file extension as attachment to a page' do + example_request '[error]' do assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'extension_whitelist_error') + expect(json_response_body).to include_response_error(:file, 'extension_whitelist_error') end end - describe do - example '[error] Add a file of which the size is too large' do - # mock the size_range method of StaticPageFileUploader to have 3 bytes as maximum size - expect_any_instance_of(StaticPageFileUploader).to receive(:size_range).and_return(1..3) + describe 'Add a file of which the size is too large' do + example '[error]' do + # Mock the `size_range` method of `Files::FileUploader` to set the maximum size to 3 bytes. + expect_any_instance_of(Files::FileUploader).to receive(:size_range).and_return(1..3) do_request assert_status 422 - json_response = json_parse response_body - expect(json_response).to include_response_error(:file, 'max_size_error') + expect(json_response_body).to include_response_error(:file, 'max_size_error') end end end delete 'web_api/v1/static_pages/:page_id/files/:file_id' do let(:page_id) { @page.id } - let(:file_id) { StaticPageFile.first.id } + let(:file_id) { @page.file_attachments.first.id } + + example 'Delete file attachment from a page' do + expect { do_request } + .to change(Files::FileAttachment, :count).by(-1) + .and not_change(StaticPageFile, :count) - example_request 'Delete a file attachment from a page' do expect(response_status).to eq 200 - expect { StaticPageFile.find file_id }.to raise_error(ActiveRecord::RecordNotFound) + + expect { Files::FileAttachment.find(file_id) } + .to raise_error(ActiveRecord::RecordNotFound) end end end