diff --git a/app/controllers/api/api_controller.rb b/app/controllers/api/api_controller.rb index 6a43a21b74..c5eda4e6cf 100644 --- a/app/controllers/api/api_controller.rb +++ b/app/controllers/api/api_controller.rb @@ -6,4 +6,4 @@ def help render plain: api_routes.map {|r| "#{r.verb}\t#{r.path.spec}" }.join("\n") end end -end \ No newline at end of file +end diff --git a/app/controllers/api/v1/bulk_export_controller.rb b/app/controllers/api/v1/bulk_export_controller.rb index ea28c915e7..d3e198cb55 100644 --- a/app/controllers/api/v1/bulk_export_controller.rb +++ b/app/controllers/api/v1/bulk_export_controller.rb @@ -17,60 +17,55 @@ def index else exports = @api_user.bulk_exports end - render json: exports.to_json(except: [:updated_at, :user_id, :collection_id], include: {:collection => {only: [:title, :slug]}}) + render json: exports.to_json(except: [:updated_at, :user_id, :collection_id], include: { collection: { only: [:title, :slug] } }) else render status: 401, json: 'You must use an API token to access bulk exports' end end - def start if @api_user collection_slug = params[:collection_slug] - if collection_slug - collection = Collection.where(slug: collection_slug).first - if collection.nil? - render status: 404, json: "No collection exists with the slug #{collection_slug}" - return - end + collection = Collection.where(slug: collection_slug).first + if collection.nil? + render status: 404, json: "No collection exists with the slug #{collection_slug}" + return + end - if collection.show_to?(@api_user) - # try to parse the bulk_export - bulk_export = BulkExport.new - bulk_export.collection = collection - bulk_export.user = @api_user - bulk_export.status = BulkExport::Status::NEW - bulk_export.plaintext_verbatim_page = !(params[:plaintext_verbatim_page].blank?) - bulk_export.plaintext_verbatim_work = !(params[:plaintext_verbatim_work].blank?) - bulk_export.plaintext_emended_page = !(params[:plaintext_emended_page].blank?) - bulk_export.plaintext_emended_work = !(params[:plaintext_emended_work].blank?) - bulk_export.plaintext_searchable_page = !(params[:plaintext_searchable_page].blank?) - bulk_export.plaintext_searchable_work = !(params[:plaintext_searchable_work].blank?) - bulk_export.tei_work = !(params[:tei_work].blank?) - bulk_export.html_page = !(params[:html_page].blank?) - bulk_export.html_work = !(params[:html_work].blank?) - bulk_export.subject_csv_collection = !(params[:subject_csv_collection].blank?) - bulk_export.subject_details_csv_collection = !(params[:subject_details_csv_collection].blank?) - bulk_export.table_csv_collection = !(params[:table_csv_collection].blank?) - bulk_export.table_csv_work = !(params[:table_csv_work].blank?) - bulk_export.notes_csv = !(params[:notes_csv].blank?) - bulk_export.save - bulk_export.submit_export_process + if collection.show_to?(@api_user) + # try to parse the bulk_export + bulk_export = BulkExport.new + bulk_export.collection = collection + bulk_export.user = @api_user + bulk_export.status = BulkExport::Status::NEW + bulk_export.plaintext_verbatim_page = params[:plaintext_verbatim_page].present? + bulk_export.plaintext_verbatim_work = params[:plaintext_verbatim_work].present? + bulk_export.plaintext_emended_page = params[:plaintext_emended_page].present? + bulk_export.plaintext_emended_work = params[:plaintext_emended_work].present? + bulk_export.plaintext_searchable_page = params[:plaintext_searchable_page].present? + bulk_export.plaintext_searchable_work = params[:plaintext_searchable_work].present? + bulk_export.tei_work = params[:tei_work].present? + bulk_export.html_page = params[:html_page].present? + bulk_export.html_work = params[:html_work].present? + bulk_export.subject_csv_collection = params[:subject_csv_collection].present? + bulk_export.subject_details_csv_collection = params[:subject_details_csv_collection].present? + bulk_export.table_csv_collection = params[:table_csv_collection].present? + bulk_export.table_csv_work = params[:table_csv_work].present? + bulk_export.notes_csv = params[:notes_csv].present? + bulk_export.save + bulk_export.submit_export_process - response = { - :id => bulk_export.id, - :status => bulk_export.status, - :status_uri => api_v1_bulk_export_status_url(bulk_export.id), - :download_uri => api_v1_bulk_export_download_url(bulk_export.id) - } + response = { + id: bulk_export.id, + status: bulk_export.status, + status_uri: api_v1_bulk_export_status_url(bulk_export.id), + download_uri: api_v1_bulk_export_download_url(bulk_export.id) + } - render status: 202, json: response.to_json + render status: 202, json: response.to_json - else - render status: 403, json: 'User #{@api_user} is not authorized to view #{collection.title}' - end else - render status: 401, json: 'You must use a collection slug in the URL to create a bulk export' + render status: 403, json: "User #{@api_user} is not authorized to view #{collection.title}" end else render status: 401, json: 'You must use an API token to access bulk exports' @@ -80,59 +75,49 @@ def start def status if @api_user bulk_export_id = params[:bulk_export_id] - if bulk_export_id - bulk_export = @api_user.bulk_exports.where(:id => bulk_export_id).first - if bulk_export - response = { - :id => bulk_export.id, - :status => bulk_export.status, - :status_uri => api_v1_bulk_export_status_url(bulk_export.id), - :download_uri => api_v1_bulk_export_download_url(bulk_export.id) - } - render status: 200, json: response.to_json - else - render status: 403, json: 'User #{@api_user} has no bulk export with ID #{bulk_export_id}' - end + bulk_export = @api_user.bulk_exports.find_by(id: bulk_export_id) + + if bulk_export + response = { + id: bulk_export.id, + status: bulk_export.status, + status_uri: api_v1_bulk_export_status_url(bulk_export.id), + download_uri: api_v1_bulk_export_download_url(bulk_export.id) + } + render status: 200, json: response.to_json else - render status: 401, json: 'You must use a bulk export ID in the URL' + render status: 403, json: "User #{@api_user} has no bulk export with ID #{bulk_export_id}" end else render status: 401, json: 'You must use an API token to access bulk exports' end end - def download if @api_user bulk_export_id = params[:bulk_export_id] - if bulk_export_id - bulk_export = @api_user.bulk_exports.where(:id => bulk_export_id).first - if bulk_export - if bulk_export.status == BulkExport::Status::FINISHED - send_file(bulk_export.zip_file_name, - filename: "fromthepage_export.zip", - :content_type => "application/zip") - else - if bulk_export.status == BulkExport::Status::CLEANED - render status: 410, json: "Bulk export #{bulk_export_id} has been deleted. Please start a new export." - elsif bulk_export.status == BulkExport::Status::ERROR - render status: 410, json: "Bulk export #{bulk_export_id} failed with an error. Please report this to support. Re-running an export with different requested formats might succeed." - else - render status: 409, json: "Bulk export #{bulk_export_id} is not ready to download. It is probably still running, but might have failed with an un-caught error." - end - end + bulk_export = @api_user.bulk_exports.find_by(id: bulk_export_id) + if bulk_export + case bulk_export.status + when BulkExport::Status::FINISHED + send_file( + bulk_export.zip_file_name, + filename: 'fromthepage_export.zip', + content_type: 'application/zip' + ) + when BulkExport::Status::CLEANED + render status: 410, json: "Bulk export #{bulk_export_id} has been deleted. Please start a new export." + when BulkExport::Status::ERROR + render status: 410, json: "Bulk export #{bulk_export_id} failed with an error. Please report this to support. Re-running an export with different requested formats might succeed." else - render status: 403, json: 'User #{@api_user} has no bulk export with ID #{bulk_export_id}' + render status: 409, json: "Bulk export #{bulk_export_id} is not ready to download. It is probably still running, but might have failed with an un-caught error." end else - render status: 401, json: 'You must use a bulk export ID in the URL' + render status: 403, json: "User #{@api_user} has no bulk export with ID #{bulk_export_id}" end else render status: 401, json: 'You must use an API token to access bulk exports' end end - - private - end -end \ No newline at end of file +end diff --git a/config/routes.rb b/config/routes.rb index d6120e148c..70f80539ed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -362,7 +362,7 @@ namespace :v1 do get 'bulk_export', to: 'bulk_export#index' get 'bulk_export/:collection_slug', to: 'bulk_export#index' - post 'bulk_export/:collection_slug', to: 'bulk_export#start' + post 'bulk_export/:collection_slug', to: 'bulk_export#start', as: 'bulk_export_start' get 'bulk_export/:bulk_export_id/status', to: 'bulk_export#status', as: 'bulk_export_status' get 'bulk_export/:bulk_export_id/download', to: 'bulk_export#download', as: 'bulk_export_download' end diff --git a/spec/factories/bulk_export.rb b/spec/factories/bulk_export.rb new file mode 100644 index 0000000000..7961e356e6 --- /dev/null +++ b/spec/factories/bulk_export.rb @@ -0,0 +1,30 @@ +FactoryBot.define do + factory :bulk_export do + collection_id { association(:collection).id } + user_id { association(:user).id } + + trait :new do + status { BulkExport::Status::NEW } + end + + trait :queued do + status { BulkExport::Status::QUEUED } + end + + trait :processing do + status { BulkExport::Status::PROCESSING } + end + + trait :finished do + status { BulkExport::Status::FINISHED } + end + + trait :cleaned do + status { BulkExport::Status::CLEANED } + end + + trait :error do + status { BulkExport::Status::ERROR } + end + end +end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index c42cb4ab31..f719870599 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -17,5 +17,24 @@ sequence(:display_name) { |n| "admin_#{n}_login" } sequence(:login) { |n| "admin_#{n}_login" } end + + factory :unique_user do + sequence(:login) { "user_#{SecureRandom.hex(2)}_login" } + sequence(:email) { "user_#{SecureRandom.hex(2)}@sample.com" } + password { 'password' } + password_confirmation { 'password' } + + trait :with_api_key do + api_key { User.generate_api_key } + end + + trait :owner do + admin { true } + end + + trait :admin do + admin { true } + end + end end end diff --git a/spec/interactors/article/combine_spec.rb b/spec/interactors/article/combine_spec.rb index c7a847cea7..68f90781dd 100644 --- a/spec/interactors/article/combine_spec.rb +++ b/spec/interactors/article/combine_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Article::Combine do - let(:user) { User.find_by(login: OWNER) } + let!(:user) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: user.id) } let!(:work) { create(:work, collection: collection, owner_user_id: user.id) } let!(:from_related_page) { create(:page, work: work, source_text: '[[Duplicate]]', source_translation: '[[Duplicate]]') } diff --git a/spec/interactors/article/destroy_spec.rb b/spec/interactors/article/destroy_spec.rb index b561a38fe3..c13a0d034b 100644 --- a/spec/interactors/article/destroy_spec.rb +++ b/spec/interactors/article/destroy_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Article::Destroy do - let(:user) { User.find_by(login: OWNER) } + let!(:user) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: user.id) } let!(:article) { create(:article, collection: collection) } @@ -25,7 +25,7 @@ end context 'when user is not owner' do - let(:other_user) { User.find_by(login: NEW_OWNER) } + let!(:other_user) { create(:unique_user) } let(:result) do described_class.call( diff --git a/spec/interactors/article/update_spec.rb b/spec/interactors/article/update_spec.rb index 6e66f242cd..0a46c33e45 100644 --- a/spec/interactors/article/update_spec.rb +++ b/spec/interactors/article/update_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Article::Update do - let(:user) { User.find_by(login: OWNER) } + let!(:user) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: user.id) } let!(:work) { create(:work, collection: collection, owner_user_id: user.id) } let!(:related_page) { create(:page, work: work, source_text: '[[Original]]', source_translation: '[[Original]]') } diff --git a/spec/interactors/work/refresh_metadata_spec.rb b/spec/interactors/work/refresh_metadata_spec.rb index c8634da697..836b678fe2 100644 --- a/spec/interactors/work/refresh_metadata_spec.rb +++ b/spec/interactors/work/refresh_metadata_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Work::RefreshMetadata do - let(:owner) { User.find_by(login: OWNER) } + let!(:owner) { create(:unique_user, :owner) } let(:collection) { create(:collection, owner_user_id: owner.id) } let(:original_metadata) { [{ label: 'en', value: ['Original Metadata'] }].to_json } let(:at_id) { 'http://example.com/manifest' } diff --git a/spec/requests/api/api_controller_spec.rb b/spec/requests/api/api_controller_spec.rb new file mode 100644 index 0000000000..35ac2df9ea --- /dev/null +++ b/spec/requests/api/api_controller_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe Api::ApiController do + describe '#help' do + let(:action_path) { api_path } + + let(:subject) { get action_path } + + it 'renders status and plain_text' do + subject + + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq('text/plain; charset=utf-8') + end + end +end diff --git a/spec/requests/api/v1/bulk_export_controller_spec.rb b/spec/requests/api/v1/bulk_export_controller_spec.rb new file mode 100644 index 0000000000..f39bdbc862 --- /dev/null +++ b/spec/requests/api/v1/bulk_export_controller_spec.rb @@ -0,0 +1,214 @@ +require 'spec_helper' +require 'fileutils' +require 'zip' + +describe Api::V1::BulkExportController do + let!(:owner) { create(:unique_user, :with_api_key, :owner) } + let(:headers) { { 'Authorization': "Bearer #{owner.api_key}" } } + + let!(:collection) { create(:collection, owner_user_id: owner.id) } + + describe '#index' do + let(:action_path) { api_v1_bulk_export_path } + let(:params) { { collection_slug: collection.slug } } + + let(:subject) { get action_path, params: params, headers: headers } + + context 'with collection_slug param' do + it 'renders status and json' do + subject + + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + + context 'when collection does not exist' do + let(:params) { { collection_slug: SecureRandom.hex(4) } } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:not_found) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end + + context 'without collection_slug param' do + let(:params) { {} } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'without api_key' do + let(:headers) { {} } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:unauthorized) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end + + describe '#start' do + let(:action_path) { api_v1_bulk_export_start_path(collection_slug: collection.slug) } + + let(:subject) { post action_path, headers: headers } + + context 'when collection can show to user' do + it 'renders status and json' do + subject + + expect(response).to have_http_status(:accepted) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'when collection cannot show to user' do + let!(:other_owner) { create(:unique_user, :with_api_key, :owner) } + let!(:collection) { create(:collection, owner_user_id: other_owner.id, restricted: true) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:forbidden) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'without api_key' do + let(:headers) { {} } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:unauthorized) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end + + describe '#status' do + let!(:bulk_export) { create(:bulk_export, collection_id: collection.id, user_id: owner.id) } + let(:action_path) { api_v1_bulk_export_status_path(bulk_export_id: bulk_export.id) } + + let(:subject) { get action_path, headers: headers } + + context 'when bulk export exists' do + it 'renders status and json' do + subject + + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'when bulk export does not exist' do + let(:action_path) { api_v1_bulk_export_status_path(bulk_export_id: SecureRandom.hex(4)) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:forbidden) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'without api_key' do + let(:headers) { {} } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:unauthorized) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end + + describe '#download' do + let!(:bulk_export) { create(:bulk_export, :finished, collection_id: collection.id, user_id: owner.id) } + let(:action_path) { api_v1_bulk_export_download_path(bulk_export_id: bulk_export.id) } + + let(:subject) { get action_path, headers: headers } + + context 'when bulk export exists' do + context 'when bulk export is finished' do + before do + FileUtils.mkdir_p(bulk_export.zip_file_path) + Zip::File.open(bulk_export.zip_file_name, Zip::File::CREATE) do |_zipfile| + # Create temp empty zip + end + end + + it 'renders status' do + subject + + expect(response).to have_http_status(:ok) + end + end + + context 'when bulk export is cleaned' do + let!(:bulk_export) { create(:bulk_export, :cleaned, collection_id: collection.id, user_id: owner.id) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:gone) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'when bulk export has error' do + let!(:bulk_export) { create(:bulk_export, :error, collection_id: collection.id, user_id: owner.id) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:gone) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'when bulk export has other statuses' do + let!(:bulk_export) { create(:bulk_export, :queued, collection_id: collection.id, user_id: owner.id) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:conflict) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end + + context 'when bulk export does not exist' do + let(:action_path) { api_v1_bulk_export_download_path(bulk_export_id: SecureRandom.hex(4)) } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:forbidden) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + + context 'without api_key' do + let(:headers) { {} } + + it 'renders status and json' do + subject + + expect(response).to have_http_status(:unauthorized) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + end + end +end diff --git a/spec/requests/article_controller_spec.rb b/spec/requests/article_controller_spec.rb index 7e5bdcaefb..30f2231d73 100644 --- a/spec/requests/article_controller_spec.rb +++ b/spec/requests/article_controller_spec.rb @@ -5,7 +5,7 @@ User.current_user = owner end - let(:owner) { User.first } + let!(:owner) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: owner.id) } let!(:work) { create(:work, collection: collection, owner_user_id: owner.id) } let!(:page) { create(:page, work: work) } diff --git a/spec/requests/collection_controller_spec.rb b/spec/requests/collection_controller_spec.rb index 3e912e0670..edb6bcfdb2 100644 --- a/spec/requests/collection_controller_spec.rb +++ b/spec/requests/collection_controller_spec.rb @@ -5,8 +5,8 @@ User.current_user = owner end - let!(:owner) { build(:owner).tap { |o| o.save(validate: false) } } - let!(:user) { build(:user).tap { |u| u.save(validate: false) } } + let!(:owner) { create(:unique_user, :owner) } + let!(:user) { create(:unique_user) } let!(:collection) { create(:collection, owner_user_id: owner.id) } describe '#search_users' do diff --git a/spec/requests/export_controller_spec.rb b/spec/requests/export_controller_spec.rb index 0348bb1f05..4723d93906 100644 --- a/spec/requests/export_controller_spec.rb +++ b/spec/requests/export_controller_spec.rb @@ -5,7 +5,7 @@ User.current_user = owner end - let(:owner) { User.first } + let!(:owner) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: owner.id) } let!(:work) { create(:work, collection: collection, owner_user_id: owner.id) } let(:source_text) do diff --git a/spec/requests/notes_controller_spec.rb b/spec/requests/notes_controller_spec.rb index b287fb66c5..adcd9d81dc 100644 --- a/spec/requests/notes_controller_spec.rb +++ b/spec/requests/notes_controller_spec.rb @@ -5,7 +5,7 @@ User.current_user = owner end - let(:owner) { User.first } + let!(:owner) { create(:unique_user, :owner) } let!(:collection) { create(:collection, owner_user_id: owner.id) } let!(:work) { create(:work, collection: collection, owner_user_id: owner.id) } let!(:page) { create(:page, work: work) }